> On 21 May 2024, at 16:13, Emma Finn wrote: > > The AVX implementation for calcualting checksums was not > > handling carry-over addition correctly in some cases. > > This patch adds an additional shuffle to add 16-bit padding > > to the final part of the calculation to handle such cases. > > This commit also adds a unit test to fuzz test the actions > > autovalidator. > > > > Signed-off-by: Emma Finn <emma.f...@intel.com> > > Reported-by: Eelco Chaudron <echau...@redhat.com> <snip> > > +AT_SETUP([datapath - Actions Autovalidator Fuzzy]) > > +AT_SKIP_IF([! $PYTHON3 -c "import scapy"], [], []) > > +AT_SKIP_IF([! $PYTHON3 $srcdir/genpkts.py 10000 fuzzy > packets]) > > Hi Emma,
Hi Eelco, Ilya & Emma (&OVS community) As you know checksums are a very "input sensitive" workload; its kind of why they're useful (hash ~uniquely represents inputset ideally) Internally, computing a hash always involves carry-bits and (expected/good) overflows, however this patch is fixing an unintended/bad overflow. Testing these corner-cases & boundary conditions is hard; for me the only way to be confident in its correctness is autovaldation and fuzzing. > As Ilya explained in the v2 reply, we do not like to have fuzzy tests in the > test suite as they are hard to reproduce. (link https://patchwork.ozlabs.org/project/openvswitch/patch/20240514134815.2576245-1-emma.f...@intel.com/#3313143) I understand that any randomness in CI is not desired, and understand that random tests can be hard to reproduce. The autovalidator tries to mitigate the "reproduce" issue by dumping the packet contents on failure, providing the erroring case to easily allow reproduction. This test *should* be 100% reliably passing, so no false-positives are expected. If not, we should fix the test to be 100% reliable. > So my request is to send out a v4 with a test modeled around the 'userspace > offload - ip csum offload' test case we have in tests/dpif-netdev.at, as per > Ilya’s request. I'll suggest 3 paths forward, and give my pro/cons list for each; whatever option is preferred by OVS community we can send as a v4; 1) Accept "SIMD code fix" part of patch only (no unit test, just fixes issue with local fuzz-testing during development) 2) Build "specific-to-this-issue unit-test" only (regression tests against this specific issue, but nothing else; low value in my opinion, as it leaves a large input space untested) 3) Accept "v3 patch as is, with fuzzing" (best test-coverage, confidence in current & future code changes. CI runtime on fuzz-tests, and if failures occur, they must be flagged & fixed, but are *actual real issues* that should be fixed.) I think 2) is what is being asked above, but wanted to make sure the tradeoffs are understood. As above, please indicate which is the preferred approach and a v4 will be on the way. > Thanks, > > Eelco Thanks & Regards, -Harry _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
