DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive
@ 2017-01-31 11:59 Ferruh Yigit
  2017-01-31 11:59 ` [dpdk-dev] [PATCH 2/2] mk: move crypto scheduler library Ferruh Yigit
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ferruh Yigit @ 2017-01-31 11:59 UTC (permalink / raw)
  To: Thomas Monjalon, Pablo de Lara; +Cc: dev, Ferruh Yigit

During app build with static library, some libraries wrapped with
--whole-archive compiler flag.

Wrapped libraries are mainly PMD libraries, this is required because PMD
APIs not directly called but run through callbacks registered via
constructor functions.

Also some set of libraries, depends to the PMD libraries needs this,
because of same reason.

But other libraries can be out of this flag, and this saves some size in
final binary.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 mk/rte.app.mk | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 0d0a970..aeadbc3 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -81,13 +81,14 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --no-whole-archive
 _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS)       += -lrte_jobstats
 _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER)          += -lrte_power
 
+_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER)          += -lrte_timer
+_LDLIBS-$(CONFIG_RTE_LIBRTE_EFD)            += -lrte_efd
+_LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)        += -lrte_cfgfile
+
 _LDLIBS-y += --whole-archive
 
-_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER)          += -lrte_timer
 _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH)           += -lrte_hash
-_LDLIBS-$(CONFIG_RTE_LIBRTE_EFD)            += -lrte_efd
 _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lrte_vhost
-
 _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS)         += -lrte_kvargs
 _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF)           += -lrte_mbuf
 _LDLIBS-$(CONFIG_RTE_LIBRTE_NET)            += -lrte_net
@@ -97,7 +98,6 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL)        += -lrte_mempool
 _LDLIBS-$(CONFIG_RTE_LIBRTE_RING)           += -lrte_ring
 _LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)            += -lrte_eal
 _LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE)        += -lrte_cmdline
-_LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)        += -lrte_cfgfile
 _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)        += -lrte_reorder
 
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)       += -lrte_pmd_bond
-- 
2.9.3

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

* [dpdk-dev] [PATCH 2/2] mk: move crypto scheduler library
  2017-01-31 11:59 [dpdk-dev] [PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive Ferruh Yigit
@ 2017-01-31 11:59 ` Ferruh Yigit
  2017-01-31 14:10   ` Thomas Monjalon
  2017-01-31 13:59 ` [dpdk-dev] [PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive Thomas Monjalon
  2017-01-31 15:01 ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
  2 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2017-01-31 11:59 UTC (permalink / raw)
  To: Thomas Monjalon, Pablo de Lara; +Cc: dev, Ferruh Yigit

There is already a block for crypto libraries, move the PMD library to
that block.

This prevents extra ifdef check for cryptodev.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 mk/rte.app.mk | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index aeadbc3..a4a09d8 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -103,10 +103,6 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)        += -lrte_reorder
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)       += -lrte_pmd_bond
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)    += -lrte_pmd_xenvirt -lxenstore
 
-ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y)
-_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_CRYPTO_SCHEDULER) += -lrte_pmd_crypto_scheduler
-endif
-
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
 # plugins (link only if static libraries)
 
@@ -153,6 +149,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ZUC)         += -lrte_pmd_zuc
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ZUC)         += -L$(LIBSSO_ZUC_PATH)/build -lsso_zuc
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO)    += -lrte_pmd_armv8
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO)    += -L$(ARMV8_CRYPTO_LIB_PATH) -larmv8_crypto
+_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_CRYPTO_SCHEDULER) += -lrte_pmd_crypto_scheduler
 endif # CONFIG_RTE_LIBRTE_CRYPTODEV
 
 endif # !CONFIG_RTE_BUILD_SHARED_LIBS
-- 
2.9.3

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

* Re: [dpdk-dev] [PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive
  2017-01-31 11:59 [dpdk-dev] [PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive Ferruh Yigit
  2017-01-31 11:59 ` [dpdk-dev] [PATCH 2/2] mk: move crypto scheduler library Ferruh Yigit
@ 2017-01-31 13:59 ` Thomas Monjalon
  2017-01-31 15:01 ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
  2 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2017-01-31 13:59 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Pablo de Lara, dev

2017-01-31 11:59, Ferruh Yigit:
> During app build with static library, some libraries wrapped with
> --whole-archive compiler flag.
> 
> Wrapped libraries are mainly PMD libraries, this is required because PMD
> APIs not directly called but run through callbacks registered via
> constructor functions.
> 
> Also some set of libraries, depends to the PMD libraries needs this,
> because of same reason.

All the libraries used by a plugin (any driver) must be in --whole-archive
to ensure that every symbols will be available for the plugin.
This should be explained in this patch (verbatim copy allowed).

> But other libraries can be out of this flag, and this saves some size in
> final binary.
[...]
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER)          += -lrte_timer
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_EFD)            += -lrte_efd
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)        += -lrte_cfgfile
> +
>  _LDLIBS-y += --whole-archive

