pcap: fix build of all-in-one shared library

PATCH NAK stepan 3 participants 8 messages Oct 6, 2014 – Dec 16, 2014 5 messages not in the archive

lore

Share & tools

lore

stepan

author

2014-10-06 23:14 UTC #0 lore raw

PATCH 1/1 pcap: fix build of all-in-one shared library

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 <[email protected]>

---
 app/cmdline_test/Makefile | 4 ++++
 app/dump_cfg/Makefile     | 4 ++++
 app/test-acl/Makefile     | 4 ++++
 app/test/Makefile         | 4 ++++
 4 files changed, 16 insertions(+)

app/cmdline_test/Makefile +4 -0

index e9eafd2..ce44fd5 100644
@@ -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

app/dump_cfg/Makefile +4 -0

index 3257127..513ce59 100644
@@ -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

app/test-acl/Makefile +4 -0

index 43dfdcb..ea063fd 100644
@@ -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

app/test/Makefile +4 -0

index 6af6d76..e456140 100644
@@ -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

Signature · 3 lines

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Neil Horman

Acked-by

2014-12-08 14:49 UTC #2 lore raw

Quoted 68 lines · stepan

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 <[email protected]>
---
app/cmdline_test/Makefile | 4 ++++
 app/dump_cfg/Makefile     | 4 ++++
 app/test-acl/Makefile     | 4 ++++
 app/test/Makefile         | 4 ++++
 4 files changed, 16 insertions(+)
app/cmdline_test/Makefile +4 -0
index e9eafd2..ce44fd5 100644
@@ -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
app/dump_cfg/Makefile +4 -0
index 3257127..513ce59 100644
@@ -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
app/test-acl/Makefile +4 -0
index 43dfdcb..ea063fd 100644
@@ -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
 
app/test/Makefile +0 -0
index 6af6d76..e456140 100644

app/test/Makefile CFLAGS_test_kni.o += -Wno-deprecated-declarations · line 154

Quoted 10 lines

app/test/Makefile +4 -0

@@ -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 <[email protected]>

A message that is not in the archive (sent off-list or not yet received): 20141208144910.GE3237-bi+AKbBUZKY6gyzm1THtWbp2dZbC/[email protected]

Thomas Monjalon

NAK

2014-12-15 22:42 UTC #4 lore raw

2014-12-08 09:49, Neil Horman:

Quoted 72 lines · stepan

Quoted 71 lines
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 <[email protected]>
---
app/cmdline_test/Makefile | 4 ++++
 app/dump_cfg/Makefile     | 4 ++++
 app/test-acl/Makefile     | 4 ++++
 app/test/Makefile         | 4 ++++
 4 files changed, 16 insertions(+)
app/cmdline_test/Makefile +4 -0
index e9eafd2..ce44fd5 100644
@@ -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
app/dump_cfg/Makefile +4 -0
index 3257127..513ce59 100644
@@ -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
app/test-acl/Makefile +4 -0
index 43dfdcb..ea063fd 100644
@@ -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
 
app/test/Makefile +0 -0
index 6af6d76..e456140 100644
@@ -145,6 +145,10 @@ CFLAGS_test_kni.o += -Wno-deprecated-declarations
 endif
 CFLAGS += -D_GNU_SOURCE
 

Quoted 8 lines

app/test/Makefile CFLAGS_test_kni.o += -Wno-deprecated-declarations · line 154
Quoted 7 lines
app/test/Makefile +4 -0
+ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
+LDFLAGS += -lpcap
+endif
+
 # this application needs libraries first
 DEPDIRS-y += lib
 
Acked-by Neil Horman <[email protected]>

NAK, it doesn't seem to be the proper fix. LDFLAGS should be automatically filled in rte.vars.mk. Will send another proposal.

Signature · 1 line

Thomas

Thomas Monjalon

2014-12-15 23:04 UTC #5 lore raw

PATCH v2 1/1 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 <[email protected]>

Signed-off-by Thomas Monjalon <[email protected]>

