From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 35DCC2142 for ; Thu, 9 Jun 2016 15:06:05 +0200 (CEST) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP; 09 Jun 2016 06:05:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,444,1459839600"; d="scan'208";a="824828971" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.221.48]) ([10.237.221.48]) by orsmga003.jf.intel.com with ESMTP; 09 Jun 2016 06:05:14 -0700 To: Thomas Monjalon References: <574872B3.6040702@intel.com> <1464367686-3475-1-git-send-email-ferruh.yigit@intel.com> <7354386.zBuGXxEf9j@xps13> Cc: dev@dpdk.org, Panu Matilainen , Christian Ehrhardt From: Ferruh Yigit Message-ID: <57596989.50601@intel.com> Date: Thu, 9 Jun 2016 14:05:13 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 MIME-Version: 1.0 In-Reply-To: <7354386.zBuGXxEf9j@xps13> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 1/2] mk: prevent overlinking in applications X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Jun 2016 13:06:05 -0000 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 >