Hi Cian, I agree with the suggestion we can use the normal AT_CHECK since we already have used the AT_CHECK_IF for checking AVX512.
Thanks > -----Original Message----- > From: Ferriter, Cian <[email protected]> > Sent: Monday, February 28, 2022 2:44 PM > To: Amber, Kumar <[email protected]>; [email protected] > Cc: [email protected]; [email protected]; Stokes, Ian > <[email protected]>; Van Haaren, Harry <[email protected]> > Subject: RE: [PATCH v1] system-dpdk.at: Improve mfex test to include dpif > avx512. > > 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
