diff mbox series

[ovs-dev,dpdk-latest,v1] build: Remove DPDK make build references.

Message ID 20201111113401.66709-1-sunil.pai.g@intel.com
State Superseded
Headers show
Series [ovs-dev,dpdk-latest,v1] build: Remove DPDK make build references. | expand

Commit Message

Pai G, Sunil Nov. 11, 2020, 11:34 a.m. UTC
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].

[1] 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.pai.g@intel.com>
---
 .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(-)

Comments

Stokes, Ian Nov. 17, 2020, 4:38 p.m. UTC | #1
> 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.pai.g@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
Ilya Maximets Nov. 18, 2020, 4:56 p.m. UTC | #2
On 11/17/20 5:38 PM, Stokes, Ian wrote:
>> 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?

Most of the commants are covered, but I didn't check if this patch
cleans everything that needs to be cleaned.  Only checked what is
in the patch.  Few comments inline.

> 
> 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.pai.g@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.

It's already removed, IIUC.  We shouldn't talk about it in the future tense.
Also it's not "preferred", but the only tool.

>> -   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/

What is $DPDK_BUILD?  Should be $DPDK_INSTALL?

>>
>>     .. 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?

I guess 20.11 docs are not yet avaialble, but, yes, 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::

There should be no '::'.

>> +   (also equivalent to --with-dpdk=yes )::

It should be quoted, i.e. ``--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

We should probably use $PYTHON3 variable here, as we're actualy don not
know if 'python3' is avaialble, or what version of python user specified.

>> $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?

Script is fine, but we likely want it in build-aux directory
instead of python/build as it's not usable for anything else
beside this particular case.

On the other hand, this looks like a very basic string processing
that could be easily done with tr and sed wihout involving
python related complications.  Maybe something lke this:

    # Ld only understands -lpthread.
    DPDK_LIB=$(echo "$DPDK_LIB" | sed 's/-pthread/-lpthread/')

    pattern='\(.*\)-Wl,--whole-archive \(.*\) -Wl,--no-whole-archive\(.*\)'
    whole_archive=$(echo "$DPDK_LIB" | sed "s/$pattern/\2/" | tr ' ' ',')
    DPDK_vswitchd_LDFLAGS=$(echo "$DPDK_LIB" | \
        sed "s/$pattern/\1 -Wl,--whole-archive,$whole_archive,--no-whole-archive \3/")

?

> 
> 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
>
Pai G, Sunil Nov. 18, 2020, 7:06 p.m. UTC | #3
Hi Ilya , Ian

Thank you for the comments , response inline.

> -----Original Message-----
> From: Ilya Maximets <i.maximets@ovn.org>
> Sent: Wednesday, November 18, 2020 10:26 PM
> To: Stokes, Ian <ian.stokes@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>;
> dev@openvswitch.org
> Cc: i.maximets@ovn.org; david.marchand@redhat.com;
> christian.ehrhardt@canonical.com; ktraynor@redhat.com
> Subject: Re: [PATCH dpdk-latest v1] build: Remove DPDK make build
> references.
> 
> On 11/17/20 5:38 PM, Stokes, Ian wrote:
> >> 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?
> 
> Most of the commants are covered, but I didn't check if this patch cleans
> everything that needs to be cleaned.  Only checked what is in the patch.  Few
> comments inline.
> 
> >
> > 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")
Sure will address this.

> >>
> >> Tested-at:
> >> https://travis-ci.org/github/Sunil-Pai-G/ovs/builds/742699277
> >> Signed-off-by: Sunil Pai G <sunil.pai.g@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.
Yes, will add for v2

> >
> >>  .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.
> 
> It's already removed, IIUC.  We shouldn't talk about it in the future tense.
> Also it's not "preferred", but the only tool.
Sure , will address this.

> 
> >> -   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/
> 
> What is $DPDK_BUILD?  Should be $DPDK_INSTALL?
Yes , will change this.

> 
> >>
> >>     .. 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?
> 
> I guess 20.11 docs are not yet avaialble, but, yes, it should be 20.11.
>
Yes , I couldn’t find 20.11 docs. Hence I put the link to 20.08.
 
> >
> >>
> >>  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::
> 
> There should be no '::'.
sure

> 
> >> +   (also equivalent to --with-dpdk=yes )::
> 
> It should be quoted, i.e. ``--with-dpdk=yes``.

Sure.
> 
> >>
> >>         $ ./configure --with-dpdk=static
> >>
> >> -   When DPDK is built using Make(for shared or static)::
> >> +   If OVS must consume DPDK shared libraries::

<snipped for brevity>

> >> @@ -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
> 
> We should probably use $PYTHON3 variable here, as we're actualy don not
> know if 'python3' is avaialble, or what version of python user specified.
> 

sure

> >> $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?
> 
> Script is fine, but we likely want it in build-aux directory instead of
> python/build as it's not usable for anything else beside this particular case.
> 
> On the other hand, this looks like a very basic string processing that could be
> easily done with tr and sed wihout involving python related complications.
> Maybe something lke this:
> 
>     # Ld only understands -lpthread.
>     DPDK_LIB=$(echo "$DPDK_LIB" | sed 's/-pthread/-lpthread/')
> 
>     pattern='\(.*\)-Wl,--whole-archive \(.*\) -Wl,--no-whole-archive\(.*\)'
>     whole_archive=$(echo "$DPDK_LIB" | sed "s/$pattern/\2/" | tr ' ' ',')
>     DPDK_vswitchd_LDFLAGS=$(echo "$DPDK_LIB" | \
>         sed "s/$pattern/\1 -Wl,--whole-archive,$whole_archive,--no-whole-
> archive \3/")
> 
> ?
I was comfortable with python as compared to sed and tr. Hence I went with it :)
Let me try this out as well.

> 
> >
> > 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
> >
Thanks and regards,
Sunil
diff mbox series

Patch

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
 
 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>
 
 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:]))