* [dpdk-dev] [PATCH] net/i40e: Eliminate weak symbols in i40e_rxtx.c
@ 2019-05-15 13:05 David Harton
2019-05-15 13:05 ` David Harton
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: David Harton @ 2019-05-15 13:05 UTC (permalink / raw)
To: dev, beilei.xing, qi.z.zhang, bruce.richardson; +Cc: David Harton
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>
---
drivers/net/i40e/Makefile | 1 +
drivers/net/i40e/i40e_rxtx.c | 52 +++++++++++++++++++-----------------
drivers/net/i40e/meson.build | 2 ++
3 files changed, 31 insertions(+), 24 deletions(-)
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..cbf31ec88 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -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,
--
2.19.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-05-15 13:05 [dpdk-dev] [PATCH] net/i40e: Eliminate weak symbols in i40e_rxtx.c David Harton
@ 2019-05-15 13:05 ` David Harton
2019-05-15 14:13 ` Bruce Richardson
2019-05-15 16:13 ` [dpdk-dev] [PATCH v2] " David Harton
2 siblings, 0 replies; 14+ messages in thread
From: David Harton @ 2019-05-15 13:05 UTC (permalink / raw)
To: dev, beilei.xing, qi.z.zhang, bruce.richardson; +Cc: David Harton
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>
---
drivers/net/i40e/Makefile | 1 +
drivers/net/i40e/i40e_rxtx.c | 52 +++++++++++++++++++-----------------
drivers/net/i40e/meson.build | 2 ++
3 files changed, 31 insertions(+), 24 deletions(-)
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..cbf31ec88 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -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,
--
2.19.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-05-15 13:05 [dpdk-dev] [PATCH] net/i40e: Eliminate weak symbols in i40e_rxtx.c David Harton
2019-05-15 13:05 ` David Harton
@ 2019-05-15 14:13 ` Bruce Richardson
2019-05-15 14:13 ` Bruce Richardson
2019-05-15 16:13 ` [dpdk-dev] [PATCH v2] " David Harton
2 siblings, 1 reply; 14+ messages in thread
From: Bruce Richardson @ 2019-05-15 14:13 UTC (permalink / raw)
To: David Harton; +Cc: dev, beilei.xing, qi.z.zhang
On Wed, May 15, 2019 at 09:05:12AM -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>
> ---
Looks reasonable to me.
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-05-15 14:13 ` Bruce Richardson
@ 2019-05-15 14:13 ` Bruce Richardson
0 siblings, 0 replies; 14+ messages in thread
From: Bruce Richardson @ 2019-05-15 14:13 UTC (permalink / raw)
To: David Harton; +Cc: dev, beilei.xing, qi.z.zhang
On Wed, May 15, 2019 at 09:05:12AM -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>
> ---
Looks reasonable to me.
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-05-15 13:05 [dpdk-dev] [PATCH] net/i40e: Eliminate weak symbols in i40e_rxtx.c David Harton
2019-05-15 13:05 ` David Harton
2019-05-15 14:13 ` Bruce Richardson
@ 2019-05-15 16:13 ` David Harton
2019-05-15 16:13 ` David Harton
` (2 more replies)
2 siblings, 3 replies; 14+ messages in thread
From: David Harton @ 2019-05-15 16:13 UTC (permalink / raw)
To: dev, beilei.xing, qi.z.zhang, bruce.richardson; +Cc: David Harton
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(-)
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,
--
2.19.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-05-15 16:13 ` [dpdk-dev] [PATCH v2] " David Harton
@ 2019-05-15 16:13 ` David Harton
2019-05-16 14:08 ` Bruce Richardson
2019-05-16 18:28 ` [dpdk-dev] [PATCH v3] " David Harton
2 siblings, 0 replies; 14+ messages in thread
From: David Harton @ 2019-05-15 16:13 UTC (permalink / raw)
To: dev, beilei.xing, qi.z.zhang, bruce.richardson; +Cc: David Harton
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(-)
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,
--
2.19.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-05-15 16:13 ` [dpdk-dev] [PATCH v2] " David Harton
2019-05-15 16:13 ` David Harton
@ 2019-05-16 14:08 ` Bruce Richardson
2019-06-04 15:59 ` Ferruh Yigit
2019-05-16 18:28 ` [dpdk-dev] [PATCH v3] " David Harton
2 siblings, 1 reply; 14+ messages in thread
From: Bruce Richardson @ 2019-05-16 14:08 UTC (permalink / raw)
To: David Harton; +Cc: dev, beilei.xing, qi.z.zhang
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>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v3] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-05-15 16:13 ` [dpdk-dev] [PATCH v2] " David Harton
2019-05-15 16:13 ` David Harton
2019-05-16 14:08 ` Bruce Richardson
@ 2019-05-16 18:28 ` David Harton
2019-05-17 10:21 ` Bruce Richardson
2 siblings, 1 reply; 14+ messages in thread
From: David Harton @ 2019-05-16 18:28 UTC (permalink / raw)
To: dev; +Cc: beilei.xing, qi.z.zhang, bruce.richardson, David Harton
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>
---
v3 - added CONFIG_RTE_LIBRTE_I40E_INC_VECTOR to makefile
v2 - added CC_AVX2_SUPPORT check to code enabling avx2 vectors
drivers/net/i40e/Makefile | 3 ++
drivers/net/i40e/i40e_rxtx.c | 60 +++++++++++++++++++-----------------
drivers/net/i40e/meson.build | 2 ++
3 files changed, 37 insertions(+), 28 deletions(-)
diff --git a/drivers/net/i40e/Makefile b/drivers/net/i40e/Makefile
index 3f869a8d6..1c3bbe329 100644
--- a/drivers/net/i40e/Makefile
+++ b/drivers/net/i40e/Makefile
@@ -89,6 +89,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += rte_pmd_i40e.c
SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_tm.c
SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_vf_representor.c
+ifeq ($(CONFIG_RTE_LIBRTE_I40E_INC_VECTOR),y)
ifeq ($(findstring RTE_MACHINE_CPUFLAG_AVX2,$(CFLAGS)),RTE_MACHINE_CPUFLAG_AVX2)
CC_AVX2_SUPPORT=1
else
@@ -103,9 +104,11 @@ else
endif
endif
endif
+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,
--
2.19.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-05-16 18:28 ` [dpdk-dev] [PATCH v3] " David Harton
@ 2019-05-17 10:21 ` Bruce Richardson
2019-05-29 17:18 ` Zhang, Qi Z
0 siblings, 1 reply; 14+ messages in thread
From: Bruce Richardson @ 2019-05-17 10:21 UTC (permalink / raw)
To: David Harton; +Cc: dev, beilei.xing, qi.z.zhang
On Thu, May 16, 2019 at 02:28:03PM -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>
> ---
>
> v3 - added CONFIG_RTE_LIBRTE_I40E_INC_VECTOR to makefile
> v2 - added CC_AVX2_SUPPORT check to code enabling avx2 vectors
>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-05-17 10:21 ` Bruce Richardson
@ 2019-05-29 17:18 ` Zhang, Qi Z
0 siblings, 0 replies; 14+ messages in thread
From: Zhang, Qi Z @ 2019-05-29 17:18 UTC (permalink / raw)
To: Richardson, Bruce, David Harton; +Cc: dev, Xing, Beilei
> -----Original Message-----
> From: Richardson, Bruce
> Sent: Friday, May 17, 2019 3:21 AM
> To: David Harton <dharton@cisco.com>
> Cc: dev@dpdk.org; Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Subject: Re: [PATCH v3] net/i40e: Eliminate weak symbols in i40e_rxtx.c
>
> On Thu, May 16, 2019 at 02:28:03PM -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>
> > ---
> >
> > v3 - added CONFIG_RTE_LIBRTE_I40E_INC_VECTOR to makefile
> > v2 - added CC_AVX2_SUPPORT check to code enabling avx2 vectors
> >
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Applied to dpdk-next-net-intel.
Thanks
Qi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-05-16 14:08 ` Bruce Richardson
@ 2019-06-04 15:59 ` Ferruh Yigit
2019-06-04 16:19 ` David Harton (dharton)
2019-06-04 16:25 ` Bruce Richardson
0 siblings, 2 replies; 14+ messages in thread
From: Ferruh Yigit @ 2019-06-04 15:59 UTC (permalink / raw)
To: Bruce Richardson, David Harton; +Cc: dev, beilei.xing, qi.z.zhang
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.
>
> With this change, you can include my ack in v3.
>
> /Bruce
>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-06-04 15:59 ` Ferruh Yigit
@ 2019-06-04 16:19 ` David Harton (dharton)
2019-06-06 9:07 ` Ferruh Yigit
2019-06-04 16:25 ` Bruce Richardson
1 sibling, 1 reply; 14+ messages in thread
From: David Harton (dharton) @ 2019-06-04 16:19 UTC (permalink / raw)
To: Ferruh Yigit, Bruce Richardson; +Cc: dev, beilei.xing, qi.z.zhang
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, June 04, 2019 12:00 PM
> To: Bruce Richardson <bruce.richardson@intel.com>; David Harton (dharton)
> <dharton@cisco.com>
> Cc: dev@dpdk.org; beilei.xing@intel.com; qi.z.zhang@intel.com
> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in
> i40e_rxtx.c
>
> 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.
Choosing a vector dynamically can be done without the use of weak symbols.
IMHO, weak symbols should really be used for cross library support where a user wants to override a 3rd party function not within a lib. In fact the weak symbol usage may preclude supporting all the variants you might need/want to select.
>
> >
> > With this change, you can include my ack in v3.
> >
> > /Bruce
> >
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> >
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-06-04 15:59 ` Ferruh Yigit
2019-06-04 16:19 ` David Harton (dharton)
@ 2019-06-04 16:25 ` Bruce Richardson
1 sibling, 0 replies; 14+ messages in thread
From: Bruce Richardson @ 2019-06-04 16:25 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: David Harton, dev, beilei.xing, qi.z.zhang
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in i40e_rxtx.c
2019-06-04 16:19 ` David Harton (dharton)
@ 2019-06-06 9:07 ` Ferruh Yigit
0 siblings, 0 replies; 14+ messages in thread
From: Ferruh Yigit @ 2019-06-06 9:07 UTC (permalink / raw)
To: David Harton (dharton), Bruce Richardson; +Cc: dev, beilei.xing, qi.z.zhang
On 6/4/2019 5:19 PM, David Harton (dharton) wrote:
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Tuesday, June 04, 2019 12:00 PM
>> To: Bruce Richardson <bruce.richardson@intel.com>; David Harton (dharton)
>> <dharton@cisco.com>
>> Cc: dev@dpdk.org; beilei.xing@intel.com; qi.z.zhang@intel.com
>> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: Eliminate weak symbols in
>> i40e_rxtx.c
>>
>> 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.
>
> Choosing a vector dynamically can be done without the use of weak symbols.
No objection then.
>
> IMHO, weak symbols should really be used for cross library support where a user wants to override a 3rd party function not within a lib. In fact the weak symbol usage may preclude supporting all the variants you might need/want to select.
>
>>
>>>
>>> With this change, you can include my ack in v3.
>>>
>>> /Bruce
>>>
>>> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
>>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-06-06 9:07 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-15 13:05 [dpdk-dev] [PATCH] net/i40e: Eliminate weak symbols in i40e_rxtx.c David Harton
2019-05-15 13:05 ` David Harton
2019-05-15 14:13 ` Bruce Richardson
2019-05-15 14:13 ` Bruce Richardson
2019-05-15 16:13 ` [dpdk-dev] [PATCH v2] " David Harton
2019-05-15 16:13 ` David Harton
2019-05-16 14:08 ` Bruce Richardson
2019-06-04 15:59 ` Ferruh Yigit
2019-06-04 16:19 ` David Harton (dharton)
2019-06-06 9:07 ` Ferruh Yigit
2019-06-04 16:25 ` Bruce Richardson
2019-05-16 18:28 ` [dpdk-dev] [PATCH v3] " David Harton
2019-05-17 10:21 ` Bruce Richardson
2019-05-29 17:18 ` Zhang, Qi Z
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).