* [dpdk-dev] [pcap PATCH] pcap: fix build of all-in-one shared library
@ 2014-10-06 23:14 stepan
2014-12-08 14:49 ` [dpdk-dev] [dpdk-dev, pcap] " Neil Horman
0 siblings, 1 reply; 8+ messages in thread
From: stepan @ 2014-10-06 23:14 UTC (permalink / raw)
To: dev
Build of some of the test binaries fails when the following flags are enabled:
CONFIG_RTE_LIBRTE_PMD_PCAP=y
CONFIG_RTE_BUILD_SHARED_LIB=y
CONFIG_RTE_BUILD_COMBINE_LIBS=y
The binarieas are missing symbols from libpcap. This
patch adds the missing '-lpcap' linker flag into the respective Makefiles.
Signed-off-by: stepan <stepan.sojka@adaptivemobile.com>
---
app/cmdline_test/Makefile | 4 ++++
app/dump_cfg/Makefile | 4 ++++
app/test-acl/Makefile | 4 ++++
app/test/Makefile | 4 ++++
4 files changed, 16 insertions(+)
diff --git a/app/cmdline_test/Makefile b/app/cmdline_test/Makefile
index e9eafd2..ce44fd5 100644
--- a/app/cmdline_test/Makefile
+++ b/app/cmdline_test/Makefile
@@ -47,6 +47,10 @@ SRCS-y += commands.c
CFLAGS += -O3
CFLAGS += $(WERROR_FLAGS)
+ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
+LDFLAGS += -lpcap
+endif
+
include $(RTE_SDK)/mk/rte.app.mk
endif
diff --git a/app/dump_cfg/Makefile b/app/dump_cfg/Makefile
index 3257127..513ce59 100644
--- a/app/dump_cfg/Makefile
+++ b/app/dump_cfg/Makefile
@@ -35,6 +35,10 @@ APP = dump_cfg
CFLAGS += $(WERROR_FLAGS)
+ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
+LDFLAGS += -lpcap
+endif
+
# all source are stored in SRCS-y
SRCS-y := main.c
diff --git a/app/test-acl/Makefile b/app/test-acl/Makefile
index 43dfdcb..ea063fd 100644
--- a/app/test-acl/Makefile
+++ b/app/test-acl/Makefile
@@ -37,6 +37,10 @@ APP = testacl
CFLAGS += $(WERROR_FLAGS)
+ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
+LDFLAGS += -lpcap
+endif
+
# all source are stored in SRCS-y
SRCS-y := main.c
diff --git a/app/test/Makefile b/app/test/Makefile
index 6af6d76..e456140 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -145,6 +145,10 @@ CFLAGS_test_kni.o += -Wno-deprecated-declarations
endif
CFLAGS += -D_GNU_SOURCE
+ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
+LDFLAGS += -lpcap
+endif
+
# this application needs libraries first
DEPDIRS-y += lib
--
1.8.4.GIT
*****************************************This email and any files transmitted with are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you have received this email in error then please delete it and notify the sender. Do not make a copy or forward it to anyone. This footnote also confirms that this email message has been swept for the presence of computer viruses. Adaptive Mobile Security Ltd, Ferry House, 48 Lower Mount Street, Dublin 2, Ireland Directors: B. Collins, G. Maclachlan (UK), N. Grierson (UK), J. Ennis (UK), D. Summers (UK). Registered in Ireland, Company No. 370343, VAT Reg.No.IE6390343O*****************************************
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [dpdk-dev, pcap] pcap: fix build of all-in-one shared library
2014-10-06 23:14 [dpdk-dev] [pcap PATCH] pcap: fix build of all-in-one shared library stepan
@ 2014-12-08 14:49 ` Neil Horman
2014-12-15 22:42 ` Thomas Monjalon
0 siblings, 1 reply; 8+ messages in thread
From: Neil Horman @ 2014-12-08 14:49 UTC (permalink / raw)
To: stepan; +Cc: dev
On Tue, Oct 07, 2014 at 12:14:04AM +0100, stepan wrote:
> Build of some of the test binaries fails when the following flags are enabled:
>
> CONFIG_RTE_LIBRTE_PMD_PCAP=y
> CONFIG_RTE_BUILD_SHARED_LIB=y
> CONFIG_RTE_BUILD_COMBINE_LIBS=y
>
> The binarieas are missing symbols from libpcap. This
> patch adds the missing '-lpcap' linker flag into the respective Makefiles.
>
> Signed-off-by: stepan <stepan.sojka@adaptivemobile.com>
>
> ---
> app/cmdline_test/Makefile | 4 ++++
> app/dump_cfg/Makefile | 4 ++++
> app/test-acl/Makefile | 4 ++++
> app/test/Makefile | 4 ++++
> 4 files changed, 16 insertions(+)
>
> diff --git a/app/cmdline_test/Makefile b/app/cmdline_test/Makefile
> index e9eafd2..ce44fd5 100644
> --- a/app/cmdline_test/Makefile
> +++ b/app/cmdline_test/Makefile
> @@ -47,6 +47,10 @@ SRCS-y += commands.c
> CFLAGS += -O3
> CFLAGS += $(WERROR_FLAGS)
>
> +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> +LDFLAGS += -lpcap
> +endif
> +
> include $(RTE_SDK)/mk/rte.app.mk
>
> endif
> diff --git a/app/dump_cfg/Makefile b/app/dump_cfg/Makefile
> index 3257127..513ce59 100644
> --- a/app/dump_cfg/Makefile
> +++ b/app/dump_cfg/Makefile
> @@ -35,6 +35,10 @@ APP = dump_cfg
>
> CFLAGS += $(WERROR_FLAGS)
>
> +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> +LDFLAGS += -lpcap
> +endif
> +
> # all source are stored in SRCS-y
>
> SRCS-y := main.c
> diff --git a/app/test-acl/Makefile b/app/test-acl/Makefile
> index 43dfdcb..ea063fd 100644
> --- a/app/test-acl/Makefile
> +++ b/app/test-acl/Makefile
> @@ -37,6 +37,10 @@ APP = testacl
>
> CFLAGS += $(WERROR_FLAGS)
>
> +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> +LDFLAGS += -lpcap
> +endif
> +
> # all source are stored in SRCS-y
> SRCS-y := main.c
>
> diff --git a/app/test/Makefile b/app/test/Makefile
> index 6af6d76..e456140 100644
> --- a/app/test/Makefile
> +++ b/app/test/Makefile
> @@ -145,6 +145,10 @@ CFLAGS_test_kni.o += -Wno-deprecated-declarations
> endif
> CFLAGS += -D_GNU_SOURCE
>
> +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> +LDFLAGS += -lpcap
> +endif
> +
> # this application needs libraries first
> DEPDIRS-y += lib
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [dpdk-dev, pcap] pcap: fix build of all-in-one shared library
2014-12-08 14:49 ` [dpdk-dev] [dpdk-dev, pcap] " Neil Horman
@ 2014-12-15 22:42 ` Thomas Monjalon
2014-12-15 23:04 ` [dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd Thomas Monjalon
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2014-12-15 22:42 UTC (permalink / raw)
To: Neil Horman, stepan; +Cc: dev
2014-12-08 09:49, Neil Horman:
> On Tue, Oct 07, 2014 at 12:14:04AM +0100, stepan wrote:
> > Build of some of the test binaries fails when the following flags are enabled:
> >
> > CONFIG_RTE_LIBRTE_PMD_PCAP=y
> > CONFIG_RTE_BUILD_SHARED_LIB=y
> > CONFIG_RTE_BUILD_COMBINE_LIBS=y
> >
> > The binarieas are missing symbols from libpcap. This
> > patch adds the missing '-lpcap' linker flag into the respective Makefiles.
> >
> > Signed-off-by: stepan <stepan.sojka@adaptivemobile.com>
> >
> > ---
> > app/cmdline_test/Makefile | 4 ++++
> > app/dump_cfg/Makefile | 4 ++++
> > app/test-acl/Makefile | 4 ++++
> > app/test/Makefile | 4 ++++
> > 4 files changed, 16 insertions(+)
> >
> > diff --git a/app/cmdline_test/Makefile b/app/cmdline_test/Makefile
> > index e9eafd2..ce44fd5 100644
> > --- a/app/cmdline_test/Makefile
> > +++ b/app/cmdline_test/Makefile
> > @@ -47,6 +47,10 @@ SRCS-y += commands.c
> > CFLAGS += -O3
> > CFLAGS += $(WERROR_FLAGS)
> >
> > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > +LDFLAGS += -lpcap
> > +endif
> > +
> > include $(RTE_SDK)/mk/rte.app.mk
> >
> > endif
> > diff --git a/app/dump_cfg/Makefile b/app/dump_cfg/Makefile
> > index 3257127..513ce59 100644
> > --- a/app/dump_cfg/Makefile
> > +++ b/app/dump_cfg/Makefile
> > @@ -35,6 +35,10 @@ APP = dump_cfg
> >
> > CFLAGS += $(WERROR_FLAGS)
> >
> > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > +LDFLAGS += -lpcap
> > +endif
> > +
> > # all source are stored in SRCS-y
> >
> > SRCS-y := main.c
> > diff --git a/app/test-acl/Makefile b/app/test-acl/Makefile
> > index 43dfdcb..ea063fd 100644
> > --- a/app/test-acl/Makefile
> > +++ b/app/test-acl/Makefile
> > @@ -37,6 +37,10 @@ APP = testacl
> >
> > CFLAGS += $(WERROR_FLAGS)
> >
> > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > +LDFLAGS += -lpcap
> > +endif
> > +
> > # all source are stored in SRCS-y
> > SRCS-y := main.c
> >
> > diff --git a/app/test/Makefile b/app/test/Makefile
> > index 6af6d76..e456140 100644
> > --- a/app/test/Makefile
> > +++ b/app/test/Makefile
> > @@ -145,6 +145,10 @@ CFLAGS_test_kni.o += -Wno-deprecated-declarations
> > endif
> > CFLAGS += -D_GNU_SOURCE
> >
> > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > +LDFLAGS += -lpcap
> > +endif
> > +
> > # this application needs libraries first
> > DEPDIRS-y += lib
> >
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
NAK, it doesn't seem to be the proper fix.
LDFLAGS should be automatically filled in rte.vars.mk.
Will send another proposal.
--
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
* [dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd
2014-12-15 22:42 ` Thomas Monjalon
@ 2014-12-15 23:04 ` Thomas Monjalon
2014-12-16 13:58 ` Neil Horman
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2014-12-15 23:04 UTC (permalink / raw)
To: dev
Some applications doesn't have the pcap link flag
when shared libraries are enabled.
Indeed in such case, pcap PMD must not be linked but pcap library should.
Actually -lpcap is always needed if pcap PMD is used,
and -lrte_pmd_pcap must be set only with static PMD library.
So the flags -lrte_pmd_pcap and -lpcap are enabled separately.
Workarounds in test-pmd/ and test-pipeline/ can be removed.
Reported-by: Stepan Sojka <stepan.sojka@adaptivemobile.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
v2:
fix in rte.app.mk
v1 from Stepan:
fix every applications
app/test-pipeline/Makefile | 4 ----
app/test-pmd/Makefile | 4 ----
mk/rte.app.mk | 6 +++++-
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/app/test-pipeline/Makefile b/app/test-pipeline/Makefile
index b81652f..aa6df0c 100644
--- a/app/test-pipeline/Makefile
+++ b/app/test-pipeline/Makefile
@@ -41,10 +41,6 @@ APP = testpipeline
CFLAGS += -O3
CFLAGS += $(WERROR_FLAGS)
-ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
-LDFLAGS += -lpcap
-endif
-
#
# all source are stored in SRCS-y
#
diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
index 97dc2e6..dcf26f4 100644
--- a/app/test-pmd/Makefile
+++ b/app/test-pmd/Makefile
@@ -41,10 +41,6 @@ APP = testpmd
CFLAGS += -O3
CFLAGS += $(WERROR_FLAGS)
-ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
-LDFLAGS += -lpcap
-endif
-
#
# all source are stored in SRCS-y
#
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 84ec4df..c5eaf82 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -119,6 +119,10 @@ LDLIBS += -lm
LDLIBS += -lrt
endif
+ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
+LDLIBS += -lpcap
+endif
+
LDLIBS += --start-group
ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
@@ -207,7 +211,7 @@ LDLIBS += -lrte_pmd_ring
endif
ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
-LDLIBS += -lrte_pmd_pcap -lpcap
+LDLIBS += -lrte_pmd_pcap
endif
ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
--
2.1.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd
2014-12-15 23:04 ` [dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd Thomas Monjalon
@ 2014-12-16 13:58 ` Neil Horman
2014-12-16 14:39 ` Thomas Monjalon
0 siblings, 1 reply; 8+ messages in thread
From: Neil Horman @ 2014-12-16 13:58 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
On Tue, Dec 16, 2014 at 12:04:44AM +0100, Thomas Monjalon wrote:
> Some applications doesn't have the pcap link flag
> when shared libraries are enabled.
> Indeed in such case, pcap PMD must not be linked but pcap library should.
>
> Actually -lpcap is always needed if pcap PMD is used,
> and -lrte_pmd_pcap must be set only with static PMD library.
> So the flags -lrte_pmd_pcap and -lpcap are enabled separately.
>
> Workarounds in test-pmd/ and test-pipeline/ can be removed.
>
> Reported-by: Stepan Sojka <stepan.sojka@adaptivemobile.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
>
> v2:
> fix in rte.app.mk
>
> v1 from Stepan:
> fix every applications
>
> app/test-pipeline/Makefile | 4 ----
> app/test-pmd/Makefile | 4 ----
> mk/rte.app.mk | 6 +++++-
> 3 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/app/test-pipeline/Makefile b/app/test-pipeline/Makefile
> index b81652f..aa6df0c 100644
> --- a/app/test-pipeline/Makefile
> +++ b/app/test-pipeline/Makefile
> @@ -41,10 +41,6 @@ APP = testpipeline
> CFLAGS += -O3
> CFLAGS += $(WERROR_FLAGS)
>
> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> -LDFLAGS += -lpcap
> -endif
> -
> #
> # all source are stored in SRCS-y
> #
> diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
> index 97dc2e6..dcf26f4 100644
> --- a/app/test-pmd/Makefile
> +++ b/app/test-pmd/Makefile
> @@ -41,10 +41,6 @@ APP = testpmd
> CFLAGS += -O3
> CFLAGS += $(WERROR_FLAGS)
>
> -ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> -LDFLAGS += -lpcap
> -endif
> -
> #
> # all source are stored in SRCS-y
> #
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 84ec4df..c5eaf82 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -119,6 +119,10 @@ LDLIBS += -lm
> LDLIBS += -lrt
> endif
>
> +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> +LDLIBS += -lpcap
> +endif
> +
> LDLIBS += --start-group
>
> ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
> @@ -207,7 +211,7 @@ LDLIBS += -lrte_pmd_ring
> endif
>
> ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> -LDLIBS += -lrte_pmd_pcap -lpcap
> +LDLIBS += -lrte_pmd_pcap
> endif
>
> ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
> --
> 2.1.3
>
>
Actually, what if we just add $(LDFLAGS) to the O_TO_S rule in mk/rte.lib.mk?
Then in lib/librte_pmd_pcap/Makefile, we can just add LDFLAGS+=-lpcap, and the
loading of the pcap pmd will itself require the loading of libpcap. That would
be a nice clean implementation that allows applications to just link the pmd and
not have to worry about dependencies. It would also allow us to clean up other
dependencies like the xenvirt pmd and vhost.
Neil
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd
2014-12-16 13:58 ` Neil Horman
@ 2014-12-16 14:39 ` Thomas Monjalon
2014-12-16 21:42 ` Neil Horman
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Monjalon @ 2014-12-16 14:39 UTC (permalink / raw)
To: Neil Horman; +Cc: dev
2014-12-16 08:58, Neil Horman:
> On Tue, Dec 16, 2014 at 12:04:44AM +0100, Thomas Monjalon wrote:
> > Some applications doesn't have the pcap link flag
> > when shared libraries are enabled.
> > Indeed in such case, pcap PMD must not be linked but pcap library should.
> >
> > Actually -lpcap is always needed if pcap PMD is used,
> > and -lrte_pmd_pcap must be set only with static PMD library.
> > So the flags -lrte_pmd_pcap and -lpcap are enabled separately.
> >
> > Workarounds in test-pmd/ and test-pipeline/ can be removed.
> >
> > Reported-by: Stepan Sojka <stepan.sojka@adaptivemobile.com>
> > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
[...]
> > --- a/mk/rte.app.mk
> > +++ b/mk/rte.app.mk
> > @@ -119,6 +119,10 @@ LDLIBS += -lm
> > LDLIBS += -lrt
> > endif
> >
> > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > +LDLIBS += -lpcap
> > +endif
> > +
> > LDLIBS += --start-group
> >
> > ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
> > @@ -207,7 +211,7 @@ LDLIBS += -lrte_pmd_ring
> > endif
> >
> > ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > -LDLIBS += -lrte_pmd_pcap -lpcap
> > +LDLIBS += -lrte_pmd_pcap
> > endif
> >
> > ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
>
> Actually, what if we just add $(LDFLAGS) to the O_TO_S rule in mk/rte.lib.mk?
> Then in lib/librte_pmd_pcap/Makefile, we can just add LDFLAGS+=-lpcap, and the
> loading of the pcap pmd will itself require the loading of libpcap. That would
> be a nice clean implementation that allows applications to just link the pmd and
> not have to worry about dependencies. It would also allow us to clean up other
> dependencies like the xenvirt pmd and vhost.
Yes it makes sense. Could you test it please?
What about applying my patch (which keep the existing logic) as a first
fix/clean-up and then move -lpcap in PMD as a second step?
Proceeding this way would allow to integrate a safe fix for 1.8.0.
Maybe that linking pcap in the PMD could unveil new bugs with some distributions,
so it would need some time to validate it.
--
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd
2014-12-16 14:39 ` Thomas Monjalon
@ 2014-12-16 21:42 ` Neil Horman
2014-12-16 23:37 ` Thomas Monjalon
0 siblings, 1 reply; 8+ messages in thread
From: Neil Horman @ 2014-12-16 21:42 UTC (permalink / raw)
To: Thomas Monjalon; +Cc: dev
On Tue, Dec 16, 2014 at 03:39:56PM +0100, Thomas Monjalon wrote:
> 2014-12-16 08:58, Neil Horman:
> > On Tue, Dec 16, 2014 at 12:04:44AM +0100, Thomas Monjalon wrote:
> > > Some applications doesn't have the pcap link flag
> > > when shared libraries are enabled.
> > > Indeed in such case, pcap PMD must not be linked but pcap library should.
> > >
> > > Actually -lpcap is always needed if pcap PMD is used,
> > > and -lrte_pmd_pcap must be set only with static PMD library.
> > > So the flags -lrte_pmd_pcap and -lpcap are enabled separately.
> > >
> > > Workarounds in test-pmd/ and test-pipeline/ can be removed.
> > >
> > > Reported-by: Stepan Sojka <stepan.sojka@adaptivemobile.com>
> > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> [...]
> > > --- a/mk/rte.app.mk
> > > +++ b/mk/rte.app.mk
> > > @@ -119,6 +119,10 @@ LDLIBS += -lm
> > > LDLIBS += -lrt
> > > endif
> > >
> > > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > > +LDLIBS += -lpcap
> > > +endif
> > > +
> > > LDLIBS += --start-group
> > >
> > > ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
> > > @@ -207,7 +211,7 @@ LDLIBS += -lrte_pmd_ring
> > > endif
> > >
> > > ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > > -LDLIBS += -lrte_pmd_pcap -lpcap
> > > +LDLIBS += -lrte_pmd_pcap
> > > endif
> > >
> > > ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
> >
> > Actually, what if we just add $(LDFLAGS) to the O_TO_S rule in mk/rte.lib.mk?
> > Then in lib/librte_pmd_pcap/Makefile, we can just add LDFLAGS+=-lpcap, and the
> > loading of the pcap pmd will itself require the loading of libpcap. That would
> > be a nice clean implementation that allows applications to just link the pmd and
> > not have to worry about dependencies. It would also allow us to clean up other
> > dependencies like the xenvirt pmd and vhost.
>
> Yes it makes sense. Could you test it please?
> What about applying my patch (which keep the existing logic) as a first
> fix/clean-up and then move -lpcap in PMD as a second step?
> Proceeding this way would allow to integrate a safe fix for 1.8.0.
> Maybe that linking pcap in the PMD could unveil new bugs with some distributions,
> so it would need some time to validate it.
>
> --
> Thomas
>
ACK, I'm fine with your patch currently. I'll revisit this after 1.8 is
released
Neil
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd
2014-12-16 21:42 ` Neil Horman
@ 2014-12-16 23:37 ` Thomas Monjalon
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2014-12-16 23:37 UTC (permalink / raw)
To: Neil Horman; +Cc: dev
2014-12-16 16:42, Neil Horman:
> On Tue, Dec 16, 2014 at 03:39:56PM +0100, Thomas Monjalon wrote:
> > 2014-12-16 08:58, Neil Horman:
> > > On Tue, Dec 16, 2014 at 12:04:44AM +0100, Thomas Monjalon wrote:
> > > > Some applications doesn't have the pcap link flag
> > > > when shared libraries are enabled.
> > > > Indeed in such case, pcap PMD must not be linked but pcap library should.
> > > >
> > > > Actually -lpcap is always needed if pcap PMD is used,
> > > > and -lrte_pmd_pcap must be set only with static PMD library.
> > > > So the flags -lrte_pmd_pcap and -lpcap are enabled separately.
> > > >
> > > > Workarounds in test-pmd/ and test-pipeline/ can be removed.
> > > >
> > > > Reported-by: Stepan Sojka <stepan.sojka@adaptivemobile.com>
> > > > Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> > [...]
> > > > --- a/mk/rte.app.mk
> > > > +++ b/mk/rte.app.mk
> > > > @@ -119,6 +119,10 @@ LDLIBS += -lm
> > > > LDLIBS += -lrt
> > > > endif
> > > >
> > > > +ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > > > +LDLIBS += -lpcap
> > > > +endif
> > > > +
> > > > LDLIBS += --start-group
> > > >
> > > > ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
> > > > @@ -207,7 +211,7 @@ LDLIBS += -lrte_pmd_ring
> > > > endif
> > > >
> > > > ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> > > > -LDLIBS += -lrte_pmd_pcap -lpcap
> > > > +LDLIBS += -lrte_pmd_pcap
> > > > endif
> > > >
> > > > ifeq ($(CONFIG_RTE_LIBRTE_PMD_AF_PACKET),y)
> > >
> > > Actually, what if we just add $(LDFLAGS) to the O_TO_S rule in mk/rte.lib.mk?
> > > Then in lib/librte_pmd_pcap/Makefile, we can just add LDFLAGS+=-lpcap, and the
> > > loading of the pcap pmd will itself require the loading of libpcap. That would
> > > be a nice clean implementation that allows applications to just link the pmd and
> > > not have to worry about dependencies. It would also allow us to clean up other
> > > dependencies like the xenvirt pmd and vhost.
> >
> > Yes it makes sense. Could you test it please?
> > What about applying my patch (which keep the existing logic) as a first
> > fix/clean-up and then move -lpcap in PMD as a second step?
> > Proceeding this way would allow to integrate a safe fix for 1.8.0.
> > Maybe that linking pcap in the PMD could unveil new bugs with some distributions,
> > so it would need some time to validate it.
> >
> ACK, I'm fine with your patch currently. I'll revisit this after 1.8 is
> released
> Neil
Applied
--
Thomas
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-12-16 23:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-06 23:14 [dpdk-dev] [pcap PATCH] pcap: fix build of all-in-one shared library stepan
2014-12-08 14:49 ` [dpdk-dev] [dpdk-dev, pcap] " Neil Horman
2014-12-15 22:42 ` Thomas Monjalon
2014-12-15 23:04 ` [dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd Thomas Monjalon
2014-12-16 13:58 ` Neil Horman
2014-12-16 14:39 ` Thomas Monjalon
2014-12-16 21:42 ` Neil Horman
2014-12-16 23:37 ` Thomas Monjalon
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).