> Building DPDK using Make is removed since DPDK 20.08.
> Hence, remove its references in OVS as well.
> While at it, address few comments on the commit [1].
> 

Thanks for the patch, just a few comments below.

@Ilya, I think Sunil has addressed most of the concerns you flagged, however 
I'd like your opinion on the python script?

I'd like to have this patch merged to dpdk-latest before creating our RFC patch 
to move OVS master to use 20.11 so it would be nice to have this 
reviewed/reworked so as to include the changes.

> [1] 540e70fba6d5 ("build: Add support for DPDK meson build")
Probably going forward it's better to use the Fixes tag, so

Fixes: 540e70fba6d5 ("build: Add support for DPDK meson build")
> 
> Tested-at: https://travis-ci.org/github/Sunil-Pai-G/ovs/builds/742699277
> Signed-off-by: Sunil Pai G <sunil.pa...@intel.com>
> ---

A summary of the changes would be good hear, they will be stripped off when be 
applied.
Although in the case it is a new patch rather than part of a revision so I 
understand why
You ma not have put it here.

>  .travis/linux-build.sh                |  4 +-
>  Documentation/intro/install/afxdp.rst |  2 +-
>  Documentation/intro/install/dpdk.rst  | 73 ++++++++-----------------
>  acinclude.m4                          | 78 ++++++++-------------------
>  python/automake.mk                    |  1 +
>  python/build/pkgcfg.py                | 24 +++++++--
>  6 files changed, 65 insertions(+), 117 deletions(-)
> 
> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh
> index 917bbf962..e085780e9 100755
> --- a/.travis/linux-build.sh
> +++ b/.travis/linux-build.sh
> @@ -145,12 +145,12 @@ function install_dpdk()
> 
>      CC=gcc meson $DPDK_OPTS build
>      ninja -C build
> -    sudo ninja -C build install
> +    ninja -C build install
> 
>      # Update the library paths.
>      sudo ldconfig
> 
> -    echo "Installed DPDK source"
> +    echo "Installed DPDK source in $(pwd)"
>      popd
>      echo "${DPDK_VER}" > ${VERSION_FILE}
>  }
> diff --git a/Documentation/intro/install/afxdp.rst
> b/Documentation/intro/install/afxdp.rst
> index 327f2b3df..aad0aeebe 100644
> --- a/Documentation/intro/install/afxdp.rst
> +++ b/Documentation/intro/install/afxdp.rst
> @@ -396,7 +396,7 @@ PVP using vhostuser device
>  --------------------------
>  First, build OVS with DPDK and AFXDP::
> 
> -  ./configure  --enable-afxdp --with-dpdk=shared|static|<dpdk path>
> +  ./configure  --enable-afxdp --with-dpdk=shared|static
>    make -j4 && make install
> 
>  Create a vhost-user port from OVS::
> diff --git a/Documentation/intro/install/dpdk.rst
> b/Documentation/intro/install/dpdk.rst
> index 7a1852bc5..ecc4c7931 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -82,60 +82,39 @@ Install DPDK
> 
>     Meson is the preferred tool to build recent DPDK releases
>     as Make support is deprecated and will be removed from DPDK 20.11.
> -   OVS supports DPDK Meson builds from DPDK 19.11 onwards.
> 
>     Build and install the DPDK library::
> 
> -       $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
> -       $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
> -       $ meson $DPDK_TARGET
> -       $ ninja -C $DPDK_TARGET
> -       $ sudo ninja -C $DPDK_TARGET install
> +       $ export DPDK_BUILD=$DPDK_DIR/build
> +       $ meson build
> +       $ ninja -C build
> +       $ sudo ninja -C build install
>         $ sudo ldconfig
> 
>     Detailed information can be found at `DPDK documentation`_.
> 
> -#. (Optional) Configure DPDK as a shared library
> +#. (Optional) Configure and export the DPDK shared library location
> 
> -   When using Meson, DPDK is built both as static and shared library.
> -   So no extra configuration is required in this case.
> +   Since DPDK is built both as static and shared library by default, no extra
> +   configuration is required for the build.
> 
> -   In case of Make, DPDK can be built as either a static library or a shared
> -   library.  By default, it is configured for the former. If you wish to use
> -   the latter, set
> -   ``CONFIG_RTE_BUILD_SHARED_LIB=y`` in
> ``$DPDK_DIR/config/common_base``.
> +   Exporting the path to library is not necessary if the DPDK libraries are
> +   system installed. For libraries installed using a prefix
> +   (assuming $DPDK_INSTALL in the below case), export the path to this
> +   library and also update the $PKG_CONFIG_PATH for use before building OVS::
> +
> +      $ export $DPDK_LIB=$DPDK_INSTALL/lib/x86_64-linux-gnu
> +      $ export LD_LIBRARY_PATH=$DPDK_LIB/:$LD_LIBRARY_PATH
> +      $ export PKG_CONFIG_PATH=$DPDK_BUILD/lib/x86_64-linux-
> gnu/pkgconfig/
> 
>     .. note::
> 
>        Minor performance loss is expected when using OVS with a shared DPDK
>        library compared to a static DPDK library.
> 
> -#. Configure and install DPDK using Make
> -
> -   Build and install the DPDK library::
> -
> -       $ export DPDK_TARGET=x86_64-native-linuxapp-gcc
> -       $ export DPDK_BUILD=$DPDK_DIR/$DPDK_TARGET
> -       $ make install T=$DPDK_TARGET DESTDIR=install
> -
> -#. (Optional) Export the DPDK shared library location
> -
> -   If DPDK was built as a shared library using Make, export the path to this
> -   library for use when building OVS::
> -
> -       $ export LD_LIBRARY_PATH=$DPDK_DIR/x86_64-native-linuxapp-gcc/lib
> -
> -   In case of Meson, exporting the path to library is not necessary if
> -   the DPDK libraries are system installed. For libraries installed using
> -   a prefix(assuming $DPDK_INSTALL in the below case), export the path to 
> this
> -   library and also update the $PKG_CONFIG_PATH for use before building OVS::
> -
> -      $ export $DPDK_LIB=$DPDK_INSTALL/lib/x86_64-linux-gnu
> -      $ export LD_LIBRARY_PATH=$DPDK_LIB/:$LD_LIBRARY_PATH
> -      $ export PKG_CONFIG_PATH=$DPDK_LIB/pkgconfig/:$PKG_CONFIG_PATH
> -
>  .. _DPDK sources: http://dpdk.org/rel
> -.. _DPDK documentation:
> https://doc.dpdk.org/guides/linux_gsg/build_dpdk.html
> +.. _DPDK documentation:
> +   https://doc.dpdk.org/guides-20.08/linux_gsg/build_dpdk.html

