[PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive

[PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive

[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

[PATCH 2/2] mk: move crypto scheduler library

[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

Re: [PATCH 1/2] mk: move libs that don't dependent PMD libs out of whole archive

[Thomas Monjalon]

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>

Re: [PATCH 2/2] mk: move crypto scheduler library

[Thomas Monjalon]

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.

Re: [PATCH 2/2] mk: move crypto scheduler library

[Ferruh Yigit]

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

[PATCH v2] mk: move libs that don't dependent PMD libs out of whole archive

[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

Re: [PATCH v2] mk: move libs that don't dependent PMD libs out of whole archive

[Thomas Monjalon]

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