[v2] net/i40e: Eliminate weak symbols in i40e_rxtx.c

Message ID 20190515161346.33080-1-dharton@cisco.com
State Superseded
Headers show
Series
  • [v2] net/i40e: Eliminate weak symbols in i40e_rxtx.c
Related show

Checks

Context Check Description
ci/Intel-compilation success Compilation OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/checkpatch warning coding style issues

Commit Message

David Harton May 15, 2019, 4:13 p.m.
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

 drivers/net/i40e/Makefile    |  1 +
 drivers/net/i40e/i40e_rxtx.c | 60 +++++++++++++++++++-----------------
 drivers/net/i40e/meson.build |  2 ++
 3 files changed, 35 insertions(+), 28 deletions(-)

Comments

Bruce Richardson May 16, 2019, 2:08 p.m. | #1
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.

With this change, you can include my ack in v3.

/Bruce 

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Patch

diff --git a/drivers/net/i40e/Makefile b/drivers/net/i40e/Makefile
index 3f869a8d6..cbcfce099 100644
--- a/drivers/net/i40e/Makefile
+++ b/drivers/net/i40e/Makefile
@@ -106,6 +106,7 @@  endif
 
 ifeq ($(CC_AVX2_SUPPORT), 1)
 	SRCS-$(CONFIG_RTE_LIBRTE_I40E_INC_VECTOR) += i40e_rxtx_vec_avx2.c
+	CFLAGS_i40e_rxtx.o += -DCC_AVX2_SUPPORT
 endif
 
 # install this header file
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 1489552da..984d322cd 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -2919,7 +2919,7 @@  i40e_txq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 static eth_rx_burst_t
 i40e_get_latest_rx_vec(bool scatter)
 {
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) && defined(CC_AVX2_SUPPORT)
 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
 		return scatter ? i40e_recv_scattered_pkts_vec_avx2 :
 				 i40e_recv_pkts_vec_avx2;
@@ -2931,7 +2931,7 @@  i40e_get_latest_rx_vec(bool scatter)
 static eth_rx_burst_t
 i40e_get_recommend_rx_vec(bool scatter)
 {
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) && defined(CC_AVX2_SUPPORT)
 	/*
 	 * since AVX frequency can be different to base frequency, limit
 	 * use of AVX2 version to later plaforms, not all those that could
@@ -3048,7 +3048,7 @@  i40e_set_tx_function_flag(struct rte_eth_dev *dev, struct i40e_tx_queue *txq)
 static eth_tx_burst_t
 i40e_get_latest_tx_vec(void)
 {
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) && defined(CC_AVX2_SUPPORT)
 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
 		return i40e_xmit_pkts_vec_avx2;
 #endif
@@ -3058,7 +3058,7 @@  i40e_get_latest_tx_vec(void)
 static eth_tx_burst_t
 i40e_get_recommend_tx_vec(void)
 {
-#ifdef RTE_ARCH_X86
+#if defined(RTE_ARCH_X86) && defined(CC_AVX2_SUPPORT)
 	/*
 	 * since AVX frequency can be different to base frequency, limit
 	 * use of AVX2 version to later plaforms, not all those that could
@@ -3181,14 +3181,15 @@  i40e_set_default_pctype_table(struct rte_eth_dev *dev)
 	}
 }
 
+#ifndef RTE_LIBRTE_I40E_INC_VECTOR
 /* Stubs needed for linkage when CONFIG_RTE_LIBRTE_I40E_INC_VECTOR is set to 'n' */
-__rte_weak int
+int
 i40e_rx_vec_dev_conf_condition_check(struct rte_eth_dev __rte_unused *dev)
 {
 	return -1;
 }
 
-__rte_weak uint16_t
+uint16_t
 i40e_recv_pkts_vec(
 	void __rte_unused *rx_queue,
 	struct rte_mbuf __rte_unused **rx_pkts,
@@ -3197,7 +3198,7 @@  i40e_recv_pkts_vec(
 	return 0;
 }
 
-__rte_weak uint16_t
+uint16_t
 i40e_recv_scattered_pkts_vec(
 	void __rte_unused *rx_queue,
 	struct rte_mbuf __rte_unused **rx_pkts,
@@ -3206,52 +3207,55 @@  i40e_recv_scattered_pkts_vec(
 	return 0;
 }
 
-__rte_weak uint16_t
-i40e_recv_pkts_vec_avx2(void __rte_unused *rx_queue,
-			struct rte_mbuf __rte_unused **rx_pkts,
-			uint16_t __rte_unused nb_pkts)
-{
-	return 0;
-}
-
-__rte_weak uint16_t
-i40e_recv_scattered_pkts_vec_avx2(void __rte_unused *rx_queue,
-			struct rte_mbuf __rte_unused **rx_pkts,
-			uint16_t __rte_unused nb_pkts)
-{
-	return 0;
-}
-
-__rte_weak int
+int
 i40e_rxq_vec_setup(struct i40e_rx_queue __rte_unused *rxq)
 {
 	return -1;
 }
 
-__rte_weak int
+int
 i40e_txq_vec_setup(struct i40e_tx_queue __rte_unused *txq)
 {
 	return -1;
 }
 
-__rte_weak void
+void
 i40e_rx_queue_release_mbufs_vec(struct i40e_rx_queue __rte_unused*rxq)
 {
 	return;
 }
 
-__rte_weak uint16_t
+uint16_t
 i40e_xmit_fixed_burst_vec(void __rte_unused * tx_queue,
 			  struct rte_mbuf __rte_unused **tx_pkts,
 			  uint16_t __rte_unused nb_pkts)
 {
 	return 0;
 }
+#endif /* ifndef RTE_LIBRTE_I40E_INC_VECTOR */
+
+#ifndef CC_AVX2_SUPPORT
+uint16_t
+i40e_recv_pkts_vec_avx2(void __rte_unused *rx_queue,
+			struct rte_mbuf __rte_unused **rx_pkts,
+			uint16_t __rte_unused nb_pkts)
+{
+	return 0;
+}
 
-__rte_weak uint16_t
+uint16_t
+i40e_recv_scattered_pkts_vec_avx2(void __rte_unused *rx_queue,
+			struct rte_mbuf __rte_unused **rx_pkts,
+			uint16_t __rte_unused nb_pkts)
+{
+	return 0;
+}
+
+uint16_t
 i40e_xmit_pkts_vec_avx2(void __rte_unused * tx_queue,
 			  struct rte_mbuf __rte_unused **tx_pkts,
 			  uint16_t __rte_unused nb_pkts)
 {
 	return 0;
 }
+#endif /* ifndef CC_AVX2_SUPPORT */
diff --git a/drivers/net/i40e/meson.build b/drivers/net/i40e/meson.build
index d783f3626..dcbca39bf 100644
--- a/drivers/net/i40e/meson.build
+++ b/drivers/net/i40e/meson.build
@@ -35,8 +35,10 @@  if arch_subdir == 'x86'
 	# a. we have AVX supported in minimum instruction set baseline
 	# b. it's not minimum instruction set, but supported by compiler
 	if dpdk_conf.has('RTE_MACHINE_CPUFLAG_AVX2')
+		cflags += ['-DCC_AVX2_SUPPORT']
 		sources += files('i40e_rxtx_vec_avx2.c')
 	elif cc.has_argument('-mavx2')
+		cflags += ['-DCC_AVX2_SUPPORT']
 		i40e_avx2_lib = static_library('i40e_avx2_lib',
 				'i40e_rxtx_vec_avx2.c',
 				dependencies: [static_rte_ethdev,