[dpdk-dev] [PATCH 0/2] remove use of weak functions from libraries

[dpdk-dev] [PATCH 0/2] remove use of weak functions from libraries

[Bruce Richardson]

Weak functions don't work well with static library builds as the linker
always picks the first version of a function irrespective of whether it is
weak or not. The solution to this is to use the "whole-archive" flag when
linking, but this has the nasty side-effect that it causes the resulting
binary to be larger than it needs to be.


A further problem with this approach of using "whole-archive" is that one
either needs to link all libraries with this flag or track what libraries
need it or not - the latter being especially a problem for apps not using
the DPDK build system itself (i.e. most apps not shipped with DPDK itself).
For meson builds this information needs to make its way all the way through
to the pkgconfig file generated - not a trivial undertaking.

Thankfully, though, the use of weak functions is limited to use for
multiple functions within a single library, meaning that when the lib is
being built, the build system itself can know whether the "weak" function
is needed or not. This allows us to change the weak function to a
conditionally compiled regular function used in fallback case. This makes
the need for "whole-archive" flag, and any special linking measures for the
library, go away.

[This set does not touch the drivers, only the libraries, since there are
other special linker flags needed for drivers in general, making the
problem less severe for driver .a files.]

Bruce Richardson (2):
  acl: remove use of weak functions
  bpf: remove use of weak functions

 lib/librte_acl/meson.build |  7 ++++++-
 lib/librte_acl/rte_acl.c   | 18 ++++++++++++++----
 lib/librte_bpf/bpf_load.c  |  4 +++-
 lib/librte_bpf/meson.build |  1 +
 mk/rte.app.mk              |  3 ---
 5 files changed, 24 insertions(+), 9 deletions(-)

-- 
2.20.1

[dpdk-dev] [PATCH 0/2] remove use of weak functions from libraries

[Bruce Richardson]

Weak functions don't work well with static library builds as the linker
always picks the first version of a function irrespective of whether it is
weak or not. The solution to this is to use the "whole-archive" flag when
linking, but this has the nasty side-effect that it causes the resulting
binary to be larger than it needs to be.


A further problem with this approach of using "whole-archive" is that one
either needs to link all libraries with this flag or track what libraries
need it or not - the latter being especially a problem for apps not using
the DPDK build system itself (i.e. most apps not shipped with DPDK itself).
For meson builds this information needs to make its way all the way through
to the pkgconfig file generated - not a trivial undertaking.

Thankfully, though, the use of weak functions is limited to use for
multiple functions within a single library, meaning that when the lib is
being built, the build system itself can know whether the "weak" function
is needed or not. This allows us to change the weak function to a
conditionally compiled regular function used in fallback case. This makes
the need for "whole-archive" flag, and any special linking measures for the
library, go away.

[This set does not touch the drivers, only the libraries, since there are
other special linker flags needed for drivers in general, making the
problem less severe for driver .a files.]

Bruce Richardson (2):
  acl: remove use of weak functions
  bpf: remove use of weak functions

 lib/librte_acl/meson.build |  7 ++++++-
 lib/librte_acl/rte_acl.c   | 18 ++++++++++++++----
 lib/librte_bpf/bpf_load.c  |  4 +++-
 lib/librte_bpf/meson.build |  1 +
 mk/rte.app.mk              |  3 ---
 5 files changed, 24 insertions(+), 9 deletions(-)

-- 
2.20.1

[dpdk-dev] [PATCH 1/2] acl: remove use of weak functions

[Bruce Richardson]

Weak functions don't work well with static libraries and require the use of
"whole-archive" flag to ensure that the correct function is used when
linking. Since the weak functions are only used as placeholders within
this library alone, we can replace them with non-weak functions using
preprocessor ifdefs.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_acl/meson.build |  7 ++++++-
 lib/librte_acl/rte_acl.c   | 18 ++++++++++++++----
 mk/rte.app.mk              |  3 ---
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
index 2207dbafe..98ece7d85 100644
--- a/lib/librte_acl/meson.build
+++ b/lib/librte_acl/meson.build
@@ -6,7 +6,7 @@ sources = files('acl_bld.c', 'acl_gen.c', 'acl_run_scalar.c',
 		'rte_acl.c', 'tb_mem.c')
 headers = files('rte_acl.h', 'rte_acl_osdep.h')
 
-if arch_subdir == 'x86'
+if dpdk_conf.has('RTE_ARCH_X86')
 	sources += files('acl_run_sse.c')
 
 	# compile AVX2 version if either:
@@ -28,4 +28,9 @@ if arch_subdir == 'x86'
 		cflags += '-DCC_AVX2_SUPPORT'
 	endif
 
+elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
+	cflags += '-flax-vector-conversions'
+	sources += files('acl_run_neon.c')
+elif dpdk_conf.has('RTE_ARCH_PPC_64')
+	sources += files('acl_run_altivec.c')
 endif
diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
index c436a9bfd..fd5bd5e4e 100644
--- a/lib/librte_acl/rte_acl.c
+++ b/lib/librte_acl/rte_acl.c
@@ -13,11 +13,13 @@ static struct rte_tailq_elem rte_acl_tailq = {
 };
 EAL_REGISTER_TAILQ(rte_acl_tailq)
 
+#ifndef RTE_ARCH_X86
+#ifndef CC_AVX2_SUPPORT
 /*
  * If the compiler doesn't support AVX2 instructions,
  * then the dummy one would be used instead for AVX2 classify method.
  */
-__rte_weak int
+int
 rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
 	__rte_unused const uint8_t **data,
 	__rte_unused uint32_t *results,
@@ -26,8 +28,9 @@ rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
 {
 	return -ENOTSUP;
 }
+#endif
 
-__rte_weak int
+int
 rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
 	__rte_unused const uint8_t **data,
 	__rte_unused uint32_t *results,
@@ -36,8 +39,11 @@ rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
 {
 	return -ENOTSUP;
 }
+#endif
 
-__rte_weak int
+#ifndef RTE_ARCH_ARM
+#ifndef RTE_ARCH_ARM64
+int
 rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
 	__rte_unused const uint8_t **data,
 	__rte_unused uint32_t *results,
@@ -46,8 +52,11 @@ rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
 {
 	return -ENOTSUP;
 }
+#endif
+#endif
 
-__rte_weak int
+#ifndef RTE_ARCH_PPC_64
+int
 rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
 	__rte_unused const uint8_t **data,
 	__rte_unused uint32_t *results,
@@ -56,6 +65,7 @@ rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
 {
 	return -ENOTSUP;
 }
+#endif
 
 static const rte_acl_classify_t classify_fns[] = {
 	[RTE_ACL_CLASSIFY_DEFAULT] = rte_acl_classify_scalar,
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 7d994bece..fdec636b4 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -46,10 +46,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)    += -lrte_distributor
 _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)        += -lrte_ip_frag
 _LDLIBS-$(CONFIG_RTE_LIBRTE_METER)          += -lrte_meter
 _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)            += -lrte_lpm
-# librte_acl needs --whole-archive because of weak functions
-_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --whole-archive
 _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += -lrte_acl
-_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --no-whole-archive
 _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --no-as-needed
 _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --whole-archive
 _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += -lrte_telemetry -ljansson
-- 
2.20.1

[dpdk-dev] [PATCH 1/2] acl: remove use of weak functions

[Bruce Richardson]

