Re: [dpdk-dev] [PATCH] examples/ethtool: fix pause configuration
Hi, all,
any comments?
在 2021/4/28 16:42, Min Hu (Connor) 写道:
From: Huisong Li
Currently, the pause command in ethtool to enable Rx/Tx pause has the
following problem. Namely, Assume that the device supports flow control
auto-negotiation to set pause parameters, which will the device that does
not support flow control auto-negotiation fails to run this command.
This patch supports the configuration of flow control auto-negotiation
and fixes the print format and style of the pause cmd to make it more
readable.
Fixes: bda68ab9d1e7 ("examples/ethtool: add user-space ethtool sample
application")
Signed-off-by: Huisong Li
Signed-off-by: Min Hu (Connor)
---
examples/ethtool/ethtool-app/ethapp.c | 59 +--
1 file changed, 36 insertions(+), 23 deletions(-)
diff --git a/examples/ethtool/ethtool-app/ethapp.c
b/examples/ethtool/ethtool-app/ethapp.c
index 36a1c37..057fa97 100644
--- a/examples/ethtool/ethtool-app/ethapp.c
+++ b/examples/ethtool/ethtool-app/ethapp.c
@@ -49,6 +49,13 @@ struct pcmd_intintint_params {
uint16_t rx;
};
+struct pcmd_pause_params {
+ cmdline_fixed_string_t cmd;
+ uint16_t port;
+ cmdline_fixed_string_t mode;
+ cmdline_fixed_string_t autoneg;
+ cmdline_fixed_string_t an_status;
+};
/* Parameter-less commands */
cmdline_parse_token_string_t pcmd_quit_token_cmd =
@@ -116,12 +123,18 @@ cmdline_parse_token_num_t pcmd_intintint_token_rx =
/* Pause commands */
cmdline_parse_token_string_t pcmd_pause_token_cmd =
- TOKEN_STRING_INITIALIZER(struct pcmd_intstr_params, cmd, "pause");
+ TOKEN_STRING_INITIALIZER(struct pcmd_pause_params, cmd, "pause");
cmdline_parse_token_num_t pcmd_pause_token_port =
- TOKEN_NUM_INITIALIZER(struct pcmd_intstr_params, port, RTE_UINT16);
-cmdline_parse_token_string_t pcmd_pause_token_opt =
- TOKEN_STRING_INITIALIZER(struct pcmd_intstr_params,
- opt, "all#tx#rx#none");
+ TOKEN_NUM_INITIALIZER(struct pcmd_pause_params, port, RTE_UINT16);
+cmdline_parse_token_string_t pcmd_pause_token_mode =
+ TOKEN_STRING_INITIALIZER(struct pcmd_pause_params,
+ mode, "full#tx#rx#none");
+cmdline_parse_token_string_t pcmd_pause_token_autoneg =
+ TOKEN_STRING_INITIALIZER(struct pcmd_pause_params,
+ autoneg, "autoneg");
+cmdline_parse_token_string_t pcmd_pause_token_an_status =
+ TOKEN_STRING_INITIALIZER(struct pcmd_pause_params,
+ an_status, "on#off");
/* VLAN commands */
cmdline_parse_token_string_t pcmd_vlan_token_cmd =
@@ -348,13 +361,12 @@ pcmd_module_eeprom_callback(void *ptr_params,
fclose(fp_eeprom);
}
-
static void
pcmd_pause_callback(void *ptr_params,
__rte_unused struct cmdline *ctx,
void *ptr_data)
{
- struct pcmd_intstr_params *params = ptr_params;
+ struct pcmd_pause_params *params = ptr_params;
struct ethtool_pauseparam info;
int stat;
@@ -366,39 +378,38 @@ pcmd_pause_callback(void *ptr_params,
stat = rte_ethtool_get_pauseparam(params->port, &info);
} else {
memset(&info, 0, sizeof(info));
- if (strcasecmp("all", params->opt) == 0) {
+ if (strcasecmp("full", params->mode) == 0) {
info.tx_pause = 1;
info.rx_pause = 1;
- } else if (strcasecmp("tx", params->opt) == 0) {
+ } else if (strcasecmp("tx", params->mode) == 0) {
info.tx_pause = 1;
info.rx_pause = 0;
- } else if (strcasecmp("rx", params->opt) == 0) {
+ } else if (strcasecmp("rx", params->mode) == 0) {
info.tx_pause = 0;
info.rx_pause = 1;
} else {
info.tx_pause = 0;
info.rx_pause = 0;
}
- /* Assume auto-negotiation wanted */
- info.autoneg = 1;
+
+ if (strcasecmp("on", params->an_status) == 0)
+ info.autoneg = 1;
+ else
+ info.autoneg = 0;
+
stat = rte_ethtool_set_pauseparam(params->port, &info);
}
if (stat == 0) {
- if (info.rx_pause && info.tx_pause)
- printf("Port %i: Tx & Rx Paused\n", params->port);
- else if (info.rx_pause)
- printf("Port %i: Rx Paused\n", params->port);
- else if (info.tx_pause)
- printf("Port %i: Tx Paused\n", params->port);
- else
- printf("Port %i: Tx & Rx not paused\n", params->port);
+ printf("Pause parameters for Port %i:\n", params->port);
+ printf("Rx pause: %s\n", info.rx_pause ? "on" : "off");
+ printf("Tx pause: %s\n", info.tx_pause ? "on" : "off"
Re: [dpdk-dev] [PATCH v1 1/2] net/i40e: improve performance for scalar Tx
> -Original Message-
> From: Feifei Wang
> Sent: Friday, June 25, 2021 5:40 PM
> To: Xing, Beilei
> Cc: dev@dpdk.org; nd ; Ruifeng Wang
> ; nd ; nd
> Subject: 回复: [PATCH v1 1/2] net/i40e: improve performance for scalar Tx
>
>
>
> > > int n = txq->tx_rs_thresh;
> > > int32_t i = 0, j = 0;
> > > const int32_t k = RTE_ALIGN_FLOOR(n, RTE_I40E_TX_MAX_FREE_BUF_SZ);
> > > const int32_t m = n % RTE_I40E_TX_MAX_FREE_BUF_SZ; struct rte_mbuf
> > > *free[RTE_I40E_TX_MAX_FREE_BUF_SZ];
> > >
> > > For FAST_FREE_MODE:
> > >
> > > if (k) {
> > > for (j = 0; j != k - RTE_I40E_TX_MAX_FREE_BUF_SZ;
> > > j += RTE_I40E_TX_MAX_FREE_BUF_SZ) {
> > > for (i = 0; i > > free[i] = txep->mbuf;
> > > txep->mbuf = NULL;
> > > }
> > > rte_mempool_put_bulk(free[0]->pool, (void **)free,
> > > RTE_I40E_TX_MAX_FREE_BUF_SZ);
> > > }
> > > }
> > >
> > > if (m) {
> > > for (i = 0; i < m; ++i, ++txep) {
> > > free[i] = txep->mbuf;
> > > txep->mbuf = NULL;
> > > }
> > > }
> > > rte_mempool_put_bulk(free[0]->pool, (void **)free, m); }
>
> > Seems no logical problem, but the code looks heavy due to for loops.
> > Did you run performance with this change when tx_rs_thresh >
> > RTE_I40E_TX_MAX_FREE_BUF_SZ?
>
> Sorry for my late rely. It takes me some time to do the test for this path and
> following is my test results:
>
> First, I come up with another way to solve this bug and compare it with
> "loop"(size of 'free' is 64).
> That is set the size of 'free' as a large constant. We know:
> tx_rs_thresh < ring_desc_size < I40E_MAX_RING_DESC(4096), so we can
> directly define as:
> struct rte_mbuf *free[RTE_I40E_TX_MAX_FREE_BUF_SZ];
>
> [1]Test Config:
> MRR Test: two porst & bi-directional flows & one core RX API:
> i40e_recv_pkts_bulk_alloc TX API: i40e_xmit_pkts_simple
> ring_descs_size: 1024
> Ring_I40E_TX_MAX_FREE_SZ: 64
>
> [2]Scheme:
> tx_rs_thresh = I40E_DEFAULT_TX_RSBIT_THRESH tx_free_thresh =
> I40E_DEFAULT_TX_FREE_THRESH tx_rs_thresh <= tx_free_thresh <
> nb_tx_desc So we change the value of 'tx_rs_thresh' by adjust
> I40E_DEFAULT_TX_RSBIT_THRESH
>
> [3]Test Results (performance improve):
> In X86:
> tx_rs_thresh/ tx_free_thresh 32/32 256/256
> 512/512
> 1.mempool_put(base) 0 0
> 0
> 2.mempool_put_bulk:loop +4.7% +5.6%
> +7.0%
> 3.mempool_put_bulk:large size for free +3.8% +2.3%
> -2.0%
> (free[I40E_MAX_RING_DESC])
>
> In Arm:
> N1SDP:
> tx_rs_thresh/ tx_free_thresh 32/32 256/256
> 512/512
> 1.mempool_put(base) 0 0
> 0
> 2.mempool_put_bulk:loop +7.9% +9.1%
> +2.9%
> 3.mempool_put_bulk:large size for free+7.1% +8.7%
> +3.4%
> (free[I40E_MAX_RING_DESC])
>
> Thunderx2:
> tx_rs_thresh/ tx_free_thresh 32/32 256/256
> 512/512
> 1.mempool_put(base) 0 0
> 0
> 2.mempool_put_bulk:loop +7.6% +10.5%
> +7.6%
> 3.mempool_put_bulk:large size for free+1.7% +18.4%
> +10.2%
> (free[I40E_MAX_RING_DESC])
>
> As a result, I feel maybe 'loop' is better and it seems not very heavy
> according to the test.
> What about your views and look forward to your reply.
> Thanks a lot.
Thanks for your patch and test.
It looks OK for me, please send V2.
[dpdk-dev] 回复: [PATCH v1 1/2] net/i40e: improve performance for scalar Tx
> -邮件原件-
> 发件人: Xing, Beilei
> 发送时间: 2021年6月28日 10:27
> 收件人: Feifei Wang
> 抄送: dev@dpdk.org; nd ; Ruifeng Wang
> ; nd ; nd
> 主题: RE: [PATCH v1 1/2] net/i40e: improve performance for scalar Tx
>
>
>
> > -Original Message-
> > From: Feifei Wang
> > Sent: Friday, June 25, 2021 5:40 PM
> > To: Xing, Beilei
> > Cc: dev@dpdk.org; nd ; Ruifeng Wang
> > ; nd ; nd
> > Subject: 回复: [PATCH v1 1/2] net/i40e: improve performance for scalar
> > Tx
> >
> >
> >
> > > > int n = txq->tx_rs_thresh;
> > > > int32_t i = 0, j = 0;
> > > > const int32_t k = RTE_ALIGN_FLOOR(n,
> RTE_I40E_TX_MAX_FREE_BUF_SZ);
> > > > const int32_t m = n % RTE_I40E_TX_MAX_FREE_BUF_SZ; struct
> rte_mbuf
> > > > *free[RTE_I40E_TX_MAX_FREE_BUF_SZ];
> > > >
> > > > For FAST_FREE_MODE:
> > > >
> > > > if (k) {
> > > > for (j = 0; j != k - RTE_I40E_TX_MAX_FREE_BUF_SZ;
> > > > j += RTE_I40E_TX_MAX_FREE_BUF_SZ) {
> > > > for (i = 0; i > > > ++txep) {
> > > > free[i] = txep->mbuf;
> > > > txep->mbuf = NULL;
> > > > }
> > > > rte_mempool_put_bulk(free[0]->pool, (void **)free,
> > > > RTE_I40E_TX_MAX_FREE_BUF_SZ);
> > > > }
> > > > }
> > > >
> > > > if (m) {
> > > > for (i = 0; i < m; ++i, ++txep) {
> > > > free[i] = txep->mbuf;
> > > > txep->mbuf = NULL;
> > > > }
> > > > }
> > > > rte_mempool_put_bulk(free[0]->pool, (void **)free, m); }
> >
> > > Seems no logical problem, but the code looks heavy due to for loops.
> > > Did you run performance with this change when tx_rs_thresh >
> > > RTE_I40E_TX_MAX_FREE_BUF_SZ?
> >
> > Sorry for my late rely. It takes me some time to do the test for this
> > path and following is my test results:
> >
> > First, I come up with another way to solve this bug and compare it
> > with "loop"(size of 'free' is 64).
> > That is set the size of 'free' as a large constant. We know:
> > tx_rs_thresh < ring_desc_size < I40E_MAX_RING_DESC(4096), so we can
> > directly define as:
> > struct rte_mbuf *free[RTE_I40E_TX_MAX_FREE_BUF_SZ];
> >
> > [1]Test Config:
> > MRR Test: two porst & bi-directional flows & one core RX API:
> > i40e_recv_pkts_bulk_alloc TX API: i40e_xmit_pkts_simple
> > ring_descs_size: 1024
> > Ring_I40E_TX_MAX_FREE_SZ: 64
> >
> > [2]Scheme:
> > tx_rs_thresh = I40E_DEFAULT_TX_RSBIT_THRESH tx_free_thresh =
> > I40E_DEFAULT_TX_FREE_THRESH tx_rs_thresh <= tx_free_thresh <
> > nb_tx_desc So we change the value of 'tx_rs_thresh' by adjust
> > I40E_DEFAULT_TX_RSBIT_THRESH
> >
> > [3]Test Results (performance improve):
> > In X86:
> > tx_rs_thresh/ tx_free_thresh 32/32 256/256
> >512/512
> > 1.mempool_put(base) 0 0
> > 0
> > 2.mempool_put_bulk:loop +4.7% +5.6%
> > +7.0%
> > 3.mempool_put_bulk:large size for free +3.8% +2.3%
> > -2.0%
> > (free[I40E_MAX_RING_DESC])
> >
> > In Arm:
> > N1SDP:
> > tx_rs_thresh/ tx_free_thresh 32/32 256/256
> >512/512
> > 1.mempool_put(base) 0 0
> > 0
> > 2.mempool_put_bulk:loop +7.9% +9.1%
> > +2.9%
> > 3.mempool_put_bulk:large size for free+7.1% +8.7%
> > +3.4%
> > (free[I40E_MAX_RING_DESC])
> >
> > Thunderx2:
> > tx_rs_thresh/ tx_free_thresh 32/32 256/256
> >512/512
> > 1.mempool_put(base) 0 0
> > 0
> > 2.mempool_put_bulk:loop +7.6% +10.5%
> >+7.6%
> > 3.mempool_put_bulk:large size for free+1.7% +18.4%
> > +10.2%
> > (free[I40E_MAX_RING_DESC])
> >
> > As a result, I feel maybe 'loop' is better and it seems not very heavy
> > according to the test.
> > What about your views and look forward to your reply.
> > Thanks a lot.
>
> Thanks for your patch and test.
> It looks OK for me, please send V2.
Thanks for the reviewing, I will update the V2 version.
[dpdk-dev] [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
If the target machine has SVE feature (e.g. '-march=armv8.2-a+sve'),
and compiler are gcc8.3, it will compile error, the error is arm_sve.h
no such file or directory.
The solution:
a. If RTE_HAS_SVE_ACLE defined (it means the minimum instruction set
support SVE ACLE) then compiles it.
b. Else if the compiler support SVE ACLE then compiles it.
c. Otherwise don't compile it.
Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
Cc: sta...@dpdk.org
Signed-off-by: Chengwen Feng
Acked-by: Ruifeng Wang
---
drivers/net/hns3/hns3_rxtx.c | 2 +-
drivers/net/hns3/meson.build | 20 +++-
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index cb9eccf..a86e105 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -2811,7 +2811,7 @@ hns3_get_default_vec_support(void)
static bool
hns3_get_sve_support(void)
{
-#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
+#if defined(RTE_HAS_SVE_ACLE)
if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
return false;
if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
index 53c7df7..a99e0db 100644
--- a/drivers/net/hns3/meson.build
+++ b/drivers/net/hns3/meson.build
@@ -35,7 +35,25 @@ deps += ['hash']
if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
sources += files('hns3_rxtx_vec.c')
-if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
+
+# compile SVE when:
+# a. support SVE in minimum instruction set baseline
+# b. it's not minimum instruction set, but compiler support
+if dpdk_conf.has('RTE_HAS_SVE_ACLE')
sources += files('hns3_rxtx_vec_sve.c')
+elif cc.has_argument('-march=armv8.2-a+sve') and
cc.check_header('arm_sve.h')
+cflags += ['-DRTE_HAS_SVE_ACLE=1']
+sve_cflags = []
+foreach flag: cflags
+if not (flag.startswith('-march=') or flag.startswith('-mcpu=') or
flag.startswith('-mtune='))
+sve_cflags += flag
+endif
+endforeach
+hns3_sve_lib = static_library('hns3_sve_lib',
+'hns3_rxtx_vec_sve.c',
+dependencies: [static_rte_ethdev],
+include_directories: includes,
+c_args: [sve_cflags, '-march=armv8.2-a+sve'])
+objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
endif
endif
--
2.8.1
[dpdk-dev] [PATCH 0/2] bugfix for SVE compile
This patch set contains two bugfixes for SVE compile. Note: 1) Because 2/2 patch needs backport to 20.11 so we separate it. 2) These two patches already acked by ARM guys from previous threads. Chengwen Feng (2): build: fix SVE compile error with gcc8.3 net/hns3: fix SVE code compile error with gcc8.3 config/arm/meson.build | 6 ++ drivers/net/hns3/hns3_rxtx.c | 2 +- drivers/net/hns3/meson.build | 20 +++- lib/eal/arm/include/rte_vect.h | 2 +- lib/lpm/rte_lpm.h | 2 +- 5 files changed, 28 insertions(+), 4 deletions(-) -- 2.8.1
[dpdk-dev] [PATCH 1/2] build: fix SVE compile error with gcc8.3
If the target machine has SVE feature (e.g. "-march=armv8.2-a+sve'),
and the compiler are gcc8.3, it will compile error:
In file included from ../dpdk-next-net/lib/eal/common/
eal_common_options.c:38:
../dpdk-next-net/lib/eal/arm/include/rte_vect.h:13:10: fatal
error: arm_sve.h: No such file or directory
#include
^~~
compilation terminated.
The root cause is that gcc8.3 supports SVE (the macro
__ARM_FEATURE_SVE was 1), but it doesn't support SVE ACLE [1].
The solution:
a) Detect compiler whether support SVE ACLE, if support then define
RTE_HAS_SVE_ACLE macro.
b) Use the RTE_HAS_SVE_ACLE macro to include SVE header file.
[1] ACLE: Arm C Language Extensions, the SVE ACLE header file is
, user should include it when writing ACLE SVE code.
Fixes: 67b68824a82d ("lpm/arm: support SVE")
Signed-off-by: Chengwen Feng
Acked-by: Ruifeng Wang
---
config/arm/meson.build | 6 ++
lib/eal/arm/include/rte_vect.h | 2 +-
lib/lpm/rte_lpm.h | 2 +-
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/config/arm/meson.build b/config/arm/meson.build
index e83a56e..7342626 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -488,3 +488,9 @@ if cc.get_define('__ARM_FEATURE_CRYPTO', args:
machine_args) != ''
compile_time_cpuflags += ['RTE_CPUFLAG_AES', 'RTE_CPUFLAG_PMULL',
'RTE_CPUFLAG_SHA1', 'RTE_CPUFLAG_SHA2']
endif
+
+# Check whether SVE ACLE is supported and set the corresponding flag which
will used with SVE ACLE code.
+if (cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != '' and
+cc.check_header('arm_sve.h'))
+dpdk_conf.set('RTE_HAS_SVE_ACLE', 1)
+endif
diff --git a/lib/eal/arm/include/rte_vect.h b/lib/eal/arm/include/rte_vect.h
index 093e912..4b705ba 100644
--- a/lib/eal/arm/include/rte_vect.h
+++ b/lib/eal/arm/include/rte_vect.h
@@ -9,7 +9,7 @@
#include "generic/rte_vect.h"
#include "rte_debug.h"
#include "arm_neon.h"
-#ifdef __ARM_FEATURE_SVE
+#ifdef RTE_HAS_SVE_ACLE
#include
#endif
diff --git a/lib/lpm/rte_lpm.h b/lib/lpm/rte_lpm.h
index 28b5768..5eb14c1 100644
--- a/lib/lpm/rte_lpm.h
+++ b/lib/lpm/rte_lpm.h
@@ -402,7 +402,7 @@ rte_lpm_lookupx4(const struct rte_lpm *lpm, xmm_t ip,
uint32_t hop[4],
uint32_t defv);
#if defined(RTE_ARCH_ARM)
-#ifdef __ARM_FEATURE_SVE
+#ifdef RTE_HAS_SVE_ACLE
#include "rte_lpm_sve.h"
#else
#include "rte_lpm_neon.h"
--
2.8.1
Re: [dpdk-dev] [PATCH v2 0/2] fix bugs for ethtool APP
Hi, all, any comments? 在 2021/5/6 11:46, Min Hu (Connor) 写道: This patch fixed fix bugs for ethtool APP. Chengwen Feng (1): examples/ethtool: fix Rx/Tx queue setup with rte socket id Huisong Li (1): examples/ethtool: add closing port operation examples/ethtool/ethtool-app/main.c | 18 ++ examples/ethtool/lib/rte_ethtool.c | 4 ++-- 2 files changed, 20 insertions(+), 2 deletions(-) --- v2: * modified commit info. add close port operation.
Re: [dpdk-dev] [PATCH V1 0/2] examples/ethtool: fix MTU set and add MTU query
Hi, all, any comments? 在 2021/4/29 18:53, Min Hu (Connor) 写道: From: Huisong Li This patchset fixes MTU data type when set MTU, and supports the query of MTU. Huisong Li (2): examples/ethtool: fix data type of MTU examples/ethtool: support the query of MTU examples/ethtool/ethtool-app/ethapp.c | 54 ++-- examples/ethtool/lib/rte_ethtool.c| 16 --- examples/ethtool/lib/rte_ethtool.h| 16 ++- 3 files changed, 63 insertions(+), 23 deletions(-)
Re: [dpdk-dev] [PATCH v2 0/2] fixes for link speed
Hi, all, any comments? 在 2021/4/28 16:36, Min Hu (Connor) 写道: This patchset contains fixes for link speed in testpmd. Huisong Li (2): app/testpmd: add link speed check before port start app/testpmd: fix link speed for a specified port app/test-pmd/cmdline.c | 50 ++-- 1 file changed, 44 insertions(+), 6 deletions(-)
Re: [dpdk-dev] [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
> -Original Message-
> From: Chengwen Feng
> Sent: Monday, June 28, 2021 10:58 AM
> To: tho...@monjalon.net; ferruh.yi...@intel.com; Ruifeng Wang
>
> Cc: dev@dpdk.org; bruce.richard...@intel.com;
> vladimir.medved...@intel.com; vikto...@rehivetech.com;
> jer...@marvell.com; Honnappa Nagarahalli
> ; jerinjac...@gmail.com;
> juraj.lin...@pantheon.tech
> Subject: [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
>
> If the target machine has SVE feature (e.g. '-march=armv8.2-a+sve'), and
> compiler are gcc8.3, it will compile error, the error is arm_sve.h no such
> file or
> directory.
>
> The solution:
> a. If RTE_HAS_SVE_ACLE defined (it means the minimum instruction set
> support SVE ACLE) then compiles it.
> b. Else if the compiler support SVE ACLE then compiles it.
> c. Otherwise don't compile it.
>
> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Chengwen Feng
> Acked-by: Ruifeng Wang
> ---
> drivers/net/hns3/hns3_rxtx.c | 2 +-
> drivers/net/hns3/meson.build | 20 +++-
> 2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
> index cb9eccf..a86e105 100644
> --- a/drivers/net/hns3/hns3_rxtx.c
> +++ b/drivers/net/hns3/hns3_rxtx.c
> @@ -2811,7 +2811,7 @@ hns3_get_default_vec_support(void)
> static bool
> hns3_get_sve_support(void)
> {
> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
> +#if defined(RTE_HAS_SVE_ACLE)
> if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
> return false;
> if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
> index 53c7df7..a99e0db 100644
> --- a/drivers/net/hns3/meson.build
> +++ b/drivers/net/hns3/meson.build
> @@ -35,7 +35,25 @@ deps += ['hash']
>
> if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
> sources += files('hns3_rxtx_vec.c')
> -if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> +
> +# compile SVE when:
> +# a. support SVE in minimum instruction set baseline
> +# b. it's not minimum instruction set, but compiler support
> +if dpdk_conf.has('RTE_HAS_SVE_ACLE')
> sources += files('hns3_rxtx_vec_sve.c')
> +elif cc.has_argument('-march=armv8.2-a+sve') and
> cc.check_header('arm_sve.h')
> +cflags += ['-DRTE_HAS_SVE_ACLE=1']
> +sve_cflags = []
Global cflags will be changed here. I think it is not very good as build of
other parts could be without SVE support.
How about " sve_cflags = ['-DRTE_HAS_SVE_ACLE=1']" and drop changes to cflags?
In this way, the additional flag will be limited to hns3_sve_lib.
> +foreach flag: cflags
> +if not (flag.startswith('-march=') or flag.startswith('-mcpu=')
> or
> flag.startswith('-mtune='))
> +sve_cflags += flag
> +endif
> +endforeach
> +hns3_sve_lib = static_library('hns3_sve_lib',
> +'hns3_rxtx_vec_sve.c',
> +dependencies: [static_rte_ethdev],
> +include_directories: includes,
> +c_args: [sve_cflags, '-march=armv8.2-a+sve'])
> +objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
> endif
> endif
> --
> 2.8.1
Re: [dpdk-dev] [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
On 2021/6/28 11:33, Ruifeng Wang wrote:
>> -Original Message-
>> From: Chengwen Feng
>> Sent: Monday, June 28, 2021 10:58 AM
>> To: tho...@monjalon.net; ferruh.yi...@intel.com; Ruifeng Wang
>>
>> Cc: dev@dpdk.org; bruce.richard...@intel.com;
>> vladimir.medved...@intel.com; vikto...@rehivetech.com;
>> jer...@marvell.com; Honnappa Nagarahalli
>> ; jerinjac...@gmail.com;
>> juraj.lin...@pantheon.tech
>> Subject: [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
>>
>> If the target machine has SVE feature (e.g. '-march=armv8.2-a+sve'), and
>> compiler are gcc8.3, it will compile error, the error is arm_sve.h no such
>> file or
>> directory.
>>
>> The solution:
>> a. If RTE_HAS_SVE_ACLE defined (it means the minimum instruction set
>> support SVE ACLE) then compiles it.
>> b. Else if the compiler support SVE ACLE then compiles it.
>> c. Otherwise don't compile it.
>>
>> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
>> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
>> Cc: sta...@dpdk.org
>>
>> Signed-off-by: Chengwen Feng
>> Acked-by: Ruifeng Wang
>> ---
>> drivers/net/hns3/hns3_rxtx.c | 2 +-
>> drivers/net/hns3/meson.build | 20 +++-
>> 2 files changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
>> index cb9eccf..a86e105 100644
>> --- a/drivers/net/hns3/hns3_rxtx.c
>> +++ b/drivers/net/hns3/hns3_rxtx.c
>> @@ -2811,7 +2811,7 @@ hns3_get_default_vec_support(void)
>> static bool
>> hns3_get_sve_support(void)
>> {
>> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
>> +#if defined(RTE_HAS_SVE_ACLE)
>> if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
>> return false;
>> if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
>> diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
>> index 53c7df7..a99e0db 100644
>> --- a/drivers/net/hns3/meson.build
>> +++ b/drivers/net/hns3/meson.build
>> @@ -35,7 +35,25 @@ deps += ['hash']
>>
>> if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
>> sources += files('hns3_rxtx_vec.c')
>> -if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
>> +
>> +# compile SVE when:
>> +# a. support SVE in minimum instruction set baseline
>> +# b. it's not minimum instruction set, but compiler support
>> +if dpdk_conf.has('RTE_HAS_SVE_ACLE')
>> sources += files('hns3_rxtx_vec_sve.c')
>> +elif cc.has_argument('-march=armv8.2-a+sve') and
>> cc.check_header('arm_sve.h')
>> +cflags += ['-DRTE_HAS_SVE_ACLE=1']
>> +sve_cflags = []
> Global cflags will be changed here. I think it is not very good as build of
> other parts could be without SVE support.
> How about " sve_cflags = ['-DRTE_HAS_SVE_ACLE=1']" and drop changes to cflags?
> In this way, the additional flag will be limited to hns3_sve_lib.
This will not change global cflags: the cflags was independent from other
drives [implemented in drivers/meson.build]
And I also check the 'build.ninja' and found the RTE_HAS_SVE_ACLE only defined
with hns3 driver source file.
PS: hns3_rxtx.c also refer the marco 'RTE_HAS_SVE_ACLE'.
>
>> +foreach flag: cflags
>> +if not (flag.startswith('-march=') or flag.startswith('-mcpu=')
>> or
>> flag.startswith('-mtune='))
>> +sve_cflags += flag
>> +endif
>> +endforeach
>> +hns3_sve_lib = static_library('hns3_sve_lib',
>> +'hns3_rxtx_vec_sve.c',
>> +dependencies: [static_rte_ethdev],
>> +include_directories: includes,
>> +c_args: [sve_cflags, '-march=armv8.2-a+sve'])
>> +objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
>> endif
>> endif
>> --
>> 2.8.1
>
>
> .
>
Re: [dpdk-dev] [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
> -Original Message-
> From: fengchengwen
> Sent: Monday, June 28, 2021 11:56 AM
> To: Ruifeng Wang ; tho...@monjalon.net;
> ferruh.yi...@intel.com
> Cc: dev@dpdk.org; bruce.richard...@intel.com;
> vladimir.medved...@intel.com; vikto...@rehivetech.com;
> jer...@marvell.com; Honnappa Nagarahalli
> ; jerinjac...@gmail.com;
> juraj.lin...@pantheon.tech; nd
> Subject: Re: [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
>
> On 2021/6/28 11:33, Ruifeng Wang wrote:
> >> -Original Message-
> >> From: Chengwen Feng
> >> Sent: Monday, June 28, 2021 10:58 AM
> >> To: tho...@monjalon.net; ferruh.yi...@intel.com; Ruifeng Wang
> >>
> >> Cc: dev@dpdk.org; bruce.richard...@intel.com;
> >> vladimir.medved...@intel.com; vikto...@rehivetech.com;
> >> jer...@marvell.com; Honnappa Nagarahalli
> >> ; jerinjac...@gmail.com;
> >> juraj.lin...@pantheon.tech
> >> Subject: [PATCH 2/2] net/hns3: fix SVE code compile error with gcc8.3
> >>
> >> If the target machine has SVE feature (e.g. '-march=armv8.2-a+sve'),
> >> and compiler are gcc8.3, it will compile error, the error is
> >> arm_sve.h no such file or directory.
> >>
> >> The solution:
> >> a. If RTE_HAS_SVE_ACLE defined (it means the minimum instruction set
> >> support SVE ACLE) then compiles it.
> >> b. Else if the compiler support SVE ACLE then compiles it.
> >> c. Otherwise don't compile it.
> >>
> >> Fixes: 8c25b02b082a ("net/hns3: fix enabling SVE Rx/Tx")
> >> Fixes: 952ebacce4f2 ("net/hns3: support SVE Rx")
> >> Cc: sta...@dpdk.org
> >>
> >> Signed-off-by: Chengwen Feng
> >> Acked-by: Ruifeng Wang
> >> ---
> >> drivers/net/hns3/hns3_rxtx.c | 2 +- drivers/net/hns3/meson.build |
> >> 20 +++-
> >> 2 files changed, 20 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/hns3/hns3_rxtx.c
> >> b/drivers/net/hns3/hns3_rxtx.c index cb9eccf..a86e105 100644
> >> --- a/drivers/net/hns3/hns3_rxtx.c
> >> +++ b/drivers/net/hns3/hns3_rxtx.c
> >> @@ -2811,7 +2811,7 @@ hns3_get_default_vec_support(void)
> >> static bool
> >> hns3_get_sve_support(void)
> >> {
> >> -#if defined(RTE_ARCH_ARM64) && defined(__ARM_FEATURE_SVE)
> >> +#if defined(RTE_HAS_SVE_ACLE)
> >>if (rte_vect_get_max_simd_bitwidth() < RTE_VECT_SIMD_256)
> >>return false;
> >>if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SVE))
> >> diff --git a/drivers/net/hns3/meson.build
> >> b/drivers/net/hns3/meson.build index 53c7df7..a99e0db 100644
> >> --- a/drivers/net/hns3/meson.build
> >> +++ b/drivers/net/hns3/meson.build
> >> @@ -35,7 +35,25 @@ deps += ['hash']
> >>
> >> if arch_subdir == 'arm' and dpdk_conf.get('RTE_ARCH_64')
> >> sources += files('hns3_rxtx_vec.c')
> >> -if cc.get_define('__ARM_FEATURE_SVE', args: machine_args) != ''
> >> +
> >> +# compile SVE when:
> >> +# a. support SVE in minimum instruction set baseline
> >> +# b. it's not minimum instruction set, but compiler support
> >> +if dpdk_conf.has('RTE_HAS_SVE_ACLE')
> >> sources += files('hns3_rxtx_vec_sve.c')
> >> +elif cc.has_argument('-march=armv8.2-a+sve') and
> >> cc.check_header('arm_sve.h')
> >> +cflags += ['-DRTE_HAS_SVE_ACLE=1']
> >> +sve_cflags = []
> > Global cflags will be changed here. I think it is not very good as build of
> other parts could be without SVE support.
> > How about " sve_cflags = ['-DRTE_HAS_SVE_ACLE=1']" and drop changes to
> cflags?
> > In this way, the additional flag will be limited to hns3_sve_lib.
>
> This will not change global cflags: the cflags was independent from other
> drives [implemented in drivers/meson.build] And I also check the
> 'build.ninja' and found the RTE_HAS_SVE_ACLE only defined with hns3 driver
> source file.
> PS: hns3_rxtx.c also refer the marco 'RTE_HAS_SVE_ACLE'.
>
Thanks for the explanation. I see cflags is restored for each driver.
Looks good to me. My tag was already added.
> >
> >> +foreach flag: cflags
> >> +if not (flag.startswith('-march=') or
> >> + flag.startswith('-mcpu=') or
> >> flag.startswith('-mtune='))
> >> +sve_cflags += flag
> >> +endif
> >> +endforeach
> >> +hns3_sve_lib = static_library('hns3_sve_lib',
> >> +'hns3_rxtx_vec_sve.c',
> >> +dependencies: [static_rte_ethdev],
> >> +include_directories: includes,
> >> +c_args: [sve_cflags, '-march=armv8.2-a+sve'])
> >> +objs += hns3_sve_lib.extract_objects('hns3_rxtx_vec_sve.c')
> >> endif
> >> endif
> >> --
> >> 2.8.1
> >
> >
> > .
> >
