[dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd
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 Signed-off-by: Thomas Monjalon --- 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
[dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd
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 > Signed-off-by: Thomas Monjalon > --- > > 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
[dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd
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 > > Signed-off-by: Thomas Monjalon [...] > > --- 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
[dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd
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 > > > Signed-off-by: Thomas Monjalon > [...] > > > --- 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
[dpdk-dev] [PATCH v2] mk: fix build with shared pcap pmd
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 > > > > Signed-off-by: Thomas Monjalon > > [...] > > > > --- 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
