[ovs-dev] [PATCH dpdk-latest v1] build: Remove DPDK make build references.
Pai G, Sunil
sunil.pai.g at intel.com
Wed Nov 18 19:06:56 UTC 2020
Hi Ilya , Ian
Thank you for the comments , response inline.
> -----Original Message-----
> From: Ilya Maximets <i.maximets at ovn.org>
> Sent: Wednesday, November 18, 2020 10:26 PM
> To: Stokes, Ian <ian.stokes at intel.com>; Pai G, Sunil <sunil.pai.g at intel.com>;
> dev at openvswitch.org
> Cc: i.maximets at ovn.org; david.marchand at redhat.com;
> christian.ehrhardt at canonical.com; ktraynor at 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 at 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
More information about the dev
mailing list