[ovs-dev] [PATCH RFC dpdk-latest] build: Add support for DPDK meson build.

Pai G, Sunil sunil.pai.g at intel.com
Thu Jul 2 15:25:02 UTC 2020


> -----Original Message-----
> From: Ilya Maximets <i.maximets at ovn.org>
> Sent: Thursday, July 2, 2020 7:26 PM
> To: Pai G, Sunil <sunil.pai.g at intel.com>; dev at openvswitch.org
> Cc: Stokes, Ian <ian.stokes at intel.com>; i.maximets at ovn.org;
> david.marchand at redhat.com; Richardson, Bruce
> <bruce.richardson at intel.com>; Tummala, Sivaprasad
> <sivaprasad.tummala at intel.com>; i.maximets at ovn.org
> Subject: Re: [PATCH RFC dpdk-latest] build: Add support for DPDK meson build.
> 
> On 7/2/20 3:13 PM, Sunil Pai G wrote:
> > Make based build is deprecated in DPDK. Meson based build to be used
> > for future DPDK releases.
> >
> > This updates travis, configure script and documentation for using DPDK
> > Meson with OVS.
> >
> > Signed-off-by: Sunil Pai G <sunil.pai.g at intel.com>
> 
> Thanks for working on this!
> Not a full review, just a few quick bits.
> 
> At first, why dpdk-latest?  Is there issue with meson build on 19.11?

The linker always picked the shared DPDK libraries over static when built with Meson
in DPDK-19.11. -Bstatic flag would get jumbled by libtool causing this.
Thanks to Bruce, there was recently merged series which fixed a bunch of issues along with this :
https://patches.dpdk.org/project/dpdk/list/?series=10690 
It is requested for a back port of this series to DPDK-19.11.
 
> 
> Few more comments inline.
> 
> Best regards, Ilya Maximets.
> 
> > ---
> >  .travis.yml                           |  3 ++
> >  .travis/linux-build.sh                | 37 +++++++++-------
> >  .travis/linux-prepare.sh              |  1 +
> >  Documentation/intro/install/afxdp.rst |  2 +-
> > Documentation/intro/install/dpdk.rst  | 56 ++++++++++++++++++++----
> >  Makefile.am                           |  3 +-
> >  acinclude.m4                          | 42 ++++++++++++------
> >  parse_pkg_cfg.py                      | 62 +++++++++++++++++++++++++++
> >  8 files changed, 167 insertions(+), 39 deletions(-)  create mode
> > 100644 parse_pkg_cfg.py
> >
> > diff --git a/.travis.yml b/.travis.yml index 97249c1ce..46d7ad9bb
> > 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -27,6 +27,9 @@ addons:
> >        - selinux-policy-dev
> >        - libunbound-dev
> >        - libunwind-dev
> > +      - python3-setuptools
> > +      - python3-wheel
> > +      - ninja-build
> >
> >  before_install: ./.travis/${TRAVIS_OS_NAME}-prepare.sh
> >
> > diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index
> > 33b359a61..7fa7e738c 100755
> > --- a/.travis/linux-build.sh
> > +++ b/.travis/linux-build.sh
> > @@ -85,17 +85,21 @@ function install_dpdk()  {
> >      local DPDK_VER=$1
> >      local VERSION_FILE="dpdk-dir/travis-dpdk-cache-version"
> > +    local DPDK_OPTS=""
> >
> > -    if [ -z "$TRAVIS_ARCH" ] ||
> > -       [ "$TRAVIS_ARCH" == "amd64" ]; then
> > -        TARGET="x86_64-native-linuxapp-gcc"
> > -    elif [ "$TRAVIS_ARCH" == "aarch64" ]; then
> > -        TARGET="arm64-armv8a-linuxapp-gcc"
> > -    else
> > +    if [ "$TRAVIS_ARCH" == "aarch64" ]; then
> > +        DPDK_OPTS="$DPDK_OPTS --cross-file
> config/arm/arm64_armv8_linux_gcc"
> 
> We're not cross compiling, we're actually building on aarch64 here.
> This option should not be needed.  IIUC, meson should detect current
> architecture and build with appropriate configuration.
> 
> We should be able to just drop all the TRAVIS_ARCH checks for DPDK here.

Sure ,let me check on this . I see a similar approach taken in DPDK travis. 
May be Bruce can comment as well ?

> 
> > +    elif [ "$TRAVIS_ARCH" != "amd64" ] && [ -n "$TRAVIS_ARCH" ]; then
> >          echo "Target is unknown"
> >          exit 1
> >      fi
> >
> > +    if [ "$DPDK_SHARED" ]; then
> > +        EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=shared"
> > +    else
> > +        EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=static"
> > +    fi
> 
> I gusee we could just change the env variable and not parse it here. i.e. have
> DPDK_LIB=static defined by travis.yml by default and redefine it for matrix
> entries where we need shared libs.

Yes , this could be done. But if we install the libraries in a custom path using a prefix, 
we would have to export the LD_LIBRARY_PATH for which querying whether to build 
as shared/static will be required.(This in relation to the below comment as well :))


