[dpdk-dev,v2] mk: fix external shared library dependencies of libraries
Commit Message
Similar to commit 5f9115e58cc6f304ff4ade694cf5823d32887d1a etc, but
for libraries. Clean up librte_vhost CFLAGS/LDFLAGS/LDLIBS confusion
while at it.
Requiring applications to know about library internal details like
dependencies to external helper libraries is a limitation of
static linkage, shared libraries should always know their own
dependencies for sane operation.
Linking with the combined library (whether shared or not) still
requires knowing the internal dependencies, and intra-dpdk
dependencies are also not currently recorded.
Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
---
v2:
- clean up librte_vhost CFLAGS/LDFLAGS/LDLIBS confusion while at it
lib/librte_eal/bsdapp/eal/Makefile | 4 ++++
lib/librte_eal/linuxapp/eal/Makefile | 6 ++++++
lib/librte_sched/Makefile | 3 +++
lib/librte_vhost/Makefile | 6 +++---
mk/rte.app.mk | 20 ++++++++------------
5 files changed, 24 insertions(+), 15 deletions(-)
Comments
On 08/12/2015 11:47, Panu Matilainen wrote:
> Similar to commit 5f9115e58cc6f304ff4ade694cf5823d32887d1a etc, but
> for libraries. Clean up librte_vhost CFLAGS/LDFLAGS/LDLIBS confusion
> while at it.
>
> Requiring applications to know about library internal details like
> dependencies to external helper libraries is a limitation of
> static linkage, shared libraries should always know their own
> dependencies for sane operation.
>
> Linking with the combined library (whether shared or not) still
> requires knowing the internal dependencies, and intra-dpdk
> dependencies are also not currently recorded.
>
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> ---
>
> v2:
> - clean up librte_vhost CFLAGS/LDFLAGS/LDLIBS confusion while at it
>
>
Hi Panu,
Patch itself looks good but there is a small side effect on BSD that results
in app/test not linking because of missing -lm.
Linuxapp links with -lm by default (EXECENV_LDLIBS), but BSD does not.
Should we just add -lm to EXECENV_LDLIBS for BSD too instead of
adding it on each app/example that uses librte_sched ?
Sergio
On 12/08/2015 06:28 PM, Sergio Gonzalez Monroy wrote:
> On 08/12/2015 11:47, Panu Matilainen wrote:
>> Similar to commit 5f9115e58cc6f304ff4ade694cf5823d32887d1a etc, but
>> for libraries. Clean up librte_vhost CFLAGS/LDFLAGS/LDLIBS confusion
>> while at it.
>>
>> Requiring applications to know about library internal details like
>> dependencies to external helper libraries is a limitation of
>> static linkage, shared libraries should always know their own
>> dependencies for sane operation.
>>
>> Linking with the combined library (whether shared or not) still
>> requires knowing the internal dependencies, and intra-dpdk
>> dependencies are also not currently recorded.
>>
>> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
>> ---
>>
>> v2:
>> - clean up librte_vhost CFLAGS/LDFLAGS/LDLIBS confusion while at it
>>
>>
> Hi Panu,
>
> Patch itself looks good but there is a small side effect on BSD that
> results
> in app/test not linking because of missing -lm.
> Linuxapp links with -lm by default (EXECENV_LDLIBS), but BSD does not.
Oh, those LIBRTE_SCHED entries were in a different if-block from the
others...
Hmm, interesting. Without this patch, on Linux -lm gets added twice
which actually causes a build failure on Fedora rawhide (related to some
libmvec related changes it seems).
> Should we just add -lm to EXECENV_LDLIBS for BSD too instead of
> adding it on each app/example that uses librte_sched ?
Linking should be based on usage, not convenience or such... but there's
no explanation why -lm is added everywhere in Linux:
commit 6da94b7a92d9706c1a4fb23a9cf54f49e6019af2
Author: Intel <intel.com>
Date: Wed Sep 18 12:00:00 2013 +0200
mk: link with libm
Signed-off-by: Intel
Certainly librte_sched should link to -lm and in static builds, all its
users, but beyond that I suppose it needs closer investigation of what
(if anything else) actually needs it.
I think we better leave it alone for 2.2, but the librte_vhost part
should be safe. I can send another version with just that if it has a
chance to make it to 2.2, otherwise lets postpone it to 2.3.
- Panu -
- Panu -
- Panu -
Add hopefully all the remaining missing DT_NEEDED entries for external
library dependencies on the libraries side: librte_vhost, librte_sched
and librte_eal.
Panu Matilainen (3):
mk: clear up libm and librt linkage confusion
mk: add DT_NEEDED entries for librte_vhost external dependencies
mk: add DT_NEEDED entries for librte_eal external dependencies
app/test/Makefile | 2 ++
lib/librte_eal/bsdapp/eal/Makefile | 4 ++++
lib/librte_eal/linuxapp/eal/Makefile | 4 ++++
lib/librte_sched/Makefile | 3 +++
lib/librte_vhost/Makefile | 7 ++++---
mk/exec-env/linuxapp/rte.vars.mk | 2 +-
mk/rte.app.mk | 20 ++++++++------------
7 files changed, 26 insertions(+), 16 deletions(-)
2016-03-10 15:15, Panu Matilainen:
> Add hopefully all the remaining missing DT_NEEDED entries for external
> library dependencies on the libraries side: librte_vhost, librte_sched
> and librte_eal.
>
> Panu Matilainen (3):
> mk: clear up libm and librt linkage confusion
> mk: add DT_NEEDED entries for librte_vhost external dependencies
> mk: add DT_NEEDED entries for librte_eal external dependencies
Applied, thanks
@@ -42,6 +42,10 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_ring
CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
CFLAGS += $(WERROR_FLAGS) -O3
+LDLIBS += -lpthread
+LDLIBS += -lexecinfo
+LDLIBS += -lgcc_s
+
EXPORT_MAP := rte_eal_version.map
LIBABIVER := 2
@@ -47,6 +47,12 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_mempool
CFLAGS += -I$(RTE_SDK)/lib/librte_ivshmem
CFLAGS += $(WERROR_FLAGS) -O3
+LDLIBS += -lpthread
+LDLIBS += -ldl
+LDLIBS += -lrt
+LDLIBS += -lm
+LDLIBS += -lgcc_s
+
# specific to linuxapp exec-env
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) := eal.c
SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_hugepage_info.c
@@ -41,6 +41,9 @@ CFLAGS += $(WERROR_FLAGS)
CFLAGS_rte_red.o := -D_GNU_SOURCE
+LDLIBS += -lm
+LDLIBS += -lrt
+
EXPORT_MAP := rte_sched_version.map
LIBABIVER := 1
@@ -42,12 +42,12 @@ CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -D_FILE_OFFSET_BITS=64
ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),y)
CFLAGS += -I vhost_user
else
-CFLAGS += -I vhost_cuse -lfuse
-LDFLAGS += -lfuse
+CFLAGS += -I vhost_cuse
+LDLIBS += -lfuse
endif
ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
-LDFLAGS += -lnuma
+LDLIBS += -lnuma
endif
# all source are stored in SRCS-y
@@ -81,23 +81,11 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM) += -lrte_lpm
_LDLIBS-$(CONFIG_RTE_LIBRTE_POWER) += -lrte_power
_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL) += -lrte_acl
_LDLIBS-$(CONFIG_RTE_LIBRTE_METER) += -lrte_meter
-
_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrte_sched
-_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lm
-_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrt
-
_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lrte_vhost
endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
-ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
-_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lnuma
-endif
-
-ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
-_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse
-endif
-
# The static libraries do not know their dependencies.
# The combined library fails also to store this information.
# So linking with static or combined library requires explicit dependencies.
@@ -111,6 +99,14 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lxenstore
_LDLIBS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += -lgxio
# QAT PMD has a dependency on libcrypto (from openssl) for calculating HMAC precomputes
_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_QAT) += -lcrypto
+_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lm
+_LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED) += -lrt
+ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
+_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lnuma
+endif
+ifeq ($(CONFIG_RTE_LIBRTE_VHOST_USER),n)
+_LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST) += -lfuse
+endif
endif # CONFIG_RTE_BUILD_COMBINE_LIBS or not CONFIG_RTE_BUILD_SHARED_LIBS
_LDLIBS-y += --start-group