[ovs-dev] [PATCH] netdev-offload-tc: Revert tunnel src/dst port masks handling

Roi Dayan roid at mellanox.com
Tue Jun 16 13:03:57 UTC 2020


The cited commit intended to add tc support for masking tunnel src/dst
ips and ports. It's not possible to do tunnel ports masking with
openflow rules and the default mask for tunnel ports set to 0 in
tnl_wc_init(), unlike tunnel ports default mask which is full mask.
So instead of never passing tunnel ports to tc, revert the changes
to tunnel ports to always pass the tunnel port.
In sw classification is done by the kernel, but for hw we must match
the tunnel dst port.

Fixes: 5f568d049130 ("netdev-offload-tc: Allow to match the IP and port mask of tunnel")
Signed-off-by: Roi Dayan <roid at mellanox.com>
Reviewed-by: Eli Britstein <elibr at mellanox.com>
---
 NEWS                        |  2 --
 include/openvswitch/match.h |  3 ---
 lib/match.c                 | 13 -------------
 lib/netdev-offload-tc.c     | 13 ++-----------
 lib/tc.c                    | 28 ++--------------------------
 tests/tunnel.at             |  4 ++--
 6 files changed, 6 insertions(+), 57 deletions(-)

diff --git a/NEWS b/NEWS
index 88b273a0af89..22cacda20ac7 100644
--- a/NEWS
+++ b/NEWS
@@ -19,8 +19,6 @@ Post-v2.13.0
    - Tunnels: TC Flower offload
      * Tunnel Local endpoint address masked match are supported.
      * Tunnel Romte endpoint address masked match are supported.
-     * Tunnel Local endpoint ports masked match are supported.
-     * Tunnel Romte endpoint ports masked match are supported.
 
 
 v2.13.0 - 14 Feb 2020
diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
index 9e480318e2b3..2e8812048e86 100644
--- a/include/openvswitch/match.h
+++ b/include/openvswitch/match.h
@@ -105,9 +105,6 @@ void match_set_tun_flags(struct match *match, uint16_t flags);
 void match_set_tun_flags_masked(struct match *match, uint16_t flags, uint16_t mask);
 void match_set_tun_tp_dst(struct match *match, ovs_be16 tp_dst);
 void match_set_tun_tp_dst_masked(struct match *match, ovs_be16 port, ovs_be16 mask);
-void match_set_tun_tp_src(struct match *match, ovs_be16 tp_src);
-void match_set_tun_tp_src_masked(struct match *match,
-                                 ovs_be16 port, ovs_be16 mask);
 void match_set_tun_gbp_id_masked(struct match *match, ovs_be16 gbp_id, ovs_be16 mask);
 void match_set_tun_gbp_id(struct match *match, ovs_be16 gbp_id);
 void match_set_tun_gbp_flags_masked(struct match *match, uint8_t flags, uint8_t mask);
diff --git a/lib/match.c b/lib/match.c
index a77554851146..ba716579d8ec 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -294,19 +294,6 @@ match_set_tun_tp_dst_masked(struct match *match, ovs_be16 port, ovs_be16 mask)
 }
 
 void
-match_set_tun_tp_src(struct match *match, ovs_be16 tp_src)
-{
-    match_set_tun_tp_src_masked(match, tp_src, OVS_BE16_MAX);
-}
-
-void
-match_set_tun_tp_src_masked(struct match *match, ovs_be16 port, ovs_be16 mask)
-{
-    match->wc.masks.tunnel.tp_src = mask;
-    match->flow.tunnel.tp_src = port & mask;
-}
-
-void
 match_set_tun_gbp_id_masked(struct match *match, ovs_be16 gbp_id, ovs_be16 mask)
 {
     match->wc.masks.tunnel.gbp_id = mask;
diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index aa6d22e74f29..258d31f54b08 100644
--- a/lib/netdev-offload-tc.c
+++ b/lib/netdev-offload-tc.c
@@ -712,15 +712,8 @@ parse_tc_flower_to_match(struct tc_flower *flower,
             match_set_tun_ttl_masked(match, flower->key.tunnel.ttl,
                                      flower->mask.tunnel.ttl);
         }
-        if (flower->mask.tunnel.tp_dst) {
-            match_set_tun_tp_dst_masked(match,
-                                        flower->key.tunnel.tp_dst,
-                                        flower->mask.tunnel.tp_dst);
-        }
-        if (flower->mask.tunnel.tp_src) {
-            match_set_tun_tp_src_masked(match,
-                                        flower->key.tunnel.tp_src,
-                                        flower->mask.tunnel.tp_src);
+        if (flower->key.tunnel.tp_dst) {
+            match_set_tun_tp_dst(match, flower->key.tunnel.tp_dst);
         }
         if (flower->key.tunnel.metadata.present.len) {
             flower_tun_opt_to_match(match, flower);
@@ -1470,8 +1463,6 @@ netdev_tc_flow_put(struct netdev *netdev, struct match *match,
         flower.mask.tunnel.ipv6.ipv6_dst = tnl_mask->ipv6_dst;
         flower.mask.tunnel.tos = tnl_mask->ip_tos;
         flower.mask.tunnel.ttl = tnl_mask->ip_ttl;
-        flower.mask.tunnel.tp_src = tnl_mask->tp_src;
-        flower.mask.tunnel.tp_dst = tnl_mask->tp_dst;
         flower.mask.tunnel.id = (tnl->flags & FLOW_TNL_F_KEY) ? tnl_mask->tun_id : 0;
         flower_match_to_tun_opt(&flower, tnl, tnl_mask);
         flower.tunnel = true;
diff --git a/lib/tc.c b/lib/tc.c
index 29b4328d8bfc..c2ab77553306 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -395,12 +395,6 @@ static const struct nl_policy tca_flower_policy[] = {
                                            .optional = true, },
     [TCA_FLOWER_KEY_ENC_UDP_DST_PORT] = { .type = NL_A_U16,
                                           .optional = true, },
-    [TCA_FLOWER_KEY_ENC_UDP_SRC_PORT] = { .type = NL_A_U16,
-                                          .optional = true, },
-    [TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK] = { .type = NL_A_U16,
-                                               .optional = true, },
-    [TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK] = { .type = NL_A_U16,
-                                               .optional = true, },
     [TCA_FLOWER_KEY_FLAGS] = { .type = NL_A_BE32, .optional = true, },
     [TCA_FLOWER_KEY_FLAGS_MASK] = { .type = NL_A_BE32, .optional = true, },
     [TCA_FLOWER_KEY_IP_TTL] = { .type = NL_A_U8,
@@ -746,15 +740,7 @@ nl_parse_flower_tunnel(struct nlattr **attrs, struct tc_flower *flower)
         flower->key.tunnel.ipv6.ipv6_dst =
             nl_attr_get_in6_addr(attrs[TCA_FLOWER_KEY_ENC_IPV6_DST]);
     }
-    if (attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK]) {
-        flower->mask.tunnel.tp_src =
-            nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK]);
-        flower->key.tunnel.tp_src =
-            nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_SRC_PORT]);
-    }
-    if (attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK]) {
-        flower->mask.tunnel.tp_dst =
-            nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK]);
+    if (attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]) {
         flower->key.tunnel.tp_dst =
             nl_attr_get_be16(attrs[TCA_FLOWER_KEY_ENC_UDP_DST_PORT]);
     }