> 
> > +
> >      if [ "${DPDK_VER##refs/*/}" != "${DPDK_VER}" ]; then
> >          # Avoid using cache for git tree build.
> >          rm -rf dpdk-dir
> > @@ -108,7 +112,8 @@ function install_dpdk()
> >          if [ -f "${VERSION_FILE}" ]; then
> >              VER=$(cat ${VERSION_FILE})
> >              if [ "${VER}" = "${DPDK_VER}" ]; then
> > -                EXTRA_OPTS="${EXTRA_OPTS} --with-dpdk=$(pwd)/dpdk-dir/build"
> > +                sudo ninja -C $(pwd)/dpdk-dir/build install
> > +                sudo ldconfig
> 
> I think that installing right here inside the cached folder and just adjusting
> environment variables should be a bit faster than re-installing DPDK every time.
> 
> This script also will be a good example for people like me, who really don't want
> to install development versions of DPDK globally on a work laptop while testing
> OVS builds.

Yes , Thanks for the suggestion. Although ,using an install to a directory with
a prefix would require this patch from Bruce: https://patches.dpdk.org/patch/72271/
 (which is not merged yet as of this writing) .without this , OVS would fail to run 
searching for few shared DPDK libraries even when built with static libraries.

> 
> >                  echo "Found cached DPDK ${VER} build in $(pwd)/dpdk-dir"
> >                  return
> >              fi
> > @@ -122,16 +127,18 @@ function install_dpdk()
> >          pushd dpdk-dir
> >      fi
> >
> > -    make config CC=gcc T=$TARGET
> > +    # Disable building DPDK kernel modules. Not needed for OVS build or tests.
> > +    DPDK_OPTS="$DPDK_OPTS -Denable_kmods=false"
> 
> We should also disable examples and tests at least.

Sure , this will reduce the build time as well.
 Will update in the next version. 

> 
> >
> > -    if [ "$DPDK_SHARED" ]; then
> > -        sed -i '/CONFIG_RTE_BUILD_SHARED_LIB=n/s/=n/=y/' build/.config
> > -        export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$(pwd)/$TARGET/lib
> > -    fi
> > +    DPDK_OPTS="$DPDK_OPTS -Dc_args=-fPIC"
> > +    CC=gcc meson $DPDK_OPTS build
> > +    ninja -C build
> > +    sudo ninja -C build install
> > +
> > +    # Update the library paths.
> > +    sudo ldconfig
> >
> > -    make -j4 CC=gcc EXTRA_CFLAGS='-fPIC'
> > -    EXTRA_OPTS="$EXTRA_OPTS --with-dpdk=$(pwd)/build"
> > -    echo "Installed DPDK source in $(pwd)"
> > +    echo "Installed DPDK source"
> >      popd
> >      echo "${DPDK_VER}" > ${VERSION_FILE}  } diff --git
> > a/.travis/linux-prepare.sh b/.travis/linux-prepare.sh index
> > 8cbbd5623..682f6234b 100755
> > --- a/.travis/linux-prepare.sh
> > +++ b/.travis/linux-prepare.sh
> > @@ -16,6 +16,7 @@ cd ..
> >
> >  pip3 install --disable-pip-version-check --user flake8 hacking
> >  pip3 install --user --upgrade docutils
> > +pip3 install --user  'meson==0.47.1'
> 
> I understand that that is the minimum required version, but why not the most
> recent one or, at least, a bit more recent?
> 

Yes , quoting David's commit message from DPDK travis :
"
    meson 0.53.0 has a compatibility issue [1] with the python 3.5.2 that comes
    in Ubuntu 16.04.
    On the other hand, the minimal version supported in dpdk is 0.47.1.

    Stick to this version to avoid getting hit by regressions in meson latest
    shiny release.

    1: https://github.com/mesonbuild/meson/issues/6427
