From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id DA7AA2C37 for ; Wed, 4 Jan 2017 12:44:23 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga101.fm.intel.com with ESMTP; 04 Jan 2017 03:44:22 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,459,1477983600"; d="scan'208";a="209512936" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.38]) ([10.237.220.38]) by fmsmga004.fm.intel.com with ESMTP; 04 Jan 2017 03:44:21 -0800 To: Jerin Jacob References: <1482833398-30145-1-git-send-email-jerin.jacob@caviumnetworks.com> <1482833398-30145-2-git-send-email-jerin.jacob@caviumnetworks.com> <20170104110144.GA8258@localhost.localdomain> Cc: dev@dpdk.org, konstantin.ananyev@intel.com, helin.zhang@intel.com, thomas.monjalon@6wind.com From: Ferruh Yigit Message-ID: <408187b4-83b0-603f-011b-583d5d0e9716@intel.com> Date: Wed, 4 Jan 2017 11:44:21 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <20170104110144.GA8258@localhost.localdomain> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH 2/2] app/testpmd: remove explicit ixgbe link request X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 04 Jan 2017 11:44:24 -0000 On 1/4/2017 11:01 AM, Jerin Jacob wrote: > On Tue, Jan 03, 2017 at 01:30:26PM +0000, Ferruh Yigit wrote: >> On 12/27/2016 10:09 AM, Jerin Jacob wrote: >>> Removed explicit ixgbe driver linkage request from >>> app/testpmd makefile to mk/rte.app.mk to >>> 1)Maintain the correct link ordering(from higher level libraries >>> to lower level libraries) >>> 2)In shared lib configuration, any application can use ixgbe >>> exposed pmd specific APIs not just testpmd. >> >> In testpmd, "explicit ixgbe driver linkage request" added because >> testpmd uses ixgbe PMD specific APIs. >> >> Overall, that line is for shared library, for static library result >> should be same. > > Unfortunately for the static library, the result is different.You can > check the testpmd.map file for the effects linking rte_pmd_ixgbe first. This was unintentional/unexpected, thanks for catching this. > > I found this while debugging the a strange issue where including ixgbe > build is dropping 300kpps/core on thunderx pmd.Finally, git bisected and saw > the following check-in causes this > > commit 425781ff5afe08b77c58ec5e4d5cf56b9ac19e02 > Author: Bernard Iremonger > Date: Wed Oct 12 18:54:12 2016 +0100 > > app/testpmd: add ixgbe VF management > > Nothing wrong in this check-in wrt functionality, but moving rte_pmd_ixgbe > to first is completely changing the symbol generation for the static build. I can guess the reason. Now "-Wl,-lrte_pmd_ixgbe" provided twice, and build system removes the duplication by keeping only first occurrence. This moves "-Wl,-lrte_pmd_ixgbe" out of "-Wl,--whole-archive" flag. Since PMD API not directly called, but register themselves via constructors, and initialized in runtime using registered callbacks, without "--whole-archive" some APIs from that PMD may be missing in final application. > > ---- > gcc -o testpmd -m64 -pthread -march=native -DRTE_MACHINE_CPUFLAG_SSE > > [snip] > > -Wcast-qual -Wformat-nonliteral -Wformat-security -Wundef > -Wwrite-strings -Werror testpmd.o parameters.o cmdline.o cmdline_flow.o > config.o iofwd.o macfwd.o macswap.o flowgen.o rxonly.o txonly.o > csumonly.o icmpecho.o -Wl,-lrte_pmd_ixgbe > ^^^^^^^^^^^^^^^^^^^^^^^^^ > -L/export/dpdk-master/build/lib -Wl,-lrte_kni -Wl,-lrte_pipeline > -Wl,-lrte_table -Wl,-lrte_port -Wl,-lrte_pdump -Wl,-lrte_distributor > -Wl,-lrte_reorder -Wl,-lrte_ip_frag -Wl,-lrte_meter -Wl,-lrte_sched > -Wl,-lrte_lpm -Wl,--whole-archive -Wl,-lrte_acl -Wl,--no-whole-archive > -Wl,-lrte_jobstats -Wl,-lrte_power -Wl,--whole-archive -Wl,-lrte_timer > -Wl,-lrte_hash -Wl,-lrte_vhost -Wl,-lrte_kvargs -Wl,-lrte_mbuf > -Wl,-lrte_net -Wl,-lrte_ethdev -Wl,-lrte_cryptodev -Wl,-lrte_eventdev > -Wl,-lrte_mempool -Wl,-lrte_ring -Wl,-lrte_eal -Wl,-lrte_cmdline > -Wl,-lrte_cfgfile -Wl,-lrte_pmd_bond -Wl,-lrte_pmd_af_packet > -Wl,-lrte_pmd_bnxt -Wl,-lrte_pmd_cxgbe -Wl,-lrte_pmd_e1000 > -Wl,-lrte_pmd_ena -Wl,-lrte_pmd_enic -Wl,-lrte_pmd_fm10k > -Wl,-lrte_pmd_i40e -Wl,-lrte_pmd_null -Wl,-lrte_pmd_qede > -Wl,-lrte_pmd_ring -Wl,-lrte_pmd_virtio -Wl,-lrte_pmd_vhost > -Wl,-lrte_pmd_vmxnet3_uio -Wl,-lrte_pmd_null_crypto > -Wl,-lrte_pmd_skeleton_event -Wl,--no-whole-archive -Wl,-lrt -Wl,-lm > -Wl,-ldl -Wl,-export-dynamic -Wl,-export-dynamic -Wl,-export-dynamic > -L/export/dpdk-master/build/lib -Wl,--as-needed -Wl,-Map=testpmd.map > -Wl,--cref > ---- > >> >> I believe it is good to keep it in testpmd Makefile, updating rte.app.mk >> to have it will: >> - link library to the applications which does not use PMD specific APIs >> and want to load PMD dynamically. >> - link library to the application that won't use driver at all. This may >> break the distributed binaries, since testpmd will now be dependent to a >> specific PMD. > > No strong opinion here as it is specific to ixgbe. But can we include > ixgbe only for shared library in testpmd so that it won't effect any > symbol generation in static build. I think this is better, I am OK with below patch, thanks. > > > [dpdk-master] $ git diff > diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile > index 5988c3e..050663a 100644 > --- a/app/test-pmd/Makefile > +++ b/app/test-pmd/Makefile > @@ -59,7 +59,9 @@ SRCS-y += csumonly.c > SRCS-y += icmpecho.c > SRCS-$(CONFIG_RTE_LIBRTE_IEEE1588) += ieee1588fwd.c > > +ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y) > _LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe > +endif > > CFLAGS_cmdline.o := -D_GNU_SOURCE > > >> >>> >>> Signed-off-by: Jerin Jacob >>> --- >>> app/test-pmd/Makefile | 2 -- >>> mk/rte.app.mk | 2 +- >>> 2 files changed, 1 insertion(+), 3 deletions(-) >>> >>> diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile >>> index 5988c3e..96e0c67 100644 >>> --- a/app/test-pmd/Makefile >>> +++ b/app/test-pmd/Makefile >>> @@ -59,8 +59,6 @@ SRCS-y += csumonly.c >>> SRCS-y += icmpecho.c >>> SRCS-$(CONFIG_RTE_LIBRTE_IEEE1588) += ieee1588fwd.c >>> >>> -_LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe >>> - >>> CFLAGS_cmdline.o := -D_GNU_SOURCE >>> >>> # this application needs libraries first >>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk >>> index f75f0e2..aee235c 100644 >>> --- a/mk/rte.app.mk >>> +++ b/mk/rte.app.mk >>> @@ -101,6 +101,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile >>> >>> _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond >>> _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt -lxenstore >>> +_LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe >>> >>> ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) >>> # plugins (link only if static libraries) >>> @@ -114,7 +115,6 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += -lrte_pmd_ena >>> _LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += -lrte_pmd_enic >>> _LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += -lrte_pmd_fm10k >>> _LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e >>> -_LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe >>> _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -lrte_pmd_mlx4 -libverbs >>> _LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += -lrte_pmd_mlx5 -libverbs >>> _LDLIBS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += -lrte_pmd_mpipe -lgxio >>> >>