Weak functions don't work well with static libraries and require the use of
"whole-archive" flag to ensure that the correct function is used when
linking. Since the weak functions are only used as placeholders within
this library alone, we can replace them with non-weak functions using
preprocessor ifdefs.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_acl/meson.build |  7 ++++++-
 lib/librte_acl/rte_acl.c   | 18 ++++++++++++++----
 mk/rte.app.mk              |  3 ---
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
index 2207dbafe..98ece7d85 100644
--- a/lib/librte_acl/meson.build
+++ b/lib/librte_acl/meson.build
@@ -6,7 +6,7 @@ sources = files('acl_bld.c', 'acl_gen.c', 'acl_run_scalar.c',
 		'rte_acl.c', 'tb_mem.c')
 headers = files('rte_acl.h', 'rte_acl_osdep.h')
 
-if arch_subdir == 'x86'
+if dpdk_conf.has('RTE_ARCH_X86')
 	sources += files('acl_run_sse.c')
 
 	# compile AVX2 version if either:
@@ -28,4 +28,9 @@ if arch_subdir == 'x86'
 		cflags += '-DCC_AVX2_SUPPORT'
 	endif
 
+elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
+	cflags += '-flax-vector-conversions'
+	sources += files('acl_run_neon.c')
+elif dpdk_conf.has('RTE_ARCH_PPC_64')
+	sources += files('acl_run_altivec.c')
 endif
diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
index c436a9bfd..fd5bd5e4e 100644
--- a/lib/librte_acl/rte_acl.c
+++ b/lib/librte_acl/rte_acl.c
@@ -13,11 +13,13 @@ static struct rte_tailq_elem rte_acl_tailq = {
 };
 EAL_REGISTER_TAILQ(rte_acl_tailq)
 
+#ifndef RTE_ARCH_X86
+#ifndef CC_AVX2_SUPPORT
 /*
  * If the compiler doesn't support AVX2 instructions,
  * then the dummy one would be used instead for AVX2 classify method.
  */
-__rte_weak int
+int
 rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
 	__rte_unused const uint8_t **data,
 	__rte_unused uint32_t *results,
@@ -26,8 +28,9 @@ rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
 {
 	return -ENOTSUP;
 }
+#endif
 
-__rte_weak int
+int
 rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
 	__rte_unused const uint8_t **data,
 	__rte_unused uint32_t *results,
@@ -36,8 +39,11 @@ rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
 {
 	return -ENOTSUP;
 }
+#endif
 
-__rte_weak int
+#ifndef RTE_ARCH_ARM
+#ifndef RTE_ARCH_ARM64
+int
 rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
 	__rte_unused const uint8_t **data,
 	__rte_unused uint32_t *results,
@@ -46,8 +52,11 @@ rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
 {
 	return -ENOTSUP;
 }
+#endif
+#endif
 
-__rte_weak int
+#ifndef RTE_ARCH_PPC_64
+int
 rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
 	__rte_unused const uint8_t **data,
 	__rte_unused uint32_t *results,
@@ -56,6 +65,7 @@ rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
 {
 	return -ENOTSUP;
 }
+#endif
 
 static const rte_acl_classify_t classify_fns[] = {
 	[RTE_ACL_CLASSIFY_DEFAULT] = rte_acl_classify_scalar,
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 7d994bece..fdec636b4 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -46,10 +46,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)    += -lrte_distributor
 _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)        += -lrte_ip_frag
 _LDLIBS-$(CONFIG_RTE_LIBRTE_METER)          += -lrte_meter
 _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)            += -lrte_lpm
-# librte_acl needs --whole-archive because of weak functions
-_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --whole-archive
 _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += -lrte_acl
-_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --no-whole-archive
 _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --no-as-needed
 _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --whole-archive
 _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += -lrte_telemetry -ljansson
-- 
2.20.1

[dpdk-dev] [PATCH 2/2] bpf: remove use of weak functions

[Bruce Richardson]

Weak functions don't work well with static libraries and require the use of
"whole-archive" flag to ensure that the correct function is used when
linking.  Since the weak function is only used as a placeholder within this
library alone, we can replace it with a non-weak version protected using
preprocessor ifdefs.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_bpf/bpf_load.c  | 4 +++-
 lib/librte_bpf/meson.build | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/librte_bpf/bpf_load.c b/lib/librte_bpf/bpf_load.c
index d9d163b7d..194103ec7 100644
--- a/lib/librte_bpf/bpf_load.c
+++ b/lib/librte_bpf/bpf_load.c
@@ -131,7 +131,8 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
 	return bpf;
 }
 
-__rte_experimental __rte_weak struct rte_bpf *
+#ifndef RTE_LIBRTE_BPF_ELF
+__rte_experimental struct rte_bpf *
 rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
 	const char *sname)
 {
@@ -146,3 +147,4 @@ rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
 	rte_errno = ENOTSUP;
 	return NULL;
 }
+#endif
diff --git a/lib/librte_bpf/meson.build b/lib/librte_bpf/meson.build
index 8a79878ff..11c1fb558 100644
--- a/lib/librte_bpf/meson.build
+++ b/lib/librte_bpf/meson.build
@@ -20,6 +20,7 @@ deps += ['mbuf', 'net', 'ethdev']
 
 dep = dependency('libelf', required: false)
 if dep.found()
+	dpdk_conf.set('RTE_LIBRTE_BPF_ELF', 1)
 	sources += files('bpf_load_elf.c')
 	ext_deps += dep
 endif
-- 
2.20.1

[dpdk-dev] [PATCH 2/2] bpf: remove use of weak functions

[Bruce Richardson]

Weak functions don't work well with static libraries and require the use of
"whole-archive" flag to ensure that the correct function is used when
linking.  Since the weak function is only used as a placeholder within this
library alone, we can replace it with a non-weak version protected using
preprocessor ifdefs.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/librte_bpf/bpf_load.c  | 4 +++-
 lib/librte_bpf/meson.build | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/librte_bpf/bpf_load.c b/lib/librte_bpf/bpf_load.c
index d9d163b7d..194103ec7 100644
--- a/lib/librte_bpf/bpf_load.c
+++ b/lib/librte_bpf/bpf_load.c
@@ -131,7 +131,8 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
 	return bpf;
 }
 
-__rte_experimental __rte_weak struct rte_bpf *
+#ifndef RTE_LIBRTE_BPF_ELF
+__rte_experimental struct rte_bpf *
 rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
 	const char *sname)
 {
@@ -146,3 +147,4 @@ rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
 	rte_errno = ENOTSUP;
 	return NULL;
 }
+#endif
diff --git a/lib/librte_bpf/meson.build b/lib/librte_bpf/meson.build
index 8a79878ff..11c1fb558 100644
--- a/lib/librte_bpf/meson.build
+++ b/lib/librte_bpf/meson.build
@@ -20,6 +20,7 @@ deps += ['mbuf', 'net', 'ethdev']
 
 dep = dependency('libelf', required: false)
 if dep.found()
+	dpdk_conf.set('RTE_LIBRTE_BPF_ELF', 1)
 	sources += files('bpf_load_elf.c')
 	ext_deps += dep
 endif
-- 
2.20.1

Re: [dpdk-dev] [PATCH 1/2] acl: remove use of weak functions

[Aaron Conole]

Bruce Richardson <bruce.richardson@intel.com> writes:

