> -----Original Message----- > From: Van Haaren, Harry > Sent: Monday, January 24, 2022 3:43 PM > To: Eelco Chaudron <echau...@redhat.com> > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org; Ferriter, Cian > <cian.ferri...@intel.com> > Subject: RE: [ovs-dev] [PATCH] dpif-netdev: fix handling of vlan and ipv4 > parsing > in avx512 > > > -----Original Message----- > > From: Eelco Chaudron <echau...@redhat.com> > > Sent: Friday, January 14, 2022 10:03 AM > > To: Van Haaren, Harry <harry.van.haa...@intel.com> > > Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org > > Subject: Re: [ovs-dev] [PATCH] dpif-netdev: fix handling of vlan and ipv4 > parsing > > in avx512 > > <snip patch contents> > > > > -#define PATTERN_IPV4_MASK PATTERN_IPV4_GEN(0xFF, 0xFE, 0xFF, 0xFF) > > > +#define PATTERN_IPV4_MASK PATTERN_IPV4_GEN(0xFF, 0xFF, 0xFF, 0xFF) > > > > I assume the original idea was not to include the may fragment bit, which I > > think > > should be fine to ignore. > > Yes - good point. > > > But the previous mask was 0xFE was masking of bit in the “Fragment offset”, > > so I think setting this to 0xDF would accomplish making sure this is not a > > fragment (or reserved bit set). > > Yes - will validate and respin a v2.
The idea of masking away the "Don’t Frag" bit (DF) is a good one. Identifying the correct bit is unfortunately more complex than one would initially think. The reason is that BE and LE, as well as "written formats" tend to disagree on exactly how things work. The suggested 0xDF above isn't correct. For example, the Wikipedia article mentions bits, but doesn't state BE/LE, or which direction to count the bits in (its left to right, aka MSB to lsb): https://en.wikipedia.org/wiki/IPv4#Flags Looking at the OVS/FreeBSD source gives a better idea, as it uses u16 values, and 0x4000 style hex values: https://www.leidinger.net/FreeBSD/dox/netinet/html/da/d2f/ip_8h_source.html#l00065 All in all, the 0xFF initially proposed will not handle packets with the DF bit set, so is not ideal. The correct hex-value to use to mask-away the "DF" bit is 0xBF: bin 0100 0000 : Location of the DF bit in the u16 frag-offset in the IPv4 header (network byte order) bin 1011 1111 : Mask value to ignore that DF bit. Binary 1011 1111 = hex 0xBF. With the DF bit now appropriately ignored, we can *match* against an IPv4 packets, however the DF bit is actually not *tracked* by the miniflow data-structure at all! (The scalar code extracts the nw_frag info from the frag_offset field, but masks away DF bit, and never pushes it into miniflow). The avx512 code must emulate that behaviour; mfex-autovalidator immediately pin-pointed the issue. The "fixup" for IPv4 DF bit is not applicable to other protocols[1], so we do not want to add it to the generic MFEX processing infrastructure. A simple bitwise AND of the blocks is all that is required, and can be done in the IPv4 parts of the switch on the protocol stack. Patch to implement & handle that DF bit-stripping on the way. -Harry [1] IPv6 Protocol Additions https://patchwork.ozlabs.org/project/openvswitch/cover/20211207110425.3873101-1-kumar.am...@intel.com/ <snip remainder of patch which didn't have new comments> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
