Hi Amber, Thanks for the patch. I've left some responses below.
Cian > -----Original Message----- > From: Amber, Kumar <[email protected]> > Sent: Monday 31 January 2022 10:52 > To: [email protected] > Cc: [email protected]; [email protected]; Stokes, Ian > <[email protected]>; Van Haaren, Harry > <[email protected]>; Amber, Kumar <[email protected]>; Ferriter, > Cian > <[email protected]> > Subject: [PATCH v1] system-dpdk.at: Improve mfex test to include dpif avx512. For the commit title, how about: system-dpdk: Fix mfex autovalidator tests. And adding a "Fixes:" tag? > > AVX512 DPIF must be active in order for the MFEX AutoValidator to be executed. > If the DPIF-AVX512 is not available, the unit test is skipped, as the > scalar DPIF does not use the MFEX function-pointer based optimizations. > > Suggested-by: Cian Ferriter <[email protected]> > Signed-off-by: Kumar Amber <[email protected]> > --- > tests/system-dpdk.at | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at > index 9384cf7f0..3d89384a3 100644 > --- a/tests/system-dpdk.at > +++ b/tests/system-dpdk.at > @@ -237,6 +237,10 @@ AT_CHECK([ovs-vsctl show], [], [stdout]) > AT_SKIP_IF([! ovs-appctl dpif-netdev/miniflow-parser-get | sed 1,4d | grep > "True"], [], [dnl > ]) > > +AT_SKIP_IF([! ovs-appctl dpif-netdev/dpif-impl-set dpif_avx512], [], [dnl > +DPIF implementation set to dpif_avx512. > +]) > + For the AT_SKIP_IF statements, only one argument should be given (one set of []). Please search "AT_SKIP_IF" on https://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Writing-Testsuites.html for more information. Also, having a look at how AT_SKIP_IF is supposed to be used and how it is actually used in OVS, I don't see anywhere that a command to change an OVS setting is run inside an AT_SKIP_IF. I think we should just use an AT_CHECK like I have below because we will only get this far if the above AT_SKIP_IF passes which means CPU ISA is available for AVX512 MFEX and DPIF impls. AT_CHECK([ovs-appctl dpif-netdev/dpif-impl-set dpif_avx512], [0], [dnl DPIF implementation set to dpif_avx512. ]) What do you think? > AT_CHECK([ovs-appctl dpif-netdev/miniflow-parser-set autovalidator], [0], > [dnl > Miniflow extract implementation set to autovalidator. > ]) > @@ -265,6 +269,10 @@ AT_CHECK([ovs-vsctl show], [], [stdout]) > AT_SKIP_IF([! ovs-appctl dpif-netdev/miniflow-parser-get | sed 1,4d | grep > "True"], [], [dnl > ]) > > +AT_SKIP_IF([! ovs-appctl dpif-netdev/dpif-impl-set dpif_avx512], [], [dnl > +DPIF implementation set to dpif_avx512. > +]) > + The above points apply for this AT_SKIP_IF too. > AT_CHECK([ovs-appctl dpif-netdev/miniflow-parser-set autovalidator], [0], > [dnl > Miniflow extract implementation set to autovalidator. > ]) > -- > 2.25.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