> Weak functions don't work well with static libraries and require the use of
> "whole-archive" flag to ensure that the correct function is used when
> linking. Since the weak functions are only used as placeholders within
> this library alone, we can replace them with non-weak functions using
> preprocessor ifdefs.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_acl/meson.build |  7 ++++++-
>  lib/librte_acl/rte_acl.c   | 18 ++++++++++++++----
>  mk/rte.app.mk              |  3 ---
>  3 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
> index 2207dbafe..98ece7d85 100644
> --- a/lib/librte_acl/meson.build
> +++ b/lib/librte_acl/meson.build
> @@ -6,7 +6,7 @@ sources = files('acl_bld.c', 'acl_gen.c', 'acl_run_scalar.c',
>  		'rte_acl.c', 'tb_mem.c')
>  headers = files('rte_acl.h', 'rte_acl_osdep.h')
>  
> -if arch_subdir == 'x86'
> +if dpdk_conf.has('RTE_ARCH_X86')
>  	sources += files('acl_run_sse.c')
>  
>  	# compile AVX2 version if either:
> @@ -28,4 +28,9 @@ if arch_subdir == 'x86'
>  		cflags += '-DCC_AVX2_SUPPORT'
>  	endif
>  
> +elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
> +	cflags += '-flax-vector-conversions'
> +	sources += files('acl_run_neon.c')

This will also need -Wno-uninitialized (otherwise it will generate
warnings about the search_neon_4 and search_neon_8 functions).

But I don't like papering over these conversions.  I'd prefer instead
the patches I posted at:

http://mails.dpdk.org/archives/dev/2019-April/129540.html
and
http://mails.dpdk.org/archives/dev/2019-April/129541.html

Are you opposed to merging those?

> +elif dpdk_conf.has('RTE_ARCH_PPC_64')
> +	sources += files('acl_run_altivec.c')
>  endif
> diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> index c436a9bfd..fd5bd5e4e 100644
> --- a/lib/librte_acl/rte_acl.c
> +++ b/lib/librte_acl/rte_acl.c
> @@ -13,11 +13,13 @@ static struct rte_tailq_elem rte_acl_tailq = {
>  };
>  EAL_REGISTER_TAILQ(rte_acl_tailq)
>  
> +#ifndef RTE_ARCH_X86
> +#ifndef CC_AVX2_SUPPORT
>  /*
>   * If the compiler doesn't support AVX2 instructions,
>   * then the dummy one would be used instead for AVX2 classify method.
>   */
> -__rte_weak int
> +int
>  rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -26,8 +28,9 @@ rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
>  
> -__rte_weak int
> +int
>  rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -36,8 +39,11 @@ rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
>  
> -__rte_weak int
> +#ifndef RTE_ARCH_ARM
> +#ifndef RTE_ARCH_ARM64
> +int
>  rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -46,8 +52,11 @@ rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
> +#endif
>  
> -__rte_weak int
> +#ifndef RTE_ARCH_PPC_64
> +int
>  rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -56,6 +65,7 @@ rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
>  
>  static const rte_acl_classify_t classify_fns[] = {
>  	[RTE_ACL_CLASSIFY_DEFAULT] = rte_acl_classify_scalar,
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 7d994bece..fdec636b4 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -46,10 +46,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)    += -lrte_distributor
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)        += -lrte_ip_frag
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_METER)          += -lrte_meter
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)            += -lrte_lpm
> -# librte_acl needs --whole-archive because of weak functions
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += -lrte_acl
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --no-whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --no-as-needed
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += -lrte_telemetry -ljansson

I think I have a solution for this that can use the weak aliasing and
not require the use of the whole-archive flag.  Would you prefer that?

Re: [dpdk-dev] [PATCH 1/2] acl: remove use of weak functions

[Aaron Conole]

Bruce Richardson <bruce.richardson@intel.com> writes:

> Weak functions don't work well with static libraries and require the use of
> "whole-archive" flag to ensure that the correct function is used when
> linking. Since the weak functions are only used as placeholders within
> this library alone, we can replace them with non-weak functions using
> preprocessor ifdefs.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_acl/meson.build |  7 ++++++-
>  lib/librte_acl/rte_acl.c   | 18 ++++++++++++++----
>  mk/rte.app.mk              |  3 ---
>  3 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
> index 2207dbafe..98ece7d85 100644
> --- a/lib/librte_acl/meson.build
> +++ b/lib/librte_acl/meson.build
> @@ -6,7 +6,7 @@ sources = files('acl_bld.c', 'acl_gen.c', 'acl_run_scalar.c',
>  		'rte_acl.c', 'tb_mem.c')
>  headers = files('rte_acl.h', 'rte_acl_osdep.h')
>  
> -if arch_subdir == 'x86'
> +if dpdk_conf.has('RTE_ARCH_X86')
>  	sources += files('acl_run_sse.c')
>  
>  	# compile AVX2 version if either:
> @@ -28,4 +28,9 @@ if arch_subdir == 'x86'
>  		cflags += '-DCC_AVX2_SUPPORT'
>  	endif
>  
> +elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
> +	cflags += '-flax-vector-conversions'
> +	sources += files('acl_run_neon.c')

This will also need -Wno-uninitialized (otherwise it will generate
warnings about the search_neon_4 and search_neon_8 functions).

But I don't like papering over these conversions.  I'd prefer instead
the patches I posted at:

http://mails.dpdk.org/archives/dev/2019-April/129540.html
and
http://mails.dpdk.org/archives/dev/2019-April/129541.html

Are you opposed to merging those?

> +elif dpdk_conf.has('RTE_ARCH_PPC_64')
> +	sources += files('acl_run_altivec.c')
>  endif
> diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> index c436a9bfd..fd5bd5e4e 100644
> --- a/lib/librte_acl/rte_acl.c
> +++ b/lib/librte_acl/rte_acl.c
> @@ -13,11 +13,13 @@ static struct rte_tailq_elem rte_acl_tailq = {
>  };
>  EAL_REGISTER_TAILQ(rte_acl_tailq)
>  
> +#ifndef RTE_ARCH_X86
> +#ifndef CC_AVX2_SUPPORT
>  /*
>   * If the compiler doesn't support AVX2 instructions,
>   * then the dummy one would be used instead for AVX2 classify method.
>   */
> -__rte_weak int
> +int
>  rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -26,8 +28,9 @@ rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
>  
> -__rte_weak int
> +int
>  rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -36,8 +39,11 @@ rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
>  
> -__rte_weak int
> +#ifndef RTE_ARCH_ARM
> +#ifndef RTE_ARCH_ARM64
> +int
>  rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -46,8 +52,11 @@ rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
> +#endif
>  
> -__rte_weak int
> +#ifndef RTE_ARCH_PPC_64
> +int
>  rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -56,6 +65,7 @@ rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
>  
>  static const rte_acl_classify_t classify_fns[] = {
>  	[RTE_ACL_CLASSIFY_DEFAULT] = rte_acl_classify_scalar,
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 7d994bece..fdec636b4 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -46,10 +46,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)    += -lrte_distributor
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)        += -lrte_ip_frag
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_METER)          += -lrte_meter
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)            += -lrte_lpm
> -# librte_acl needs --whole-archive because of weak functions
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += -lrte_acl
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --no-whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --no-as-needed
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += -lrte_telemetry -ljansson

I think I have a solution for this that can use the weak aliasing and
not require the use of the whole-archive flag.  Would you prefer that?

Re: [dpdk-dev] [PATCH 1/2] acl: remove use of weak functions

[Bruce Richardson]