Yes we can move these libraries out of --whole-archive as they are not
used by any driver.

Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

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

* Re: [dpdk-dev] [PATCH 2/2] mk: move crypto scheduler library
  2017-01-31 11:59 ` [dpdk-dev] [PATCH 2/2] mk: move crypto scheduler library Ferruh Yigit
@ 2017-01-31 14:10   ` Thomas Monjalon
  2017-01-31 14:20     ` Ferruh Yigit
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Monjalon @ 2017-01-31 14:10 UTC (permalink / raw)
  To: Ferruh Yigit, Pablo de Lara, Fan Zhang; +Cc: dev

2017-01-31 11:59, Ferruh Yigit:
> There is already a block for crypto libraries, move the PMD library to
> that block.
> 
> This prevents extra ifdef check for cryptodev.

That's why I thought also when reading this patch:
	http://dpdk.org/commit/dbb336407
Then I've read its message:
"Different than other cryptodev PMDs, scheduler PMD is required to be built
as shared libraries."

I guess the explanation is that it has an API (like bonding has):
	drivers/crypto/scheduler/rte_cryptodev_scheduler.h

However, it is neither referenced in doc/api/doxy-api.conf nor
doc/api/doxy-api-index.md.


> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -103,10 +103,6 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)        += -lrte_reorder
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)       += -lrte_pmd_bond
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT)    += -lrte_pmd_xenvirt -lxenstore
>  
> -ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y)
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_CRYPTO_SCHEDULER) += -lrte_pmd_crypto_scheduler
> -endif
> -
>  ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
>  # plugins (link only if static libraries)
>  
> @@ -153,6 +149,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ZUC)         += -lrte_pmd_zuc
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ZUC)         += -L$(LIBSSO_ZUC_PATH)/build -lsso_zuc
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO)    += -lrte_pmd_armv8
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO)    += -L$(ARMV8_CRYPTO_LIB_PATH) -larmv8_crypto
> +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_CRYPTO_SCHEDULER) += -lrte_pmd_crypto_scheduler
>  endif # CONFIG_RTE_LIBRTE_CRYPTODEV
>  
>  endif # !CONFIG_RTE_BUILD_SHARED_LIBS


I must say the quick overview I had on this PMD is not very promising.
Please Pablo and Fan, try to better explain things in the patches
and get more review on framework integration.

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

* Re: [dpdk-dev] [PATCH 2/2] mk: move crypto scheduler library
  2017-01-31 14:10   ` Thomas Monjalon
