| Message ID | 20190124163813.23928-1-christian.ehrhardt@canonical.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] acinclude: Also use LIBS from dpkg pkg-config | expand |
On Thu, Jan 24, 2019 at 5:38 PM Christian Ehrhardt <christian.ehrhardt@canonical.com> wrote: > > DPDK 18.11 builds using the more modern meson build system no more > provide the -ldpdk linker script. Instead it is expected to use > pkgconfig for linker options as well. A single patch didn't seem right for a 0/1 into, but let me add some further data to help your review in a reply. Debian/Ubuntu already use the meson build system and therefore will need this or a similar change. FYI here an Ubuntu Openvswitch build log of OVS branch-2.11 with dpdk enabled and this patch applied to get it building. amd64: https://launchpadlibrarian.net/407992531/buildlog_ubuntu-disco-amd64.openvswitch_2.11.0~git20190121.4e4f80ec2-0ubuntu1~ppa4_BUILDING.txt.gz > This change will set DPDK_LIB from pkg-config (if pkg-config was > available) and since that already carries the whole-archive flags around > the PMDs skips the further wrapping in more whole-archive if that is > already part of DPDK_LIB. FYI this is an example content a meson generated pkg-config file for DPDK: prefix=/usr libdir=${prefix}/lib/x86_64-linux-gnu includedir=${prefix}/include/dpdk Name: DPDK Description: The Data Plane Development Kit (DPDK). Note that CFLAGS might contain an -march flag higher than typical baseline. This is required for a number of static inline functions in the public headers. Version: 18.11.0 Requires.private: libbsd, zlib, libmnl, libmlx4, libibverbs, libmlx5, libcrypto Libs: -L${libdir} -lrte_telemetry -lrte_bpf -lrte_flow_classify -lrte_pipeline -lrte_table -lrte_port -lrte_vhost -lrte_security -lrte_sched -lrte_reorder -lrte_rawdev -lrte_pdump -lrte_power -lrte_meter -lrte_member -lrte_lpm -lrte_latencystats -lrte_kni -lrte_jobstats - lrte_ip_frag -lrte_gso -lrte_gro -lrte_eventdev -lrte_efd -lrte_distributor -lrte_cryptodev -lrte_compressdev -lrte_cfgfile -lrte_bitratestats -lrte_bbdev -lrte_acl -lrte_timer -lrte_hash -lrte_metrics -lrte_pci -lrte_ethdev -lrte_net -lrte_mbuf -lrte_mempool -lrte_ring - lrte_cmdline -lrte_eal -lrte_kvargs Libs.private: -L${libdir} -lrte_kvargs -lrte_eal -lrte_ring -lrte_mempool -lrte_mbuf -lrte_pci -lrte_cryptodev -lrte_net -lrte_cmdline -lrte_ethdev -lrte_hash -lrte_timer -lrte_common_dpaax -lrte_eventdev -lrte_rawdev -lrte_bus_dpaa -lrte_bus_fslmc -lrte_bus_pci -lrte_com mon_octeontx -lrte_bus_vdev -lrte_meter -lrte_sched -lrte_ip_frag -lrte_mempool_dpaa -lrte_mempool_dpaa2 -lrte_vhost -lrte_security -lrte_kni -lrte_bus_vmbus -lrte_mempool_octeontx -lpcap -lrte_port -lrte_lpm -lrte_acl -lrte_table -lrte_pipeline -lrte_gso -lIPSec_MB -lIPS ec_MB -lrte_common_cpt -lrte_reorder -lrte_compressdev -lrte_pmd_dpaa -lrte_pmd_dpaa2 -lrte_pmd_dpaa2_sec -lrte_pmd_octeontx -lrte_bbdev -lrte_bus_ifpga -Wl,--whole-archive -lrte_mempool_bucket -lrte_mempool_ring -lrte_mempool_stack -lrte_pmd_af_packet -lrte_pmd_ark -lrte _pmd_atlantic -lrte_pmd_avf -lrte_pmd_avp -lrte_pmd_axgbe -lrte_pmd_bond -lrte_pmd_bnx2x -lrte_pmd_bnxt -lrte_pmd_cxgbe -lrte_pmd_e1000 -lrte_pmd_ena -lrte_pmd_enetc -lrte_pmd_enic -lrte_pmd_failsafe -lrte_pmd_fm10k -lrte_pmd_i40e -lrte_pmd_ifc -lrte_pmd_ixgbe -lrte_pmd_k ni -lrte_pmd_liquidio -lrte_pmd_mlx4 -lrte_pmd_mlx5 -lrte_pmd_netvsc -lrte_pmd_nfp -lrte_pmd_null -lrte_pmd_pcap -lrte_pmd_qede -lrte_pmd_ring -lrte_pmd_sfc -lrte_pmd_softnic -lrte_pmd_tap -lrte_pmd_thunderx -lrte_pmd_vdev_netvsc -lrte_pmd_vhost -lrte_pmd_virtio -lrte_pmd _vmxnet3 -lrte_pmd_aesni_gcm -lrte_pmd_aesni_mb -lrte_pmd_caam_jr -lrte_pmd_ccp -lrte_pmd_dpaa_sec -lrte_pmd_null_crypto -lrte_pmd_octeontx_crypto -lrte_pmd_openssl -lrte_pmd_crypto_scheduler -lrte_pmd_virtio_crypto -lrte_pmd_octeontx_compress -lrte_pmd_qat -lrte_pmd_zlib -lrte_pmd_dpaa_event -lrte_pmd_dpaa2_event -lrte_pmd_octeontx_event -lrte_pmd_opdl_event -lrte_pmd_skeleton_event -lrte_pmd_sw_event -lrte_pmd_dsw_event -lrte_pmd_bbdev_null -lrte_pmd_skeleton_rawdev -lrte_pmd_dpaa2_cmdif -lrte_pmd_dpaa2_qdma -lrte_pmd_ifpga_rawdev -Wl,- -no-whole-archive -Wl,-Bdynamic -Wl,--no-as-needed -pthread -lm -ldl -lnuma Cflags: -I${includedir}/../x86_64-linux-gnu/dpdk -I${includedir} -include rte_config.h -march=corei7 > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> > --- > acinclude.m4 | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index f038fd457..a45411860 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > case "$with_dpdk" in > yes) > DPDK_AUTO_DISCOVER="true" > - PKG_CHECK_MODULES([DPDK], [libdpdk], > - [DPDK_INCLUDE="$DPDK_CFLAGS"], > - [DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"]) > + 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"]) > ;; > *) > DPDK_AUTO_DISCOVER="false" > @@ -238,10 +238,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH/dpdk" > fi > DPDK_LIB_DIR="$with_dpdk/lib" > + DPDK_LIB="-ldpdk" > ;; > esac > > - DPDK_LIB="-ldpdk" > DPDK_EXTRA_LIB="" > > ovs_save_CFLAGS="$CFLAGS" > @@ -346,7 +346,14 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > # > # These options are specified inside a single -Wl directive to prevent > # autotools from reordering them. > - DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no-whole-archive > + # > + # 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 > AC_SUBST([DPDK_vswitchd_LDFLAGS]) > AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.]) > fi > -- > 2.17.1 > -- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
On Thu, 2019-01-24 at 17:38 +0100, Christian Ehrhardt wrote: > DPDK 18.11 builds using the more modern meson build system no more > provide the -ldpdk linker script. Instead it is expected to use > pkgconfig for linker options as well. > > This change will set DPDK_LIB from pkg-config (if pkg-config was > available) and since that already carries the whole-archive flags > around > the PMDs skips the further wrapping in more whole-archive if that is > already part of DPDK_LIB. > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> > --- > acinclude.m4 | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index f038fd457..a45411860 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > case "$with_dpdk" in > yes) > DPDK_AUTO_DISCOVER="true" > - PKG_CHECK_MODULES([DPDK], [libdpdk], > - [DPDK_INCLUDE="$DPDK_CFLAGS"], > - [DPDK_INCLUDE="-I/usr/local/include/dpdk > -I/usr/include/dpdk"]) > + 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"]) > ;; > *) > DPDK_AUTO_DISCOVER="false" > @@ -238,10 +238,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH/dpdk" > fi > DPDK_LIB_DIR="$with_dpdk/lib" > + DPDK_LIB="-ldpdk" > ;; > esac > > - DPDK_LIB="-ldpdk" > DPDK_EXTRA_LIB="" > > ovs_save_CFLAGS="$CFLAGS" > @@ -346,7 +346,14 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > # > # These options are specified inside a single -Wl directive to > prevent > # autotools from reordering them. > - DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no-whole- > archive > + # > + # 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 > AC_SUBST([DPDK_vswitchd_LDFLAGS]) > AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.]) > fi Acked-by: Luca Boccassi <bluca@debian.org>
Christian Ehrhardt <christian.ehrhardt@canonical.com> writes: > DPDK 18.11 builds using the more modern meson build system no more > provide the -ldpdk linker script. Instead it is expected to use > pkgconfig for linker options as well. > > This change will set DPDK_LIB from pkg-config (if pkg-config was > available) and since that already carries the whole-archive flags around > the PMDs skips the further wrapping in more whole-archive if that is > already part of DPDK_LIB. > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> > --- > acinclude.m4 | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index f038fd457..a45411860 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > case "$with_dpdk" in > yes) > DPDK_AUTO_DISCOVER="true" > - PKG_CHECK_MODULES([DPDK], [libdpdk], > - [DPDK_INCLUDE="$DPDK_CFLAGS"], > - [DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"]) > + 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"]) Something about this autoconf check caused all of the travis builds to fail: https://travis-ci.org/ovsrobot/ovs/builds/483986081 The PKG_CHECK_MODULES_STATIC seems to have been added in a later version of autoconf than is shipped in the travis environment, so it might be something we need to test against and provide an alternative implementation, ala the following bug report: https://bugs.freedesktop.org/show_bug.cgi?id=19541 Thinking about it, is there a reason to change from dynamic linking to static linking? Especially since we can achieve something similar via the `PKG_CONFIG="$PKG_CONFIG --static"` override? > ;; > *) > DPDK_AUTO_DISCOVER="false" > @@ -238,10 +238,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH/dpdk" > fi > DPDK_LIB_DIR="$with_dpdk/lib" > + DPDK_LIB="-ldpdk" > ;; > esac > > - DPDK_LIB="-ldpdk" > DPDK_EXTRA_LIB="" > > ovs_save_CFLAGS="$CFLAGS" > @@ -346,7 +346,14 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > # > # These options are specified inside a single -Wl directive to prevent > # autotools from reordering them. > - DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no-whole-archive > + # > + # 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 > AC_SUBST([DPDK_vswitchd_LDFLAGS]) > AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.]) > fi
On Fri, 2019-01-25 at 11:28 -0500, Aaron Conole wrote: > Christian Ehrhardt <christian.ehrhardt@canonical.com> writes: > > > DPDK 18.11 builds using the more modern meson build system no more > > provide the -ldpdk linker script. Instead it is expected to use > > pkgconfig for linker options as well. > > > > This change will set DPDK_LIB from pkg-config (if pkg-config was > > available) and since that already carries the whole-archive flags > > around > > the PMDs skips the further wrapping in more whole-archive if that > > is > > already part of DPDK_LIB. > > > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com > > > > > --- > > acinclude.m4 | 17 ++++++++++++----- > > 1 file changed, 12 insertions(+), 5 deletions(-) > > > > diff --git a/acinclude.m4 b/acinclude.m4 > > index f038fd457..a45411860 100644 > > --- a/acinclude.m4 > > +++ b/acinclude.m4 > > @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > > case "$with_dpdk" in > > yes) > > DPDK_AUTO_DISCOVER="true" > > - PKG_CHECK_MODULES([DPDK], [libdpdk], > > - [DPDK_INCLUDE="$DPDK_CFLAGS"], > > - [DPDK_INCLUDE="-I/usr/local/include/dpdk > > -I/usr/include/dpdk"]) > > + 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"]) > > Something about this autoconf check caused all of the travis builds > to > fail: > > https://travis-ci.org/ovsrobot/ovs/builds/483986081 > > The PKG_CHECK_MODULES_STATIC seems to have been added in a later > version > of autoconf than is shipped in the travis environment, so it might be > something we need to test against and provide an alternative > implementation, ala the following bug report: > > https://bugs.freedesktop.org/show_bug.cgi?id=19541 Hi, The PKG_CHECK* macros come from the pkg-config package, not autoconf, in version 0.29. The version of pkg-config in Ubuntu 14.04 does not indeed contain it, but 16.04 does. If you go down the reimplementation route, I would recommend checking for the macro version first and add a comment, so that you can then drop it when compatibility with Ubuntu 14.04 and Debian 8 is no longer required. > Thinking about it, is there a reason to change from dynamic linking > to > static linking? Especially since we can achieve something similar > via > the `PKG_CONFIG="$PKG_CONFIG --static"` override? Note that this is not doing static linking - it's just getting the full list of libs (which includes all the PMDs). The _STATIC macro is implemented exactly as you suggested above.
On Fri, 2019-01-25 at 16:39 +0000, Luca Boccassi wrote: > On Fri, 2019-01-25 at 11:28 -0500, Aaron Conole wrote: > > Christian Ehrhardt <christian.ehrhardt@canonical.com> writes: > > > > > DPDK 18.11 builds using the more modern meson build system no > > > more > > > provide the -ldpdk linker script. Instead it is expected to use > > > pkgconfig for linker options as well. > > > > > > This change will set DPDK_LIB from pkg-config (if pkg-config was > > > available) and since that already carries the whole-archive flags > > > around > > > the PMDs skips the further wrapping in more whole-archive if that > > > is > > > already part of DPDK_LIB. > > > > > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.c > > > om > > > > > > > > > > --- > > > acinclude.m4 | 17 ++++++++++++----- > > > 1 file changed, 12 insertions(+), 5 deletions(-) > > > > > > diff --git a/acinclude.m4 b/acinclude.m4 > > > index f038fd457..a45411860 100644 > > > --- a/acinclude.m4 > > > +++ b/acinclude.m4 > > > @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > > > case "$with_dpdk" in > > > yes) > > > DPDK_AUTO_DISCOVER="true" > > > - PKG_CHECK_MODULES([DPDK], [libdpdk], > > > - [DPDK_INCLUDE="$DPDK_CFLAGS"], > > > - [DPDK_INCLUDE="- > > > I/usr/local/include/dpdk > > > -I/usr/include/dpdk"]) > > > + 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"]) > > > > Something about this autoconf check caused all of the travis builds > > to > > fail: > > > > https://travis-ci.org/ovsrobot/ovs/builds/483986081 > > > > The PKG_CHECK_MODULES_STATIC seems to have been added in a later > > version > > of autoconf than is shipped in the travis environment, so it might > > be > > something we need to test against and provide an alternative > > implementation, ala the following bug report: > > > > https://bugs.freedesktop.org/show_bug.cgi?id=19541 > > Hi, > > The PKG_CHECK* macros come from the pkg-config package, not autoconf, > in version 0.29. The version of pkg-config in Ubuntu 14.04 does not > indeed contain it, but 16.04 does. > > If you go down the reimplementation route, I would recommend checking > for the macro version first and add a comment, so that you can then > drop it when compatibility with Ubuntu 14.04 and Debian 8 is no > longer > required. Also, these macro files are all versioned - autoconf is smart enough to check the version if you have it locally, and if the system's is higher, it will use that one instead, so there's no drawback to backporting. In other words, you can drop pkg.m4 version 0.29 in the m4/ directory in the repository, add "m4_include([m4/pkg.m4])" to configure.ac and then not worry about it anymore until compatibility with Ubuntu 14.04 and Debian 8 can be dropped. > > Thinking about it, is there a reason to change from dynamic linking > > to > > static linking? Especially since we can achieve something similar > > via > > the `PKG_CONFIG="$PKG_CONFIG --static"` override? > > Note that this is not doing static linking - it's just getting the > full > list of libs (which includes all the PMDs). The _STATIC macro is > implemented exactly as you suggested above. >
On Fri, Jan 25, 2019 at 04:39:15PM +0000, Luca Boccassi wrote: > On Fri, 2019-01-25 at 11:28 -0500, Aaron Conole wrote: > > Christian Ehrhardt <christian.ehrhardt@canonical.com> writes: > > > > > DPDK 18.11 builds using the more modern meson build system no more > > > provide the -ldpdk linker script. Instead it is expected to use > > > pkgconfig for linker options as well. > > > > > > This change will set DPDK_LIB from pkg-config (if pkg-config was > > > available) and since that already carries the whole-archive flags > > > around > > > the PMDs skips the further wrapping in more whole-archive if that > > > is > > > already part of DPDK_LIB. > > > > > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com > > > > > > > --- > > > acinclude.m4 | 17 ++++++++++++----- > > > 1 file changed, 12 insertions(+), 5 deletions(-) > > > > > > diff --git a/acinclude.m4 b/acinclude.m4 > > > index f038fd457..a45411860 100644 > > > --- a/acinclude.m4 > > > +++ b/acinclude.m4 > > > @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > > > case "$with_dpdk" in > > > yes) > > > DPDK_AUTO_DISCOVER="true" > > > - PKG_CHECK_MODULES([DPDK], [libdpdk], > > > - [DPDK_INCLUDE="$DPDK_CFLAGS"], > > > - [DPDK_INCLUDE="-I/usr/local/include/dpdk > > > -I/usr/include/dpdk"]) > > > + 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"]) > > > > Something about this autoconf check caused all of the travis builds > > to > > fail: > > > > https://travis-ci.org/ovsrobot/ovs/builds/483986081 > > > > The PKG_CHECK_MODULES_STATIC seems to have been added in a later > > version > > of autoconf than is shipped in the travis environment, so it might be > > something we need to test against and provide an alternative > > implementation, ala the following bug report: > > > > https://bugs.freedesktop.org/show_bug.cgi?id=19541 > > Hi, > > The PKG_CHECK* macros come from the pkg-config package, not autoconf, > in version 0.29. The version of pkg-config in Ubuntu 14.04 does not > indeed contain it, but 16.04 does. Probably one can just copy pkg.m4 from pkg-config into the OVS m4/ directory.
Luca Boccassi <bluca@debian.org> writes: > On Fri, 2019-01-25 at 16:39 +0000, Luca Boccassi wrote: >> On Fri, 2019-01-25 at 11:28 -0500, Aaron Conole wrote: >> > Christian Ehrhardt <christian.ehrhardt@canonical.com> writes: >> > >> > > DPDK 18.11 builds using the more modern meson build system no >> > > more >> > > provide the -ldpdk linker script. Instead it is expected to use >> > > pkgconfig for linker options as well. >> > > >> > > This change will set DPDK_LIB from pkg-config (if pkg-config was >> > > available) and since that already carries the whole-archive flags >> > > around >> > > the PMDs skips the further wrapping in more whole-archive if that >> > > is >> > > already part of DPDK_LIB. >> > > >> > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.c >> > > om >> > > > >> > > >> > > --- >> > > acinclude.m4 | 17 ++++++++++++----- >> > > 1 file changed, 12 insertions(+), 5 deletions(-) >> > > >> > > diff --git a/acinclude.m4 b/acinclude.m4 >> > > index f038fd457..a45411860 100644 >> > > --- a/acinclude.m4 >> > > +++ b/acinclude.m4 >> > > @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ >> > > case "$with_dpdk" in >> > > yes) >> > > DPDK_AUTO_DISCOVER="true" >> > > - PKG_CHECK_MODULES([DPDK], [libdpdk], >> > > - [DPDK_INCLUDE="$DPDK_CFLAGS"], >> > > - [DPDK_INCLUDE="- >> > > I/usr/local/include/dpdk >> > > -I/usr/include/dpdk"]) >> > > + 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"]) >> > >> > Something about this autoconf check caused all of the travis builds >> > to >> > fail: >> > >> > https://travis-ci.org/ovsrobot/ovs/builds/483986081 >> > >> > The PKG_CHECK_MODULES_STATIC seems to have been added in a later >> > version >> > of autoconf than is shipped in the travis environment, so it might >> > be >> > something we need to test against and provide an alternative >> > implementation, ala the following bug report: >> > >> > https://bugs.freedesktop.org/show_bug.cgi?id=19541 >> >> Hi, >> >> The PKG_CHECK* macros come from the pkg-config package, not autoconf, >> in version 0.29. The version of pkg-config in Ubuntu 14.04 does not >> indeed contain it, but 16.04 does. >> >> If you go down the reimplementation route, I would recommend checking >> for the macro version first and add a comment, so that you can then >> drop it when compatibility with Ubuntu 14.04 and Debian 8 is no >> longer >> required. > > Also, these macro files are all versioned - autoconf is smart enough to > check the version if you have it locally, and if the system's is > higher, it will use that one instead, so there's no drawback to > backporting. Cool, I didn't know that! > In other words, you can drop pkg.m4 version 0.29 in the m4/ directory > in the repository, add "m4_include([m4/pkg.m4])" to configure.ac and > then not worry about it anymore until compatibility with Ubuntu 14.04 > and Debian 8 can be dropped. That sounds like a solution. >> > Thinking about it, is there a reason to change from dynamic linking >> > to >> > static linking? Especially since we can achieve something similar >> > via >> > the `PKG_CONFIG="$PKG_CONFIG --static"` override? >> >> Note that this is not doing static linking - it's just getting the >> full >> list of libs (which includes all the PMDs). The _STATIC macro is >> implemented exactly as you suggested above. >>
On Fri, Jan 25, 2019 at 5:53 PM Aaron Conole <aconole@redhat.com> wrote: > > Luca Boccassi <bluca@debian.org> writes: > > > On Fri, 2019-01-25 at 16:39 +0000, Luca Boccassi wrote: > >> On Fri, 2019-01-25 at 11:28 -0500, Aaron Conole wrote: > >> > Christian Ehrhardt <christian.ehrhardt@canonical.com> writes: > >> > > >> > > DPDK 18.11 builds using the more modern meson build system no > >> > > more > >> > > provide the -ldpdk linker script. Instead it is expected to use > >> > > pkgconfig for linker options as well. > >> > > > >> > > This change will set DPDK_LIB from pkg-config (if pkg-config was > >> > > available) and since that already carries the whole-archive flags > >> > > around > >> > > the PMDs skips the further wrapping in more whole-archive if that > >> > > is > >> > > already part of DPDK_LIB. > >> > > > >> > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.c > >> > > om > >> > > > > >> > > > >> > > --- > >> > > acinclude.m4 | 17 ++++++++++++----- > >> > > 1 file changed, 12 insertions(+), 5 deletions(-) > >> > > > >> > > diff --git a/acinclude.m4 b/acinclude.m4 > >> > > index f038fd457..a45411860 100644 > >> > > --- a/acinclude.m4 > >> > > +++ b/acinclude.m4 > >> > > @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > >> > > case "$with_dpdk" in > >> > > yes) > >> > > DPDK_AUTO_DISCOVER="true" > >> > > - PKG_CHECK_MODULES([DPDK], [libdpdk], > >> > > - [DPDK_INCLUDE="$DPDK_CFLAGS"], > >> > > - [DPDK_INCLUDE="- > >> > > I/usr/local/include/dpdk > >> > > -I/usr/include/dpdk"]) > >> > > + 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"]) > >> > > >> > Something about this autoconf check caused all of the travis builds > >> > to > >> > fail: > >> > > >> > https://travis-ci.org/ovsrobot/ovs/builds/483986081 > >> > > >> > The PKG_CHECK_MODULES_STATIC seems to have been added in a later > >> > version > >> > of autoconf than is shipped in the travis environment, so it might > >> > be > >> > something we need to test against and provide an alternative > >> > implementation, ala the following bug report: > >> > > >> > https://bugs.freedesktop.org/show_bug.cgi?id=19541 > >> > >> Hi, > >> > >> The PKG_CHECK* macros come from the pkg-config package, not autoconf, > >> in version 0.29. The version of pkg-config in Ubuntu 14.04 does not > >> indeed contain it, but 16.04 does. > >> > >> If you go down the reimplementation route, I would recommend checking > >> for the macro version first and add a comment, so that you can then > >> drop it when compatibility with Ubuntu 14.04 and Debian 8 is no > >> longer > >> required. > > > > Also, these macro files are all versioned - autoconf is smart enough to > > check the version if you have it locally, and if the system's is > > higher, it will use that one instead, so there's no drawback to > > backporting. > > Cool, I didn't know that! > > > In other words, you can drop pkg.m4 version 0.29 in the m4/ directory > > in the repository, add "m4_include([m4/pkg.m4])" to configure.ac and > > then not worry about it anymore until compatibility with Ubuntu 14.04 > > and Debian 8 can be dropped. > > That sounds like a solution. Ben/Aaron, thanks for sorting out that aspect of it! That said, does it mean that the Travis failures now can be considered passing? And if so what does it mean for the patch itself, do you consider it ok and would merge it or are there other challenges that we need to tackle? > >> > Thinking about it, is there a reason to change from dynamic linking > >> > to > >> > static linking? Especially since we can achieve something similar > >> > via > >> > the `PKG_CONFIG="$PKG_CONFIG --static"` override? > >> > >> Note that this is not doing static linking - it's just getting the > >> full > >> list of libs (which includes all the PMDs). The _STATIC macro is > >> implemented exactly as you suggested above. > >>
Christian Ehrhardt <christian.ehrhardt@canonical.com> writes: > On Fri, Jan 25, 2019 at 5:53 PM Aaron Conole <aconole@redhat.com> wrote: >> >> Luca Boccassi <bluca@debian.org> writes: >> >> > On Fri, 2019-01-25 at 16:39 +0000, Luca Boccassi wrote: >> >> On Fri, 2019-01-25 at 11:28 -0500, Aaron Conole wrote: >> >> > Christian Ehrhardt <christian.ehrhardt@canonical.com> writes: >> >> > >> >> > > DPDK 18.11 builds using the more modern meson build system no >> >> > > more >> >> > > provide the -ldpdk linker script. Instead it is expected to use >> >> > > pkgconfig for linker options as well. >> >> > > >> >> > > This change will set DPDK_LIB from pkg-config (if pkg-config was >> >> > > available) and since that already carries the whole-archive flags >> >> > > around >> >> > > the PMDs skips the further wrapping in more whole-archive if that >> >> > > is >> >> > > already part of DPDK_LIB. >> >> > > >> >> > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.c >> >> > > om >> >> > > > >> >> > > >> >> > > --- >> >> > > acinclude.m4 | 17 ++++++++++++----- >> >> > > 1 file changed, 12 insertions(+), 5 deletions(-) >> >> > > >> >> > > diff --git a/acinclude.m4 b/acinclude.m4 >> >> > > index f038fd457..a45411860 100644 >> >> > > --- a/acinclude.m4 >> >> > > +++ b/acinclude.m4 >> >> > > @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ >> >> > > case "$with_dpdk" in >> >> > > yes) >> >> > > DPDK_AUTO_DISCOVER="true" >> >> > > - PKG_CHECK_MODULES([DPDK], [libdpdk], >> >> > > - [DPDK_INCLUDE="$DPDK_CFLAGS"], >> >> > > - [DPDK_INCLUDE="- >> >> > > I/usr/local/include/dpdk >> >> > > -I/usr/include/dpdk"]) >> >> > > + 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"]) >> >> > >> >> > Something about this autoconf check caused all of the travis builds >> >> > to >> >> > fail: >> >> > >> >> > https://travis-ci.org/ovsrobot/ovs/builds/483986081 >> >> > >> >> > The PKG_CHECK_MODULES_STATIC seems to have been added in a later >> >> > version >> >> > of autoconf than is shipped in the travis environment, so it might >> >> > be >> >> > something we need to test against and provide an alternative >> >> > implementation, ala the following bug report: >> >> > >> >> > https://bugs.freedesktop.org/show_bug.cgi?id=19541 >> >> >> >> Hi, >> >> >> >> The PKG_CHECK* macros come from the pkg-config package, not autoconf, >> >> in version 0.29. The version of pkg-config in Ubuntu 14.04 does not >> >> indeed contain it, but 16.04 does. >> >> >> >> If you go down the reimplementation route, I would recommend checking >> >> for the macro version first and add a comment, so that you can then >> >> drop it when compatibility with Ubuntu 14.04 and Debian 8 is no >> >> longer >> >> required. >> > >> > Also, these macro files are all versioned - autoconf is smart enough to >> > check the version if you have it locally, and if the system's is >> > higher, it will use that one instead, so there's no drawback to >> > backporting. >> >> Cool, I didn't know that! >> >> > In other words, you can drop pkg.m4 version 0.29 in the m4/ directory >> > in the repository, add "m4_include([m4/pkg.m4])" to configure.ac and >> > then not worry about it anymore until compatibility with Ubuntu 14.04 >> > and Debian 8 can be dropped. >> >> That sounds like a solution. > > Ben/Aaron, thanks for sorting out that aspect of it! > > That said, does it mean that the Travis failures now can be considered passing? Not quite yet. > And if so what does it mean for the patch itself, do you consider it ok and > would merge it or are there other challenges that we need to tackle? I think if you made the adjustments suggested by Luca (copy pkg.m4 into the m4/ directory in the ovs tree, add the m4_include([m4/pkg.m4]) directive to configure.ac) and then post a v2, it would be acceptable. >> >> > Thinking about it, is there a reason to change from dynamic linking >> >> > to >> >> > static linking? Especially since we can achieve something similar >> >> > via >> >> > the `PKG_CONFIG="$PKG_CONFIG --static"` override? >> >> >> >> Note that this is not doing static linking - it's just getting the >> >> full >> >> list of libs (which includes all the PMDs). The _STATIC macro is >> >> implemented exactly as you suggested above. >> >>
On Wed, 2019-01-30 at 11:08 +0100, Christian Ehrhardt wrote: > DPDK 18.11 can be built using the more modern meson build system. > In that case it no more provides the -ldpdk linker script. Instead > it is expected to use pkgconfig for linker options as well. > > FYI here a build log on Ubuntu 19.04 with the three patches applied: > https://launchpadlibrarian.net/409021278/buildlog_ubuntu-disco-amd64. > openvswitch_2.11.0~git20190121.4e4f80ec2- > 0ubuntu1~ppa5_BUILDING.txt.gz > > Updates from v1: > - add updated pkg.m4 to support PKG_CHECK_MODULES_STATIC > - include local pkg.m4 in configure.ac > > Christian Ehrhardt (3): > acinclude: Also use LIBS from dpkg pkg-config > m4: update pkg.m4 to pkg-config 0.29.1. > configure.ac: use the locally provided pkg.m4 > > acinclude.m4 | 17 ++-- > configure.ac | 2 + > m4/pkg.m4 | 217 +++++++++++++++++++++++++++++++++-------------- > ---- > 3 files changed, 153 insertions(+), 83 deletions(-) Series-acked-by: Luca Boccassi <bluca@debian.org>
diff --git a/acinclude.m4 b/acinclude.m4 index f038fd457..a45411860 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -223,9 +223,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ case "$with_dpdk" in yes) DPDK_AUTO_DISCOVER="true" - PKG_CHECK_MODULES([DPDK], [libdpdk], - [DPDK_INCLUDE="$DPDK_CFLAGS"], - [DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"]) + 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"]) ;; *) DPDK_AUTO_DISCOVER="false" @@ -238,10 +238,10 @@ AC_DEFUN([OVS_CHECK_DPDK], [ DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH/dpdk" fi DPDK_LIB_DIR="$with_dpdk/lib" + DPDK_LIB="-ldpdk" ;; esac - DPDK_LIB="-ldpdk" DPDK_EXTRA_LIB="" ovs_save_CFLAGS="$CFLAGS" @@ -346,7 +346,14 @@ AC_DEFUN([OVS_CHECK_DPDK], [ # # These options are specified inside a single -Wl directive to prevent # autotools from reordering them. - DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no-whole-archive + # + # 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 AC_SUBST([DPDK_vswitchd_LDFLAGS]) AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.]) fi
DPDK 18.11 builds using the more modern meson build system no more provide the -ldpdk linker script. Instead it is expected to use pkgconfig for linker options as well. This change will set DPDK_LIB from pkg-config (if pkg-config was available) and since that already carries the whole-archive flags around the PMDs skips the further wrapping in more whole-archive if that is already part of DPDK_LIB. Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- acinclude.m4 | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)