On Wed, Apr 10, 2019 at 09:54:02AM -0400, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
> 
> > Weak functions don't work well with static libraries and require the
> > use of "whole-archive" flag to ensure that the correct function is used
> > when linking. Since the weak functions are only used as placeholders
> > within this library alone, we can replace them with non-weak functions
> > using preprocessor ifdefs.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
> > lib/librte_acl/meson.build |  7 ++++++- lib/librte_acl/rte_acl.c   | 18
> > ++++++++++++++---- mk/rte.app.mk              |  3 --- 3 files changed,
> > 20 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
> > index 2207dbafe..98ece7d85 100644 --- a/lib/librte_acl/meson.build +++
> > b/lib/librte_acl/meson.build @@ -6,7 +6,7 @@ sources =
> > files('acl_bld.c', 'acl_gen.c', 'acl_run_scalar.c', 'rte_acl.c',
> > 'tb_mem.c') headers = files('rte_acl.h', 'rte_acl_osdep.h')
> >  
> > -if arch_subdir == 'x86' +if dpdk_conf.has('RTE_ARCH_X86') sources +=
> > files('acl_run_sse.c')
> >  
> >  	# compile AVX2 version if either: @@ -28,4 +28,9 @@ if arch_subdir
> >  	== 'x86' cflags += '-DCC_AVX2_SUPPORT' endif
> >  
> > +elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
> > +	cflags += '-flax-vector-conversions' +	sources +=
> > files('acl_run_neon.c')
> 
> This will also need -Wno-uninitialized (otherwise it will generate
> warnings about the search_neon_4 and search_neon_8 functions).
> 
> But I don't like papering over these conversions.  I'd prefer instead the
> patches I posted at:
> 
> http://mails.dpdk.org/archives/dev/2019-April/129540.html and
> http://mails.dpdk.org/archives/dev/2019-April/129541.html
> 
> Are you opposed to merging those?
> 
Nope, not in the least. I'm happy enough to rework this patch on top of
those - I'd just had forgotten about them in my rush to get a potential
solution out here.  I did these up quickly to show how easy it is to remove
the need for the weak functions and the subsequent linker "--whole-archive"
flag.

/Bruce

PS: I see your patch 2 does not include the Wno-uninitialized flag, is it
not needed in your patch, or just an oversight?

Re: [dpdk-dev] [PATCH 1/2] acl: remove use of weak functions

[Bruce Richardson]

On Wed, Apr 10, 2019 at 09:54:02AM -0400, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
> 
> > Weak functions don't work well with static libraries and require the
> > use of "whole-archive" flag to ensure that the correct function is used
> > when linking. Since the weak functions are only used as placeholders
> > within this library alone, we can replace them with non-weak functions
> > using preprocessor ifdefs.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
> > lib/librte_acl/meson.build |  7 ++++++- lib/librte_acl/rte_acl.c   | 18
> > ++++++++++++++---- mk/rte.app.mk              |  3 --- 3 files changed,
> > 20 insertions(+), 8 deletions(-)
> >
> > diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
> > index 2207dbafe..98ece7d85 100644 --- a/lib/librte_acl/meson.build +++
> > b/lib/librte_acl/meson.build @@ -6,7 +6,7 @@ sources =
> > files('acl_bld.c', 'acl_gen.c', 'acl_run_scalar.c', 'rte_acl.c',
> > 'tb_mem.c') headers = files('rte_acl.h', 'rte_acl_osdep.h')
> >  
> > -if arch_subdir == 'x86' +if dpdk_conf.has('RTE_ARCH_X86') sources +=
> > files('acl_run_sse.c')
> >  
> >  	# compile AVX2 version if either: @@ -28,4 +28,9 @@ if arch_subdir
> >  	== 'x86' cflags += '-DCC_AVX2_SUPPORT' endif
> >  
> > +elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
> > +	cflags += '-flax-vector-conversions' +	sources +=
> > files('acl_run_neon.c')
> 
> This will also need -Wno-uninitialized (otherwise it will generate
> warnings about the search_neon_4 and search_neon_8 functions).
> 
> But I don't like papering over these conversions.  I'd prefer instead the
> patches I posted at:
> 
> http://mails.dpdk.org/archives/dev/2019-April/129540.html and
> http://mails.dpdk.org/archives/dev/2019-April/129541.html
> 
> Are you opposed to merging those?
> 
Nope, not in the least. I'm happy enough to rework this patch on top of
those - I'd just had forgotten about them in my rush to get a potential
solution out here.  I did these up quickly to show how easy it is to remove
the need for the weak functions and the subsequent linker "--whole-archive"
flag.

/Bruce

PS: I see your patch 2 does not include the Wno-uninitialized flag, is it
not needed in your patch, or just an oversight?

Re: [dpdk-dev] [PATCH 2/2] bpf: remove use of weak functions

[Aaron Conole]

Bruce Richardson <bruce.richardson@intel.com> writes:

> Weak functions don't work well with static libraries and require the use of
> "whole-archive" flag to ensure that the correct function is used when
> linking.  Since the weak function is only used as a placeholder within this
> library alone, we can replace it with a non-weak version protected using
> preprocessor ifdefs.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

I agree with dropping the weak implementations.

But, can't we adjust the order of objects when building with multiple
strong definitions and the linker will choose the first?  I know this
works for the GNU linker.

I do find this information to help support this statement:

https://stackoverflow.com/questions/51656838/attribute-weak-and-static-libraries

Unfortunately, I don't find anything in the C standard to govern entity
precedence.  This means it isn't something that works across linker
implementations (but compatible implementations like clang will maintain
that behavior).

>  lib/librte_bpf/bpf_load.c  | 4 +++-
>  lib/librte_bpf/meson.build | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_bpf/bpf_load.c b/lib/librte_bpf/bpf_load.c
> index d9d163b7d..194103ec7 100644
> --- a/lib/librte_bpf/bpf_load.c
> +++ b/lib/librte_bpf/bpf_load.c
> @@ -131,7 +131,8 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
>  	return bpf;
>  }
>  
> -__rte_experimental __rte_weak struct rte_bpf *
> +#ifndef RTE_LIBRTE_BPF_ELF
> +__rte_experimental struct rte_bpf *
>  rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
>  	const char *sname)
>  {
> @@ -146,3 +147,4 @@ rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
>  	rte_errno = ENOTSUP;
>  	return NULL;
>  }
> +#endif
> diff --git a/lib/librte_bpf/meson.build b/lib/librte_bpf/meson.build
> index 8a79878ff..11c1fb558 100644
> --- a/lib/librte_bpf/meson.build
> +++ b/lib/librte_bpf/meson.build
> @@ -20,6 +20,7 @@ deps += ['mbuf', 'net', 'ethdev']
>  
>  dep = dependency('libelf', required: false)
>  if dep.found()
> +	dpdk_conf.set('RTE_LIBRTE_BPF_ELF', 1)
>  	sources += files('bpf_load_elf.c')
>  	ext_deps += dep
>  endif

Re: [dpdk-dev] [PATCH 2/2] bpf: remove use of weak functions

[Aaron Conole]

Bruce Richardson <bruce.richardson@intel.com> writes:

> Weak functions don't work well with static libraries and require the use of
> "whole-archive" flag to ensure that the correct function is used when
> linking.  Since the weak function is only used as a placeholder within this
> library alone, we can replace it with a non-weak version protected using
> preprocessor ifdefs.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

I agree with dropping the weak implementations.

But, can't we adjust the order of objects when building with multiple
strong definitions and the linker will choose the first?  I know this
works for the GNU linker.

I do find this information to help support this statement:

https://stackoverflow.com/questions/51656838/attribute-weak-and-static-libraries