@ 2017-01-31 14:20     ` Ferruh Yigit
  0 siblings, 0 replies; 7+ messages in thread
From: Ferruh Yigit @ 2017-01-31 14:20 UTC (permalink / raw)
  To: Thomas Monjalon, Pablo de Lara, Fan Zhang; +Cc: dev

On 1/31/2017 2:10 PM, Thomas Monjalon wrote:
> 2017-01-31 11:59, Ferruh Yigit:
>> There is already a block for crypto libraries, move the PMD library to
>> that block.
>>
>> This prevents extra ifdef check for cryptodev.
> 
> That's why I thought also when reading this patch:
> 	http://dpdk.org/commit/dbb336407
> Then I've read its message:
> "Different than other cryptodev PMDs, scheduler PMD is required to be built
> as shared libraries."

This patch requires following patch for shared library compilation:
http://dpdk.org/dev/patchwork/patch/20091/

I tried to separate patches, but it seems not able to completely.

Overall, for this patch and above referenced one, need to decide how to
link PMDs with APIs (like bonding, xenvirt)

> 
> I guess the explanation is that it has an API (like bonding has):
> 	drivers/crypto/scheduler/rte_cryptodev_scheduler.h
> 
> However, it is neither referenced in doc/api/doxy-api.conf nor
> doc/api/doxy-api-index.md.
> 
> 
<...>

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

* [dpdk-dev] [PATCH v2] mk: move libs that don't dependent PMD libs out of whole archive
  2017-01-31 11:59 [dpdk-dev] [PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive Ferruh Yigit
  2017-01-31 11:59 ` [dpdk-dev] [PATCH 2/2] mk: move crypto scheduler library Ferruh Yigit
  2017-01-31 13:59 ` [dpdk-dev] [PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive Thomas Monjalon
@ 2017-01-31 15:01 ` Ferruh Yigit
  2017-02-09 21:48   ` Thomas Monjalon
  2 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2017-01-31 15:01 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Ferruh Yigit

During app build with static library, some libraries wrapped with
--whole-archive compiler flag.

Wrapped libraries are mainly PMD libraries, this is required because PMD
APIs not called directly but run through callbacks registered via
constructor functions.

Also some set of libraries, depends to the PMD libraries needs this,
because of same reason.

All the libraries used by a plugin (any driver) must be in
--whole-archive to ensure that every symbols will be available for the
plugin.

But other libraries can be out of this flag, and this saves some bytes
in final binary.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 mk/rte.app.mk | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 0d0a970..aeadbc3 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -81,13 +81,14 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --no-whole-archive
 _LDLIBS-$(CONFIG_RTE_LIBRTE_JOBSTATS)       += -lrte_jobstats
 _LDLIBS-$(CONFIG_RTE_LIBRTE_POWER)          += -lrte_power
 
+_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER)          += -lrte_timer
+_LDLIBS-$(CONFIG_RTE_LIBRTE_EFD)            += -lrte_efd
+_LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)        += -lrte_cfgfile
+
 _LDLIBS-y += --whole-archive
 
-_LDLIBS-$(CONFIG_RTE_LIBRTE_TIMER)          += -lrte_timer
 _LDLIBS-$(CONFIG_RTE_LIBRTE_HASH)           += -lrte_hash
-_LDLIBS-$(CONFIG_RTE_LIBRTE_EFD)            += -lrte_efd
 _LDLIBS-$(CONFIG_RTE_LIBRTE_VHOST)          += -lrte_vhost
-
 _LDLIBS-$(CONFIG_RTE_LIBRTE_KVARGS)         += -lrte_kvargs
 _LDLIBS-$(CONFIG_RTE_LIBRTE_MBUF)           += -lrte_mbuf
 _LDLIBS-$(CONFIG_RTE_LIBRTE_NET)            += -lrte_net
@@ -97,7 +98,6 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL)        += -lrte_mempool
 _LDLIBS-$(CONFIG_RTE_LIBRTE_RING)           += -lrte_ring
 _LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)            += -lrte_eal
 _LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE)        += -lrte_cmdline
-_LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE)        += -lrte_cfgfile
 _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)        += -lrte_reorder
 
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)       += -lrte_pmd_bond
-- 
2.9.3

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

* Re: [dpdk-dev] [PATCH v2] mk: move libs that don't dependent PMD libs out of whole archive
  2017-01-31 15:01 ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
@ 2017-02-09 21:48   ` Thomas Monjalon
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2017-02-09 21:48 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev

2017-01-31 15:01, Ferruh Yigit:
> During app build with static library, some libraries wrapped with
> --whole-archive compiler flag.
> 
> Wrapped libraries are mainly PMD libraries, this is required because PMD
> APIs not called directly but run through callbacks registered via
> constructor functions.
> 
> Also some set of libraries, depends to the PMD libraries needs this,
> because of same reason.
> 
> All the libraries used by a plugin (any driver) must be in
> --whole-archive to ensure that every symbols will be available for the
> plugin.
> 
> But other libraries can be out of this flag, and this saves some bytes
> in final binary.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Thomas Monjalon <thomas.monjalon@6wind.com>

Applied, thanks

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

end of thread, other threads:[~2017-02-09 21:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-31 11:59 [dpdk-dev] [PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive Ferruh Yigit
2017-01-31 11:59 ` [dpdk-dev] [PATCH 2/2] mk: move crypto scheduler library Ferruh Yigit
2017-01-31 14:10   ` Thomas Monjalon
2017-01-31 14:20     ` Ferruh Yigit
2017-01-31 13:59 ` [dpdk-dev] [PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive Thomas Monjalon
2017-01-31 15:01 ` [dpdk-dev] [PATCH v2] " Ferruh Yigit
2017-02-09 21:48   ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).