netdev-offload-tc: Revert tunnel src/dst port masks handling

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@mellanox.com>
Reviewed-by: Eli Britstein <elibr@mellanox.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>

diff --git a/NEWS b/NEWS
index 88b273a0af8974ddf2682859d37fc64cfa2e0d3e..22cacda20ac7216a153ce7424deecc858311cb03 100644 (file)
--- 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 9e480318e2b33f862514f914c3db0480d871b812..2e8812048e86b83fda805e03207a7bb38cdb98ab 100644 (file)
--- 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 a775548511460baa64e3124507fd342dda78567a..ba716579d8ecde56d79aaa10d9f6e22d21de1aa3 100644 (file)
--- a/lib/match.c
+++ b/lib/match.c
@@ -293,19 +293,6 @@ match_set_tun_tp_dst_masked(struct match *match, ovs_be16 port, ovs_be16 mask)
     match->flow.tunnel.tp_dst = port & 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)
 {

diff --git a/lib/netdev-offload-tc.c b/lib/netdev-offload-tc.c
index aa6d22e74f29d0bdbec797e125438c743455f9f0..258d31f54b08ee76d7bd902d189295a5d37543fd 100644 (file)
--- 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 29b4328d8bfc20799eeaf122cb37077e628f295e..c2ab77553306e927b284c68ddf48b3ab049487ae 100644 (file)
--- 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 a74a67aa81239dbebdc09cb956f67aa505554c2d..e08fd1e048f85f5d48b5808a1f59cb621853129e 100644 (file)
--- 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