From patchwork Tue May 24 21:35:23 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Aaron Conole X-Patchwork-Id: 1635165 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=AMrlhrOM; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4L76v03bf1z9sGl for ; Wed, 25 May 2022 07:35:39 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 5DC4541A07; Tue, 24 May 2022 21:35:36 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id OX23wmg4Uxuu; Tue, 24 May 2022 21:35:35 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 096DC415E3; Tue, 24 May 2022 21:35:33 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id CE17BC0039; Tue, 24 May 2022 21:35:33 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id D8521C002D for ; Tue, 24 May 2022 21:35:31 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id C6DA2415E5 for ; Tue, 24 May 2022 21:35:31 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id XdhULOJz-oGv for ; Tue, 24 May 2022 21:35:30 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id 16E1F415E3 for ; Tue, 24 May 2022 21:35:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1653428128; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=wqypQGZUe7vxv+CHAdEjf6z67Mr58/D+MKWN357AH0I=; b=AMrlhrOMTU8nqDX8FxIhR0str+a5NLFFZSWGU63U1zYCNTyFV6SAS+OYA8d5hbXIES7HAD kRsjCFxe3q3M4JyMnyMadHqKRGW8EyjJFgkyKrgcVLpNYiKzQCYH/N74uK7HXaSfh2PV80 ld+tC9+sxgGy/HiqZ7YyRTF5ds4tSKg= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-460-McBI7XM1Mmqn8SFD6BzoJQ-1; Tue, 24 May 2022 17:35:24 -0400 X-MC-Unique: McBI7XM1Mmqn8SFD6BzoJQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 69F283C025BD; Tue, 24 May 2022 21:35:24 +0000 (UTC) Received: from RHTPC1VM0NT.lan (unknown [10.22.32.159]) by smtp.corp.redhat.com (Postfix) with ESMTP id EF39F82872; Tue, 24 May 2022 21:35:23 +0000 (UTC) From: Aaron Conole To: dev@openvswitch.org Date: Tue, 24 May 2022 17:35:23 -0400 Message-Id: <20220524213523.637938-1-aconole@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=aconole@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: Ilya Maximets , Flavio Leitner , Ben Pfaff , Rashid Khan Subject: [ovs-dev] [PATCH v2] classifier: adjust segment boundary to execute prerequisite processing 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: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" During flow processing, the flow wildcards are checked as a series of stages, and these stages are intended to carry dependencies in a single direction. But when the neighbor discovery processing, for example, was executed there is an incorrect dependency chain - we need fields from stage 4 to determine whether we need fields from stage 3. We can build a set of flow rules to demonstrate this: table=0,priority=100,ipv6,ipv6_src=1000::/10 actions=resubmit(,1) table=0,priority=0 actions=NORMAL table=1,priority=110,ipv6,ipv6_dst=1000::3 actions=resubmit(,2) table=1,priority=100,ipv6,ipv6_dst=1000::4 actions=resubmit(,2) table=1,priority=0 actions=NORMAL table=2,priority=120,icmp6,nw_ttl=255,icmp_type=135,icmp_code=0,nd_sll=10:de:ad:be:ef:10 actions=NORMAL table=2,priority=100,tcp actions=NORMAL table=2,priority=100,icmp6 actions=NORMAL table=2,priority=0 actions=NORMAL With this set of flows, any IPv6 packet that executes through this pipeline will have the corresponding nd_sll field flagged as required match for classification even if that field doesn't make sense in such a context (for example, TCP packets). When the corresponding flow is installed into the kernel datapath, this field is not reflected when the revalidator executes the dump stage (see net/openvswitch/flow_netlink.c for more details). During the sweep stage, revalidator will compare the dumped WC with a generated WC - these will mismatch because the generated WC will match on the Neighbor Discovery fields, while the datapath WC will not match on these fields. We will then invalidate the flow and as a side effect force an upcall. By redefining the boundary, we shift these fields to the l4 subtable, and cause masks to be generated matching just the requisite fields. The list of fields being shifted: struct in6_addr nd_target; struct eth_addr arp_sha; struct eth_addr arp_tha; ovs_be16 tcp_flags; ovs_be16 pad2; struct ovs_key_nsh nsh; A standout field would be tcp_flags moving from l3 subtable matches to the l4 subtable matches. This reverts a partial performance optimization in the case of stateless firewalling. The tcp_flags field might have been a good candidate to retain in the l3 segment, but it got overloaded with ICMPv6 ND matching, and therefore we can't preserve this kind of optimization. Two other approaches were considered - moving the nd_target field alone and collapsing the l3/l4 segments into a single subtable for matching. Moving any field individually introduces ABI mismatch, and doesn't completely address the problems with other neighbor discovery related fields (such as nd_sll/nd_tll). Collapsing the two subtables creates an issue with datapath flow explosion, since the l3 and l4 fields will be unwildcarded together (this can be seen with some of the existing classifier tests). A simple test is added to showcase the behavior. Fixes: 476f36e83bc5 ("Classifier: Staged subtable matching.") Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2081773 Reported-by: Numan Siddique Suggested-by: Ilya Maximets Signed-off-by: Aaron Conole Acked-by: Eelco Chaudron Acked-by: Cian Ferriter Tested-by: Numan Siddique --- v1->v2: Added missing OVS_VSWITCHD_STOP and adjust whitespace on test case include/openvswitch/flow.h | 7 +++---- tests/classifier.at | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h index 3054015d93..df10cf579e 100644 --- a/include/openvswitch/flow.h +++ b/include/openvswitch/flow.h @@ -141,15 +141,14 @@ struct flow { uint8_t nw_tos; /* IP ToS (including DSCP and ECN). */ uint8_t nw_ttl; /* IP TTL/Hop Limit. */ uint8_t nw_proto; /* IP protocol or low 8 bits of ARP opcode. */ + /* L4 (64-bit aligned) */ struct in6_addr nd_target; /* IPv6 neighbor discovery (ND) target. */ struct eth_addr arp_sha; /* ARP/ND source hardware address. */ struct eth_addr arp_tha; /* ARP/ND target hardware address. */ - ovs_be16 tcp_flags; /* TCP flags/ICMPv6 ND options type. - * With L3 to avoid matching L4. */ + ovs_be16 tcp_flags; /* TCP flags/ICMPv6 ND options type. */ ovs_be16 pad2; /* Pad to 64 bits. */ struct ovs_key_nsh nsh; /* Network Service Header keys */ - /* L4 (64-bit aligned) */ ovs_be16 tp_src; /* TCP/UDP/SCTP source port/ICMP type. */ ovs_be16 tp_dst; /* TCP/UDP/SCTP destination port/ICMP code. */ ovs_be16 ct_tp_src; /* CT original tuple source port/ICMP type. */ @@ -179,7 +178,7 @@ BUILD_ASSERT_DECL(offsetof(struct flow, igmp_group_ip4) + sizeof(uint32_t) enum { FLOW_SEGMENT_1_ENDS_AT = offsetof(struct flow, dl_dst), FLOW_SEGMENT_2_ENDS_AT = offsetof(struct flow, nw_src), - FLOW_SEGMENT_3_ENDS_AT = offsetof(struct flow, tp_src), + FLOW_SEGMENT_3_ENDS_AT = offsetof(struct flow, nd_target), }; BUILD_ASSERT_DECL(FLOW_SEGMENT_1_ENDS_AT % sizeof(uint64_t) == 0); BUILD_ASSERT_DECL(FLOW_SEGMENT_2_ENDS_AT % sizeof(uint64_t) == 0); diff --git a/tests/classifier.at b/tests/classifier.at index cdcd72c156..57a1feb389 100644 --- a/tests/classifier.at +++ b/tests/classifier.at @@ -129,6 +129,31 @@ Datapath actions: 3 OVS_VSWITCHD_STOP(["/'prefixes' with incompatible field: ipv6_label/d"]) AT_CLEANUP +AT_SETUP([flow classifier - ipv6 ND dependancy]) +OVS_VSWITCHD_START +add_of_ports br0 1 2 +AT_DATA([flows.txt], [dnl + table=0,priority=100,ipv6,ipv6_src=1000::/10 actions=resubmit(,1) + table=0,priority=0 actions=NORMAL + table=1,priority=110,ipv6,ipv6_dst=1000::3 actions=resubmit(,2) + table=1,priority=100,ipv6,ipv6_dst=1000::4 actions=resubmit(,2) + table=1,priority=0 actions=NORMAL + table=2,priority=120,icmp6,nw_ttl=255,icmp_type=135,icmp_code=0,nd_target=1000::1 actions=NORMAL + table=2,priority=100,tcp actions=NORMAL + table=2,priority=100,icmp6 actions=NORMAL + table=2,priority=0 actions=NORMAL +]) +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) + +# test ICMPv6 echo request (which should have no nd_target field) +AT_CHECK([ovs-appctl ofproto/trace br0 "in_port=1,eth_src=f6:d2:b0:19:5e:7b,eth_dst=d2:49:19:91:78:fe,dl_type=0x86dd,ipv6_src=1000::3,ipv6_dst=1000::4,nw_proto=58,icmpv6_type=128,icmpv6_code=0"], [0], [stdout]) +AT_CHECK([tail -2 stdout], [0], + [Megaflow: recirc_id=0,eth,icmp6,in_port=1,dl_src=f6:d2:b0:19:5e:7b,dl_dst=d2:49:19:91:78:fe,ipv6_src=1000::/10,ipv6_dst=1000::4,nw_ttl=0,nw_frag=no +Datapath actions: 100,2 +]) +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_BANNER([conjunctive match]) AT_SETUP([single conjunctive match])