"

> >
> >  if [ "$M32" ]; then
> >      # Installing 32-bit libraries.
> > diff --git a/Documentation/intro/install/afxdp.rst
> > b/Documentation/intro/install/afxdp.rst
> > index 99003e4db..f422685ba 100644
> > --- a/Documentation/intro/install/afxdp.rst
> > +++ b/Documentation/intro/install/afxdp.rst
> > @@ -387,7 +387,7 @@ PVP using vhostuser device
> >  --------------------------
> >  First, build OVS with DPDK and AFXDP::
> >
> > -  ./configure  --enable-afxdp --with-dpdk=<dpdk path>
> > +  ./configure  --enable-afxdp --with-dpdk=shared|static|<dpdk path>
> >    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 dbf88ec43..46e63ddf9 100644
> > --- a/Documentation/intro/install/dpdk.rst
> > +++ b/Documentation/intro/install/dpdk.rst
> > @@ -76,9 +76,31 @@ Install DPDK
> >         $ export DPDK_DIR=/usr/src/dpdk-19.11
> >         $ cd $DPDK_DIR
> >
> > +#. Configure and install DPDK using Meson
> > +
> > +   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 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
> > +       $ sudo ldconfig
> > +
> > +   Detailed information can be found at `DPDK documentation`_.
> > +
> > +   .. _DPDK documentation:
> > + https://doc.dpdk.org/guides/linux_gsg/build_dpdk.html
> > +
> >  #. (Optional) Configure DPDK as a shared library
> >
> > -   DPDK can be built as either a static library or a shared library.  By
> > +   When using Meson, DPDK is built both as static and shared library.
> > +   So no extra configuration is required in this case.
> > +
> > +   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``.
> >
> > @@ -87,7 +109,7 @@ Install DPDK
> >        Minor performance loss is expected when using OVS with a shared DPDK
> >        library compared to a static DPDK library.
> >
> > -#. Configure and install DPDK
> > +#. Configure and install DPDK using Make
> >
> >     Build and install the DPDK library::
> >
> > @@ -97,11 +119,19 @@ Install DPDK
> >
> >  #. (Optional) Export the DPDK shared library location
> >
> > -   If DPDK was built as a shared library, export the path to this library for
> > +   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 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 update the $PKG_CONFIG_PATH for use when building OVS::
> > +
> > +      $ export LD_LIBRARY_PATH=$DPDK_INSTALL/lib/x86_64-linux-
> gnu/:$LD_LIBRARY_PATH
> > +      $ export
> > + PKG_CONFIG_PATH=$DPDK_INSTALL/lib/x86_64-linux-
> gnu/pkgconfig/:$PKG_C
> > + ONFIG_PATH
> > +
> >  .. _DPDK sources: http://dpdk.org/rel
> >
> >  Install OVS
> > @@ -121,17 +151,27 @@ has to be configured to build against the DPDK
> library (``--with-dpdk``).
> >
> >  #. Bootstrap, if required, as described in
> > :ref:`general-bootstrapping`
> >
> > -#. Configure the package using the ``--with-dpdk`` flag::
> > +#. 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::
> > +
> > +       $ ./configure --with-dpdk=static
> > +
> > +   When DPDK is built using Make(for shared or static)::
> >
> >         $ ./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.
> >
> > -   If no path is provided to ``--with-dpdk``, but a pkg-config configuration
> > -   for libdpdk is available the include paths will be generated via an
> > -   equivalent ``pkg-config --cflags libdpdk``.
> > -
> >     .. note::
> >       While ``--with-dpdk`` is required, you can pass any other configuration
> >       option described in :ref:`general-configuring`.
> > diff --git a/Makefile.am b/Makefile.am index b279303d1..13141b946
> > 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -92,7 +92,8 @@ EXTRA_DIST = \
> >  	$(MAN_ROOTS) \
> >  	Vagrantfile \
> >  	Vagrantfile-FreeBSD \
> > -	.mailmap
> > +	.mailmap\
> > +	parse_pkg_cfg.py
> >  bin_PROGRAMS =
> >  sbin_PROGRAMS =
> >  bin_SCRIPTS =
> > diff --git a/acinclude.m4 b/acinclude.m4 index 3b0eea020..66d1d3583
> > 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -306,8 +306,8 @@ dnl
> >  dnl Configure DPDK source tree
> >  AC_DEFUN([OVS_CHECK_DPDK], [
> >    AC_ARG_WITH([dpdk],
> > -              [AC_HELP_STRING([--with-dpdk=/path/to/dpdk],
> > -                              [Specify the DPDK build directory])],
> > +              [AC_HELP_STRING([--with-dpdk=static|shared|/path/to/dpdk],
> > +                              [Specify whether to use static or
> > + shared DPDK libraries only if built using meson OR the DPDK build
> > + directory in case of Make])],
> >                [have_dpdk=true])
> >
> >    AC_MSG_CHECKING([whether dpdk is enabled]) @@ -317,13 +317,24 @@
> > AC_DEFUN([OVS_CHECK_DPDK], [
> >    else
> >      AC_MSG_RESULT([yes])
> >      case "$with_dpdk" in
> > -      yes)
> > +      "shared" | "static" | "yes")
> >          DPDK_AUTO_DISCOVER="true"
> > -        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"])
> > +        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"
> > @@ -397,8 +408,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [
> >        [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, Use --with-dpdk
> > -            to specify the DPDK library installed in non-standard location]))
> > +            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
> > @@ -424,10 +436,12 @@ AC_DEFUN([OVS_CHECK_DPDK], [
> >      # 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.
> > -    case "$DPDK_LIB" in
> > -      *whole-archive*) DPDK_vswitchd_LDFLAGS=$DPDK_LIB;;
> > -      *) DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no-whole-
> archive
> > -    esac
> > +    if [[ "$pkg_failed" != "no" ]];then
> > +      DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no-whole-
> archive
> > +    else
> > +      DPDK_vswitchd_LDFLAGS=`python3 parse_pkg_cfg.py $DPDK_LIB`
> > +    fi
> > +
> >      AC_SUBST([DPDK_vswitchd_LDFLAGS])
> >      AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.])
> >    fi
> > diff --git a/parse_pkg_cfg.py b/parse_pkg_cfg.py new file mode 100644
> > index 000000000..7f7e1430d
> > --- /dev/null
> > +++ b/parse_pkg_cfg.py
> > @@ -0,0 +1,62 @@
> > +#
> > +#   BSD LICENSE
> > +#
> > +#   Copyright(c) 2020 Intel Corporation. All rights reserved.
> > +#   All rights reserved.
> > +#
> > +#   Redistribution and use in source and binary forms, with or without
> > +#   modification, are permitted provided that the following conditions
> > +#   are met:
> > +#
> > +#     * Redistributions of source code must retain the above copyright
> > +#       notice, this list of conditions and the following disclaimer.
> > +#      * Redistributions in binary form must reproduce the above copyright
> > +#        notice, this list of conditions and the following disclaimer in
> > +#        the documentation and/or other materials provided with the
> > +#        distribution.
> > +#      * Neither the name of Intel Corporation nor the names of its
> > +#        contributors may be used to endorse or promote products derived
> > +#        from this software without specific prior written permission.
> > +#
> > +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> > +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> > +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS FOR
> > +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> > +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> > +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> > +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF USE,
> > +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
> AND ON ANY
> > +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> > +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
> THE USE
> > +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> > +#
> > +
> > +# The purpose of this script is to parse the libraries # From
> > +pkg-config in case of dpdk meson builds.
> > +import sys
> > +def parse_pkg_cfg_libs(arg):
> > +    final_string = ""
> > +    first_linker_flag_ind=False
> > +    for text in arg:
> > +        text = text.strip()
> > +        # Ld only understands -lpthread.
> > +        if text == "-pthread":
> > +            text = "-lpthread"
> > +        if '-L/' in text:
> > +            final_string = final_string + text + " "
> > +        elif '-Wl,' == text[:4]:
> > +            # Libtool requires -Wl to only appear once after
> > +            # Which everything else is treated as linker input.
> > +            if not first_linker_flag_ind:
> > +                first_linker_flag_ind = True
> > +                final_string = final_string + text + ","
> > +            else:
> > +                # Omit succeeding -Wl.
> > +                final_string = final_string + text[4:] + ","
> > +        else:
> > +            # Libtool expects comma separated libraries.
> > +            final_string = final_string + text + ","
> > +    return final_string
> > +
> > +if __name__ == "__main__":
> > +    print(parse_pkg_cfg_libs(sys.argv[1:])[:-1])
> >

Thank you for the review Ilya!
Please find my response inline.
Thanks and regards,
Sunil


More information about the dev mailing list