Hi Aaron! This will move the following fields from the L4 segment to
the L3 segment:
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 pad2; /* Pad to 64 bits. */
struct ovs_key_nsh nsh; /* Network Service Header keys */
The one that stands out to me is tcp_flags. I remember that we had a
reason that we wanted to include this in the L3 segment. I think it was
because we might want to match on flags without matching on anything
else in L4, but I don't remember the exact details. There is a comment
to that effect above.
I suggest including the list of fields being moved in the commit message
(i.e. the code excerpt above) so that it's clearer what's changing.
Some more below:
On Wed, May 11, 2022 at 10:52:41AM -0400, Aaron Conole wrote:
> 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.
I find "include" (or "exclude") a little ambiguous here. I think that
saying that the generated or dumped WC "matches" or "does not match" on
a particular field would be clearer. Some more like this below:
> 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]>
Thanks for adding a test!
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
