Re: [dpdk-dev] [PATCH 2/2] app/testpmd: remove explicit ixgbe link request

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

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.

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

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


[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.jacob@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
> > 
>