| Message ID | 20210617162754.2028048-6-kumar.amber@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MFEX Infrastructure + Optimizations | expand |
> This commit adds a new command to allow the user to enable > autovalidatior by default at build time thus allowing for > runnig unit test by default. > > $ ./configure --enable-mfex-default-autovalidator > Hi Amber, thanks for the patch, comments inline below. > Signed-off-by: Kumar Amber <kumar.amber@intel.com> > Co-authored-by: Harry van Haaren <harry.van.haaren@intel.com> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> > --- > Documentation/topics/dpdk/bridge.rst | 5 +++++ > NEWS | 12 +++++++++++- > acinclude.m4 | 16 ++++++++++++++++ > configure.ac | 1 + > lib/dpif-netdev-private-extract.c | 24 ++++++++++++++++++++++++ > lib/dpif-netdev-private-extract.h | 10 ++++++++++ > lib/dpif-netdev.c | 7 +++++-- > 7 files changed, 72 insertions(+), 3 deletions(-) > > diff --git a/Documentation/topics/dpdk/bridge.rst > b/Documentation/topics/dpdk/bridge.rst > index b262b98f8..1c78adc75 100644 > --- a/Documentation/topics/dpdk/bridge.rst > +++ b/Documentation/topics/dpdk/bridge.rst > @@ -307,6 +307,11 @@ To set the Miniflow autovalidator, use this command :: > > $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator > > +A compile time option is available in order to test it with the OVS unit > +test suite. Use the following configure option :: > + > + $ ./configure --enable-mfex-default-autovalidator > + > Unit Test Miniflow Extract > ++++++++++++++++++++++++++ > > diff --git a/NEWS b/NEWS > index 63a485309..ed9f4d4c4 100644 > --- a/NEWS > +++ b/NEWS > @@ -24,6 +24,17 @@ Post-v2.15.0 > * An optimized miniflow extract (mfex) implementation is now available, > which uses CPU SIMD ISA to parse specific traffic profiles efficiently. > Refer to the documentation for details on how to enable it at runtime. The NEWS section in general here seems wrong, seems to have features from other patches in both this patch series and the DPIF patch series added which shouldn't be the case. Only features added by this patch should be added to NEWS. > + * Cache results for CPU ISA checks, reduces overhead on repeated lookups. > + * Add command line option to switch between mfex function pointers. > + * Add miniflow extract auto-validator function to compare different > + miniflow extract implementations against default implementation. > + * Add study function to miniflow function table which studies packet > + and automatically chooses the best miniflow implementation for that > + traffic. > + * Add AVX512 based optimized miniflow extract function for traffic type > + IP/UDP. The line below is the only item that should be added to NEWS in this patch. > + * Add build time configure command to enable auto-validatior as default > + miniflow implementation at build time. > - ovs-ctl: > * New option '--no-record-hostname' to disable hostname configuration > in ovsdb on startup. > @@ -35,7 +46,6 @@ Post-v2.15.0 > * New option '--election-timer' to the 'create-cluster' command to set the > leader election timer during cluster creation. > > - > v2.15.0 - 15 Feb 2021 > --------------------- > - OVSDB: > diff --git a/acinclude.m4 b/acinclude.m4 > index 5fbcd9872..e2704cfda 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -14,6 +14,22 @@ > # See the License for the specific language governing permissions and > # limitations under the License. > > +dnl Set OVS MFEX Autovalidator as default miniflow extract at compile time? > +dnl This enables automatically running all unit tests with all MFEX > +dnl implementations. > +AC_DEFUN([OVS_CHECK_MFEX_AUTOVALIDATOR], [ > + AC_ARG_ENABLE([mfex-default-autovalidator], > + [AC_HELP_STRING([--enable-mfex-default-autovalidator], [Enable > MFEX autovalidator as default miniflow_extract implementation.])], > + [autovalidator=yes],[autovalidator=no]) > + AC_MSG_CHECKING([whether MFEX Autovalidator is default > implementation]) > + if test "$autovalidator" != yes; then > + AC_MSG_RESULT([no]) > + else > + OVS_CFLAGS="$OVS_CFLAGS -DMFEX_AUTOVALIDATOR_DEFAULT" > + AC_MSG_RESULT([yes]) > + fi > +]) > + > dnl Set OVS DPCLS Autovalidator as default subtable search at compile time? > dnl This enables automatically running all unit tests with all DPCLS > dnl implementations. > diff --git a/configure.ac b/configure.ac > index e45685a6c..46c402892 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -186,6 +186,7 @@ OVS_ENABLE_SPARSE > OVS_CTAGS_IDENTIFIERS > OVS_CHECK_DPCLS_AUTOVALIDATOR > OVS_CHECK_DPIF_AVX512_DEFAULT > +OVS_CHECK_MFEX_AUTOVALIDATOR > OVS_CHECK_BINUTILS_AVX512 > > AC_ARG_VAR(KARCH, [Kernel Architecture String]) > diff --git a/lib/dpif-netdev-private-extract.c b/lib/dpif-netdev-private-extract.c > index d86268a1d..2008e5ee5 100644 > --- a/lib/dpif-netdev-private-extract.c > +++ b/lib/dpif-netdev-private-extract.c > @@ -230,3 +230,27 @@ dpif_miniflow_extract_autovalidator(struct > dp_packet_batch *packets, > */ > return 0; > } > + > +/* Variable to hold the defaualt mfex implementation. */ Typo in default above. > +static miniflow_extract_func default_mfex_func = NULL; I'd prefer if above was declared at the top of the file rather than here. > + > +void > +dpif_miniflow_extract_set_default(miniflow_extract_func func) > +{ > + default_mfex_func = func; > +} > + > +miniflow_extract_func > +dpif_miniflow_extract_get_default(void) > +{ > + > +#ifdef MFEX_AUTOVALIDATOR_DEFAULT > + ovs_assert(mfex_impls[0].extract_func == > + dpif_miniflow_extract_autovalidator); > + VLOG_INFO("Default miniflow Extract implementation %s \n", > + mfex_impls[0].name); > + return mfex_impls[0].extract_func; > +#else > + return default_mfex_func; > +#endif > +} > diff --git a/lib/dpif-netdev-private-extract.h b/lib/dpif-netdev-private-extract.h > index 3ada413bb..d8a284db7 100644 > --- a/lib/dpif-netdev-private-extract.h > +++ b/lib/dpif-netdev-private-extract.h > @@ -118,4 +118,14 @@ mfex_study_traffic(struct dp_packet_batch *packets, > uint32_t keys_size, odp_port_t in_port, > void *pmd_handle); > > +/* Retrieve the default miniflow extract or auto-validator > + * based upon build time configuration choosen by the user. */ Typo above, chosen. > +miniflow_extract_func > +dpif_miniflow_extract_get_default(void); > + > +/* Returns the default MFEX which is first ./configure selected, but can be > + * overridden at runtime. */ > +void > +dpif_miniflow_extract_set_default(miniflow_extract_func func); > + > #endif /* DPIF_NETDEV_AVX512_EXTRACT */ > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 4f4ab2790..716e0debf 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -1177,6 +1177,9 @@ dpif_miniflow_extract_impl_set(struct unixctl_conn > *conn, int argc, > > ovs_mutex_unlock(&dp_netdev_mutex); > > + /* Set the default implementation for PMD threads created in the future. */ > + dpif_miniflow_extract_set_default(*new_func); > + > /* Reply with success to command. */ > struct ds reply = DS_EMPTY_INITIALIZER; > ds_put_format(&reply, "Miniflow implementation set to %s.\n", mfex_name); > @@ -6282,8 +6285,8 @@ dp_netdev_configure_pmd(struct > dp_netdev_pmd_thread *pmd, struct dp_netdev *dp, > /* Initialize DPIF function pointer to the default configured version. */ > pmd->netdev_input_func = dp_netdev_impl_get_default(); > > - /*Init default miniflow_extract function */ > - pmd->miniflow_extract_opt = NULL; > + /* Init default miniflow_extract function */ Minor, missing period at end of comment. BR Ian > + pmd->miniflow_extract_opt = dpif_miniflow_extract_get_default(); > > /* init the 'flow_cache' since there is no > * actual thread created for NON_PMD_CORE_ID. */ > -- > 2.25.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Ian, Thanks again , replies Inline <Snipped> > > which uses CPU SIMD ISA to parse specific traffic profiles efficiently. > > Refer to the documentation for details on how to enable it at > runtime. > The NEWS section in general here seems wrong, seems to have features from > other patches in both this patch series and the DPIF patch series added > which shouldn't be the case. > Only features added by this patch should be added to NEWS. Taken care of in v5 split news with each commit. > > > + * Cache results for CPU ISA checks, reduces overhead on repeated > lookups. > > + * Add command line option to switch between mfex function pointers. > > + * Add miniflow extract auto-validator function to compare different > > + miniflow extract implementations against default implementation. > > + * Add study function to miniflow function table which studies packet > > + and automatically chooses the best miniflow implementation for that > > + traffic. > > + * Add AVX512 based optimized miniflow extract function for traffic > type > > + IP/UDP. > > The line below is the only item that should be added to NEWS in this patch. Fixed in v5. > > > + * Add build time configure command to enable auto-validatior as > default > > + miniflow implementation at build time. > > - ovs-ctl: > > * New option '--no-record-hostname' to disable hostname > configuration > > in ovsdb on startup. > > @@ -35,7 +46,6 @@ Post-v2.15.0 > > * New option '--election-timer' to the 'create-cluster' command to set > the > > leader election timer during cluster creation. > > > > - > > v2.15.0 - 15 Feb 2021 > > --------------------- > > - OVSDB: > > diff --git a/acinclude.m4 b/acinclude.m4 > > index 5fbcd9872..e2704cfda 100644 > > --- a/acinclude.m4 > > +++ b/acinclude.m4 > > @@ -14,6 +14,22 @@ > > # See the License for the specific language governing permissions and > > # limitations under the License. > > > > +dnl Set OVS MFEX Autovalidator as default miniflow extract at compile > time? > > +dnl This enables automatically running all unit tests with all MFEX > > +dnl implementations. > > +AC_DEFUN([OVS_CHECK_MFEX_AUTOVALIDATOR], [ > > + AC_ARG_ENABLE([mfex-default-autovalidator], > > + [AC_HELP_STRING([--enable-mfex-default-autovalidator], > [Enable > > MFEX autovalidator as default miniflow_extract implementation.])], > > + [autovalidator=yes],[autovalidator=no]) > > + AC_MSG_CHECKING([whether MFEX Autovalidator is default > > implementation]) > > + if test "$autovalidator" != yes; then > > + AC_MSG_RESULT([no]) > > + else > > + OVS_CFLAGS="$OVS_CFLAGS -DMFEX_AUTOVALIDATOR_DEFAULT" > > + AC_MSG_RESULT([yes]) > > + fi > > +]) > > + > > dnl Set OVS DPCLS Autovalidator as default subtable search at compile > time? > > dnl This enables automatically running all unit tests with all DPCLS > > dnl implementations. > > diff --git a/configure.ac b/configure.ac > > index e45685a6c..46c402892 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -186,6 +186,7 @@ OVS_ENABLE_SPARSE > > OVS_CTAGS_IDENTIFIERS > > OVS_CHECK_DPCLS_AUTOVALIDATOR > > OVS_CHECK_DPIF_AVX512_DEFAULT > > +OVS_CHECK_MFEX_AUTOVALIDATOR > > OVS_CHECK_BINUTILS_AVX512 > > > > AC_ARG_VAR(KARCH, [Kernel Architecture String]) > > diff --git a/lib/dpif-netdev-private-extract.c b/lib/dpif-netdev-private- > extract.c > > index d86268a1d..2008e5ee5 100644 > > --- a/lib/dpif-netdev-private-extract.c > > +++ b/lib/dpif-netdev-private-extract.c > > @@ -230,3 +230,27 @@ dpif_miniflow_extract_autovalidator(struct > > dp_packet_batch *packets, > > */ > > return 0; > > } > > + > > +/* Variable to hold the defaualt mfex implementation. */ > Typo in default above. > > +static miniflow_extract_func default_mfex_func = NULL; > I'd prefer if above was declared at the top of the file rather than here. > Fixed in v5. > > + > > +void > > +dpif_miniflow_extract_set_default(miniflow_extract_func func) > > +{ > > + default_mfex_func = func; > > +} > > + > > +miniflow_extract_func > > +dpif_miniflow_extract_get_default(void) > > +{ > > + > > +#ifdef MFEX_AUTOVALIDATOR_DEFAULT > > + ovs_assert(mfex_impls[0].extract_func == > > + dpif_miniflow_extract_autovalidator); > > + VLOG_INFO("Default miniflow Extract implementation %s \n", > > + mfex_impls[0].name); > > + return mfex_impls[0].extract_func; > > +#else > > + return default_mfex_func; > > +#endif > > +} > > diff --git a/lib/dpif-netdev-private-extract.h b/lib/dpif-netdev-private- > extract.h > > index 3ada413bb..d8a284db7 100644 > > --- a/lib/dpif-netdev-private-extract.h > > +++ b/lib/dpif-netdev-private-extract.h > > @@ -118,4 +118,14 @@ mfex_study_traffic(struct dp_packet_batch > *packets, > > uint32_t keys_size, odp_port_t in_port, > > void *pmd_handle); > > > > +/* Retrieve the default miniflow extract or auto-validator > > + * based upon build time configuration choosen by the user. */ > Typo above, chosen. Fixed in v5. > > > +miniflow_extract_func > > +dpif_miniflow_extract_get_default(void); > > + > > +/* Returns the default MFEX which is first ./configure selected, but can be > > + * overridden at runtime. */ > > +void > > +dpif_miniflow_extract_set_default(miniflow_extract_func func); > > + > > #endif /* DPIF_NETDEV_AVX512_EXTRACT */ > > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > > index 4f4ab2790..716e0debf 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -1177,6 +1177,9 @@ dpif_miniflow_extract_impl_set(struct > unixctl_conn > > *conn, int argc, > > > > ovs_mutex_unlock(&dp_netdev_mutex); > > > > + /* Set the default implementation for PMD threads created in the > future. */ > > + dpif_miniflow_extract_set_default(*new_func); > > + > > /* Reply with success to command. */ > > struct ds reply = DS_EMPTY_INITIALIZER; > > ds_put_format(&reply, "Miniflow implementation set to %s.\n", > mfex_name); > > @@ -6282,8 +6285,8 @@ dp_netdev_configure_pmd(struct > > dp_netdev_pmd_thread *pmd, struct dp_netdev *dp, > > /* Initialize DPIF function pointer to the default configured version. */ > > pmd->netdev_input_func = dp_netdev_impl_get_default(); > > > > - /*Init default miniflow_extract function */ > > - pmd->miniflow_extract_opt = NULL; > > + /* Init default miniflow_extract function */ > > Minor, missing period at end of comment. Fixed in v5. > > > BR > Ian > > + pmd->miniflow_extract_opt = dpif_miniflow_extract_get_default(); > > > > /* init the 'flow_cache' since there is no > > * actual thread created for NON_PMD_CORE_ID. */ > > -- > > 2.25.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi, I think Ian covered all issues and I suspect this patch might change a bit due to comments on previous patches. fbl On Thu, Jun 17, 2021 at 09:57:47PM +0530, Kumar Amber wrote: > This commit adds a new command to allow the user to enable > autovalidatior by default at build time thus allowing for > runnig unit test by default. > > $ ./configure --enable-mfex-default-autovalidator > > Signed-off-by: Kumar Amber <kumar.amber@intel.com> > Co-authored-by: Harry van Haaren <harry.van.haaren@intel.com> > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> > --- > Documentation/topics/dpdk/bridge.rst | 5 +++++ > NEWS | 12 +++++++++++- > acinclude.m4 | 16 ++++++++++++++++ > configure.ac | 1 + > lib/dpif-netdev-private-extract.c | 24 ++++++++++++++++++++++++ > lib/dpif-netdev-private-extract.h | 10 ++++++++++ > lib/dpif-netdev.c | 7 +++++-- > 7 files changed, 72 insertions(+), 3 deletions(-) > > diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst > index b262b98f8..1c78adc75 100644 > --- a/Documentation/topics/dpdk/bridge.rst > +++ b/Documentation/topics/dpdk/bridge.rst > @@ -307,6 +307,11 @@ To set the Miniflow autovalidator, use this command :: > > $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator > > +A compile time option is available in order to test it with the OVS unit > +test suite. Use the following configure option :: > + > + $ ./configure --enable-mfex-default-autovalidator > + > Unit Test Miniflow Extract > ++++++++++++++++++++++++++ > > diff --git a/NEWS b/NEWS > index 63a485309..ed9f4d4c4 100644 > --- a/NEWS > +++ b/NEWS > @@ -24,6 +24,17 @@ Post-v2.15.0 > * An optimized miniflow extract (mfex) implementation is now available, > which uses CPU SIMD ISA to parse specific traffic profiles efficiently. > Refer to the documentation for details on how to enable it at runtime. > + * Cache results for CPU ISA checks, reduces overhead on repeated lookups. > + * Add command line option to switch between mfex function pointers. > + * Add miniflow extract auto-validator function to compare different > + miniflow extract implementations against default implementation. > + * Add study function to miniflow function table which studies packet > + and automatically chooses the best miniflow implementation for that > + traffic. > + * Add AVX512 based optimized miniflow extract function for traffic type > + IP/UDP. > + * Add build time configure command to enable auto-validatior as default > + miniflow implementation at build time. > - ovs-ctl: > * New option '--no-record-hostname' to disable hostname configuration > in ovsdb on startup. > @@ -35,7 +46,6 @@ Post-v2.15.0 > * New option '--election-timer' to the 'create-cluster' command to set the > leader election timer during cluster creation. > > - > v2.15.0 - 15 Feb 2021 > --------------------- > - OVSDB: > diff --git a/acinclude.m4 b/acinclude.m4 > index 5fbcd9872..e2704cfda 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -14,6 +14,22 @@ > # See the License for the specific language governing permissions and > # limitations under the License. > > +dnl Set OVS MFEX Autovalidator as default miniflow extract at compile time? > +dnl This enables automatically running all unit tests with all MFEX > +dnl implementations. > +AC_DEFUN([OVS_CHECK_MFEX_AUTOVALIDATOR], [ > + AC_ARG_ENABLE([mfex-default-autovalidator], > + [AC_HELP_STRING([--enable-mfex-default-autovalidator], [Enable MFEX autovalidator as default miniflow_extract implementation.])], > + [autovalidator=yes],[autovalidator=no]) > + AC_MSG_CHECKING([whether MFEX Autovalidator is default implementation]) > + if test "$autovalidator" != yes; then > + AC_MSG_RESULT([no]) > + else > + OVS_CFLAGS="$OVS_CFLAGS -DMFEX_AUTOVALIDATOR_DEFAULT" > + AC_MSG_RESULT([yes]) > + fi > +]) > + > dnl Set OVS DPCLS Autovalidator as default subtable search at compile time? > dnl This enables automatically running all unit tests with all DPCLS > dnl implementations. > diff --git a/configure.ac b/configure.ac > index e45685a6c..46c402892 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -186,6 +186,7 @@ OVS_ENABLE_SPARSE > OVS_CTAGS_IDENTIFIERS > OVS_CHECK_DPCLS_AUTOVALIDATOR > OVS_CHECK_DPIF_AVX512_DEFAULT > +OVS_CHECK_MFEX_AUTOVALIDATOR > OVS_CHECK_BINUTILS_AVX512 > > AC_ARG_VAR(KARCH, [Kernel Architecture String]) > diff --git a/lib/dpif-netdev-private-extract.c b/lib/dpif-netdev-private-extract.c > index d86268a1d..2008e5ee5 100644 > --- a/lib/dpif-netdev-private-extract.c > +++ b/lib/dpif-netdev-private-extract.c > @@ -230,3 +230,27 @@ dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets, > */ > return 0; > } > + > +/* Variable to hold the defaualt mfex implementation. */ > +static miniflow_extract_func default_mfex_func = NULL; > + > +void > +dpif_miniflow_extract_set_default(miniflow_extract_func func) > +{ > + default_mfex_func = func; > +} > + > +miniflow_extract_func > +dpif_miniflow_extract_get_default(void) > +{ > + > +#ifdef MFEX_AUTOVALIDATOR_DEFAULT > + ovs_assert(mfex_impls[0].extract_func == > + dpif_miniflow_extract_autovalidator); > + VLOG_INFO("Default miniflow Extract implementation %s \n", > + mfex_impls[0].name); > + return mfex_impls[0].extract_func; > +#else > + return default_mfex_func; > +#endif > +} > diff --git a/lib/dpif-netdev-private-extract.h b/lib/dpif-netdev-private-extract.h > index 3ada413bb..d8a284db7 100644 > --- a/lib/dpif-netdev-private-extract.h > +++ b/lib/dpif-netdev-private-extract.h > @@ -118,4 +118,14 @@ mfex_study_traffic(struct dp_packet_batch *packets, > uint32_t keys_size, odp_port_t in_port, > void *pmd_handle); > > +/* Retrieve the default miniflow extract or auto-validator > + * based upon build time configuration choosen by the user. */ > +miniflow_extract_func > +dpif_miniflow_extract_get_default(void); > + > +/* Returns the default MFEX which is first ./configure selected, but can be > + * overridden at runtime. */ > +void > +dpif_miniflow_extract_set_default(miniflow_extract_func func); > + > #endif /* DPIF_NETDEV_AVX512_EXTRACT */ > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index 4f4ab2790..716e0debf 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -1177,6 +1177,9 @@ dpif_miniflow_extract_impl_set(struct unixctl_conn *conn, int argc, > > ovs_mutex_unlock(&dp_netdev_mutex); > > + /* Set the default implementation for PMD threads created in the future. */ > + dpif_miniflow_extract_set_default(*new_func); > + > /* Reply with success to command. */ > struct ds reply = DS_EMPTY_INITIALIZER; > ds_put_format(&reply, "Miniflow implementation set to %s.\n", mfex_name); > @@ -6282,8 +6285,8 @@ dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd, struct dp_netdev *dp, > /* Initialize DPIF function pointer to the default configured version. */ > pmd->netdev_input_func = dp_netdev_impl_get_default(); > > - /*Init default miniflow_extract function */ > - pmd->miniflow_extract_opt = NULL; > + /* Init default miniflow_extract function */ > + pmd->miniflow_extract_opt = dpif_miniflow_extract_get_default(); > > /* init the 'flow_cache' since there is no > * actual thread created for NON_PMD_CORE_ID. */ > -- > 2.25.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Flavio, All the refactored changes are now in v5 done accordingly to previous patches changes. > -----Original Message----- > From: Flavio Leitner <fbl@sysclose.org> > Sent: Monday, June 28, 2021 8:24 AM > To: Amber, Kumar <kumar.amber@intel.com> > Cc: dev@openvswitch.org; i.maximets@ovn.org > Subject: Re: [ovs-dev] [v4 05/12] dpif-netdev: Add configure to enable > autovalidator at build time. > > > > Hi, > > I think Ian covered all issues and I suspect this patch might change a bit due > to comments on previous patches. > > fbl > > On Thu, Jun 17, 2021 at 09:57:47PM +0530, Kumar Amber wrote: > > This commit adds a new command to allow the user to enable > > autovalidatior by default at build time thus allowing for runnig unit > > test by default. > > > > $ ./configure --enable-mfex-default-autovalidator > > > > Signed-off-by: Kumar Amber <kumar.amber@intel.com> > > Co-authored-by: Harry van Haaren <harry.van.haaren@intel.com> > > Signed-off-by: Harry van Haaren <harry.van.haaren@intel.com> > > --- > > Documentation/topics/dpdk/bridge.rst | 5 +++++ > > NEWS | 12 +++++++++++- > > acinclude.m4 | 16 ++++++++++++++++ > > configure.ac | 1 + > > lib/dpif-netdev-private-extract.c | 24 ++++++++++++++++++++++++ > > lib/dpif-netdev-private-extract.h | 10 ++++++++++ > > lib/dpif-netdev.c | 7 +++++-- > > 7 files changed, 72 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/topics/dpdk/bridge.rst > > b/Documentation/topics/dpdk/bridge.rst > > index b262b98f8..1c78adc75 100644 > > --- a/Documentation/topics/dpdk/bridge.rst > > +++ b/Documentation/topics/dpdk/bridge.rst > > @@ -307,6 +307,11 @@ To set the Miniflow autovalidator, use this > command :: > > > > $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator > > > > +A compile time option is available in order to test it with the OVS > > +unit test suite. Use the following configure option :: > > + > > + $ ./configure --enable-mfex-default-autovalidator > > + > > Unit Test Miniflow Extract > > ++++++++++++++++++++++++++ > > > > diff --git a/NEWS b/NEWS > > index 63a485309..ed9f4d4c4 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -24,6 +24,17 @@ Post-v2.15.0 > > * An optimized miniflow extract (mfex) implementation is now > available, > > which uses CPU SIMD ISA to parse specific traffic profiles efficiently. > > Refer to the documentation for details on how to enable it at > runtime. > > + * Cache results for CPU ISA checks, reduces overhead on repeated > lookups. > > + * Add command line option to switch between mfex function pointers. > > + * Add miniflow extract auto-validator function to compare different > > + miniflow extract implementations against default implementation. > > + * Add study function to miniflow function table which studies packet > > + and automatically chooses the best miniflow implementation for that > > + traffic. > > + * Add AVX512 based optimized miniflow extract function for traffic > type > > + IP/UDP. > > + * Add build time configure command to enable auto-validatior as > default > > + miniflow implementation at build time. > > - ovs-ctl: > > * New option '--no-record-hostname' to disable hostname > configuration > > in ovsdb on startup. > > @@ -35,7 +46,6 @@ Post-v2.15.0 > > * New option '--election-timer' to the 'create-cluster' command to set > the > > leader election timer during cluster creation. > > > > - > > v2.15.0 - 15 Feb 2021 > > --------------------- > > - OVSDB: > > diff --git a/acinclude.m4 b/acinclude.m4 index 5fbcd9872..e2704cfda > > 100644 > > --- a/acinclude.m4 > > +++ b/acinclude.m4 > > @@ -14,6 +14,22 @@ > > # See the License for the specific language governing permissions and > > # limitations under the License. > > > > +dnl Set OVS MFEX Autovalidator as default miniflow extract at compile > time? > > +dnl This enables automatically running all unit tests with all MFEX > > +dnl implementations. > > +AC_DEFUN([OVS_CHECK_MFEX_AUTOVALIDATOR], [ > > + AC_ARG_ENABLE([mfex-default-autovalidator], > > + [AC_HELP_STRING([--enable-mfex-default-autovalidator], > [Enable MFEX autovalidator as default miniflow_extract implementation.])], > > + [autovalidator=yes],[autovalidator=no]) > > + AC_MSG_CHECKING([whether MFEX Autovalidator is default > > +implementation]) > > + if test "$autovalidator" != yes; then > > + AC_MSG_RESULT([no]) > > + else > > + OVS_CFLAGS="$OVS_CFLAGS -DMFEX_AUTOVALIDATOR_DEFAULT" > > + AC_MSG_RESULT([yes]) > > + fi > > +]) > > + > > dnl Set OVS DPCLS Autovalidator as default subtable search at compile > time? > > dnl This enables automatically running all unit tests with all DPCLS > > dnl implementations. > > diff --git a/configure.ac b/configure.ac index e45685a6c..46c402892 > > 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -186,6 +186,7 @@ OVS_ENABLE_SPARSE > > OVS_CTAGS_IDENTIFIERS > > OVS_CHECK_DPCLS_AUTOVALIDATOR > > OVS_CHECK_DPIF_AVX512_DEFAULT > > +OVS_CHECK_MFEX_AUTOVALIDATOR > > OVS_CHECK_BINUTILS_AVX512 > > > > AC_ARG_VAR(KARCH, [Kernel Architecture String]) diff --git > > a/lib/dpif-netdev-private-extract.c > > b/lib/dpif-netdev-private-extract.c > > index d86268a1d..2008e5ee5 100644 > > --- a/lib/dpif-netdev-private-extract.c > > +++ b/lib/dpif-netdev-private-extract.c > > @@ -230,3 +230,27 @@ dpif_miniflow_extract_autovalidator(struct > dp_packet_batch *packets, > > */ > > return 0; > > } > > + > > +/* Variable to hold the defaualt mfex implementation. */ static > > +miniflow_extract_func default_mfex_func = NULL; > > + > > +void > > +dpif_miniflow_extract_set_default(miniflow_extract_func func) { > > + default_mfex_func = func; > > +} > > + > > +miniflow_extract_func > > +dpif_miniflow_extract_get_default(void) > > +{ > > + > > +#ifdef MFEX_AUTOVALIDATOR_DEFAULT > > + ovs_assert(mfex_impls[0].extract_func == > > + dpif_miniflow_extract_autovalidator); > > + VLOG_INFO("Default miniflow Extract implementation %s \n", > > + mfex_impls[0].name); > > + return mfex_impls[0].extract_func; #else > > + return default_mfex_func; > > +#endif > > +} > > diff --git a/lib/dpif-netdev-private-extract.h > > b/lib/dpif-netdev-private-extract.h > > index 3ada413bb..d8a284db7 100644 > > --- a/lib/dpif-netdev-private-extract.h > > +++ b/lib/dpif-netdev-private-extract.h > > @@ -118,4 +118,14 @@ mfex_study_traffic(struct dp_packet_batch > *packets, > > uint32_t keys_size, odp_port_t in_port, > > void *pmd_handle); > > > > +/* Retrieve the default miniflow extract or auto-validator > > + * based upon build time configuration choosen by the user. */ > > +miniflow_extract_func dpif_miniflow_extract_get_default(void); > > + > > +/* Returns the default MFEX which is first ./configure selected, but > > +can be > > + * overridden at runtime. */ > > +void > > +dpif_miniflow_extract_set_default(miniflow_extract_func func); > > + > > #endif /* DPIF_NETDEV_AVX512_EXTRACT */ diff --git > > a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 4f4ab2790..716e0debf > > 100644 > > --- a/lib/dpif-netdev.c > > +++ b/lib/dpif-netdev.c > > @@ -1177,6 +1177,9 @@ dpif_miniflow_extract_impl_set(struct > > unixctl_conn *conn, int argc, > > > > ovs_mutex_unlock(&dp_netdev_mutex); > > > > + /* Set the default implementation for PMD threads created in the > future. */ > > + dpif_miniflow_extract_set_default(*new_func); > > + > > /* Reply with success to command. */ > > struct ds reply = DS_EMPTY_INITIALIZER; > > ds_put_format(&reply, "Miniflow implementation set to %s.\n", > > mfex_name); @@ -6282,8 +6285,8 @@ dp_netdev_configure_pmd(struct > dp_netdev_pmd_thread *pmd, struct dp_netdev *dp, > > /* Initialize DPIF function pointer to the default configured version. */ > > pmd->netdev_input_func = dp_netdev_impl_get_default(); > > > > - /*Init default miniflow_extract function */ > > - pmd->miniflow_extract_opt = NULL; > > + /* Init default miniflow_extract function */ > > + pmd->miniflow_extract_opt = dpif_miniflow_extract_get_default(); > > > > /* init the 'flow_cache' since there is no > > * actual thread created for NON_PMD_CORE_ID. */ > > -- > > 2.25.1 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > -- > fbl
diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst index b262b98f8..1c78adc75 100644 --- a/Documentation/topics/dpdk/bridge.rst +++ b/Documentation/topics/dpdk/bridge.rst @@ -307,6 +307,11 @@ To set the Miniflow autovalidator, use this command :: $ ovs-appctl dpif-netdev/miniflow-parser-set autovalidator +A compile time option is available in order to test it with the OVS unit +test suite. Use the following configure option :: + + $ ./configure --enable-mfex-default-autovalidator + Unit Test Miniflow Extract ++++++++++++++++++++++++++ diff --git a/NEWS b/NEWS index 63a485309..ed9f4d4c4 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,17 @@ Post-v2.15.0 * An optimized miniflow extract (mfex) implementation is now available, which uses CPU SIMD ISA to parse specific traffic profiles efficiently. Refer to the documentation for details on how to enable it at runtime. + * Cache results for CPU ISA checks, reduces overhead on repeated lookups. + * Add command line option to switch between mfex function pointers. + * Add miniflow extract auto-validator function to compare different + miniflow extract implementations against default implementation. + * Add study function to miniflow function table which studies packet + and automatically chooses the best miniflow implementation for that + traffic. + * Add AVX512 based optimized miniflow extract function for traffic type + IP/UDP. + * Add build time configure command to enable auto-validatior as default + miniflow implementation at build time. - ovs-ctl: * New option '--no-record-hostname' to disable hostname configuration in ovsdb on startup. @@ -35,7 +46,6 @@ Post-v2.15.0 * New option '--election-timer' to the 'create-cluster' command to set the leader election timer during cluster creation. - v2.15.0 - 15 Feb 2021 --------------------- - OVSDB: diff --git a/acinclude.m4 b/acinclude.m4 index 5fbcd9872..e2704cfda 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -14,6 +14,22 @@ # See the License for the specific language governing permissions and # limitations under the License. +dnl Set OVS MFEX Autovalidator as default miniflow extract at compile time? +dnl This enables automatically running all unit tests with all MFEX +dnl implementations. +AC_DEFUN([OVS_CHECK_MFEX_AUTOVALIDATOR], [ + AC_ARG_ENABLE([mfex-default-autovalidator], + [AC_HELP_STRING([--enable-mfex-default-autovalidator], [Enable MFEX autovalidator as default miniflow_extract implementation.])], + [autovalidator=yes],[autovalidator=no]) + AC_MSG_CHECKING([whether MFEX Autovalidator is default implementation]) + if test "$autovalidator" != yes; then + AC_MSG_RESULT([no]) + else + OVS_CFLAGS="$OVS_CFLAGS -DMFEX_AUTOVALIDATOR_DEFAULT" + AC_MSG_RESULT([yes]) + fi +]) + dnl Set OVS DPCLS Autovalidator as default subtable search at compile time? dnl This enables automatically running all unit tests with all DPCLS dnl implementations. diff --git a/configure.ac b/configure.ac index e45685a6c..46c402892 100644 --- a/configure.ac +++ b/configure.ac @@ -186,6 +186,7 @@ OVS_ENABLE_SPARSE OVS_CTAGS_IDENTIFIERS OVS_CHECK_DPCLS_AUTOVALIDATOR OVS_CHECK_DPIF_AVX512_DEFAULT +OVS_CHECK_MFEX_AUTOVALIDATOR OVS_CHECK_BINUTILS_AVX512 AC_ARG_VAR(KARCH, [Kernel Architecture String]) diff --git a/lib/dpif-netdev-private-extract.c b/lib/dpif-netdev-private-extract.c index d86268a1d..2008e5ee5 100644 --- a/lib/dpif-netdev-private-extract.c +++ b/lib/dpif-netdev-private-extract.c @@ -230,3 +230,27 @@ dpif_miniflow_extract_autovalidator(struct dp_packet_batch *packets, */ return 0; } + +/* Variable to hold the defaualt mfex implementation. */ +static miniflow_extract_func default_mfex_func = NULL; + +void +dpif_miniflow_extract_set_default(miniflow_extract_func func) +{ + default_mfex_func = func; +} + +miniflow_extract_func +dpif_miniflow_extract_get_default(void) +{ + +#ifdef MFEX_AUTOVALIDATOR_DEFAULT + ovs_assert(mfex_impls[0].extract_func == + dpif_miniflow_extract_autovalidator); + VLOG_INFO("Default miniflow Extract implementation %s \n", + mfex_impls[0].name); + return mfex_impls[0].extract_func; +#else + return default_mfex_func; +#endif +} diff --git a/lib/dpif-netdev-private-extract.h b/lib/dpif-netdev-private-extract.h index 3ada413bb..d8a284db7 100644 --- a/lib/dpif-netdev-private-extract.h +++ b/lib/dpif-netdev-private-extract.h @@ -118,4 +118,14 @@ mfex_study_traffic(struct dp_packet_batch *packets, uint32_t keys_size, odp_port_t in_port, void *pmd_handle); +/* Retrieve the default miniflow extract or auto-validator + * based upon build time configuration choosen by the user. */ +miniflow_extract_func +dpif_miniflow_extract_get_default(void); + +/* Returns the default MFEX which is first ./configure selected, but can be + * overridden at runtime. */ +void +dpif_miniflow_extract_set_default(miniflow_extract_func func); + #endif /* DPIF_NETDEV_AVX512_EXTRACT */ diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 4f4ab2790..716e0debf 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -1177,6 +1177,9 @@ dpif_miniflow_extract_impl_set(struct unixctl_conn *conn, int argc, ovs_mutex_unlock(&dp_netdev_mutex); + /* Set the default implementation for PMD threads created in the future. */ + dpif_miniflow_extract_set_default(*new_func); + /* Reply with success to command. */ struct ds reply = DS_EMPTY_INITIALIZER; ds_put_format(&reply, "Miniflow implementation set to %s.\n", mfex_name); @@ -6282,8 +6285,8 @@ dp_netdev_configure_pmd(struct dp_netdev_pmd_thread *pmd, struct dp_netdev *dp, /* Initialize DPIF function pointer to the default configured version. */ pmd->netdev_input_func = dp_netdev_impl_get_default(); - /*Init default miniflow_extract function */ - pmd->miniflow_extract_opt = NULL; + /* Init default miniflow_extract function */ + pmd->miniflow_extract_opt = dpif_miniflow_extract_get_default(); /* init the 'flow_cache' since there is no * actual thread created for NON_PMD_CORE_ID. */