Unfortunately, I don't find anything in the C standard to govern entity
precedence.  This means it isn't something that works across linker
implementations (but compatible implementations like clang will maintain
that behavior).

>  lib/librte_bpf/bpf_load.c  | 4 +++-
>  lib/librte_bpf/meson.build | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_bpf/bpf_load.c b/lib/librte_bpf/bpf_load.c
> index d9d163b7d..194103ec7 100644
> --- a/lib/librte_bpf/bpf_load.c
> +++ b/lib/librte_bpf/bpf_load.c
> @@ -131,7 +131,8 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
>  	return bpf;
>  }
>  
> -__rte_experimental __rte_weak struct rte_bpf *
> +#ifndef RTE_LIBRTE_BPF_ELF
> +__rte_experimental struct rte_bpf *
>  rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
>  	const char *sname)
>  {
> @@ -146,3 +147,4 @@ rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
>  	rte_errno = ENOTSUP;
>  	return NULL;
>  }
> +#endif
> diff --git a/lib/librte_bpf/meson.build b/lib/librte_bpf/meson.build
> index 8a79878ff..11c1fb558 100644
> --- a/lib/librte_bpf/meson.build
> +++ b/lib/librte_bpf/meson.build
> @@ -20,6 +20,7 @@ deps += ['mbuf', 'net', 'ethdev']
>  
>  dep = dependency('libelf', required: false)
>  if dep.found()
> +	dpdk_conf.set('RTE_LIBRTE_BPF_ELF', 1)
>  	sources += files('bpf_load_elf.c')
>  	ext_deps += dep
>  endif

Re: [dpdk-dev] [PATCH 1/2] acl: remove use of weak functions

[Aaron Conole]

Bruce Richardson <bruce.richardson@intel.com> writes:

> On Wed, Apr 10, 2019 at 09:54:02AM -0400, Aaron Conole wrote:
>> Bruce Richardson <bruce.richardson@intel.com> writes:
>> 
>> > Weak functions don't work well with static libraries and require the
>> > use of "whole-archive" flag to ensure that the correct function is used
>> > when linking. Since the weak functions are only used as placeholders
>> > within this library alone, we can replace them with non-weak functions
>> > using preprocessor ifdefs.
>> >
>> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
>> > lib/librte_acl/meson.build |  7 ++++++- lib/librte_acl/rte_acl.c   | 18
>> > ++++++++++++++---- mk/rte.app.mk              |  3 --- 3 files changed,
>> > 20 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
>> > index 2207dbafe..98ece7d85 100644 --- a/lib/librte_acl/meson.build +++
>> > b/lib/librte_acl/meson.build @@ -6,7 +6,7 @@ sources =
>> > files('acl_bld.c', 'acl_gen.c', 'acl_run_scalar.c', 'rte_acl.c',
>> > 'tb_mem.c') headers = files('rte_acl.h', 'rte_acl_osdep.h')
>> >  
>> > -if arch_subdir == 'x86' +if dpdk_conf.has('RTE_ARCH_X86') sources +=
>> > files('acl_run_sse.c')
>> >  
>> >  	# compile AVX2 version if either: @@ -28,4 +28,9 @@ if arch_subdir
>> >  	== 'x86' cflags += '-DCC_AVX2_SUPPORT' endif
>> >  
>> > +elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
>> > +	cflags += '-flax-vector-conversions' +	sources +=
>> > files('acl_run_neon.c')
>> 
>> This will also need -Wno-uninitialized (otherwise it will generate
>> warnings about the search_neon_4 and search_neon_8 functions).
>> 
>> But I don't like papering over these conversions.  I'd prefer instead the
>> patches I posted at:
>> 
>> http://mails.dpdk.org/archives/dev/2019-April/129540.html and
>> http://mails.dpdk.org/archives/dev/2019-April/129541.html
>> 
>> Are you opposed to merging those?
>> 
> Nope, not in the least. I'm happy enough to rework this patch on top of
> those - I'd just had forgotten about them in my rush to get a potential
> solution out here.  I did these up quickly to show how easy it is to remove
> the need for the weak functions and the subsequent linker "--whole-archive"
> flag.
>
> /Bruce
>
> PS: I see your patch 2 does not include the Wno-uninitialized flag, is it
> not needed in your patch, or just an oversight?

It isn't needed, I resolved the issue with an explicit initialization.

Re: [dpdk-dev] [PATCH 1/2] acl: remove use of weak functions

[Aaron Conole]

Bruce Richardson <bruce.richardson@intel.com> writes:

> On Wed, Apr 10, 2019 at 09:54:02AM -0400, Aaron Conole wrote:
>> Bruce Richardson <bruce.richardson@intel.com> writes:
>> 
>> > Weak functions don't work well with static libraries and require the
>> > use of "whole-archive" flag to ensure that the correct function is used
>> > when linking. Since the weak functions are only used as placeholders
>> > within this library alone, we can replace them with non-weak functions
>> > using preprocessor ifdefs.
>> >
>> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
>> > lib/librte_acl/meson.build |  7 ++++++- lib/librte_acl/rte_acl.c   | 18
>> > ++++++++++++++---- mk/rte.app.mk              |  3 --- 3 files changed,
>> > 20 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
>> > index 2207dbafe..98ece7d85 100644 --- a/lib/librte_acl/meson.build +++
>> > b/lib/librte_acl/meson.build @@ -6,7 +6,7 @@ sources =
>> > files('acl_bld.c', 'acl_gen.c', 'acl_run_scalar.c', 'rte_acl.c',
>> > 'tb_mem.c') headers = files('rte_acl.h', 'rte_acl_osdep.h')
>> >  
>> > -if arch_subdir == 'x86' +if dpdk_conf.has('RTE_ARCH_X86') sources +=
>> > files('acl_run_sse.c')
>> >  
>> >  	# compile AVX2 version if either: @@ -28,4 +28,9 @@ if arch_subdir
>> >  	== 'x86' cflags += '-DCC_AVX2_SUPPORT' endif
>> >  
>> > +elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
>> > +	cflags += '-flax-vector-conversions' +	sources +=
>> > files('acl_run_neon.c')
>> 
>> This will also need -Wno-uninitialized (otherwise it will generate
>> warnings about the search_neon_4 and search_neon_8 functions).
>> 
>> But I don't like papering over these conversions.  I'd prefer instead the
>> patches I posted at:
>> 
>> http://mails.dpdk.org/archives/dev/2019-April/129540.html and
>> http://mails.dpdk.org/archives/dev/2019-April/129541.html
>> 
>> Are you opposed to merging those?
>> 
> Nope, not in the least. I'm happy enough to rework this patch on top of
> those - I'd just had forgotten about them in my rush to get a potential
> solution out here.  I did these up quickly to show how easy it is to remove
> the need for the weak functions and the subsequent linker "--whole-archive"
> flag.
>
> /Bruce
>
> PS: I see your patch 2 does not include the Wno-uninitialized flag, is it
> not needed in your patch, or just an oversight?

It isn't needed, I resolved the issue with an explicit initialization.

Re: [dpdk-dev] [PATCH 2/2] bpf: remove use of weak functions

[Bruce Richardson]

On Wed, Apr 10, 2019 at 10:07:33AM -0400, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
> 
> > Weak functions don't work well with static libraries and require the use of
> > "whole-archive" flag to ensure that the correct function is used when
> > linking.  Since the weak function is only used as a placeholder within this
> > library alone, we can replace it with a non-weak version protected using
> > preprocessor ifdefs.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> 
> I agree with dropping the weak implementations.
> 
> But, can't we adjust the order of objects when building with multiple
> strong definitions and the linker will choose the first?  I know this
> works for the GNU linker.
> 
> I do find this information to help support this statement:
> 
> https://stackoverflow.com/questions/51656838/attribute-weak-and-static-libraries
> 
> Unfortunately, I don't find anything in the C standard to govern entity
> precedence.  This means it isn't something that works across linker
> implementations (but compatible implementations like clang will maintain
> that behavior).
> 
It may be possible, but I'd be worried about the fragility of the support.
Is it a good idea to have multiple implementations of a function in a .a
file. Without running the resulting binary, we have no way of knowing the
correct function was picked up. At least with the weak versions, we can see
from nm what is used.

/Bruce

Re: [dpdk-dev] [PATCH 2/2] bpf: remove use of weak functions

[Bruce Richardson]

On Wed, Apr 10, 2019 at 10:07:33AM -0400, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
> 
> > Weak functions don't work well with static libraries and require the use of
> > "whole-archive" flag to ensure that the correct function is used when
> > linking.  Since the weak function is only used as a placeholder within this
> > library alone, we can replace it with a non-weak version protected using
> > preprocessor ifdefs.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> 
> I agree with dropping the weak implementations.
> 
> But, can't we adjust the order of objects when building with multiple
> strong definitions and the linker will choose the first?  I know this
> works for the GNU linker.
> 
> I do find this information to help support this statement:
> 
> https://stackoverflow.com/questions/51656838/attribute-weak-and-static-libraries
> 
> Unfortunately, I don't find anything in the C standard to govern entity
> precedence.  This means it isn't something that works across linker
> implementations (but compatible implementations like clang will maintain
> that behavior).
> 
It may be possible, but I'd be worried about the fragility of the support.
Is it a good idea to have multiple implementations of a function in a .a
file. Without running the resulting binary, we have no way of knowing the
correct function was picked up. At least with the weak versions, we can see
from nm what is used.

/Bruce

Re: [dpdk-dev] [PATCH 1/2] acl: remove use of weak functions

[Ananyev, Konstantin]

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Wednesday, April 10, 2019 2:45 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; aconole@redhat.com
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [PATCH 1/2] acl: remove use of weak functions
> 
> Weak functions don't work well with static libraries and require the use of
> "whole-archive" flag to ensure that the correct function is used when
> linking. Since the weak functions are only used as placeholders within
> this library alone, we can replace them with non-weak functions using
> preprocessor ifdefs.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_acl/meson.build |  7 ++++++-
>  lib/librte_acl/rte_acl.c   | 18 ++++++++++++++----
>  mk/rte.app.mk              |  3 ---
>  3 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
> index 2207dbafe..98ece7d85 100644
> --- a/lib/librte_acl/meson.build
> +++ b/lib/librte_acl/meson.build
> @@ -6,7 +6,7 @@ sources = files('acl_bld.c', 'acl_gen.c', 'acl_run_scalar.c',
>  		'rte_acl.c', 'tb_mem.c')
>  headers = files('rte_acl.h', 'rte_acl_osdep.h')
> 
> -if arch_subdir == 'x86'
> +if dpdk_conf.has('RTE_ARCH_X86')
>  	sources += files('acl_run_sse.c')
> 
>  	# compile AVX2 version if either:
> @@ -28,4 +28,9 @@ if arch_subdir == 'x86'
>  		cflags += '-DCC_AVX2_SUPPORT'
>  	endif
> 
> +elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
> +	cflags += '-flax-vector-conversions'
> +	sources += files('acl_run_neon.c')
> +elif dpdk_conf.has('RTE_ARCH_PPC_64')
> +	sources += files('acl_run_altivec.c')
>  endif
> diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> index c436a9bfd..fd5bd5e4e 100644
> --- a/lib/librte_acl/rte_acl.c
> +++ b/lib/librte_acl/rte_acl.c
> @@ -13,11 +13,13 @@ static struct rte_tailq_elem rte_acl_tailq = {
>  };
>  EAL_REGISTER_TAILQ(rte_acl_tailq)
> 
> +#ifndef RTE_ARCH_X86
> +#ifndef CC_AVX2_SUPPORT
>  /*
>   * If the compiler doesn't support AVX2 instructions,
>   * then the dummy one would be used instead for AVX2 classify method.
>   */
> -__rte_weak int
> +int
>  rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -26,8 +28,9 @@ rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
> 
> -__rte_weak int
> +int
>  rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -36,8 +39,11 @@ rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
> 
> -__rte_weak int
> +#ifndef RTE_ARCH_ARM
> +#ifndef RTE_ARCH_ARM64
> +int
>  rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -46,8 +52,11 @@ rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
> +#endif
> 
> -__rte_weak int
> +#ifndef RTE_ARCH_PPC_64
> +int
>  rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -56,6 +65,7 @@ rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
> 
>  static const rte_acl_classify_t classify_fns[] = {
>  	[RTE_ACL_CLASSIFY_DEFAULT] = rte_acl_classify_scalar,
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 7d994bece..fdec636b4 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -46,10 +46,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)    += -lrte_distributor
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)        += -lrte_ip_frag
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_METER)          += -lrte_meter
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)            += -lrte_lpm
> -# librte_acl needs --whole-archive because of weak functions
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += -lrte_acl
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --no-whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --no-as-needed
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += -lrte_telemetry -ljansson
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Tested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.20.1

Re: [dpdk-dev] [PATCH 1/2] acl: remove use of weak functions

[Ananyev, Konstantin]

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Wednesday, April 10, 2019 2:45 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; aconole@redhat.com
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [PATCH 1/2] acl: remove use of weak functions
> 
> Weak functions don't work well with static libraries and require the use of
> "whole-archive" flag to ensure that the correct function is used when
> linking. Since the weak functions are only used as placeholders within
> this library alone, we can replace them with non-weak functions using
> preprocessor ifdefs.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_acl/meson.build |  7 ++++++-
>  lib/librte_acl/rte_acl.c   | 18 ++++++++++++++----
>  mk/rte.app.mk              |  3 ---
>  3 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
> index 2207dbafe..98ece7d85 100644
> --- a/lib/librte_acl/meson.build
> +++ b/lib/librte_acl/meson.build
> @@ -6,7 +6,7 @@ sources = files('acl_bld.c', 'acl_gen.c', 'acl_run_scalar.c',
>  		'rte_acl.c', 'tb_mem.c')
>  headers = files('rte_acl.h', 'rte_acl_osdep.h')
> 
> -if arch_subdir == 'x86'
> +if dpdk_conf.has('RTE_ARCH_X86')
>  	sources += files('acl_run_sse.c')
> 
>  	# compile AVX2 version if either:
> @@ -28,4 +28,9 @@ if arch_subdir == 'x86'
>  		cflags += '-DCC_AVX2_SUPPORT'
>  	endif
> 
> +elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
> +	cflags += '-flax-vector-conversions'
> +	sources += files('acl_run_neon.c')
> +elif dpdk_conf.has('RTE_ARCH_PPC_64')
> +	sources += files('acl_run_altivec.c')
>  endif
> diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
> index c436a9bfd..fd5bd5e4e 100644
> --- a/lib/librte_acl/rte_acl.c
> +++ b/lib/librte_acl/rte_acl.c
> @@ -13,11 +13,13 @@ static struct rte_tailq_elem rte_acl_tailq = {
>  };
>  EAL_REGISTER_TAILQ(rte_acl_tailq)
> 
> +#ifndef RTE_ARCH_X86
> +#ifndef CC_AVX2_SUPPORT
>  /*
>   * If the compiler doesn't support AVX2 instructions,
>   * then the dummy one would be used instead for AVX2 classify method.
>   */
> -__rte_weak int
> +int
>  rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -26,8 +28,9 @@ rte_acl_classify_avx2(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
> 
> -__rte_weak int
> +int
>  rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -36,8 +39,11 @@ rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
> 
> -__rte_weak int
> +#ifndef RTE_ARCH_ARM
> +#ifndef RTE_ARCH_ARM64
> +int
>  rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -46,8 +52,11 @@ rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
> +#endif
> 
> -__rte_weak int
> +#ifndef RTE_ARCH_PPC_64
> +int
>  rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
>  	__rte_unused const uint8_t **data,
>  	__rte_unused uint32_t *results,
> @@ -56,6 +65,7 @@ rte_acl_classify_altivec(__rte_unused const struct rte_acl_ctx *ctx,
>  {
>  	return -ENOTSUP;
>  }
> +#endif
> 
>  static const rte_acl_classify_t classify_fns[] = {
>  	[RTE_ACL_CLASSIFY_DEFAULT] = rte_acl_classify_scalar,
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 7d994bece..fdec636b4 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -46,10 +46,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR)    += -lrte_distributor
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_IP_FRAG)        += -lrte_ip_frag
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_METER)          += -lrte_meter
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_LPM)            += -lrte_lpm
> -# librte_acl needs --whole-archive because of weak functions
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += -lrte_acl
> -_LDLIBS-$(CONFIG_RTE_LIBRTE_ACL)            += --no-whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --no-as-needed
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += --whole-archive
>  _LDLIBS-$(CONFIG_RTE_LIBRTE_TELEMETRY)      += -lrte_telemetry -ljansson
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Tested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.20.1

Re: [dpdk-dev] [PATCH 2/2] bpf: remove use of weak functions

[Ananyev, Konstantin]

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Wednesday, April 10, 2019 2:45 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; aconole@redhat.com
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [PATCH 2/2] bpf: remove use of weak functions
> 
> Weak functions don't work well with static libraries and require the use of
> "whole-archive" flag to ensure that the correct function is used when
> linking.  Since the weak function is only used as a placeholder within this
> library alone, we can replace it with a non-weak version protected using
> preprocessor ifdefs.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_bpf/bpf_load.c  | 4 +++-
>  lib/librte_bpf/meson.build | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_bpf/bpf_load.c b/lib/librte_bpf/bpf_load.c
> index d9d163b7d..194103ec7 100644
> --- a/lib/librte_bpf/bpf_load.c
> +++ b/lib/librte_bpf/bpf_load.c
> @@ -131,7 +131,8 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
>  	return bpf;
>  }
> 
> -__rte_experimental __rte_weak struct rte_bpf *
> +#ifndef RTE_LIBRTE_BPF_ELF
> +__rte_experimental struct rte_bpf *
>  rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
>  	const char *sname)
>  {
> @@ -146,3 +147,4 @@ rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
>  	rte_errno = ENOTSUP;
>  	return NULL;
>  }
> +#endif
> diff --git a/lib/librte_bpf/meson.build b/lib/librte_bpf/meson.build
> index 8a79878ff..11c1fb558 100644
> --- a/lib/librte_bpf/meson.build
> +++ b/lib/librte_bpf/meson.build
> @@ -20,6 +20,7 @@ deps += ['mbuf', 'net', 'ethdev']
> 
>  dep = dependency('libelf', required: false)
>  if dep.found()
> +	dpdk_conf.set('RTE_LIBRTE_BPF_ELF', 1)
>  	sources += files('bpf_load_elf.c')
>  	ext_deps += dep
>  endif
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Tested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.20.1

Re: [dpdk-dev] [PATCH 2/2] bpf: remove use of weak functions

