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.  When the neighbor discovery processing, for example, was
executed there is an incorrect dependancy 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 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 include
such dependent ND 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 by including the requisite fields.  Two
other approaches were considered - moving the nd_target field alone
(which leaves arp_sha/nd_sll, arp_tha/nd_tll, and tcp_flags still as
issues so this was avoided) and collapsing the l3/l4 segments into one
(which would result in flow explosion in real scenarios, so it was also
discarded as a solution).

Fixes: 476f36e83bc5 ("Classifier: Staged subtable matching.")
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2081773
Reported-by: Numan Siddique <[email protected]>
Suggested-by: Ilya Maximets <[email protected]>
Signed-off-by: Aaron Conole <[email protected]>
---
 include/openvswitch/flow.h |  2 +-
 tests/classifier.at        | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
index 3054015d93..a966dde216 100644
--- a/include/openvswitch/flow.h
+++ b/include/openvswitch/flow.h
@@ -179,7 +179,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..a0a8fe0359 100644
--- a/tests/classifier.at
+++ b/tests/classifier.at
@@ -129,6 +129,33 @@ 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
+])
+
+AT_CLEANUP
+
+
+
 AT_BANNER([conjunctive match])
 
 AT_SETUP([single conjunctive match])
-- 
2.31.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to