---

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

app/test-pipeline/Makefile +0 -4

index b81652f..aa6df0c 100644
@@ -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
 #

app/test-pmd/Makefile +0 -4

index 97dc2e6..dcf26f4 100644
@@ -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
 #

mk/rte.app.mk +5 -1

index 84ec4df..c5eaf82 100644
@@ -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)

Signature · 1 line

2.1.3

A message that is not in the archive (sent off-list or not yet received): 1418684684-23642-1-git-send-email-thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org

Neil Horman

2014-12-16 13:58 UTC #7 lore raw

Re: [PATCH v2] mk: fix build with shared pcap pmd

Quoted 83 lines · Thomas Monjalon

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 <[email protected]>
Signed-off-by Thomas Monjalon <[email protected]>
---
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(-)
app/test-pipeline/Makefile +0 -4
index b81652f..aa6df0c 100644
@@ -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
 #
app/test-pmd/Makefile +0 -4
index 97dc2e6..dcf26f4 100644
@@ -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
 #
mk/rte.app.mk +5 -1
index 84ec4df..c5eaf82 100644
@@ -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)
Signature · 1 line
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

A message that is not in the archive (sent off-list or not yet received): 20141216135822.GC13806-B26myB8xz7F8NnZeBjwnZQMhkBWG/[email protected]

Thomas Monjalon

2014-12-16 14:39 UTC #9 lore raw

Re: [PATCH v2] mk: fix build with shared pcap pmd

2014-12-16 08:58, Neil Horman:

Quoted 13 lines · Thomas Monjalon

Quoted 12 lines
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 <[email protected]>
Signed-off-by Thomas Monjalon <[email protected]>

[...]

Quoted 21 lines

mk/rte.app.mk +5 -1

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

Quoted 8 lines

mk/rte.app.mk LDLIBS += -lrte_pmd_ring · line 217
Quoted 1 line
mk/rte.app.mk +0 -0
 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.

Signature · 1 line

Thomas

Neil Horman

2014-12-16 21:42 UTC #10 lore raw

Re: [PATCH v2] mk: fix build with shared pcap pmd

Quoted 45 lines · Thomas Monjalon

2014-12-16 08:58, Neil Horman:
Quoted 13 lines · Thomas Monjalon
Quoted 12 lines
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 <[email protected]>
Signed-off-by Thomas Monjalon <[email protected]>
[...]
Quoted 21 lines
Quoted 21 lines
mk/rte.app.mk +5 -1
@@ -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
 
Quoted 8 lines
mk/rte.app.mk LDLIBS += -lrte_pmd_ring · line 217
Quoted 1 line
mk/rte.app.mk +0 -0
 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.

Quoted 10 lines

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.
Signature · 1 line
Thomas

ACK, I'm fine with your patch currently. I'll revisit this after 1.8 is released
Neil

A message that is not in the archive (sent off-list or not yet received): 20141216214213.GF13806-B26myB8xz7F8NnZeBjwnZQMhkBWG/[email protected]

Thomas Monjalon

Applied

2014-12-16 23:37 UTC #12 lore raw

Re: [PATCH v2] mk: fix build with shared pcap pmd

2014-12-16 16:42, Neil Horman:

Quoted 53 lines · Thomas Monjalon

Quoted 45 lines
2014-12-16 08:58, Neil Horman:
Quoted 13 lines · Thomas Monjalon
Quoted 12 lines
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 <[email protected]>
Signed-off-by Thomas Monjalon <[email protected]>
[...]
Quoted 21 lines
Quoted 21 lines
mk/rte.app.mk +5 -1
@@ -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
 
Quoted 8 lines
mk/rte.app.mk LDLIBS += -lrte_pmd_ring · line 217
Quoted 1 line
mk/rte.app.mk +0 -0
 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.
Quoted 7 lines
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.

Quoted 3 lines

ACK, I'm fine with your patch currently. I'll revisit this after 1.8 is released
Neil

Applied

Signature · 1 line

Thomas