All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] net/ixgbe: remove unused global variable
@ 2016-12-27 10:09 Jerin Jacob
  2016-12-27 10:09 ` [PATCH 2/2] app/testpmd: remove explicit ixgbe link request Jerin Jacob
  2017-01-03 13:23 ` [PATCH 1/2] net/ixgbe: remove unused global variable Ferruh Yigit
  0 siblings, 2 replies; 8+ messages in thread
From: Jerin Jacob @ 2016-12-27 10:09 UTC (permalink / raw)
  To: dev; +Cc: konstantin.ananyev, helin.zhang, thomas.monjalon, Jerin Jacob

Removed unused "reg_info" global variable from ixgbe driver.

cat build/app/testpmd.map | grep "Allocating common symbols" -A 15
Allocating common symbols
Common symbol   size    file
reg_info        0x18    build/lib/librte_pmd_ixgbe.a(ixgbe_ethdev.o)

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 drivers/net/ixgbe/ixgbe_regs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_regs.h b/drivers/net/ixgbe/ixgbe_regs.h
index 773e169..2aa4820 100644
--- a/drivers/net/ixgbe/ixgbe_regs.h
+++ b/drivers/net/ixgbe/ixgbe_regs.h
@@ -41,7 +41,7 @@ struct reg_info {
 	uint32_t count;
 	uint32_t stride;
 	const char *name;
-} reg_info;
+};
 
 static const struct reg_info ixgbe_regs_general[] = {
 	{IXGBE_CTRL, 1, 1, "IXGBE_CTRL"},
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] app/testpmd: remove explicit ixgbe link request
  2016-12-27 10:09 [PATCH 1/2] net/ixgbe: remove unused global variable Jerin Jacob
@ 2016-12-27 10:09 ` Jerin Jacob
  2017-01-03 13:30   ` Ferruh Yigit
  2017-01-03 13:23 ` [PATCH 1/2] net/ixgbe: remove unused global variable Ferruh Yigit
  1 sibling, 1 reply; 8+ messages in thread
From: Jerin Jacob @ 2016-12-27 10:09 UTC (permalink / raw)
  To: dev; +Cc: konstantin.ananyev, helin.zhang, thomas.monjalon, Jerin Jacob

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.

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] net/ixgbe: remove unused global variable
  2016-12-27 10:09 [PATCH 1/2] net/ixgbe: remove unused global variable Jerin Jacob
  2016-12-27 10:09 ` [PATCH 2/2] app/testpmd: remove explicit ixgbe link request Jerin Jacob
@ 2017-01-03 13:23 ` Ferruh Yigit
  2017-01-03 15:44   ` Ferruh Yigit
  1 sibling, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2017-01-03 13:23 UTC (permalink / raw)
  To: Jerin Jacob, dev; +Cc: konstantin.ananyev, helin.zhang, thomas.monjalon

On 12/27/2016 10:09 AM, Jerin Jacob wrote:
> Removed unused "reg_info" global variable from ixgbe driver.
> 
> cat build/app/testpmd.map | grep "Allocating common symbols" -A 15
> Allocating common symbols
> Common symbol   size    file
> reg_info        0x18    build/lib/librte_pmd_ixgbe.a(ixgbe_ethdev.o)
> 
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] app/testpmd: remove explicit ixgbe link request
  2016-12-27 10:09 ` [PATCH 2/2] app/testpmd: remove explicit ixgbe link request Jerin Jacob
@ 2017-01-03 13:30   ` Ferruh Yigit
  2017-01-04 11:01     ` Jerin Jacob
  0 siblings, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2017-01-03 13:30 UTC (permalink / raw)
  To: Jerin Jacob, dev; +Cc: konstantin.ananyev, helin.zhang, thomas.monjalon

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.

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.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] net/ixgbe: remove unused global variable
  2017-01-03 13:23 ` [PATCH 1/2] net/ixgbe: remove unused global variable Ferruh Yigit
@ 2017-01-03 15:44   ` Ferruh Yigit
  0 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2017-01-03 15:44 UTC (permalink / raw)
  To: Jerin Jacob, dev; +Cc: konstantin.ananyev, helin.zhang, thomas.monjalon

On 1/3/2017 1:23 PM, Ferruh Yigit wrote:
> On 12/27/2016 10:09 AM, Jerin Jacob wrote:
>> Removed unused "reg_info" global variable from ixgbe driver.
>>
>> cat build/app/testpmd.map | grep "Allocating common symbols" -A 15
>> Allocating common symbols
>> Common symbol   size    file
>> reg_info        0x18    build/lib/librte_pmd_ixgbe.a(ixgbe_ethdev.o)
>>
>> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> 
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] app/testpmd: remove explicit ixgbe link request
  2017-01-03 13:30   ` Ferruh Yigit
@ 2017-01-04 11:01     ` Jerin Jacob
  2017-01-04 11:44       ` Ferruh Yigit
  0 siblings, 1 reply; 8+ messages in thread
From: Jerin Jacob @ 2017-01-04 11:01 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, konstantin.ananyev, helin.zhang, thomas.monjalon

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

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] app/testpmd: remove explicit ixgbe link request
  2017-01-04 11:01     ` Jerin Jacob
@ 2017-01-04 11:44       ` Ferruh Yigit
  2017-01-04 12:59         ` Jerin Jacob
  0 siblings, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2017-01-04 11:44 UTC (permalink / raw)
  To: Jerin Jacob; +Cc: dev, konstantin.ananyev, helin.zhang, thomas.monjalon

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] app/testpmd: remove explicit ixgbe link request
  2017-01-04 11:44       ` Ferruh Yigit
@ 2017-01-04 12:59         ` Jerin Jacob
  0 siblings, 0 replies; 8+ messages in thread
From: Jerin Jacob @ 2017-01-04 12:59 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, konstantin.ananyev, helin.zhang, thomas.monjalon

On Wed, Jan 04, 2017 at 11:44:21AM +0000, Ferruh Yigit wrote:
> 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.
> > ----
> > 
> >>
> >> 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.

OK. I will post the v2 based on following patch then.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-01-04 12:59 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-27 10:09 [PATCH 1/2] net/ixgbe: remove unused global variable Jerin Jacob
2016-12-27 10:09 ` [PATCH 2/2] app/testpmd: remove explicit ixgbe link request Jerin Jacob
2017-01-03 13:30   ` Ferruh Yigit
2017-01-04 11:01     ` Jerin Jacob
2017-01-04 11:44       ` Ferruh Yigit
2017-01-04 12:59         ` Jerin Jacob
2017-01-03 13:23 ` [PATCH 1/2] net/ixgbe: remove unused global variable Ferruh Yigit
2017-01-03 15:44   ` Ferruh Yigit

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.