[dpdk-dev] [PATCH 1/2] mk: prevent overlinking in applications
On 6/9/2016 11:10 AM, Thomas Monjalon wrote: > Hi Ferruh, > > 2016-05-27 17:48, Ferruh Yigit: >> Replace --no-as-needed linker flag with --as-needed flag, which will >> only link libraries directly called by application. This requires inter >> library dependencies resolved correctly. >> >> Not linking all libraries cause a compile error for lpcap and possible >> to have other similar compiler errors, so increasing the scope of >> --start-group argument. > > What is the error? > This is with -as-needed flag, and static library compilation: .../dpdk/build/lib/librte_pmd_pcap.a(rte_eth_pcap.o): In function `eth_dev_stop': rte_eth_pcap.c:(.text+0xcc1): undefined reference to `pcap_dump_close' rte_eth_pcap.c:(.text+0xcd6): undefined reference to `pcap_close' rte_eth_pcap.c:(.text+0xd19): undefined reference to `pcap_close' rte_eth_pcap.c:(.text+0xd58): undefined reference to `pcap_close' /root/development/dpdk/build/lib/librte_pmd_pcap.a(rte_eth_pcap.o): In function `open_tx_pcap': rte_eth_pcap.c:(.text+0x190b): undefined reference to `pcap_open_dead' rte_eth_pcap.c:(.text+0x191b): undefined reference to `pcap_dump_open' ... pcap pmd has libpcap calls, but while linking final application (testpmd), linker is not able to find objects that has these. The command line for compilation is: gcc ... -o test ... -Wl,--whole-archive -Wl,-lpcap -Wl,--start-group -Wl,-lrte_pmd_pcap -Wl,--end-group -Wl,--no-whole-archive -lpcap is provided, but since before -lrte_pmd_pcap, references not resolved. Previously, with "-no-as-needed" flag, all shared libraries linked always, which was preventing compile error. >> cmdline_test application causes compile error because of cyclic >> dependency between librte_eal <-> librte_mempool. A workaround added to >> cmdline_test for compile error. >> >> Signed-off-by: Ferruh Yigit > >> --- a/app/cmdline_test/Makefile >> +++ b/app/cmdline_test/Makefile >> @@ -46,6 +46,7 @@ SRCS-y += commands.c >> >> CFLAGS += -O3 >> CFLAGS += $(WERROR_FLAGS) > > A comment is required here to explain the workaround. > Sure, I will add a comment and send a new version of the patch. >> +LDFLAGS += -no-as-needed > > The option should be --no-as-needed. Right > >> --- a/mk/exec-env/linuxapp/rte.vars.mk >> +++ b/mk/exec-env/linuxapp/rte.vars.mk >> @@ -46,7 +46,7 @@ EXECENV_CFLAGS = -pthread >> endif >> >> # Workaround lack of DT_NEEDED entry > > This comment is obsolete now. Will remove. > >> -EXECENV_LDFLAGS = --no-as-needed >> +EXECENV_LDFLAGS = --as-needed > > Why put this option for Linux only? When this option not defined at all, different compilers behave different, some has --as-needed as default and cause compilation issues, I guess that is why for Linux --no-as-needed is forces. I assume BSD compilers by default use --no-as-needed, so we don't have any compilation problem. And it looks like there is no specific reason to not force BSD to --as-needed, I will test it. > Shouldn't we restrict this option to app.mk only? Having --no-needed flag only for app should be OK. Let me test first, and as a result should we move this to app.mk? > >> --- a/mk/rte.app.mk >> +++ b/mk/rte.app.mk >> @@ -58,6 +58,7 @@ _LDLIBS-y += -L$(RTE_SDK_BIN)/lib >> # >> >> _LDLIBS-y += --whole-archive >> +_LDLIBS-y += --start-group > > --start-group must be used only to solve circular dependencies. > Ideally we should not use it. I think it's needed only because > of EAL logs using mempool (must be removed). No, this is not the reason, please check above lpcap compile error, that is the reason of this update. > Why extending it? > I'm afraid we are masking some issues. But if we have a convention to add external libraries after dpdk libraries, that also should solve this issue, but that is more update than just updating --start-group. > >> _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor >> _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder >> @@ -111,8 +112,6 @@ _LDLIBS-y += -lcrypto >> endif >> endif # !CONFIG_RTE_BUILD_SHARED_LIBS >> >> -_LDLIBS-y += --start-group >> - >> _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs >> _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf >> _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)+= -lrte_ip_frag >
[dpdk-dev] [PATCH 1/2] mk: prevent overlinking in applications
Hi Ferruh, 2016-05-27 17:48, Ferruh Yigit: > Replace --no-as-needed linker flag with --as-needed flag, which will > only link libraries directly called by application. This requires inter > library dependencies resolved correctly. > > Not linking all libraries cause a compile error for lpcap and possible > to have other similar compiler errors, so increasing the scope of > --start-group argument. What is the error? > cmdline_test application causes compile error because of cyclic > dependency between librte_eal <-> librte_mempool. A workaround added to > cmdline_test for compile error. > > Signed-off-by: Ferruh Yigit > --- a/app/cmdline_test/Makefile > +++ b/app/cmdline_test/Makefile > @@ -46,6 +46,7 @@ SRCS-y += commands.c > > CFLAGS += -O3 > CFLAGS += $(WERROR_FLAGS) A comment is required here to explain the workaround. > +LDFLAGS += -no-as-needed The option should be --no-as-needed. > --- a/mk/exec-env/linuxapp/rte.vars.mk > +++ b/mk/exec-env/linuxapp/rte.vars.mk > @@ -46,7 +46,7 @@ EXECENV_CFLAGS = -pthread > endif > > # Workaround lack of DT_NEEDED entry This comment is obsolete now. > -EXECENV_LDFLAGS = --no-as-needed > +EXECENV_LDFLAGS = --as-needed Why put this option for Linux only? Shouldn't we restrict this option to app.mk only? > --- a/mk/rte.app.mk > +++ b/mk/rte.app.mk > @@ -58,6 +58,7 @@ _LDLIBS-y += -L$(RTE_SDK_BIN)/lib > # > > _LDLIBS-y += --whole-archive > +_LDLIBS-y += --start-group --start-group must be used only to solve circular dependencies. Ideally we should not use it. I think it's needed only because of EAL logs using mempool (must be removed). Why extending it? I'm afraid we are masking some issues. > _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor > _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder > @@ -111,8 +112,6 @@ _LDLIBS-y += -lcrypto > endif > endif # !CONFIG_RTE_BUILD_SHARED_LIBS > > -_LDLIBS-y += --start-group > - > _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs > _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf > _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)+= -lrte_ip_frag
[dpdk-dev] [PATCH 1/2] mk: prevent overlinking in applications
Replace --no-as-needed linker flag with --as-needed flag, which will only link libraries directly called by application. This requires inter library dependencies resolved correctly. Not linking all libraries cause a compile error for lpcap and possible to have other similar compiler errors, so increasing the scope of --start-group argument. cmdline_test application causes compile error because of cyclic dependency between librte_eal <-> librte_mempool. A workaround added to cmdline_test for compile error. Signed-off-by: Ferruh Yigit --- This patch is on top of patch: http://dpdk.org/dev/patchwork/patch/12987/ --- app/cmdline_test/Makefile| 1 + mk/exec-env/linuxapp/rte.vars.mk | 2 +- mk/rte.app.mk| 3 +-- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/cmdline_test/Makefile b/app/cmdline_test/Makefile index c6169f5..5b7a2a2 100644 --- a/app/cmdline_test/Makefile +++ b/app/cmdline_test/Makefile @@ -46,6 +46,7 @@ SRCS-y += commands.c CFLAGS += -O3 CFLAGS += $(WERROR_FLAGS) +LDFLAGS += -no-as-needed # this application needs libraries first DEPDIRS-y += lib drivers diff --git a/mk/exec-env/linuxapp/rte.vars.mk b/mk/exec-env/linuxapp/rte.vars.mk index d51bd17..10d37d5 100644 --- a/mk/exec-env/linuxapp/rte.vars.mk +++ b/mk/exec-env/linuxapp/rte.vars.mk @@ -46,7 +46,7 @@ EXECENV_CFLAGS = -pthread endif # Workaround lack of DT_NEEDED entry -EXECENV_LDFLAGS = --no-as-needed +EXECENV_LDFLAGS = --as-needed EXECENV_LDLIBS = EXECENV_ASFLAGS = diff --git a/mk/rte.app.mk b/mk/rte.app.mk index b84b56d..e12226c 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -58,6 +58,7 @@ _LDLIBS-y += -L$(RTE_SDK_BIN)/lib # _LDLIBS-y += --whole-archive +_LDLIBS-y += --start-group _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)+= -lrte_distributor _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)+= -lrte_reorder @@ -111,8 +112,6 @@ _LDLIBS-y += -lcrypto endif endif # !CONFIG_RTE_BUILD_SHARED_LIBS -_LDLIBS-y += --start-group - _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS) += -lrte_kvargs _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF) += -lrte_mbuf _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)+= -lrte_ip_frag -- 2.5.5