This references the DPDK 20.08 documentation, I think it should be 20.11?

> 
>  Install OVS
>  ~~~~~~~~~~~
> @@ -156,24 +135,14 @@ has to be configured to build against the DPDK library
> (``--with-dpdk``).
> 
>  #. Configure the package using the ``--with-dpdk`` flag:
> 
> -   Depending on the tool used to build DPDK and the type of
> -   DPDK library to use, one can configure OVS as follows:
> -
> -   When DPDK is built using Meson, and OVS must consume DPDK shared
> libraries
> -   (also equivalent to leaving --with-dpdk option empty)::
> -
> -       $ ./configure --with-dpdk=shared
> -
> -   When DPDK is built using Meson, and OVS must consume DPDK static
> libraries::
> +   If OVS must consume DPDK static libraries::
> +   (also equivalent to --with-dpdk=yes )::
> 
>         $ ./configure --with-dpdk=static
> 
> -   When DPDK is built using Make(for shared or static)::
> +   If OVS must consume DPDK shared libraries::
> 
> -       $ ./configure --with-dpdk=$DPDK_BUILD
> -
> -   where ``DPDK_BUILD`` is the path to the built DPDK library. This can be
> -   skipped if DPDK library is installed in its default location.
> +       $ ./configure --with-dpdk=shared
> 
>     .. note::
>       While ``--with-dpdk`` is required, you can pass any other configuration
> diff --git a/acinclude.m4 b/acinclude.m4
> index 061afda4e..3479c5010 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -334,10 +334,9 @@ dnl
>  dnl Configure DPDK source tree
>  AC_DEFUN([OVS_CHECK_DPDK], [
>    AC_ARG_WITH([dpdk],
> -              [AC_HELP_STRING([--with-dpdk=static|shared|/path/to/dpdk],
> +              [AC_HELP_STRING([--with-dpdk=static|shared|yes],
>                                [Specify "static" or "shared" depending on the
> -                              DPDK libraries to use only if built using Meson
> -                              OR the DPDK build directory in case of Make])],
> +                              DPDK libraries to use])],
>                [have_dpdk=true])
> 
>    AC_MSG_CHECKING([whether dpdk is enabled])
> @@ -347,46 +346,25 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>    else
>      AC_MSG_RESULT([yes])
>      case "$with_dpdk" in
> -      "shared" | "static" | "yes")
> -        DPDK_AUTO_DISCOVER="true"
> -        case "$with_dpdk" in
> -          "shared" | "yes")
> -             PKG_CHECK_MODULES([DPDK], [libdpdk], [
> -                 DPDK_INCLUDE="$DPDK_CFLAGS"
> -                 DPDK_LIB="$DPDK_LIBS"], [
> -                 DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
> -                 DPDK_LIB="-ldpdk"])
> -                 ;;
> -           "static")
> -             PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk], [
> -                 DPDK_INCLUDE="$DPDK_CFLAGS"
> -                 DPDK_LIB="$DPDK_LIBS"], [
> -                 DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
> -                 DPDK_LIB="-ldpdk"])
> -                ;;
> -        esac
> -        ;;
> -      *)
> -        DPDK_AUTO_DISCOVER="false"
> -        DPDK_INCLUDE_PATH="$with_dpdk/include"
> -        # If 'with_dpdk' is passed install directory, point to headers
> -        # installed in $DESTDIR/$prefix/include/dpdk
> -        if test -e "$DPDK_INCLUDE_PATH/rte_config.h"; then
> -           DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH"
> -        elif test -e "$DPDK_INCLUDE_PATH/dpdk/rte_config.h"; then
> -           DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH/dpdk"
> -        fi
> -        DPDK_LIB_DIR="$with_dpdk/lib"
> -        DPDK_LIB="-ldpdk"
> -        ;;
> +      "shared")
> +          PKG_CHECK_MODULES([DPDK], [libdpdk], [
> +              DPDK_INCLUDE="$DPDK_CFLAGS"
> +              DPDK_LIB="$DPDK_LIBS"], [
> +              DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
> +              DPDK_LIB="-ldpdk"])
> +              ;;
> +      "static" | "yes")
> +          PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk], [
> +              DPDK_INCLUDE="$DPDK_CFLAGS"
> +              DPDK_LIB="$DPDK_LIBS"], [
> +              DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"
> +              DPDK_LIB="-ldpdk"])
> +              ;;
>      esac
> 
>      ovs_save_CFLAGS="$CFLAGS"
>      ovs_save_LDFLAGS="$LDFLAGS"
>      CFLAGS="$CFLAGS $DPDK_INCLUDE"
> -    if test "$DPDK_AUTO_DISCOVER" = "false"; then
> -      LDFLAGS="$LDFLAGS -L${DPDK_LIB_DIR}"
> -    fi
> 
>      AC_CHECK_HEADERS([rte_config.h], [], [
>        AC_MSG_ERROR([unable to find rte_config.h in $with_dpdk])
> @@ -435,21 +413,14 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>        [AC_MSG_RESULT([yes])
>         DPDKLIB_FOUND=true],
>        [AC_MSG_RESULT([no])
> -       if test "$DPDK_AUTO_DISCOVER" = "true"; then
> -         AC_MSG_ERROR(m4_normalize([
> -            Could not find DPDK library in default search path, update
> -            PKG_CONFIG_PATH for pkg-config to find the .pc file in
> -            non-standard location]))
> -       else
> -         AC_MSG_ERROR([Could not find DPDK libraries in $DPDK_LIB_DIR])
> -       fi
> +       AC_MSG_ERROR(m4_normalize([
> +          Could not find DPDK library in default search path, update
> +          PKG_CONFIG_PATH for pkg-config to find the .pc file in
> +          non-standard location]))
>        ])
> 
>      CFLAGS="$ovs_save_CFLAGS"
>      LDFLAGS="$ovs_save_LDFLAGS"
> -    if test "$DPDK_AUTO_DISCOVER" = "false"; then
> -      OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR"
> -    fi
>      OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE"
>      OVS_ENABLE_OPTION([-mssse3])
> 
> @@ -462,14 +433,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>      # These options are specified inside a single -Wl directive to prevent
>      # autotools from reordering them.
>      #
> -    # OTOH newer versions of dpdk pkg-config (generated with Meson)
> -    # will already have flagged just the right set of libs with
> -    # --whole-archive - in those cases do not wrap it once more.
> -    if [[ "$pkg_failed" != "no" ]];then
> -      DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no-whole-
> archive
> -    else
> -      DPDK_vswitchd_LDFLAGS=`python3 ${srcdir}/python/build/pkgcfg.py
> $DPDK_LIB`
> -    fi
> +    DPDK_vswitchd_LDFLAGS=`python3 ${srcdir}/python/build/pkgcfg.py
> $DPDK_LIB`
> 
>      AC_SUBST([DPDK_vswitchd_LDFLAGS])
>      AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.])
> diff --git a/python/automake.mk b/python/automake.mk
> index 69d9800f9..f805e1a17 100644
> --- a/python/automake.mk
> +++ b/python/automake.mk
> @@ -68,6 +68,7 @@ FLAKE8_PYFILES += \
>       python/setup.py \
>       python/build/__init__.py \
>       python/build/nroff.py \
> +     python/build/pkgcfg.py \
>       python/ovs/dirs.py.template
> 
>  nobase_pkgdata_DATA = $(ovs_pyfiles) $(ovstest_pyfiles)
> diff --git a/python/build/pkgcfg.py b/python/build/pkgcfg.py
> index 7cee3cb03..8401df4d2 100644
> --- a/python/build/pkgcfg.py
> +++ b/python/build/pkgcfg.py
> @@ -1,3 +1,5 @@
> +#!/usr/bin/env python3
> +
>  # Copyright (c) 2020 Intel, Inc.
>  #
>  # Licensed under the Apache License, Version 2.0 (the "License")
> @@ -12,19 +14,31 @@
>  # See the License for the specific language governing permissions and
>  # Limitations under the License.
> 
> -# The purpose of this script is to parse the libraries
> -# From pkg-config in case of DPDK Meson builds.
> +# This script wraps the DPDK libraries inside a single -Wl directive
> +# after comma separation to prevent autotools from reordering them.
> +# Ex: -L/usr/local/lib/x86_64-linux-gnu
> +#     -Wl,--whole-archive <libs>
> +#     -Wl,--no-whole-archive <libs>
> +#
> +#     is changed to :
> +#
> +#     -L/usr/local/lib/x86_64-linux-gnu
> +#     -Wl,--whole-archive,<comma separated libs>
> +#     --no-whole-archive,<comma separated libs>
> 

I don't have an issue with above, I think the comment explains the purpose, 
however Ilya might have a different approach without the script?

Regards
Ian


>  import sys
> +
> +
>  def parse_pkg_cfg_libs(arg):
>      linker_prefix = "-Wl,"
>      # Libtool expects libraries to be comma separated
>      # And -Wl must appear only once.
> -    final_string = ','.join(map(str.strip,arg[1:])).replace('-Wl,','')
> -    final_string = arg[0]+" "+linker_prefix+final_string
> +    final_string = ','.join(map(str.strip, arg[1:])).replace('-Wl,', '')
> +    final_string = arg[0] + " " + linker_prefix + final_string
>      # Ld only understands -lpthread.
> -    final_string = final_string.replace('-pthread','-lpthread')
> +    final_string = final_string.replace('-pthread', '-lpthread')
>      return final_string
> 
> +
>  if __name__ == "__main__":
>      print(parse_pkg_cfg_libs(sys.argv[1:]))
> --
> 2.17.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to