From patchwork Tue Jun 16 13:03:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roi Dayan X-Patchwork-Id: 1310313 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=mellanox.com Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49mT2j5P8Lz9sRN for ; Tue, 16 Jun 2020 23:05:33 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 4A4B987231; Tue, 16 Jun 2020 13:05:32 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UNU4eaXMaF9Z; Tue, 16 Jun 2020 13:05:31 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 6232D86B8A; Tue, 16 Jun 2020 13:05:31 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 476FDC07FF; Tue, 16 Jun 2020 13:05:31 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 29636C016E for ; Tue, 16 Jun 2020 13:05:30 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 262DE86FF8 for ; Tue, 16 Jun 2020 13:05:30 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ITtYUTUyYnpl for ; Tue, 16 Jun 2020 13:05:29 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mellanox.co.il (mail-il-dmz.mellanox.com [193.47.165.129]) by fraxinus.osuosl.org (Postfix) with ESMTP id 8373986B8A for ; Tue, 16 Jun 2020 13:05:28 +0000 (UTC) Received: from Internal Mail-Server by MTLPINE1 (envelope-from roid@mellanox.com) with SMTP; 16 Jun 2020 16:05:23 +0300 Received: from mtr-vdi-191.wap.labs.mlnx. (mtr-vdi-191.wap.labs.mlnx [10.209.100.28]) by labmailer.mlnx (8.13.8/8.13.8) with ESMTP id 05GD5Mm1003043; Tue, 16 Jun 2020 16:05:22 +0300 From: Roi Dayan To: dev@openvswitch.org Date: Tue, 16 Jun 2020 16:03:57 +0300 Message-Id: <20200616130357.23719-1-roid@mellanox.com> X-Mailer: git-send-email 2.8.4 Cc: Simon Horman , Ilya Maximets Subject: [ovs-dev] [PATCH] netdev-offload-tc: Revert tunnel src/dst port masks handling X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" 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 Reviewed-by: Eli Britstein --- 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