DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] mk: fix external shared library dependencies of libraries
@ 2015-12-08  8:30 Panu Matilainen
  2015-12-08 10:11 ` Thomas Monjalon
  0 siblings, 1 reply; 4+ messages in thread
From: Panu Matilainen @ 2015-12-08  8:30 UTC (permalink / raw)
  To: dev

Similar to commit 5f9115e58cc6f304ff4ade694cf5823d32887d1a etc, but
for libraries.

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

Note: I haven't tested on FreeBSD, it should be straightforward but
it wouldn't hurt if Bruce / Sergio can sanity-check that side...

 lib/librte_eal/bsdapp/eal/Makefile   |  4 ++++
 lib/librte_eal/linuxapp/eal/Makefile |  6 ++++++
 lib/librte_sched/Makefile            |  3 +++
 lib/librte_vhost/Makefile            |  2 ++
 mk/rte.app.mk                        | 20 ++++++++------------
 5 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
index 65b293f..d7e06fd 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -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
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 26eced5..2a0fa2b 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -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
diff --git a/lib/librte_sched/Makefile b/lib/librte_sched/Makefile
index b1cb285..4d631f6 100644
--- a/lib/librte_sched/Makefile
+++ b/lib/librte_sched/Makefile
@@ -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
diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile
index 6681f22..a3bdca4 100644
--- a/lib/librte_vhost/Makefile
+++ b/lib/librte_vhost/Makefile
@@ -44,10 +44,12 @@ CFLAGS += -I vhost_user
 else
 CFLAGS += -I vhost_cuse -lfuse
 LDFLAGS += -lfuse
+LDLIBS += -lfuse
 endif
 
 ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
 LDFLAGS += -lnuma
+LDLIBS += -lnuma
 endif
 
 # all source are stored in SRCS-y
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 8ecab41..4ecaa6c 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -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
-- 
2.5.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix external shared library dependencies of libraries
  2015-12-08  8:30 [dpdk-dev] [PATCH] mk: fix external shared library dependencies of libraries Panu Matilainen
@ 2015-12-08 10:11 ` Thomas Monjalon
  2015-12-08 11:19   ` Panu Matilainen
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Monjalon @ 2015-12-08 10:11 UTC (permalink / raw)
  To: Panu Matilainen; +Cc: dev

Hi Panu,

2015-12-08 10:30, Panu Matilainen:
> --- a/lib/librte_vhost/Makefile
> +++ b/lib/librte_vhost/Makefile
> @@ -44,10 +44,12 @@ CFLAGS += -I vhost_user
>  else
>  CFLAGS += -I vhost_cuse -lfuse
>  LDFLAGS += -lfuse
> +LDLIBS += -lfuse
>  endif
>  
>  ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
>  LDFLAGS += -lnuma
> +LDLIBS += -lnuma
>  endif

It looks weird to have to declare the dependencies both in
LDFLAGS and LDLIBS. What is the reason?
Can we improve it?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix external shared library dependencies of libraries
  2015-12-08 10:11 ` Thomas Monjalon
@ 2015-12-08 11:19   ` Panu Matilainen
  2015-12-08 11:39     ` Panu Matilainen
  0 siblings, 1 reply; 4+ messages in thread
From: Panu Matilainen @ 2015-12-08 11:19 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 12/08/2015 12:11 PM, Thomas Monjalon wrote:
> Hi Panu,
>
> 2015-12-08 10:30, Panu Matilainen:
>> --- a/lib/librte_vhost/Makefile
>> +++ b/lib/librte_vhost/Makefile
>> @@ -44,10 +44,12 @@ CFLAGS += -I vhost_user
>>   else
>>   CFLAGS += -I vhost_cuse -lfuse
>>   LDFLAGS += -lfuse
>> +LDLIBS += -lfuse
>>   endif
>>
>>   ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
>>   LDFLAGS += -lnuma
>> +LDLIBS += -lnuma
>>   endif
>
> It looks weird to have to declare the dependencies both in
> LDFLAGS and LDLIBS. What is the reason?
> Can we improve it?

I'd say its just an artifact of the dpdk build system evolution and 
surely we can improve it, but I'd leave it post 2.2 to avoid breaking 
anything now.

I'm planning further work in this area and one of the things on my TODO 
list is to look into the LDFLAGS/LDLIBS duplication. Technically, LDLIBS 
should only contain the libraries to link, and all the others directives 
(such as linker path etc) should go to LDFLAGS.

	- Panu -

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] mk: fix external shared library dependencies of libraries
  2015-12-08 11:19   ` Panu Matilainen
@ 2015-12-08 11:39     ` Panu Matilainen
  0 siblings, 0 replies; 4+ messages in thread
From: Panu Matilainen @ 2015-12-08 11:39 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

On 12/08/2015 01:19 PM, Panu Matilainen wrote:
> On 12/08/2015 12:11 PM, Thomas Monjalon wrote:
>> Hi Panu,
>>
>> 2015-12-08 10:30, Panu Matilainen:
>>> --- a/lib/librte_vhost/Makefile
>>> +++ b/lib/librte_vhost/Makefile
>>> @@ -44,10 +44,12 @@ CFLAGS += -I vhost_user
>>>   else
>>>   CFLAGS += -I vhost_cuse -lfuse
>>>   LDFLAGS += -lfuse
>>> +LDLIBS += -lfuse
>>>   endif
>>>
>>>   ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
>>>   LDFLAGS += -lnuma
>>> +LDLIBS += -lnuma
>>>   endif
>>
>> It looks weird to have to declare the dependencies both in
>> LDFLAGS and LDLIBS. What is the reason?
>> Can we improve it?
>
> I'd say its just an artifact of the dpdk build system evolution and
> surely we can improve it, but I'd leave it post 2.2 to avoid breaking
> anything now.

Actually, scratch that. That librte_vhost has used LDFLAGS instead of 
LDLIBS is likely just a mistake that happens to work, but there should 
be no reason for it.

I'll send a v2 with that changed, and while at it, remove the bogus 
-lfuse from vhost_cuse CFLAGS too.

	- Panu -

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-12-08 11:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-08  8:30 [dpdk-dev] [PATCH] mk: fix external shared library dependencies of libraries Panu Matilainen
2015-12-08 10:11 ` Thomas Monjalon
2015-12-08 11:19   ` Panu Matilainen
2015-12-08 11:39     ` Panu Matilainen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).