Re: [ovs-dev] [v8 05/12] dpif-netdev: Add configure to enable autovalidator at build time.

2021-07-11 Thread Amber, Kumar
Hi Flavio,


> -Original Message-
> From: Flavio Leitner 
> Sent: Sunday, July 11, 2021 6:14 AM
> To: Amber, Kumar 
> Cc: ovs-dev@openvswitch.org; i.maxim...@ovn.org
> Subject: Re: [ovs-dev] [v8 05/12] dpif-netdev: Add configure to enable
> autovalidator at build time.
> 
> On Fri, Jul 09, 2021 at 05:35:55PM +0530, kumar Amber wrote:
> > From: Kumar Amber 
> >
> > 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 
> > Co-authored-by: Harry van Haaren 
> > Signed-off-by: Harry van Haaren 
> >
> > ---
> > v7:
> > - fix review commens(Eelco, Flavio)
> > v5:
> > - fix review comments(Ian, Flavio, Eelco)
> > ---
> > ---
> >  Documentation/topics/dpdk/bridge.rst |  5 +
> >  NEWS |  5 +++--
> >  acinclude.m4 | 16 
> >  configure.ac |  1 +
> >  lib/dpif-netdev-private-extract.c|  9 -
> >  5 files changed, 33 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/topics/dpdk/bridge.rst
> > b/Documentation/topics/dpdk/bridge.rst
> > index 7c618cf1f..4db416ddd 100644
> > --- a/Documentation/topics/dpdk/bridge.rst
> > +++ b/Documentation/topics/dpdk/bridge.rst
> > @@ -307,3 +307,8 @@ implementations provide the same results.
> >  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
> > diff --git a/NEWS b/NEWS
> > index 95bf386e3..3addb8616 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -31,9 +31,11 @@ Post-v2.15.0
> >   * 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
> > + * Add study function to miniflow function table which studies
> > + packet
> 
> I guess I commented out in the patch introducing this.
> Please fix there instead.
>

Fixed there.
 
> > and automatically chooses the best miniflow implementation for that
> > traffic.
> > + * 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.
> > @@ -50,7 +52,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 343303447..5a48f0335
> > 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/c

Re: [ovs-dev] [v8 05/12] dpif-netdev: Add configure to enable autovalidator at build time.

2021-07-10 Thread Flavio Leitner
On Fri, Jul 09, 2021 at 05:35:55PM +0530, kumar Amber wrote:
> From: Kumar Amber 
> 
> 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 
> Co-authored-by: Harry van Haaren 
> Signed-off-by: Harry van Haaren 
> 
> ---
> v7:
> - fix review commens(Eelco, Flavio)
> v5:
> - fix review comments(Ian, Flavio, Eelco)
> ---
> ---
>  Documentation/topics/dpdk/bridge.rst |  5 +
>  NEWS |  5 +++--
>  acinclude.m4 | 16 
>  configure.ac |  1 +
>  lib/dpif-netdev-private-extract.c|  9 -
>  5 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/bridge.rst 
> b/Documentation/topics/dpdk/bridge.rst
> index 7c618cf1f..4db416ddd 100644
> --- a/Documentation/topics/dpdk/bridge.rst
> +++ b/Documentation/topics/dpdk/bridge.rst
> @@ -307,3 +307,8 @@ implementations provide the same results.
>  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
> diff --git a/NEWS b/NEWS
> index 95bf386e3..3addb8616 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -31,9 +31,11 @@ Post-v2.15.0
>   * 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
> + * Add study function to miniflow function table which studies packet

I guess I commented out in the patch introducing this.
Please fix there instead.

> and automatically chooses the best miniflow implementation for that
> traffic.
> + * 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.
> @@ -50,7 +52,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 343303447..5a48f0335 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 bb7c98f31..be8c69408 100644
> --- a/lib/dpif-netdev-private-extract.c
> +++ b/lib/dpif-netdev-private-extract.c
> @@ -80,13 +80,20 @@ dp_mfex_impl_get_default(void)
>  /* For the first call, this will be NULL. Compute the compile time 
> default.
>   */
>  if (OVS_UNLIKELY(!default_mfex_func_set)) {
> +
> +#ifdef MFEX_AUTOVALIDATOR_DEFAULT
> +VLOG_INFO("Default miniflow Extract implementation %s",
> +  mfex_impls[MFEX_IMPL_AUTOVALIDATOR].name);
> +atomic_store_relaxed(mfex_func, (uintptr_t) mfex_impls
> + [MFEX_IMPL_AUTOVALIDATOR].extract_func);
> +#else
>  VLOG_INFO("Default MFEX implementation is %s.\n",
>mfex_impls[MFEX_IM