@@ -2713,10 +2699,7 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower)
     struct in6_addr *ipv6_dst_mask = &flower->mask.tunnel.ipv6.ipv6_dst;
     struct in6_addr *ipv6_src = &flower->key.tunnel.ipv6.ipv6_src;
     struct in6_addr *ipv6_dst = &flower->key.tunnel.ipv6.ipv6_dst;
-    ovs_be16 tp_dst_mask = flower->mask.tunnel.tp_dst;
-    ovs_be16 tp_src_mask = flower->mask.tunnel.tp_src;
     ovs_be16 tp_dst = flower->key.tunnel.tp_dst;
-    ovs_be16 tp_src = flower->key.tunnel.tp_src;
     ovs_be32 id = be64_to_be32(flower->key.tunnel.id);
     uint8_t tos = flower->key.tunnel.tos;
     uint8_t ttl = flower->key.tunnel.ttl;
@@ -2748,16 +2731,9 @@ nl_msg_put_flower_tunnel(struct ofpbuf *request, struct tc_flower *flower)
         nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL, ttl);
         nl_msg_put_u8(request, TCA_FLOWER_KEY_ENC_IP_TTL_MASK, ttl_mask);
     }
-    if (tp_dst_mask) {
-        nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT_MASK,
-                        tp_dst_mask);
+    if (tp_dst) {
         nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_DST_PORT, tp_dst);
     }
-    if (tp_src_mask) {
-        nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_SRC_PORT_MASK,
-                        tp_src_mask);
-        nl_msg_put_be16(request, TCA_FLOWER_KEY_ENC_UDP_SRC_PORT, tp_src);
-    }
     if (id_mask) {
         nl_msg_put_be32(request, TCA_FLOWER_KEY_ENC_KEY_ID, id);
     }
diff --git a/tests/tunnel.at b/tests/tunnel.at
index a74a67aa8123..e08fd1e048f8 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -123,10 +123,10 @@ AT_CHECK([ovs-appctl dpif/show | tail -n +3], [0], [dnl
     p2 2/2: (dummy)
 ])
 
-AT_CHECK([ovs-appctl dpctl/add-flow "tunnel(dst=1.1.1.1,src=3.3.3.200/255.255.255.0,tp_dst=123,tp_src=1/0xf,ttl=64),recirc_id(0),in_port(1),eth(),eth_type(0x0800),ipv4()" "2"])
+AT_CHECK([ovs-appctl dpctl/add-flow "tunnel(dst=1.1.1.1,src=3.3.3.200/255.255.255.0,tp_dst=123,tp_src=1,ttl=64),recirc_id(0),in_port(1),eth(),eth_type(0x0800),ipv4()" "2"])
 
 AT_CHECK([ovs-appctl dpctl/dump-flows | tail -1], [0], [dnl
-tunnel(src=3.3.3.200/255.255.255.0,dst=1.1.1.1,ttl=64,tp_src=1/0xf,tp_dst=123),recirc_id(0),in_port(1),eth_type(0x0800), packets:0, bytes:0, used:never, actions:2
+tunnel(src=3.3.3.200/255.255.255.0,dst=1.1.1.1,ttl=64,tp_src=1,tp_dst=123),recirc_id(0),in_port(1),eth_type(0x0800), packets:0, bytes:0, used:never, actions:2
 ])
 
 OVS_VSWITCHD_STOP
-- 
2.8.4



More information about the dev mailing list