[dpdk-dev] drivers: fix shared library dependencies to external libraries

Message ID ee5b1caf719262b06bbf0b8b134b250abd0865d3.1445437070.git.pmatilai@redhat.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Panu Matilainen Oct. 21, 2015, 2:18 p.m. UTC
  Similar to commit 113c8e13c4201eee207723571f83aaf285277d75, but
for bnx2x and pcap PMDs.

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. This is especially highlighted
with dlopen()'ed items, having applications link against about plugin
internal dependencies goes on the side of absurd.

Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
---
 drivers/net/bnx2x/Makefile | 1 +
 drivers/net/pcap/Makefile  | 1 +
 mk/rte.app.mk              | 5 ++---
 3 files changed, 4 insertions(+), 3 deletions(-)
  

Comments

Nicolás Pernas Maradei Oct. 21, 2015, 4:30 p.m. UTC | #1
Hi,

Are those the only two libraries with external dependencies? I took a 
quick look to the rte.app.mk file and there seem to be some others like 
-lfuse and -lnuma. Would it be possible to move those to their specific 
Makefiles as well?

Thanks,
Nico.

On 10/21/2015 03:18 PM, Panu Matilainen wrote:
> Similar to commit 113c8e13c4201eee207723571f83aaf285277d75, but
> for bnx2x and pcap PMDs.
>
> 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. This is especially highlighted
> with dlopen()'ed items, having applications link against about plugin
> internal dependencies goes on the side of absurd.
>
> Signed-off-by: Panu Matilainen <pmatilai@redhat.com>
> ---
>   drivers/net/bnx2x/Makefile | 1 +
>   drivers/net/pcap/Makefile  | 1 +
>   mk/rte.app.mk              | 5 ++---
>   3 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/bnx2x/Makefile b/drivers/net/bnx2x/Makefile
> index 87f31b6..d895d8c 100644
> --- a/drivers/net/bnx2x/Makefile
> +++ b/drivers/net/bnx2x/Makefile
> @@ -8,6 +8,7 @@ LIB = librte_pmd_bnx2x.a
>   CFLAGS += -O3 -g
>   CFLAGS += $(WERROR_FLAGS)
>   CFLAGS += -DZLIB_CONST
> +LDLIBS += -lz
>   
>   EXPORT_MAP := rte_pmd_bnx2x_version.map
>   
> diff --git a/drivers/net/pcap/Makefile b/drivers/net/pcap/Makefile
> index 48be913..b41d8a2 100644
> --- a/drivers/net/pcap/Makefile
> +++ b/drivers/net/pcap/Makefile
> @@ -39,6 +39,7 @@ LIB = librte_pmd_pcap.a
>   
>   CFLAGS += -O3
>   CFLAGS += $(WERROR_FLAGS)
> +LDLIBS += -lpcap
>   
>   EXPORT_MAP := rte_pmd_pcap_version.map
>   
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 9e1909e..c0d574f 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -90,7 +90,6 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lrte_vhost
>   
>   endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>   
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)       += -lpcap
>   
>   ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lnuma
> @@ -101,11 +100,11 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lfuse
>   endif
>   
>   ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)       += -lpcap
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lz
>   _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)       += -libverbs
>   endif # ! CONFIG_RTE_BUILD_SHARED_LIBS
>   
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lz
> -
>   _LDLIBS-y += --start-group
>   
>   ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
  
Panu Matilainen Oct. 22, 2015, 4:44 a.m. UTC | #2
On 10/21/2015 07:30 PM, Nicolas Pernas Maradei wrote:
> Hi,
>
> Are those the only two libraries with external dependencies? I took a
> quick look to the rte.app.mk file and there seem to be some others like
> -lfuse and -lnuma. Would it be possible to move those to their specific
> Makefiles as well?

AFAICS those were only remaining *drivers* with external dependencies.

The libraries have dependencies of their own like you noted, but they're 
more scattered, and things start getting more complicated because of 
CONFIG_RTE_BUILD_COMBINE_LIBS etc. I plan to get to that later when time 
permits but wanted to get the driver side out of the way because they're 
the worst offenders, and one driver already does this so the situation 
is inconsistent too.

	- Panu -
  

Patch

diff --git a/drivers/net/bnx2x/Makefile b/drivers/net/bnx2x/Makefile
index 87f31b6..d895d8c 100644
--- a/drivers/net/bnx2x/Makefile
+++ b/drivers/net/bnx2x/Makefile
@@ -8,6 +8,7 @@  LIB = librte_pmd_bnx2x.a
 CFLAGS += -O3 -g
 CFLAGS += $(WERROR_FLAGS)
 CFLAGS += -DZLIB_CONST
+LDLIBS += -lz
 
 EXPORT_MAP := rte_pmd_bnx2x_version.map
 
diff --git a/drivers/net/pcap/Makefile b/drivers/net/pcap/Makefile
index 48be913..b41d8a2 100644
--- a/drivers/net/pcap/Makefile
+++ b/drivers/net/pcap/Makefile
@@ -39,6 +39,7 @@  LIB = librte_pmd_pcap.a
 
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+LDLIBS += -lpcap
 
 EXPORT_MAP := rte_pmd_pcap_version.map
 
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 9e1909e..c0d574f 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -90,7 +90,6 @@  _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lrte_vhost
 
 endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
 
-_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)       += -lpcap
 
 ifeq ($(CONFIG_RTE_LIBRTE_VHOST_NUMA),y)
 _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lnuma
@@ -101,11 +100,11 @@  _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lfuse
 endif
 
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP)       += -lpcap
+_LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lz
 _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD)       += -libverbs
 endif # ! CONFIG_RTE_BUILD_SHARED_LIBS
 
-_LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD)      += -lz
-
 _LDLIBS-y += --start-group
 
 ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)