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

Stokes, Ian ian.stokes at intel.com
Thu Jun 24 17:18:51 UTC 2021


> 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 at intel.com>
> Co-authored-by: Harry van Haaren <harry.van.haaren at intel.com>
> Signed-off-by: Harry van Haaren <harry.van.haaren at 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 at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list