[ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather implementation

[Van Haaren, Harry]

> -----Original Message-----
> From: dev <ovs-dev-bounces at openvswitch.org> On Behalf Of Van Haaren, Harry
> Sent: Tuesday, May 26, 2020 3:52 PM
> To: William Tu <u9012063 at gmail.com>
> Cc: ovs-dev at openvswitch.org; i.maximets at ovn.org
> Subject: Re: [ovs-dev] [PATCH v2 5/5] dpif-lookup: add avx512 gather
> implementation

<snip>

> > Why ukey is related here? Does you avx512 patch make any change to ukey?
> 
> No AVX512 doesn't make any ukey changes - but issues in the hashing of the
> miniflow data blocks cause ukeys to be installed in different locations than
> where they are looked up - hence "ukey install fail" == "issue in miniflow iteration" in
> this context.

The ukey install fails are due to a mismatch in compile flags (with/without SSE 4.2),
combined with the fact that the hashing in OVS changes its implementation depending
on the availability of the SSE 4.2  ISA (and other defines for other architectures).

The mismatch comes from upcall code being compiled without SSE4.2 (so using mhash hash code)
while the AVX512 lookup hash routines have SSE4.2 enabled (so using CRC32 hash code).
As a result, hashing identical data in different .c files produces a different hash values.

>From OVS docs (http://docs.openvswitch.org/en/latest/intro/install/general/) the following
enables native ISA for your build, or else just enable SSE4.2 and popcount:
./configure CFLAGS="-g -O2 -march=native"
./configure CFLAGS="-g -O2 -march=nehalem"

To continue your testing William, I suggest using the above workaround - compile OVS and explicitly
enable SSE4.2, aligning all hashing code in OVS to use the more performant CRC32 hashing.

I will work on a proper solution to avoid this issue in the v3 patchset.

Thanks for reporting, -Harry

> > > There is an alternative - set the "autovalidator" DPCLS implementation to
> > > the highest priority, and it should ovs_assert() if the scalar/AVX512
> > implementations
> > > mismatch. Then a dump of the OVS miniflow will give what's needed to verify
> > root cause.
> > >
> > that's a cool feature.
> > When setting
> > ovs-appctl dpif-netdev/subtable-lookup-set autovalidator 100
> > log shows
> > 2020-05-21T22:28:55.964Z|77007|dpif_lookup_autovalidator(pmd-
> > c00/id:9)|ERR|matches_good
> > 7 != matches_test 0 for func avx512_gather
> 
> Brilliant - this is exactly why the autovalidator is there. It has correctly flagged
> an issue here - I've reproduced using a pcap and your commands above. I will
> investigate a fix and include in the v3.
> 
> Thanks for the details - will keep you all posted on progress. -Harry
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev