Re: [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in i40e_rxtx.c

[Bruce Richardson <bruce.richardson@intel.com>]

On Tue, Jun 04, 2019 at 04:59:47PM +0100, Ferruh Yigit wrote:
> On 5/16/2019 3:08 PM, Bruce Richardson wrote:
> > On Wed, May 15, 2019 at 12:13:46PM -0400, David Harton wrote:
> >> Use of weak symbols can hide makefile errors especially when
> >> custom makefiles are used.  Removing the use of weak symbols
> >> to avoid a stub function being linked in production code.
> >>
> >> Signed-off-by: David Harton <dharton@cisco.com>
> >> ---
> >>
> >> v2 - added CC_AVX2_SUPPORT check to code enabling avx2 vectors
> >>
> > Testing a few compiles here, this breaks when vector mode is disabled,
> > because it's possible that CC_AVX2_SUPPORT=1 when VECTOR=n. I'd suggest
> > adding "ifeq ($(CONFIG_RTE_LIBRTE_I40E_INC_VECTOR),y) ... endif" around the
> > block in the makefile checking for AVX2 support, so that we never set AVX2
> > unless we have vector support.
> 
> Concern is this is pushing vectorization support more to compile time
> configuration. Do we really have to select if to use vector PMD or not in
> compile time?
> 
> Can't we get rid of the 'CONFIG_RTE_LIBRTE_I40E_INC_VECTOR' config option
> completely? As done in the ICE driver now.
> 
> Isn't it better to compile vectorization support in as much as possible and do
> the vector or scalar path selection in runtime, this patch may prevent us to do
> that, weak functions enables us being more dynamic.
> 
Weak functions are not needed to do the runtime selection - they are
needed for compilation only. They have the downside of potentially causing
runtime problems due to a mis-configured compile, which is only seen later
by the end user. By using real functions rather than weak functions it
means that any mischosen compile paths will flag a compile error rather
than silently succeeding and then accidentally using an incorrect function
at runtime.