[Ananyev, Konstantin]

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Wednesday, April 10, 2019 2:45 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; aconole@redhat.com
> Cc: dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [PATCH 2/2] bpf: remove use of weak functions
> 
> Weak functions don't work well with static libraries and require the use of
> "whole-archive" flag to ensure that the correct function is used when
> linking.  Since the weak function is only used as a placeholder within this
> library alone, we can replace it with a non-weak version protected using
> preprocessor ifdefs.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_bpf/bpf_load.c  | 4 +++-
>  lib/librte_bpf/meson.build | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_bpf/bpf_load.c b/lib/librte_bpf/bpf_load.c
> index d9d163b7d..194103ec7 100644
> --- a/lib/librte_bpf/bpf_load.c
> +++ b/lib/librte_bpf/bpf_load.c
> @@ -131,7 +131,8 @@ rte_bpf_load(const struct rte_bpf_prm *prm)
>  	return bpf;
>  }
> 
> -__rte_experimental __rte_weak struct rte_bpf *
> +#ifndef RTE_LIBRTE_BPF_ELF
> +__rte_experimental struct rte_bpf *
>  rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
>  	const char *sname)
>  {
> @@ -146,3 +147,4 @@ rte_bpf_elf_load(const struct rte_bpf_prm *prm, const char *fname,
>  	rte_errno = ENOTSUP;
>  	return NULL;
>  }
> +#endif
> diff --git a/lib/librte_bpf/meson.build b/lib/librte_bpf/meson.build
> index 8a79878ff..11c1fb558 100644
> --- a/lib/librte_bpf/meson.build
> +++ b/lib/librte_bpf/meson.build
> @@ -20,6 +20,7 @@ deps += ['mbuf', 'net', 'ethdev']
> 
>  dep = dependency('libelf', required: false)
>  if dep.found()
> +	dpdk_conf.set('RTE_LIBRTE_BPF_ELF', 1)
>  	sources += files('bpf_load_elf.c')
>  	ext_deps += dep
>  endif
> --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Tested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.20.1

Re: [dpdk-dev] [PATCH 0/2] remove use of weak functions from libraries

[David Marchand]

Hello,

On Wed, Apr 10, 2019 at 3:45 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> Weak functions don't work well with static library builds as the linker
> always picks the first version of a function irrespective of whether it is
> weak or not. The solution to this is to use the "whole-archive" flag when
> linking, but this has the nasty side-effect that it causes the resulting
> binary to be larger than it needs to be.
>
>
> A further problem with this approach of using "whole-archive" is that one
> either needs to link all libraries with this flag or track what libraries
> need it or not - the latter being especially a problem for apps not using
> the DPDK build system itself (i.e. most apps not shipped with DPDK itself).
> For meson builds this information needs to make its way all the way through
> to the pkgconfig file generated - not a trivial undertaking.
>
> Thankfully, though, the use of weak functions is limited to use for
> multiple functions within a single library, meaning that when the lib is
> being built, the build system itself can know whether the "weak" function
> is needed or not. This allows us to change the weak function to a
> conditionally compiled regular function used in fallback case. This makes
> the need for "whole-archive" flag, and any special linking measures for the
> library, go away.
>
> [This set does not touch the drivers, only the libraries, since there are
> other special linker flags needed for drivers in general, making the
> problem less severe for driver .a files.]
>
>
Was there something preventing this patchset from getting merged ?


-- 
David Marchand

Re: [dpdk-dev] [PATCH 0/2] remove use of weak functions from libraries

[Bruce Richardson]

On Mon, May 27, 2019 at 04:13:53PM +0200, David Marchand wrote:
>    Hello,
>    On Wed, Apr 10, 2019 at 3:45 PM Bruce Richardson
>    <[1]bruce.richardson@intel.com> wrote:
> 
>      Weak functions don't work well with static library builds as the
>      linker
>      always picks the first version of a function irrespective of whether
>      it is
>      weak or not. The solution to this is to use the "whole-archive" flag
>      when
>      linking, but this has the nasty side-effect that it causes the
>      resulting
>      binary to be larger than it needs to be.
>      A further problem with this approach of using "whole-archive" is
>      that one
>      either needs to link all libraries with this flag or track what
>      libraries
>      need it or not - the latter being especially a problem for apps not
>      using
>      the DPDK build system itself (i.e. most apps not shipped with DPDK
>      itself).
>      For meson builds this information needs to make its way all the way
>      through
>      to the pkgconfig file generated - not a trivial undertaking.
>      Thankfully, though, the use of weak functions is limited to use for
>      multiple functions within a single library, meaning that when the
>      lib is
>      being built, the build system itself can know whether the "weak"
>      function
>      is needed or not. This allows us to change the weak function to a
>      conditionally compiled regular function used in fallback case. This
>      makes
>      the need for "whole-archive" flag, and any special linking measures
>      for the
>      library, go away.
>      [This set does not touch the drivers, only the libraries, since
>      there are
>      other special linker flags needed for drivers in general, making the
>      problem less severe for driver .a files.]
> 
>    Was there something preventing this patchset from getting merged ?
>    --
>    David Marchand
> 
I believe Aaron Conole had some changes in the same area and was looking to
roll these changes into a bigger patchset of his. Aaron, can you please
confirm?

/Bruce

Re: [dpdk-dev] [PATCH 0/2] remove use of weak functions from libraries

[Aaron Conole]

Bruce Richardson <bruce.richardson@intel.com> writes:

> On Mon, May 27, 2019 at 04:13:53PM +0200, David Marchand wrote:
>>    Hello,
>>    On Wed, Apr 10, 2019 at 3:45 PM Bruce Richardson
>>    <[1]bruce.richardson@intel.com> wrote:
>> 
>>      Weak functions don't work well with static library builds as the
>>      linker
>>      always picks the first version of a function irrespective of whether
>>      it is
>>      weak or not. The solution to this is to use the "whole-archive" flag
>>      when
>>      linking, but this has the nasty side-effect that it causes the
>>      resulting
>>      binary to be larger than it needs to be.
>>      A further problem with this approach of using "whole-archive" is
>>      that one
>>      either needs to link all libraries with this flag or track what
>>      libraries
>>      need it or not - the latter being especially a problem for apps not
>>      using
>>      the DPDK build system itself (i.e. most apps not shipped with DPDK
>>      itself).
>>      For meson builds this information needs to make its way all the way
>>      through
>>      to the pkgconfig file generated - not a trivial undertaking.
>>      Thankfully, though, the use of weak functions is limited to use for
>>      multiple functions within a single library, meaning that when the
>>      lib is
>>      being built, the build system itself can know whether the "weak"
>>      function
>>      is needed or not. This allows us to change the weak function to a
>>      conditionally compiled regular function used in fallback case. This
>>      makes
>>      the need for "whole-archive" flag, and any special linking measures
>>      for the
>>      library, go away.
>>      [This set does not touch the drivers, only the libraries, since
>>      there are
>>      other special linker flags needed for drivers in general, making the
>>      problem less severe for driver .a files.]
>> 
>>    Was there something preventing this patchset from getting merged ?
>>    --
>>    David Marchand
>> 
> I believe Aaron Conole had some changes in the same area and was looking to
> roll these changes into a bigger patchset of his. Aaron, can you please
> confirm?

Yes - Sorry the patches are in my queue.  Maybe it just makes sense to
merge these though?

> /Bruce

Re: [dpdk-dev] [PATCH 0/2] remove use of weak functions from libraries

[Bruce Richardson]

On Mon, May 27, 2019 at 04:57:15PM -0400, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
> 
> > On Mon, May 27, 2019 at 04:13:53PM +0200, David Marchand wrote:
> >>    Hello,
> >>    On Wed, Apr 10, 2019 at 3:45 PM Bruce Richardson
> >>    <[1]bruce.richardson@intel.com> wrote:
> >> 
> >>      Weak functions don't work well with static library builds as the
> >>      linker
> >>      always picks the first version of a function irrespective of whether
> >>      it is
> >>      weak or not. The solution to this is to use the "whole-archive" flag
> >>      when
> >>      linking, but this has the nasty side-effect that it causes the
> >>      resulting
> >>      binary to be larger than it needs to be.
> >>      A further problem with this approach of using "whole-archive" is
> >>      that one
> >>      either needs to link all libraries with this flag or track what
> >>      libraries
> >>      need it or not - the latter being especially a problem for apps not
> >>      using
> >>      the DPDK build system itself (i.e. most apps not shipped with DPDK
> >>      itself).
> >>      For meson builds this information needs to make its way all the way
> >>      through
> >>      to the pkgconfig file generated - not a trivial undertaking.
> >>      Thankfully, though, the use of weak functions is limited to use for
> >>      multiple functions within a single library, meaning that when the
> >>      lib is
> >>      being built, the build system itself can know whether the "weak"
> >>      function
> >>      is needed or not. This allows us to change the weak function to a
> >>      conditionally compiled regular function used in fallback case. This
> >>      makes
> >>      the need for "whole-archive" flag, and any special linking measures
> >>      for the
> >>      library, go away.
> >>      [This set does not touch the drivers, only the libraries, since
> >>      there are
> >>      other special linker flags needed for drivers in general, making the
> >>      problem less severe for driver .a files.]
> >> 
> >>    Was there something preventing this patchset from getting merged ?
> >>    --
> >>    David Marchand
> >> 
> > I believe Aaron Conole had some changes in the same area and was looking to
> > roll these changes into a bigger patchset of his. Aaron, can you please
> > confirm?
> 
> Yes - Sorry the patches are in my queue.  Maybe it just makes sense to
> merge these though?
>
Funnily enough, I've no objections to that :-) 

Re: [dpdk-dev] [PATCH 0/2] remove use of weak functions from libraries

[Thomas Monjalon]

10/04/2019 15:45, Bruce Richardson:
> Weak functions don't work well with static library builds as the linker
> always picks the first version of a function irrespective of whether it is
> weak or not. The solution to this is to use the "whole-archive" flag when
> linking, but this has the nasty side-effect that it causes the resulting
> binary to be larger than it needs to be.
> 
> 
> A further problem with this approach of using "whole-archive" is that one
> either needs to link all libraries with this flag or track what libraries
> need it or not - the latter being especially a problem for apps not using
> the DPDK build system itself (i.e. most apps not shipped with DPDK itself).
> For meson builds this information needs to make its way all the way through
> to the pkgconfig file generated - not a trivial undertaking.
> 
> Thankfully, though, the use of weak functions is limited to use for
> multiple functions within a single library, meaning that when the lib is
> being built, the build system itself can know whether the "weak" function
> is needed or not. This allows us to change the weak function to a
> conditionally compiled regular function used in fallback case. This makes
> the need for "whole-archive" flag, and any special linking measures for the
> library, go away.
> 
> [This set does not touch the drivers, only the libraries, since there are
> other special linker flags needed for drivers in general, making the
> problem less severe for driver .a files.]
> 
> Bruce Richardson (2):
>   acl: remove use of weak functions
>   bpf: remove use of weak functions

Applied, thanks