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 <bernard.iremon...@intel.com> > 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 <jerin.ja...@caviumnetworks.com> >>> --- >>> 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 >>> >>
