* [PATCH 1/3] app/testpmd: fix displaying RSS info
@ 2022-05-25 17:37 Ferruh Yigit
2022-05-25 17:37 ` [PATCH 2/3] app/testpmd: remove duplicated flow type to string table Ferruh Yigit
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Ferruh Yigit @ 2022-05-25 17:37 UTC (permalink / raw)
To: Xiaoyun Li, Aman Singh, Yuying Zhang, Helin Zhang, Jingjing Wu,
Thomas Monjalon
Cc: dev, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko, stable
When supported RSS offload flow types are printed via 'show port info #'
command, flow names are get from flow type array which is wrong and
causing some RSS flow types not being displayed.
Instead RSS flow type array should be used. Also helper functions added
and existing code updated to use helpers.
Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
Cc: stable@dpdk.org
Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
---
Cc: helin.zhang@intel.com
Note:
In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may need
to check that too.
---
app/test-pmd/config.c | 92 ++++++++++++++++++++++++++++++-------------
1 file changed, 64 insertions(+), 28 deletions(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 72d2606d19d5..c353d224ef06 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
{ NULL, 0 },
};
+static const char *
+rsstype_to_str(uint64_t rss_type)
+{
+ int i;
+
+ for (i = 0; rss_type_table[i].str != NULL; i++) {
+ if (rss_type_table[i].rss_type == rss_type)
+ return rss_type_table[i].str;
+ }
+
+ return NULL;
+}
+
+static uint64_t
+str_to_rsstype(const char *str)
+{
+ int i;
+
+ for (i = 0; rss_type_table[i].str != NULL; i++) {
+ if (!strcmp(rss_type_table[i].str, str))
+ return rss_type_table[i].rss_type;
+ }
+
+ return 0;
+}
+
static const struct {
enum rte_eth_fec_mode mode;
const char *name;
@@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
if (!dev_info.flow_type_rss_offloads)
printf("No RSS offload flow type is supported.\n");
else {
+ uint64_t rss_types = dev_info.flow_type_rss_offloads;
uint16_t i;
- char *p;
printf("Supported RSS offload flow types:\n");
- for (i = RTE_ETH_FLOW_UNKNOWN + 1;
- i < sizeof(dev_info.flow_type_rss_offloads) * CHAR_BIT; i++) {
- if (!(dev_info.flow_type_rss_offloads & (1ULL << i)))
- continue;
- p = flowtype_to_str(i);
- if (p)
- printf(" %s\n", p);
- else
- printf(" user defined %d\n", i);
+ for (i = 0; rss_types != 0; i++) {
+ if (rss_types & 1) {
+ uint64_t rss_type = 1ULL << i;
+ const char *p = rsstype_to_str(rss_type);
+
+ if (p)
+ printf(" %s\n", p);
+ else
+ printf(" user defined 0x%"PRIx64"\n", rss_type);
+ }
+ rss_types >>= 1;
}
}
@@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error *error)
static void
rss_config_display(struct rte_flow_action_rss *rss_conf)
{
+ uint64_t rss_types;
uint8_t i;
if (rss_conf == NULL) {
@@ -1582,16 +1611,23 @@ rss_config_display(struct rte_flow_action_rss *rss_conf)
}
printf(" types:\n");
- if (rss_conf->types == 0) {
+ rss_types = rss_conf->types;
+ if (rss_types == 0) {
printf(" none\n");
return;
}
- for (i = 0; rss_type_table[i].str; i++) {
- if ((rss_conf->types &
- rss_type_table[i].rss_type) ==
- rss_type_table[i].rss_type &&
- rss_type_table[i].rss_type != 0)
- printf(" %s\n", rss_type_table[i].str);
+
+ for (i = 0; rss_types != 0; i++) {
+ if (rss_types & 1) {
+ uint64_t rss_type = 1ULL << i;
+ const char *p = rsstype_to_str(rss_type);
+
+ if (p)
+ printf(" %s\n", p);
+ else
+ printf(" user defined 0x%"PRIx64"\n", rss_type);
+ }
+ rss_types >>= 1;
}
}
@@ -3823,11 +3859,16 @@ port_rss_hash_conf_show(portid_t port_id, int show_rss_key)
return;
}
printf("RSS functions:\n ");
- for (i = 0; rss_type_table[i].str; i++) {
- if (rss_type_table[i].rss_type == 0)
- continue;
- if ((rss_hf & rss_type_table[i].rss_type) == rss_type_table[i].rss_type)
- printf("%s ", rss_type_table[i].str);
+ for (i = 0; rss_hf != 0; i++) {
+ if (rss_hf & 1) {
+ uint64_t rss_type = 1ULL << i;
+ const char *p = rsstype_to_str(rss_type);
+ if (p)
+ printf("%s ", p);
+ else
+ printf("0x%"PRIx64" ", rss_type);
+ }
+ rss_hf >>= 1;
}
printf("\n");
if (!show_rss_key)
@@ -3844,15 +3885,10 @@ port_rss_hash_key_update(portid_t port_id, char rss_type[], uint8_t *hash_key,
{
struct rte_eth_rss_conf rss_conf;
int diag;
- unsigned int i;
rss_conf.rss_key = NULL;
rss_conf.rss_key_len = 0;
- rss_conf.rss_hf = 0;
- for (i = 0; rss_type_table[i].str; i++) {
- if (!strcmp(rss_type_table[i].str, rss_type))
- rss_conf.rss_hf = rss_type_table[i].rss_type;
- }
+ rss_conf.rss_hf = str_to_rsstype(rss_type);
diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
if (diag == 0) {
rss_conf.rss_key = hash_key;
--
2.27.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/3] app/testpmd: remove duplicated flow type to string table
2022-05-25 17:37 [PATCH 1/3] app/testpmd: fix displaying RSS info Ferruh Yigit
@ 2022-05-25 17:37 ` Ferruh Yigit
2022-05-25 17:37 ` [PATCH 3/3] app/testpmd: compact RSS flow type output in port info Ferruh Yigit
` (2 subsequent siblings)
3 siblings, 0 replies; 14+ messages in thread
From: Ferruh Yigit @ 2022-05-25 17:37 UTC (permalink / raw)
To: Xiaoyun Li, Aman Singh, Yuying Zhang
Cc: dev, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko, Kirill Rybalchenko
Flow type table has two instance, one is used for flow type to string
conversion, and other is used for string to flow type conversion.
And tables are diverged by time.
Unifying tables to prevent maintaining two different tables.
Note: made 'flowtype_to_str()' non-static to prevent build error for the
case PMDs using it disables. Making function generic, not for some PMDs.
Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
---
Cc: Kirill Rybalchenko <kirill.rybalchenko@intel.com>
---
app/test-pmd/cmdline.c | 41 +-------------------
app/test-pmd/config.c | 87 +++++++++++++++++++++++++-----------------
app/test-pmd/testpmd.h | 2 +
3 files changed, 55 insertions(+), 75 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index d8900a0d0704..becb3d772fad 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -10572,45 +10572,6 @@ do { \
#ifdef RTE_NET_I40E
-static uint16_t
-str2flowtype(char *string)
-{
- uint8_t i = 0;
- static const struct {
- char str[32];
- uint16_t type;
- } flowtype_str[] = {
- {"raw", RTE_ETH_FLOW_RAW},
- {"ipv4", RTE_ETH_FLOW_IPV4},
- {"ipv4-frag", RTE_ETH_FLOW_FRAG_IPV4},
- {"ipv4-tcp", RTE_ETH_FLOW_NONFRAG_IPV4_TCP},
- {"ipv4-udp", RTE_ETH_FLOW_NONFRAG_IPV4_UDP},
- {"ipv4-sctp", RTE_ETH_FLOW_NONFRAG_IPV4_SCTP},
- {"ipv4-other", RTE_ETH_FLOW_NONFRAG_IPV4_OTHER},
- {"ipv6", RTE_ETH_FLOW_IPV6},
- {"ipv6-frag", RTE_ETH_FLOW_FRAG_IPV6},
- {"ipv6-tcp", RTE_ETH_FLOW_NONFRAG_IPV6_TCP},
- {"ipv6-udp", RTE_ETH_FLOW_NONFRAG_IPV6_UDP},
- {"ipv6-sctp", RTE_ETH_FLOW_NONFRAG_IPV6_SCTP},
- {"ipv6-other", RTE_ETH_FLOW_NONFRAG_IPV6_OTHER},
- {"l2_payload", RTE_ETH_FLOW_L2_PAYLOAD},
- {"ipv6-ex", RTE_ETH_FLOW_IPV6_EX},
- {"ipv6-tcp-ex", RTE_ETH_FLOW_IPV6_TCP_EX},
- {"ipv6-udp-ex", RTE_ETH_FLOW_IPV6_UDP_EX},
- {"gtpu", RTE_ETH_FLOW_GTPU},
- };
-
- for (i = 0; i < RTE_DIM(flowtype_str); i++) {
- if (!strcmp(flowtype_str[i].str, string))
- return flowtype_str[i].type;
- }
-
- if (isdigit(string[0]) && atoi(string) > 0 && atoi(string) < 64)
- return (uint16_t)atoi(string);
-
- return RTE_ETH_FLOW_UNKNOWN;
-}
-
/* *** deal with flow director filter *** */
struct cmd_flow_director_result {
cmdline_fixed_string_t flow_director_filter;
@@ -10639,7 +10600,7 @@ cmd_flow_director_filter_parsed(void *parsed_result,
struct rte_pmd_i40e_flow_type_mapping
mapping[RTE_PMD_I40E_FLOW_TYPE_MAX];
struct rte_pmd_i40e_pkt_template_conf conf;
- uint16_t flow_type = str2flowtype(res->flow_type);
+ uint16_t flow_type = str_to_flowtype(res->flow_type);
uint16_t i, port = res->port_id;
uint8_t add;
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index c353d224ef06..47de5b6d9458 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -66,8 +66,6 @@
#define NS_PER_SEC 1E9
-static char *flowtype_to_str(uint16_t flow_type);
-
static const struct {
enum tx_pkt_split split;
const char *name;
@@ -5640,40 +5638,39 @@ set_record_burst_stats(uint8_t on_off)
record_burst_stats = on_off;
}
-static char*
+static const struct {
+ char str[32];
+ uint16_t ftype;
+} flowtype_str_table[] = {
+ {"raw", RTE_ETH_FLOW_RAW},
+ {"ipv4", RTE_ETH_FLOW_IPV4},
+ {"ipv4-frag", RTE_ETH_FLOW_FRAG_IPV4},
+ {"ipv4-tcp", RTE_ETH_FLOW_NONFRAG_IPV4_TCP},
+ {"ipv4-udp", RTE_ETH_FLOW_NONFRAG_IPV4_UDP},
+ {"ipv4-sctp", RTE_ETH_FLOW_NONFRAG_IPV4_SCTP},
+ {"ipv4-other", RTE_ETH_FLOW_NONFRAG_IPV4_OTHER},
+ {"ipv6", RTE_ETH_FLOW_IPV6},
+ {"ipv6-frag", RTE_ETH_FLOW_FRAG_IPV6},
+ {"ipv6-tcp", RTE_ETH_FLOW_NONFRAG_IPV6_TCP},
+ {"ipv6-udp", RTE_ETH_FLOW_NONFRAG_IPV6_UDP},
+ {"ipv6-sctp", RTE_ETH_FLOW_NONFRAG_IPV6_SCTP},
+ {"ipv6-other", RTE_ETH_FLOW_NONFRAG_IPV6_OTHER},
+ {"l2_payload", RTE_ETH_FLOW_L2_PAYLOAD},
+ {"ipv6-ex", RTE_ETH_FLOW_IPV6_EX},
+ {"ipv6-tcp-ex", RTE_ETH_FLOW_IPV6_TCP_EX},
+ {"ipv6-udp-ex", RTE_ETH_FLOW_IPV6_UDP_EX},
+ {"port", RTE_ETH_FLOW_PORT},
+ {"vxlan", RTE_ETH_FLOW_VXLAN},
+ {"geneve", RTE_ETH_FLOW_GENEVE},
+ {"nvgre", RTE_ETH_FLOW_NVGRE},
+ {"vxlan-gpe", RTE_ETH_FLOW_VXLAN_GPE},
+ {"gtpu", RTE_ETH_FLOW_GTPU},
+};
+
+const char*
flowtype_to_str(uint16_t flow_type)
{
- struct flow_type_info {
- char str[32];
- uint16_t ftype;
- };
-
uint8_t i;
- static struct flow_type_info flowtype_str_table[] = {
- {"raw", RTE_ETH_FLOW_RAW},
- {"ipv4", RTE_ETH_FLOW_IPV4},
- {"ipv4-frag", RTE_ETH_FLOW_FRAG_IPV4},
- {"ipv4-tcp", RTE_ETH_FLOW_NONFRAG_IPV4_TCP},
- {"ipv4-udp", RTE_ETH_FLOW_NONFRAG_IPV4_UDP},
- {"ipv4-sctp", RTE_ETH_FLOW_NONFRAG_IPV4_SCTP},
- {"ipv4-other", RTE_ETH_FLOW_NONFRAG_IPV4_OTHER},
- {"ipv6", RTE_ETH_FLOW_IPV6},
- {"ipv6-frag", RTE_ETH_FLOW_FRAG_IPV6},
- {"ipv6-tcp", RTE_ETH_FLOW_NONFRAG_IPV6_TCP},
- {"ipv6-udp", RTE_ETH_FLOW_NONFRAG_IPV6_UDP},
- {"ipv6-sctp", RTE_ETH_FLOW_NONFRAG_IPV6_SCTP},
- {"ipv6-other", RTE_ETH_FLOW_NONFRAG_IPV6_OTHER},
- {"l2_payload", RTE_ETH_FLOW_L2_PAYLOAD},
- {"ipv6-ex", RTE_ETH_FLOW_IPV6_EX},
- {"ipv6-tcp-ex", RTE_ETH_FLOW_IPV6_TCP_EX},
- {"ipv6-udp-ex", RTE_ETH_FLOW_IPV6_UDP_EX},
- {"port", RTE_ETH_FLOW_PORT},
- {"vxlan", RTE_ETH_FLOW_VXLAN},
- {"geneve", RTE_ETH_FLOW_GENEVE},
- {"nvgre", RTE_ETH_FLOW_NVGRE},
- {"vxlan-gpe", RTE_ETH_FLOW_VXLAN_GPE},
- {"gtpu", RTE_ETH_FLOW_GTPU},
- };
for (i = 0; i < RTE_DIM(flowtype_str_table); i++) {
if (flowtype_str_table[i].ftype == flow_type)
@@ -5683,6 +5680,26 @@ flowtype_to_str(uint16_t flow_type)
return NULL;
}
+uint16_t
+str_to_flowtype(const char *string)
+{
+ uint8_t i;
+
+ for (i = 0; i < RTE_DIM(flowtype_str_table); i++) {
+ if (!strcmp(flowtype_str_table[i].str, string))
+ return flowtype_str_table[i].ftype;
+ }
+
+ if (isdigit(string[0])) {
+ int val = atoi(string);
+
+ if (val > 0 && val < 64)
+ return (uint16_t)val;
+ }
+
+ return RTE_ETH_FLOW_UNKNOWN;
+}
+
#if defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE)
static inline void
@@ -5749,7 +5766,7 @@ print_fdir_flex_mask(struct rte_eth_fdir_flex_conf *flex_conf, uint32_t num)
{
struct rte_eth_fdir_flex_mask *mask;
uint32_t i, j;
- char *p;
+ const char *p;
for (i = 0; i < flex_conf->nb_flexmasks; i++) {
mask = &flex_conf->flex_mask[i];
@@ -5765,7 +5782,7 @@ static inline void
print_fdir_flow_type(uint32_t flow_types_mask)
{
int i;
- char *p;
+ const char *p;
for (i = RTE_ETH_FLOW_UNKNOWN; i < RTE_ETH_FLOW_MAX; i++) {
if (!(flow_types_mask & (1 << i)))
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 203311bcc29e..5ca297215514 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1086,6 +1086,8 @@ void fdir_set_flex_payload(portid_t port_id,
void port_rss_reta_info(portid_t port_id,
struct rte_eth_rss_reta_entry64 *reta_conf,
uint16_t nb_entries);
+uint16_t str_to_flowtype(const char *string);
+const char *flowtype_to_str(uint16_t flow_type);
void set_vf_traffic(portid_t port_id, uint8_t is_rx, uint16_t vf, uint8_t on);
--
2.27.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/3] app/testpmd: compact RSS flow type output in port info
2022-05-25 17:37 [PATCH 1/3] app/testpmd: fix displaying RSS info Ferruh Yigit
2022-05-25 17:37 ` [PATCH 2/3] app/testpmd: remove duplicated flow type to string table Ferruh Yigit
@ 2022-05-25 17:37 ` Ferruh Yigit
2022-05-31 16:35 ` Andrew Rybchenko
2022-05-26 3:52 ` [PATCH 1/3] app/testpmd: fix displaying RSS info lihuisong (C)
2022-05-27 2:30 ` lihuisong (C)
3 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2022-05-25 17:37 UTC (permalink / raw)
To: Xiaoyun Li, Aman Singh, Yuying Zhang
Cc: dev, Thomas Monjalon, Ferruh Yigit, Andrew Rybchenko
In port info command output, 'show port info all', supported RSS flow
types printed one type per line, and although this information is not
most important part of the command it takes big part of the command
output.
Compacting the supported RSS flow type output by printing 6 (hardcoded
value) items per line, instead of one per line. Output becomes as
following:
Supported RSS offload flow types:
ipv4 ipv4-frag ipv4-tcp ipv4-udp ipv4-sctp ipv4-other
ipv6 ipv6-frag ipv6-tcp ipv6-udp ipv6-sctp ipv6-other
l2-payload ipv6-ex ipv6-tcp-ex ipv6-udp-ex port vxlan
geneve nvgre mpls
Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
---
app/test-pmd/config.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 47de5b6d9458..5496ccd7f8ad 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -805,6 +805,7 @@ port_infos_display(portid_t port_id)
else {
uint64_t rss_types = dev_info.flow_type_rss_offloads;
uint16_t i;
+ uint16_t len = 0;
printf("Supported RSS offload flow types:\n");
for (i = 0; rss_types != 0; i++) {
@@ -813,12 +814,21 @@ port_infos_display(portid_t port_id)
const char *p = rsstype_to_str(rss_type);
if (p)
- printf(" %s\n", p);
+ printf(" %s", p);
else
- printf(" user defined 0x%"PRIx64"\n", rss_type);
+ printf(" user defined 0x%"PRIx64, rss_type);
+
+ len++;
+ /* wrap on every 6 items */
+ if (len == 6) {
+ printf("\n");
+ len = 0;
+ }
}
rss_types >>= 1;
}
+ if (len)
+ printf("\n");
}
printf("Minimum size of RX buffer: %u\n", dev_info.min_rx_bufsize);
--
2.27.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] app/testpmd: fix displaying RSS info
2022-05-25 17:37 [PATCH 1/3] app/testpmd: fix displaying RSS info Ferruh Yigit
2022-05-25 17:37 ` [PATCH 2/3] app/testpmd: remove duplicated flow type to string table Ferruh Yigit
2022-05-25 17:37 ` [PATCH 3/3] app/testpmd: compact RSS flow type output in port info Ferruh Yigit
@ 2022-05-26 3:52 ` lihuisong (C)
2022-05-26 8:56 ` Ferruh Yigit
2022-05-27 2:30 ` lihuisong (C)
3 siblings, 1 reply; 14+ messages in thread
From: lihuisong (C) @ 2022-05-26 3:52 UTC (permalink / raw)
To: Ferruh Yigit, Xiaoyun Li, Aman Singh, Yuying Zhang, Helin Zhang,
Jingjing Wu, Thomas Monjalon
Cc: dev, Thomas Monjalon, Andrew Rybchenko, stable
Hi, Ferruh
This patch is the same as the following patch:
http://patches.dpdk.org/project/dpdk/patch/20220429102445.23711-2-lihuisong@huawei.com/
It was delegated to you.😂
在 2022/5/26 1:37, Ferruh Yigit 写道:
> When supported RSS offload flow types are printed via 'show port info #'
> command, flow names are get from flow type array which is wrong and
> causing some RSS flow types not being displayed.
>
> Instead RSS flow type array should be used. Also helper functions added
> and existing code updated to use helpers.
>
> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
> ---
> Cc: helin.zhang@intel.com
>
> Note:
> In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
> 'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may need
> to check that too.
> ---
> app/test-pmd/config.c | 92 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 64 insertions(+), 28 deletions(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 72d2606d19d5..c353d224ef06 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
> { NULL, 0 },
> };
>
> +static const char *
> +rsstype_to_str(uint64_t rss_type)
> +{
> + int i;
> +
> + for (i = 0; rss_type_table[i].str != NULL; i++) {
> + if (rss_type_table[i].rss_type == rss_type)
> + return rss_type_table[i].str;
> + }
> +
> + return NULL;
> +}
> +
> +static uint64_t
> +str_to_rsstype(const char *str)
> +{
> + int i;
> +
> + for (i = 0; rss_type_table[i].str != NULL; i++) {
> + if (!strcmp(rss_type_table[i].str, str))
> + return rss_type_table[i].rss_type;
> + }
> +
> + return 0;
> +}
> +
> static const struct {
> enum rte_eth_fec_mode mode;
> const char *name;
> @@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
> if (!dev_info.flow_type_rss_offloads)
> printf("No RSS offload flow type is supported.\n");
> else {
> + uint64_t rss_types = dev_info.flow_type_rss_offloads;
> uint16_t i;
> - char *p;
>
> printf("Supported RSS offload flow types:\n");
> - for (i = RTE_ETH_FLOW_UNKNOWN + 1;
> - i < sizeof(dev_info.flow_type_rss_offloads) * CHAR_BIT; i++) {
> - if (!(dev_info.flow_type_rss_offloads & (1ULL << i)))
> - continue;
> - p = flowtype_to_str(i);
> - if (p)
> - printf(" %s\n", p);
> - else
> - printf(" user defined %d\n", i);
> + for (i = 0; rss_types != 0; i++) {
> + if (rss_types & 1) {
> + uint64_t rss_type = 1ULL << i;
> + const char *p = rsstype_to_str(rss_type);
> +
> + if (p)
> + printf(" %s\n", p);
> + else
> + printf(" user defined 0x%"PRIx64"\n", rss_type);
> + }
> + rss_types >>= 1;
> }
> }
>
> @@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error *error)
> static void
> rss_config_display(struct rte_flow_action_rss *rss_conf)
> {
> + uint64_t rss_types;
> uint8_t i;
>
> if (rss_conf == NULL) {
> @@ -1582,16 +1611,23 @@ rss_config_display(struct rte_flow_action_rss *rss_conf)
> }
>
> printf(" types:\n");
> - if (rss_conf->types == 0) {
> + rss_types = rss_conf->types;
> + if (rss_types == 0) {
> printf(" none\n");
> return;
> }
> - for (i = 0; rss_type_table[i].str; i++) {
> - if ((rss_conf->types &
> - rss_type_table[i].rss_type) ==
> - rss_type_table[i].rss_type &&
> - rss_type_table[i].rss_type != 0)
> - printf(" %s\n", rss_type_table[i].str);
> +
> + for (i = 0; rss_types != 0; i++) {
> + if (rss_types & 1) {
> + uint64_t rss_type = 1ULL << i;
> + const char *p = rsstype_to_str(rss_type);
> +
> + if (p)
> + printf(" %s\n", p);
> + else
> + printf(" user defined 0x%"PRIx64"\n", rss_type);
> + }
> + rss_types >>= 1;
> }
> }
>
> @@ -3823,11 +3859,16 @@ port_rss_hash_conf_show(portid_t port_id, int show_rss_key)
> return;
> }
> printf("RSS functions:\n ");
> - for (i = 0; rss_type_table[i].str; i++) {
> - if (rss_type_table[i].rss_type == 0)
> - continue;
> - if ((rss_hf & rss_type_table[i].rss_type) == rss_type_table[i].rss_type)
> - printf("%s ", rss_type_table[i].str);
> + for (i = 0; rss_hf != 0; i++) {
> + if (rss_hf & 1) {
> + uint64_t rss_type = 1ULL << i;
> + const char *p = rsstype_to_str(rss_type);
> + if (p)
> + printf("%s ", p);
> + else
> + printf("0x%"PRIx64" ", rss_type);
> + }
> + rss_hf >>= 1;
> }
> printf("\n");
> if (!show_rss_key)
> @@ -3844,15 +3885,10 @@ port_rss_hash_key_update(portid_t port_id, char rss_type[], uint8_t *hash_key,
> {
> struct rte_eth_rss_conf rss_conf;
> int diag;
> - unsigned int i;
>
> rss_conf.rss_key = NULL;
> rss_conf.rss_key_len = 0;
> - rss_conf.rss_hf = 0;
> - for (i = 0; rss_type_table[i].str; i++) {
> - if (!strcmp(rss_type_table[i].str, rss_type))
> - rss_conf.rss_hf = rss_type_table[i].rss_type;
> - }
> + rss_conf.rss_hf = str_to_rsstype(rss_type);
> diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
> if (diag == 0) {
> rss_conf.rss_key = hash_key;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] app/testpmd: fix displaying RSS info
2022-05-26 3:52 ` [PATCH 1/3] app/testpmd: fix displaying RSS info lihuisong (C)
@ 2022-05-26 8:56 ` Ferruh Yigit
2022-05-27 2:48 ` lihuisong (C)
0 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2022-05-26 8:56 UTC (permalink / raw)
To: lihuisong (C),
Xiaoyun Li, Aman Singh, Yuying Zhang, Helin Zhang, Jingjing Wu
Cc: dev, Thomas Monjalon, Andrew Rybchenko, stable
On 5/26/2022 4:52 AM, lihuisong (C) wrote:
> CAUTION: This message has originated from an External Source. Please use
> proper judgment and caution when opening attachments, clicking links, or
> responding to this email.
>
>
> Hi, Ferruh
>
> This patch is the same as the following patch:
>
> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20220429102445.23711-2-lihuisong%40huawei.com%2F&data=05%7C01%7CFerruh.Yigit%40amd.com%7Cda898bd1b39b438ce3e308da3ecb3123%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637891340148254654%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=kixRff%2BQavl0SYS7woT5YYJELrrocoSoV4sE7TUK9Is%3D&reserved=0
>
>
> It was delegated to you.😂
>
Yes indeed, I missed it. They are fixing same issue.
But there is slight difference in the implementation, can you please
review the set? We can share the sign off for final set.
>
> 在 2022/5/26 1:37, Ferruh Yigit 写道:
>> When supported RSS offload flow types are printed via 'show port info #'
>> command, flow names are get from flow type array which is wrong and
>> causing some RSS flow types not being displayed.
>>
>> Instead RSS flow type array should be used. Also helper functions added
>> and existing code updated to use helpers.
>>
>> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
>> ---
>> Cc: helin.zhang@intel.com
>>
>> Note:
>> In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
>> 'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may need
>> to check that too.
>> ---
>> app/test-pmd/config.c | 92 ++++++++++++++++++++++++++++++-------------
>> 1 file changed, 64 insertions(+), 28 deletions(-)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 72d2606d19d5..c353d224ef06 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
>> { NULL, 0 },
>> };
>>
>> +static const char *
>> +rsstype_to_str(uint64_t rss_type)
>> +{
>> + int i;
>> +
>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>> + if (rss_type_table[i].rss_type == rss_type)
>> + return rss_type_table[i].str;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static uint64_t
>> +str_to_rsstype(const char *str)
>> +{
>> + int i;
>> +
>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>> + if (!strcmp(rss_type_table[i].str, str))
>> + return rss_type_table[i].rss_type;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static const struct {
>> enum rte_eth_fec_mode mode;
>> const char *name;
>> @@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
>> if (!dev_info.flow_type_rss_offloads)
>> printf("No RSS offload flow type is supported.\n");
>> else {
>> + uint64_t rss_types = dev_info.flow_type_rss_offloads;
>> uint16_t i;
>> - char *p;
>>
>> printf("Supported RSS offload flow types:\n");
>> - for (i = RTE_ETH_FLOW_UNKNOWN + 1;
>> - i < sizeof(dev_info.flow_type_rss_offloads) *
>> CHAR_BIT; i++) {
>> - if (!(dev_info.flow_type_rss_offloads & (1ULL <<
>> i)))
>> - continue;
>> - p = flowtype_to_str(i);
>> - if (p)
>> - printf(" %s\n", p);
>> - else
>> - printf(" user defined %d\n", i);
>> + for (i = 0; rss_types != 0; i++) {
>> + if (rss_types & 1) {
>> + uint64_t rss_type = 1ULL << i;
>> + const char *p = rsstype_to_str(rss_type);
>> +
>> + if (p)
>> + printf(" %s\n", p);
>> + else
>> + printf(" user defined
>> 0x%"PRIx64"\n", rss_type);
>> + }
>> + rss_types >>= 1;
>> }
>> }
>>
>> @@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error *error)
>> static void
>> rss_config_display(struct rte_flow_action_rss *rss_conf)
>> {
>> + uint64_t rss_types;
>> uint8_t i;
>>
>> if (rss_conf == NULL) {
>> @@ -1582,16 +1611,23 @@ rss_config_display(struct rte_flow_action_rss
>> *rss_conf)
>> }
>>
>> printf(" types:\n");
>> - if (rss_conf->types == 0) {
>> + rss_types = rss_conf->types;
>> + if (rss_types == 0) {
>> printf(" none\n");
>> return;
>> }
>> - for (i = 0; rss_type_table[i].str; i++) {
>> - if ((rss_conf->types &
>> - rss_type_table[i].rss_type) ==
>> - rss_type_table[i].rss_type &&
>> - rss_type_table[i].rss_type != 0)
>> - printf(" %s\n", rss_type_table[i].str);
>> +
>> + for (i = 0; rss_types != 0; i++) {
>> + if (rss_types & 1) {
>> + uint64_t rss_type = 1ULL << i;
>> + const char *p = rsstype_to_str(rss_type);
>> +
>> + if (p)
>> + printf(" %s\n", p);
>> + else
>> + printf(" user defined 0x%"PRIx64"\n",
>> rss_type);
>> + }
>> + rss_types >>= 1;
>> }
>> }
>>
>> @@ -3823,11 +3859,16 @@ port_rss_hash_conf_show(portid_t port_id, int
>> show_rss_key)
>> return;
>> }
>> printf("RSS functions:\n ");
>> - for (i = 0; rss_type_table[i].str; i++) {
>> - if (rss_type_table[i].rss_type == 0)
>> - continue;
>> - if ((rss_hf & rss_type_table[i].rss_type) ==
>> rss_type_table[i].rss_type)
>> - printf("%s ", rss_type_table[i].str);
>> + for (i = 0; rss_hf != 0; i++) {
>> + if (rss_hf & 1) {
>> + uint64_t rss_type = 1ULL << i;
>> + const char *p = rsstype_to_str(rss_type);
>> + if (p)
>> + printf("%s ", p);
>> + else
>> + printf("0x%"PRIx64" ", rss_type);
>> + }
>> + rss_hf >>= 1;
>> }
>> printf("\n");
>> if (!show_rss_key)
>> @@ -3844,15 +3885,10 @@ port_rss_hash_key_update(portid_t port_id,
>> char rss_type[], uint8_t *hash_key,
>> {
>> struct rte_eth_rss_conf rss_conf;
>> int diag;
>> - unsigned int i;
>>
>> rss_conf.rss_key = NULL;
>> rss_conf.rss_key_len = 0;
>> - rss_conf.rss_hf = 0;
>> - for (i = 0; rss_type_table[i].str; i++) {
>> - if (!strcmp(rss_type_table[i].str, rss_type))
>> - rss_conf.rss_hf = rss_type_table[i].rss_type;
>> - }
>> + rss_conf.rss_hf = str_to_rsstype(rss_type);
>> diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
>> if (diag == 0) {
>> rss_conf.rss_key = hash_key;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] app/testpmd: fix displaying RSS info
2022-05-25 17:37 [PATCH 1/3] app/testpmd: fix displaying RSS info Ferruh Yigit
` (2 preceding siblings ...)
2022-05-26 3:52 ` [PATCH 1/3] app/testpmd: fix displaying RSS info lihuisong (C)
@ 2022-05-27 2:30 ` lihuisong (C)
2022-05-30 10:43 ` Ferruh Yigit
3 siblings, 1 reply; 14+ messages in thread
From: lihuisong (C) @ 2022-05-27 2:30 UTC (permalink / raw)
To: Ferruh Yigit, Xiaoyun Li, Aman Singh, Yuying Zhang, Helin Zhang,
Jingjing Wu, Thomas Monjalon
Cc: dev, Thomas Monjalon, Andrew Rybchenko, stable
在 2022/5/26 1:37, Ferruh Yigit 写道:
> When supported RSS offload flow types are printed via 'show port info #'
> command, flow names are get from flow type array which is wrong and
> causing some RSS flow types not being displayed.
>
> Instead RSS flow type array should be used. Also helper functions added
> and existing code updated to use helpers.
>
> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
> ---
> Cc: helin.zhang@intel.com
>
> Note:
> In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
> 'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may need
> to check that too.
> ---
> app/test-pmd/config.c | 92 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 64 insertions(+), 28 deletions(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 72d2606d19d5..c353d224ef06 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
> { NULL, 0 },
> };
>
> +static const char *
> +rsstype_to_str(uint64_t rss_type)
> +{
> + int i;
> +
> + for (i = 0; rss_type_table[i].str != NULL; i++) {
> + if (rss_type_table[i].rss_type == rss_type)
> + return rss_type_table[i].str;
> + }
> +
> + return NULL;
> +}
> +
> +static uint64_t
> +str_to_rsstype(const char *str)
> +{
> + int i;
> +
> + for (i = 0; rss_type_table[i].str != NULL; i++) {
> + if (!strcmp(rss_type_table[i].str, str))
> + return rss_type_table[i].rss_type;
> + }
> +
> + return 0;
> +}
> +
> static const struct {
> enum rte_eth_fec_mode mode;
> const char *name;
> @@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
> if (!dev_info.flow_type_rss_offloads)
> printf("No RSS offload flow type is supported.\n");
> else {
> + uint64_t rss_types = dev_info.flow_type_rss_offloads;
> uint16_t i;
> - char *p;
>
> printf("Supported RSS offload flow types:\n");
> - for (i = RTE_ETH_FLOW_UNKNOWN + 1;
> - i < sizeof(dev_info.flow_type_rss_offloads) * CHAR_BIT; i++) {
> - if (!(dev_info.flow_type_rss_offloads & (1ULL << i)))
> - continue;
> - p = flowtype_to_str(i);
> - if (p)
> - printf(" %s\n", p);
> - else
> - printf(" user defined %d\n", i);
> + for (i = 0; rss_types != 0; i++) {
> + if (rss_types & 1) {
> + uint64_t rss_type = 1ULL << i;
> + const char *p = rsstype_to_str(rss_type);
> +
> + if (p)
> + printf(" %s\n", p);
> + else
> + printf(" user defined 0x%"PRIx64"\n", rss_type);
> + }
> + rss_types >>= 1;
> }
> }
>
> @@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error *error)
> static void
> rss_config_display(struct rte_flow_action_rss *rss_conf)
> {
> + uint64_t rss_types;
> uint8_t i;
>
> if (rss_conf == NULL) {
> @@ -1582,16 +1611,23 @@ rss_config_display(struct rte_flow_action_rss *rss_conf)
> }
>
> printf(" types:\n");
> - if (rss_conf->types == 0) {
> + rss_types = rss_conf->types;
> + if (rss_types == 0) {
> printf(" none\n");
> return;
> }
> - for (i = 0; rss_type_table[i].str; i++) {
> - if ((rss_conf->types &
> - rss_type_table[i].rss_type) ==
> - rss_type_table[i].rss_type &&
> - rss_type_table[i].rss_type != 0)
> - printf(" %s\n", rss_type_table[i].str);
> +
> + for (i = 0; rss_types != 0; i++) {
> + if (rss_types & 1) {
> + uint64_t rss_type = 1ULL << i;
> + const char *p = rsstype_to_str(rss_type);
It seems that we can't use one bit to get rss type name.
Because part of name in rss_type_table[] consist of multiple bits.
Maybe it's better to use the original method to display RSS type name.
On the other hand, RSS type name are printed in many places as this
patch modified.
It is recommended that you encapsulate a funcion to display RSS type.
> +
> + if (p)
> + printf(" %s\n", p);
> + else
> + printf(" user defined 0x%"PRIx64"\n", rss_type);
> + }
> + rss_types >>= 1;
> }
> }
>
> @@ -3823,11 +3859,16 @@ port_rss_hash_conf_show(portid_t port_id, int show_rss_key)
> return;
> }
> printf("RSS functions:\n ");
> - for (i = 0; rss_type_table[i].str; i++) {
> - if (rss_type_table[i].rss_type == 0)
> - continue;
> - if ((rss_hf & rss_type_table[i].rss_type) == rss_type_table[i].rss_type)
> - printf("%s ", rss_type_table[i].str);
> + for (i = 0; rss_hf != 0; i++) {
> + if (rss_hf & 1) {
> + uint64_t rss_type = 1ULL << i;
> + const char *p = rsstype_to_str(rss_type);
> + if (p)
> + printf("%s ", p);
> + else
> + printf("0x%"PRIx64" ", rss_type);
> + }
> + rss_hf >>= 1;
> }
> printf("\n");
> if (!show_rss_key)
> @@ -3844,15 +3885,10 @@ port_rss_hash_key_update(portid_t port_id, char rss_type[], uint8_t *hash_key,
> {
> struct rte_eth_rss_conf rss_conf;
> int diag;
> - unsigned int i;
>
> rss_conf.rss_key = NULL;
> rss_conf.rss_key_len = 0;
> - rss_conf.rss_hf = 0;
> - for (i = 0; rss_type_table[i].str; i++) {
> - if (!strcmp(rss_type_table[i].str, rss_type))
> - rss_conf.rss_hf = rss_type_table[i].rss_type;
> - }
> + rss_conf.rss_hf = str_to_rsstype(rss_type);
> diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
> if (diag == 0) {
> rss_conf.rss_key = hash_key;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] app/testpmd: fix displaying RSS info
2022-05-26 8:56 ` Ferruh Yigit
@ 2022-05-27 2:48 ` lihuisong (C)
0 siblings, 0 replies; 14+ messages in thread
From: lihuisong (C) @ 2022-05-27 2:48 UTC (permalink / raw)
To: Ferruh Yigit, Xiaoyun Li, Aman Singh, Yuying Zhang, Helin Zhang,
Jingjing Wu
Cc: dev, Thomas Monjalon, Andrew Rybchenko, stable
在 2022/5/26 16:56, Ferruh Yigit 写道:
> On 5/26/2022 4:52 AM, lihuisong (C) wrote:
>> CAUTION: This message has originated from an External Source. Please
>> use proper judgment and caution when opening attachments, clicking
>> links, or responding to this email.
>>
>>
>> Hi, Ferruh
>>
>> This patch is the same as the following patch:
>>
>> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatches.dpdk.org%2Fproject%2Fdpdk%2Fpatch%2F20220429102445.23711-2-lihuisong%40huawei.com%2F&data=05%7C01%7CFerruh.Yigit%40amd.com%7Cda898bd1b39b438ce3e308da3ecb3123%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637891340148254654%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=kixRff%2BQavl0SYS7woT5YYJELrrocoSoV4sE7TUK9Is%3D&reserved=0
>>
>>
>> It was delegated to you.😂
>>
>
> Yes indeed, I missed it. They are fixing same issue.
> But there is slight difference in the implementation, can you please
> review the set? We can share the sign off for final set.
ok.
>
>>
>> 在 2022/5/26 1:37, Ferruh Yigit 写道:
>>> When supported RSS offload flow types are printed via 'show port
>>> info #'
>>> command, flow names are get from flow type array which is wrong and
>>> causing some RSS flow types not being displayed.
>>>
>>> Instead RSS flow type array should be used. Also helper functions added
>>> and existing code updated to use helpers.
>>>
>>> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
>>> ---
>>> Cc: helin.zhang@intel.com
>>>
>>> Note:
>>> In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
>>> 'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may need
>>> to check that too.
>>> ---
>>> app/test-pmd/config.c | 92
>>> ++++++++++++++++++++++++++++++-------------
>>> 1 file changed, 64 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>> index 72d2606d19d5..c353d224ef06 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
>>> { NULL, 0 },
>>> };
>>>
>>> +static const char *
>>> +rsstype_to_str(uint64_t rss_type)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>>> + if (rss_type_table[i].rss_type == rss_type)
>>> + return rss_type_table[i].str;
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> +static uint64_t
>>> +str_to_rsstype(const char *str)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>>> + if (!strcmp(rss_type_table[i].str, str))
>>> + return rss_type_table[i].rss_type;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static const struct {
>>> enum rte_eth_fec_mode mode;
>>> const char *name;
>>> @@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
>>> if (!dev_info.flow_type_rss_offloads)
>>> printf("No RSS offload flow type is supported.\n");
>>> else {
>>> + uint64_t rss_types = dev_info.flow_type_rss_offloads;
>>> uint16_t i;
>>> - char *p;
>>>
>>> printf("Supported RSS offload flow types:\n");
>>> - for (i = RTE_ETH_FLOW_UNKNOWN + 1;
>>> - i < sizeof(dev_info.flow_type_rss_offloads) *
>>> CHAR_BIT; i++) {
>>> - if (!(dev_info.flow_type_rss_offloads & (1ULL
>>> << i)))
>>> - continue;
>>> - p = flowtype_to_str(i);
>>> - if (p)
>>> - printf(" %s\n", p);
>>> - else
>>> - printf(" user defined %d\n", i);
>>> + for (i = 0; rss_types != 0; i++) {
>>> + if (rss_types & 1) {
>>> + uint64_t rss_type = 1ULL << i;
>>> + const char *p = rsstype_to_str(rss_type);
>>> +
>>> + if (p)
>>> + printf(" %s\n", p);
>>> + else
>>> + printf(" user defined
>>> 0x%"PRIx64"\n", rss_type);
>>> + }
>>> + rss_types >>= 1;
>>> }
>>> }
>>>
>>> @@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error *error)
>>> static void
>>> rss_config_display(struct rte_flow_action_rss *rss_conf)
>>> {
>>> + uint64_t rss_types;
>>> uint8_t i;
>>>
>>> if (rss_conf == NULL) {
>>> @@ -1582,16 +1611,23 @@ rss_config_display(struct
>>> rte_flow_action_rss *rss_conf)
>>> }
>>>
>>> printf(" types:\n");
>>> - if (rss_conf->types == 0) {
>>> + rss_types = rss_conf->types;
>>> + if (rss_types == 0) {
>>> printf(" none\n");
>>> return;
>>> }
>>> - for (i = 0; rss_type_table[i].str; i++) {
>>> - if ((rss_conf->types &
>>> - rss_type_table[i].rss_type) ==
>>> - rss_type_table[i].rss_type &&
>>> - rss_type_table[i].rss_type != 0)
>>> - printf(" %s\n", rss_type_table[i].str);
>>> +
>>> + for (i = 0; rss_types != 0; i++) {
>>> + if (rss_types & 1) {
>>> + uint64_t rss_type = 1ULL << i;
>>> + const char *p = rsstype_to_str(rss_type);
>>> +
>>> + if (p)
>>> + printf(" %s\n", p);
>>> + else
>>> + printf(" user defined 0x%"PRIx64"\n",
>>> rss_type);
>>> + }
>>> + rss_types >>= 1;
>>> }
>>> }
>>>
>>> @@ -3823,11 +3859,16 @@ port_rss_hash_conf_show(portid_t port_id,
>>> int show_rss_key)
>>> return;
>>> }
>>> printf("RSS functions:\n ");
>>> - for (i = 0; rss_type_table[i].str; i++) {
>>> - if (rss_type_table[i].rss_type == 0)
>>> - continue;
>>> - if ((rss_hf & rss_type_table[i].rss_type) ==
>>> rss_type_table[i].rss_type)
>>> - printf("%s ", rss_type_table[i].str);
>>> + for (i = 0; rss_hf != 0; i++) {
>>> + if (rss_hf & 1) {
>>> + uint64_t rss_type = 1ULL << i;
>>> + const char *p = rsstype_to_str(rss_type);
>>> + if (p)
>>> + printf("%s ", p);
>>> + else
>>> + printf("0x%"PRIx64" ", rss_type);
>>> + }
>>> + rss_hf >>= 1;
>>> }
>>> printf("\n");
>>> if (!show_rss_key)
>>> @@ -3844,15 +3885,10 @@ port_rss_hash_key_update(portid_t port_id,
>>> char rss_type[], uint8_t *hash_key,
>>> {
>>> struct rte_eth_rss_conf rss_conf;
>>> int diag;
>>> - unsigned int i;
>>>
>>> rss_conf.rss_key = NULL;
>>> rss_conf.rss_key_len = 0;
>>> - rss_conf.rss_hf = 0;
>>> - for (i = 0; rss_type_table[i].str; i++) {
>>> - if (!strcmp(rss_type_table[i].str, rss_type))
>>> - rss_conf.rss_hf = rss_type_table[i].rss_type;
>>> - }
>>> + rss_conf.rss_hf = str_to_rsstype(rss_type);
>>> diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
>>> if (diag == 0) {
>>> rss_conf.rss_key = hash_key;
>
> .
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] app/testpmd: fix displaying RSS info
2022-05-27 2:30 ` lihuisong (C)
@ 2022-05-30 10:43 ` Ferruh Yigit
2022-05-30 12:32 ` lihuisong (C)
0 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2022-05-30 10:43 UTC (permalink / raw)
To: lihuisong (C),
Xiaoyun Li, Aman Singh, Yuying Zhang, Helin Zhang, Jingjing Wu,
Thomas Monjalon
Cc: dev, Thomas Monjalon, Andrew Rybchenko, stable
On 5/27/2022 3:30 AM, lihuisong (C) wrote:
> CAUTION: This message has originated from an External Source. Please use
> proper judgment and caution when opening attachments, clicking links, or
> responding to this email.
>
>
> 在 2022/5/26 1:37, Ferruh Yigit 写道:
>> When supported RSS offload flow types are printed via 'show port info #'
>> command, flow names are get from flow type array which is wrong and
>> causing some RSS flow types not being displayed.
>>
>> Instead RSS flow type array should be used. Also helper functions added
>> and existing code updated to use helpers.
>>
>> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
>> ---
>> Cc: helin.zhang@intel.com
>>
>> Note:
>> In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
>> 'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may need
>> to check that too.
>> ---
>> app/test-pmd/config.c | 92 ++++++++++++++++++++++++++++++-------------
>> 1 file changed, 64 insertions(+), 28 deletions(-)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 72d2606d19d5..c353d224ef06 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
>> { NULL, 0 },
>> };
>>
>> +static const char *
>> +rsstype_to_str(uint64_t rss_type)
>> +{
>> + int i;
>> +
>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>> + if (rss_type_table[i].rss_type == rss_type)
>> + return rss_type_table[i].str;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static uint64_t
>> +str_to_rsstype(const char *str)
>> +{
>> + int i;
>> +
>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>> + if (!strcmp(rss_type_table[i].str, str))
>> + return rss_type_table[i].rss_type;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static const struct {
>> enum rte_eth_fec_mode mode;
>> const char *name;
>> @@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
>> if (!dev_info.flow_type_rss_offloads)
>> printf("No RSS offload flow type is supported.\n");
>> else {
>> + uint64_t rss_types = dev_info.flow_type_rss_offloads;
>> uint16_t i;
>> - char *p;
>>
>> printf("Supported RSS offload flow types:\n");
>> - for (i = RTE_ETH_FLOW_UNKNOWN + 1;
>> - i < sizeof(dev_info.flow_type_rss_offloads) *
>> CHAR_BIT; i++) {
>> - if (!(dev_info.flow_type_rss_offloads & (1ULL <<
>> i)))
>> - continue;
>> - p = flowtype_to_str(i);
>> - if (p)
>> - printf(" %s\n", p);
>> - else
>> - printf(" user defined %d\n", i);
>> + for (i = 0; rss_types != 0; i++) {
>> + if (rss_types & 1) {
>> + uint64_t rss_type = 1ULL << i;
>> + const char *p = rsstype_to_str(rss_type);
>> +
>> + if (p)
>> + printf(" %s\n", p);
>> + else
>> + printf(" user defined
>> 0x%"PRIx64"\n", rss_type);
>> + }
>> + rss_types >>= 1;
>> }
>> }
>>
>> @@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error *error)
>> static void
>> rss_config_display(struct rte_flow_action_rss *rss_conf)
>> {
>> + uint64_t rss_types;
>> uint8_t i;
>>
>> if (rss_conf == NULL) {
>> @@ -1582,16 +1611,23 @@ rss_config_display(struct rte_flow_action_rss
>> *rss_conf)
>> }
>>
>> printf(" types:\n");
>> - if (rss_conf->types == 0) {
>> + rss_types = rss_conf->types;
>> + if (rss_types == 0) {
>> printf(" none\n");
>> return;
>> }
>> - for (i = 0; rss_type_table[i].str; i++) {
>> - if ((rss_conf->types &
>> - rss_type_table[i].rss_type) ==
>> - rss_type_table[i].rss_type &&
>> - rss_type_table[i].rss_type != 0)
>> - printf(" %s\n", rss_type_table[i].str);
>> +
>> + for (i = 0; rss_types != 0; i++) {
>> + if (rss_types & 1) {
>> + uint64_t rss_type = 1ULL << i;
>> + const char *p = rsstype_to_str(rss_type);
> It seems that we can't use one bit to get rss type name.
> Because part of name in rss_type_table[] consist of multiple bits.
> Maybe it's better to use the original method to display RSS type name.
>
Right, it doesn't cover 'all' case, but thinking twice how useful 'all'
case is, it doesn't really cover all options, and for user it is hard to
know which functions are set when it displays 'all'.
What about to remove 'all' item from 'rss_type_table[]'?
> On the other hand, RSS type name are printed in many places as this
> patch modified.
> It is recommended that you encapsulate a funcion to display RSS type.
Yes but each are formatting slightly different, so to let various
formatting possible, common function returns string instead of printing
them. As 3/3 of this set does some formatting.
>> +
>> + if (p)
>> + printf(" %s\n", p);
>> + else
>> + printf(" user defined 0x%"PRIx64"\n",
>> rss_type);
>> + }
>> + rss_types >>= 1;
>> }
>> }
>>
>> @@ -3823,11 +3859,16 @@ port_rss_hash_conf_show(portid_t port_id, int
>> show_rss_key)
>> return;
>> }
>> printf("RSS functions:\n ");
>> - for (i = 0; rss_type_table[i].str; i++) {
>> - if (rss_type_table[i].rss_type == 0)
>> - continue;
>> - if ((rss_hf & rss_type_table[i].rss_type) ==
>> rss_type_table[i].rss_type)
>> - printf("%s ", rss_type_table[i].str);
>> + for (i = 0; rss_hf != 0; i++) {
>> + if (rss_hf & 1) {
>> + uint64_t rss_type = 1ULL << i;
>> + const char *p = rsstype_to_str(rss_type);
>> + if (p)
>> + printf("%s ", p);
>> + else
>> + printf("0x%"PRIx64" ", rss_type);
>> + }
>> + rss_hf >>= 1;
>> }
>> printf("\n");
>> if (!show_rss_key)
>> @@ -3844,15 +3885,10 @@ port_rss_hash_key_update(portid_t port_id,
>> char rss_type[], uint8_t *hash_key,
>> {
>> struct rte_eth_rss_conf rss_conf;
>> int diag;
>> - unsigned int i;
>>
>> rss_conf.rss_key = NULL;
>> rss_conf.rss_key_len = 0;
>> - rss_conf.rss_hf = 0;
>> - for (i = 0; rss_type_table[i].str; i++) {
>> - if (!strcmp(rss_type_table[i].str, rss_type))
>> - rss_conf.rss_hf = rss_type_table[i].rss_type;
>> - }
>> + rss_conf.rss_hf = str_to_rsstype(rss_type);
>> diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
>> if (diag == 0) {
>> rss_conf.rss_key = hash_key;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] app/testpmd: fix displaying RSS info
2022-05-30 10:43 ` Ferruh Yigit
@ 2022-05-30 12:32 ` lihuisong (C)
2022-05-30 13:02 ` Ferruh Yigit
0 siblings, 1 reply; 14+ messages in thread
From: lihuisong (C) @ 2022-05-30 12:32 UTC (permalink / raw)
To: Ferruh Yigit, Xiaoyun Li, Aman Singh, Yuying Zhang, Helin Zhang,
Jingjing Wu, Thomas Monjalon
Cc: dev, Thomas Monjalon, Andrew Rybchenko, stable
在 2022/5/30 18:43, Ferruh Yigit 写道:
> On 5/27/2022 3:30 AM, lihuisong (C) wrote:
>> CAUTION: This message has originated from an External Source. Please
>> use proper judgment and caution when opening attachments, clicking
>> links, or responding to this email.
>>
>>
>> 在 2022/5/26 1:37, Ferruh Yigit 写道:
>>> When supported RSS offload flow types are printed via 'show port
>>> info #'
>>> command, flow names are get from flow type array which is wrong and
>>> causing some RSS flow types not being displayed.
>>>
>>> Instead RSS flow type array should be used. Also helper functions added
>>> and existing code updated to use helpers.
>>>
>>> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
>>> ---
>>> Cc: helin.zhang@intel.com
>>>
>>> Note:
>>> In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
>>> 'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may need
>>> to check that too.
>>> ---
>>> app/test-pmd/config.c | 92
>>> ++++++++++++++++++++++++++++++-------------
>>> 1 file changed, 64 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>> index 72d2606d19d5..c353d224ef06 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
>>> { NULL, 0 },
>>> };
>>>
>>> +static const char *
>>> +rsstype_to_str(uint64_t rss_type)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>>> + if (rss_type_table[i].rss_type == rss_type)
>>> + return rss_type_table[i].str;
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>>> +static uint64_t
>>> +str_to_rsstype(const char *str)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>>> + if (!strcmp(rss_type_table[i].str, str))
>>> + return rss_type_table[i].rss_type;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> static const struct {
>>> enum rte_eth_fec_mode mode;
>>> const char *name;
>>> @@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
>>> if (!dev_info.flow_type_rss_offloads)
>>> printf("No RSS offload flow type is supported.\n");
>>> else {
>>> + uint64_t rss_types = dev_info.flow_type_rss_offloads;
>>> uint16_t i;
>>> - char *p;
>>>
>>> printf("Supported RSS offload flow types:\n");
>>> - for (i = RTE_ETH_FLOW_UNKNOWN + 1;
>>> - i < sizeof(dev_info.flow_type_rss_offloads) *
>>> CHAR_BIT; i++) {
>>> - if (!(dev_info.flow_type_rss_offloads & (1ULL
>>> << i)))
>>> - continue;
>>> - p = flowtype_to_str(i);
>>> - if (p)
>>> - printf(" %s\n", p);
>>> - else
>>> - printf(" user defined %d\n", i);
>>> + for (i = 0; rss_types != 0; i++) {
>>> + if (rss_types & 1) {
>>> + uint64_t rss_type = 1ULL << i;
>>> + const char *p = rsstype_to_str(rss_type);
>>> +
>>> + if (p)
>>> + printf(" %s\n", p);
>>> + else
>>> + printf(" user defined
>>> 0x%"PRIx64"\n", rss_type);
Maybe we need to add a mapping table between RSS offload and name for
'flow_type_rss_offloads'.
>>> + }
>>> + rss_types >>= 1;
>>> }
>>> }
>>>
>>> @@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error *error)
>>> static void
>>> rss_config_display(struct rte_flow_action_rss *rss_conf)
>>> {
>>> + uint64_t rss_types;
>>> uint8_t i;
>>>
>>> if (rss_conf == NULL) {
>>> @@ -1582,16 +1611,23 @@ rss_config_display(struct
>>> rte_flow_action_rss *rss_conf)
>>> }
>>>
>>> printf(" types:\n");
>>> - if (rss_conf->types == 0) {
>>> + rss_types = rss_conf->types;
>>> + if (rss_types == 0) {
>>> printf(" none\n");
>>> return;
>>> }
>>> - for (i = 0; rss_type_table[i].str; i++) {
>>> - if ((rss_conf->types &
>>> - rss_type_table[i].rss_type) ==
>>> - rss_type_table[i].rss_type &&
>>> - rss_type_table[i].rss_type != 0)
>>> - printf(" %s\n", rss_type_table[i].str);
>>> +
>>> + for (i = 0; rss_types != 0; i++) {
>>> + if (rss_types & 1) {
>>> + uint64_t rss_type = 1ULL << i;
>>> + const char *p = rsstype_to_str(rss_type);
>> It seems that we can't use one bit to get rss type name.
>> Because part of name in rss_type_table[] consist of multiple bits.
>> Maybe it's better to use the original method to display RSS type name.
>>
>
> Right, it doesn't cover 'all' case, but thinking twice how useful
> 'all' case is, it doesn't really cover all options, and for user it is
> hard to know which functions are set when it displays 'all'.
> What about to remove 'all' item from 'rss_type_table[]'?
It seems that remove 'all' item isn't the way to resolve this issue.
There are other iterms in 'rss_type_table[], such as, 'ip', 'udp',
'tcp' and so on. The 'rss_type_table[]' should primarily serve
the "show port rss xxx" and "port confg rss xxx" commands.
They all need these.
>
>> On the other hand, RSS type name are printed in many places as this
>> patch modified.
>> It is recommended that you encapsulate a funcion to display RSS type.
>
> Yes but each are formatting slightly different, so to let various
> formatting possible, common function returns string instead of
> printing them. As 3/3 of this set does some formatting.
We can unify this display if we add the mapping table mentioned above.
>
>>> +
>>> + if (p)
>>> + printf(" %s\n", p);
>>> + else
>>> + printf(" user defined 0x%"PRIx64"\n",
>>> rss_type);
>>> + }
>>> + rss_types >>= 1;
>>> }
>>> }
>>>
>>> @@ -3823,11 +3859,16 @@ port_rss_hash_conf_show(portid_t port_id,
>>> int show_rss_key)
>>> return;
>>> }
>>> printf("RSS functions:\n ");
>>> - for (i = 0; rss_type_table[i].str; i++) {
>>> - if (rss_type_table[i].rss_type == 0)
>>> - continue;
>>> - if ((rss_hf & rss_type_table[i].rss_type) ==
>>> rss_type_table[i].rss_type)
>>> - printf("%s ", rss_type_table[i].str);
>>> + for (i = 0; rss_hf != 0; i++) {
>>> + if (rss_hf & 1) {
>>> + uint64_t rss_type = 1ULL << i;
>>> + const char *p = rsstype_to_str(rss_type);
>>> + if (p)
>>> + printf("%s ", p);
>>> + else
>>> + printf("0x%"PRIx64" ", rss_type);
>>> + }
>>> + rss_hf >>= 1;
>>> }
>>> printf("\n");
>>> if (!show_rss_key)
>>> @@ -3844,15 +3885,10 @@ port_rss_hash_key_update(portid_t port_id,
>>> char rss_type[], uint8_t *hash_key,
>>> {
>>> struct rte_eth_rss_conf rss_conf;
>>> int diag;
>>> - unsigned int i;
>>>
>>> rss_conf.rss_key = NULL;
>>> rss_conf.rss_key_len = 0;
>>> - rss_conf.rss_hf = 0;
>>> - for (i = 0; rss_type_table[i].str; i++) {
>>> - if (!strcmp(rss_type_table[i].str, rss_type))
>>> - rss_conf.rss_hf = rss_type_table[i].rss_type;
>>> - }
>>> + rss_conf.rss_hf = str_to_rsstype(rss_type);
>>> diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
>>> if (diag == 0) {
>>> rss_conf.rss_key = hash_key;
>
> .
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] app/testpmd: fix displaying RSS info
2022-05-30 12:32 ` lihuisong (C)
@ 2022-05-30 13:02 ` Ferruh Yigit
2022-05-31 2:07 ` lihuisong (C)
0 siblings, 1 reply; 14+ messages in thread
From: Ferruh Yigit @ 2022-05-30 13:02 UTC (permalink / raw)
To: lihuisong (C),
Xiaoyun Li, Aman Singh, Yuying Zhang, Helin Zhang, Jingjing Wu,
Thomas Monjalon
Cc: dev, Thomas Monjalon, Andrew Rybchenko, stable
On 5/30/2022 1:32 PM, lihuisong (C) wrote:
> CAUTION: This message has originated from an External Source. Please use
> proper judgment and caution when opening attachments, clicking links, or
> responding to this email.
>
>
> 在 2022/5/30 18:43, Ferruh Yigit 写道:
>> On 5/27/2022 3:30 AM, lihuisong (C) wrote:
>>> CAUTION: This message has originated from an External Source. Please
>>> use proper judgment and caution when opening attachments, clicking
>>> links, or responding to this email.
>>>
>>>
>>> 在 2022/5/26 1:37, Ferruh Yigit 写道:
>>>> When supported RSS offload flow types are printed via 'show port
>>>> info #'
>>>> command, flow names are get from flow type array which is wrong and
>>>> causing some RSS flow types not being displayed.
>>>>
>>>> Instead RSS flow type array should be used. Also helper functions added
>>>> and existing code updated to use helpers.
>>>>
>>>> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
>>>> ---
>>>> Cc: helin.zhang@intel.com
>>>>
>>>> Note:
>>>> In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
>>>> 'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may need
>>>> to check that too.
>>>> ---
>>>> app/test-pmd/config.c | 92
>>>> ++++++++++++++++++++++++++++++-------------
>>>> 1 file changed, 64 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>>> index 72d2606d19d5..c353d224ef06 100644
>>>> --- a/app/test-pmd/config.c
>>>> +++ b/app/test-pmd/config.c
>>>> @@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
>>>> { NULL, 0 },
>>>> };
>>>>
>>>> +static const char *
>>>> +rsstype_to_str(uint64_t rss_type)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>>>> + if (rss_type_table[i].rss_type == rss_type)
>>>> + return rss_type_table[i].str;
>>>> + }
>>>> +
>>>> + return NULL;
>>>> +}
>>>> +
>>>> +static uint64_t
>>>> +str_to_rsstype(const char *str)
>>>> +{
>>>> + int i;
>>>> +
>>>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>>>> + if (!strcmp(rss_type_table[i].str, str))
>>>> + return rss_type_table[i].rss_type;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> static const struct {
>>>> enum rte_eth_fec_mode mode;
>>>> const char *name;
>>>> @@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
>>>> if (!dev_info.flow_type_rss_offloads)
>>>> printf("No RSS offload flow type is supported.\n");
>>>> else {
>>>> + uint64_t rss_types = dev_info.flow_type_rss_offloads;
>>>> uint16_t i;
>>>> - char *p;
>>>>
>>>> printf("Supported RSS offload flow types:\n");
>>>> - for (i = RTE_ETH_FLOW_UNKNOWN + 1;
>>>> - i < sizeof(dev_info.flow_type_rss_offloads) *
>>>> CHAR_BIT; i++) {
>>>> - if (!(dev_info.flow_type_rss_offloads & (1ULL
>>>> << i)))
>>>> - continue;
>>>> - p = flowtype_to_str(i);
>>>> - if (p)
>>>> - printf(" %s\n", p);
>>>> - else
>>>> - printf(" user defined %d\n", i);
>>>> + for (i = 0; rss_types != 0; i++) {
>>>> + if (rss_types & 1) {
>>>> + uint64_t rss_type = 1ULL << i;
>>>> + const char *p = rsstype_to_str(rss_type);
>>>> +
>>>> + if (p)
>>>> + printf(" %s\n", p);
>>>> + else
>>>> + printf(" user defined
>>>> 0x%"PRIx64"\n", rss_type);
> Maybe we need to add a mapping table between RSS offload and name for
> 'flow_type_rss_offloads'.
>>>> + }
>>>> + rss_types >>= 1;
>>>> }
>>>> }
>>>>
>>>> @@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error *error)
>>>> static void
>>>> rss_config_display(struct rte_flow_action_rss *rss_conf)
>>>> {
>>>> + uint64_t rss_types;
>>>> uint8_t i;
>>>>
>>>> if (rss_conf == NULL) {
>>>> @@ -1582,16 +1611,23 @@ rss_config_display(struct
>>>> rte_flow_action_rss *rss_conf)
>>>> }
>>>>
>>>> printf(" types:\n");
>>>> - if (rss_conf->types == 0) {
>>>> + rss_types = rss_conf->types;
>>>> + if (rss_types == 0) {
>>>> printf(" none\n");
>>>> return;
>>>> }
>>>> - for (i = 0; rss_type_table[i].str; i++) {
>>>> - if ((rss_conf->types &
>>>> - rss_type_table[i].rss_type) ==
>>>> - rss_type_table[i].rss_type &&
>>>> - rss_type_table[i].rss_type != 0)
>>>> - printf(" %s\n", rss_type_table[i].str);
>>>> +
>>>> + for (i = 0; rss_types != 0; i++) {
>>>> + if (rss_types & 1) {
>>>> + uint64_t rss_type = 1ULL << i;
>>>> + const char *p = rsstype_to_str(rss_type);
>>> It seems that we can't use one bit to get rss type name.
>>> Because part of name in rss_type_table[] consist of multiple bits.
>>> Maybe it's better to use the original method to display RSS type name.
>>>
>>
>> Right, it doesn't cover 'all' case, but thinking twice how useful
>> 'all' case is, it doesn't really cover all options, and for user it is
>> hard to know which functions are set when it displays 'all'.
>> What about to remove 'all' item from 'rss_type_table[]'?
> It seems that remove 'all' item isn't the way to resolve this issue.
> There are other iterms in 'rss_type_table[], such as, 'ip', 'udp',
> 'tcp' and so on. The 'rss_type_table[]' should primarily serve
> the "show port rss xxx" and "port confg rss xxx" commands.
> They all need these.
>
You are right, it is not just 'all'.
>>
>>> On the other hand, RSS type name are printed in many places as this
>>> patch modified.
>>> It is recommended that you encapsulate a funcion to display RSS type.
>>
>> Yes but each are formatting slightly different, so to let various
>> formatting possible, common function returns string instead of
>> printing them. As 3/3 of this set does some formatting.
>
> We can unify this display if we add the mapping table mentioned above.
>
If it can be unified, agree that this simplifies the code [2].
Both above two changes already exists in your set [1], if you don't mind
to adapt 2/3 & 3/3 of this set into yours, we can continue with your
set, is this OK for you?
[1]
https://patches.dpdk.org/project/dpdk/list/?series=22735
[2]
In your 'show_rss_types()', it can get additional parameter for
formatting, like after how many items to break to next line, this can
cover existing formatting options, but not sure how elegant a solution
it is.
>>
>>>> +
>>>> + if (p)
>>>> + printf(" %s\n", p);
>>>> + else
>>>> + printf(" user defined 0x%"PRIx64"\n",
>>>> rss_type);
>>>> + }
>>>> + rss_types >>= 1;
>>>> }
>>>> }
>>>>
>>>> @@ -3823,11 +3859,16 @@ port_rss_hash_conf_show(portid_t port_id,
>>>> int show_rss_key)
>>>> return;
>>>> }
>>>> printf("RSS functions:\n ");
>>>> - for (i = 0; rss_type_table[i].str; i++) {
>>>> - if (rss_type_table[i].rss_type == 0)
>>>> - continue;
>>>> - if ((rss_hf & rss_type_table[i].rss_type) ==
>>>> rss_type_table[i].rss_type)
>>>> - printf("%s ", rss_type_table[i].str);
>>>> + for (i = 0; rss_hf != 0; i++) {
>>>> + if (rss_hf & 1) {
>>>> + uint64_t rss_type = 1ULL << i;
>>>> + const char *p = rsstype_to_str(rss_type);
>>>> + if (p)
>>>> + printf("%s ", p);
>>>> + else
>>>> + printf("0x%"PRIx64" ", rss_type);
>>>> + }
>>>> + rss_hf >>= 1;
>>>> }
>>>> printf("\n");
>>>> if (!show_rss_key)
>>>> @@ -3844,15 +3885,10 @@ port_rss_hash_key_update(portid_t port_id,
>>>> char rss_type[], uint8_t *hash_key,
>>>> {
>>>> struct rte_eth_rss_conf rss_conf;
>>>> int diag;
>>>> - unsigned int i;
>>>>
>>>> rss_conf.rss_key = NULL;
>>>> rss_conf.rss_key_len = 0;
>>>> - rss_conf.rss_hf = 0;
>>>> - for (i = 0; rss_type_table[i].str; i++) {
>>>> - if (!strcmp(rss_type_table[i].str, rss_type))
>>>> - rss_conf.rss_hf = rss_type_table[i].rss_type;
>>>> - }
>>>> + rss_conf.rss_hf = str_to_rsstype(rss_type);
>>>> diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
>>>> if (diag == 0) {
>>>> rss_conf.rss_key = hash_key;
>>
>> .
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] app/testpmd: fix displaying RSS info
2022-05-30 13:02 ` Ferruh Yigit
@ 2022-05-31 2:07 ` lihuisong (C)
2022-05-31 16:35 ` Andrew Rybchenko
0 siblings, 1 reply; 14+ messages in thread
From: lihuisong (C) @ 2022-05-31 2:07 UTC (permalink / raw)
To: Ferruh Yigit, Xiaoyun Li, Aman Singh, Yuying Zhang, Helin Zhang,
Jingjing Wu, Thomas Monjalon
Cc: dev, Thomas Monjalon, Andrew Rybchenko, stable
在 2022/5/30 21:02, Ferruh Yigit 写道:
> On 5/30/2022 1:32 PM, lihuisong (C) wrote:
>> CAUTION: This message has originated from an External Source. Please
>> use proper judgment and caution when opening attachments, clicking
>> links, or responding to this email.
>>
>>
>> 在 2022/5/30 18:43, Ferruh Yigit 写道:
>>> On 5/27/2022 3:30 AM, lihuisong (C) wrote:
>>>> CAUTION: This message has originated from an External Source. Please
>>>> use proper judgment and caution when opening attachments, clicking
>>>> links, or responding to this email.
>>>>
>>>>
>>>> 在 2022/5/26 1:37, Ferruh Yigit 写道:
>>>>> When supported RSS offload flow types are printed via 'show port
>>>>> info #'
>>>>> command, flow names are get from flow type array which is wrong and
>>>>> causing some RSS flow types not being displayed.
>>>>>
>>>>> Instead RSS flow type array should be used. Also helper functions
>>>>> added
>>>>> and existing code updated to use helpers.
>>>>>
>>>>> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
>>>>> ---
>>>>> Cc: helin.zhang@intel.com
>>>>>
>>>>> Note:
>>>>> In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
>>>>> 'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may
>>>>> need
>>>>> to check that too.
>>>>> ---
>>>>> app/test-pmd/config.c | 92
>>>>> ++++++++++++++++++++++++++++++-------------
>>>>> 1 file changed, 64 insertions(+), 28 deletions(-)
>>>>>
>>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>>>> index 72d2606d19d5..c353d224ef06 100644
>>>>> --- a/app/test-pmd/config.c
>>>>> +++ b/app/test-pmd/config.c
>>>>> @@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
>>>>> { NULL, 0 },
>>>>> };
>>>>>
>>>>> +static const char *
>>>>> +rsstype_to_str(uint64_t rss_type)
>>>>> +{
>>>>> + int i;
>>>>> +
>>>>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>>>>> + if (rss_type_table[i].rss_type == rss_type)
>>>>> + return rss_type_table[i].str;
>>>>> + }
>>>>> +
>>>>> + return NULL;
>>>>> +}
>>>>> +
>>>>> +static uint64_t
>>>>> +str_to_rsstype(const char *str)
>>>>> +{
>>>>> + int i;
>>>>> +
>>>>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>>>>> + if (!strcmp(rss_type_table[i].str, str))
>>>>> + return rss_type_table[i].rss_type;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> static const struct {
>>>>> enum rte_eth_fec_mode mode;
>>>>> const char *name;
>>>>> @@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
>>>>> if (!dev_info.flow_type_rss_offloads)
>>>>> printf("No RSS offload flow type is supported.\n");
>>>>> else {
>>>>> + uint64_t rss_types = dev_info.flow_type_rss_offloads;
>>>>> uint16_t i;
>>>>> - char *p;
>>>>>
>>>>> printf("Supported RSS offload flow types:\n");
>>>>> - for (i = RTE_ETH_FLOW_UNKNOWN + 1;
>>>>> - i < sizeof(dev_info.flow_type_rss_offloads) *
>>>>> CHAR_BIT; i++) {
>>>>> - if (!(dev_info.flow_type_rss_offloads & (1ULL
>>>>> << i)))
>>>>> - continue;
>>>>> - p = flowtype_to_str(i);
>>>>> - if (p)
>>>>> - printf(" %s\n", p);
>>>>> - else
>>>>> - printf(" user defined %d\n", i);
>>>>> + for (i = 0; rss_types != 0; i++) {
>>>>> + if (rss_types & 1) {
>>>>> + uint64_t rss_type = 1ULL << i;
>>>>> + const char *p =
>>>>> rsstype_to_str(rss_type);
>>>>> +
>>>>> + if (p)
>>>>> + printf(" %s\n", p);
>>>>> + else
>>>>> + printf(" user defined
>>>>> 0x%"PRIx64"\n", rss_type);
>> Maybe we need to add a mapping table between RSS offload and name for
>> 'flow_type_rss_offloads'.
>>>>> + }
>>>>> + rss_types >>= 1;
>>>>> }
>>>>> }
>>>>>
>>>>> @@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error
>>>>> *error)
>>>>> static void
>>>>> rss_config_display(struct rte_flow_action_rss *rss_conf)
>>>>> {
>>>>> + uint64_t rss_types;
>>>>> uint8_t i;
>>>>>
>>>>> if (rss_conf == NULL) {
>>>>> @@ -1582,16 +1611,23 @@ rss_config_display(struct
>>>>> rte_flow_action_rss *rss_conf)
>>>>> }
>>>>>
>>>>> printf(" types:\n");
>>>>> - if (rss_conf->types == 0) {
>>>>> + rss_types = rss_conf->types;
>>>>> + if (rss_types == 0) {
>>>>> printf(" none\n");
>>>>> return;
>>>>> }
>>>>> - for (i = 0; rss_type_table[i].str; i++) {
>>>>> - if ((rss_conf->types &
>>>>> - rss_type_table[i].rss_type) ==
>>>>> - rss_type_table[i].rss_type &&
>>>>> - rss_type_table[i].rss_type != 0)
>>>>> - printf(" %s\n", rss_type_table[i].str);
>>>>> +
>>>>> + for (i = 0; rss_types != 0; i++) {
>>>>> + if (rss_types & 1) {
>>>>> + uint64_t rss_type = 1ULL << i;
>>>>> + const char *p = rsstype_to_str(rss_type);
>>>> It seems that we can't use one bit to get rss type name.
>>>> Because part of name in rss_type_table[] consist of multiple bits.
>>>> Maybe it's better to use the original method to display RSS type name.
>>>>
>>>
>>> Right, it doesn't cover 'all' case, but thinking twice how useful
>>> 'all' case is, it doesn't really cover all options, and for user it is
>>> hard to know which functions are set when it displays 'all'.
>>> What about to remove 'all' item from 'rss_type_table[]'?
>> It seems that remove 'all' item isn't the way to resolve this issue.
>> There are other iterms in 'rss_type_table[], such as, 'ip', 'udp',
>> 'tcp' and so on. The 'rss_type_table[]' should primarily serve
>> the "show port rss xxx" and "port confg rss xxx" commands.
>> They all need these.
> >
>
> You are right, it is not just 'all'.
>
>>>
>>>> On the other hand, RSS type name are printed in many places as this
>>>> patch modified.
>>>> It is recommended that you encapsulate a funcion to display RSS type.
>>>
>>> Yes but each are formatting slightly different, so to let various
>>> formatting possible, common function returns string instead of
>>> printing them. As 3/3 of this set does some formatting.
>>
>> We can unify this display if we add the mapping table mentioned above.
>>
>
> If it can be unified, agree that this simplifies the code [2].
Ack.
>
> Both above two changes already exists in your set [1], if you don't
> mind to adapt 2/3 & 3/3 of this set into yours, we can continue with
> your set, is this OK for you?
ok. It's my pleasure to work with you.
>
>
>
> [1]
> https://patches.dpdk.org/project/dpdk/list/?series=22735
>
> [2]
> In your 'show_rss_types()', it can get additional parameter for
> formatting, like after how many items to break to next line, this can
> cover existing formatting options, but not sure how elegant a solution
> it is.
>
>>>
>>>>> +
>>>>> + if (p)
>>>>> + printf(" %s\n", p);
>>>>> + else
>>>>> + printf(" user defined 0x%"PRIx64"\n",
>>>>> rss_type);
>>>>> + }
>>>>> + rss_types >>= 1;
>>>>> }
>>>>> }
>>>>>
>>>>> @@ -3823,11 +3859,16 @@ port_rss_hash_conf_show(portid_t port_id,
>>>>> int show_rss_key)
>>>>> return;
>>>>> }
>>>>> printf("RSS functions:\n ");
>>>>> - for (i = 0; rss_type_table[i].str; i++) {
>>>>> - if (rss_type_table[i].rss_type == 0)
>>>>> - continue;
>>>>> - if ((rss_hf & rss_type_table[i].rss_type) ==
>>>>> rss_type_table[i].rss_type)
>>>>> - printf("%s ", rss_type_table[i].str);
>>>>> + for (i = 0; rss_hf != 0; i++) {
>>>>> + if (rss_hf & 1) {
>>>>> + uint64_t rss_type = 1ULL << i;
>>>>> + const char *p = rsstype_to_str(rss_type);
>>>>> + if (p)
>>>>> + printf("%s ", p);
>>>>> + else
>>>>> + printf("0x%"PRIx64" ", rss_type);
>>>>> + }
>>>>> + rss_hf >>= 1;
>>>>> }
>>>>> printf("\n");
>>>>> if (!show_rss_key)
>>>>> @@ -3844,15 +3885,10 @@ port_rss_hash_key_update(portid_t port_id,
>>>>> char rss_type[], uint8_t *hash_key,
>>>>> {
>>>>> struct rte_eth_rss_conf rss_conf;
>>>>> int diag;
>>>>> - unsigned int i;
>>>>>
>>>>> rss_conf.rss_key = NULL;
>>>>> rss_conf.rss_key_len = 0;
>>>>> - rss_conf.rss_hf = 0;
>>>>> - for (i = 0; rss_type_table[i].str; i++) {
>>>>> - if (!strcmp(rss_type_table[i].str, rss_type))
>>>>> - rss_conf.rss_hf = rss_type_table[i].rss_type;
>>>>> - }
>>>>> + rss_conf.rss_hf = str_to_rsstype(rss_type);
>>>>> diag = rte_eth_dev_rss_hash_conf_get(port_id, &rss_conf);
>>>>> if (diag == 0) {
>>>>> rss_conf.rss_key = hash_key;
>>>
>>> .
>
> .
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] app/testpmd: compact RSS flow type output in port info
2022-05-25 17:37 ` [PATCH 3/3] app/testpmd: compact RSS flow type output in port info Ferruh Yigit
@ 2022-05-31 16:35 ` Andrew Rybchenko
2022-06-01 7:08 ` lihuisong (C)
0 siblings, 1 reply; 14+ messages in thread
From: Andrew Rybchenko @ 2022-05-31 16:35 UTC (permalink / raw)
To: Ferruh Yigit, Xiaoyun Li, Aman Singh, Yuying Zhang; +Cc: dev, Thomas Monjalon
On 5/25/22 20:37, Ferruh Yigit wrote:
> In port info command output, 'show port info all', supported RSS flow
> types printed one type per line, and although this information is not
> most important part of the command it takes big part of the command
> output.
>
> Compacting the supported RSS flow type output by printing 6 (hardcoded
> value) items per line, instead of one per line. Output becomes as
> following:
>
> Supported RSS offload flow types:
> ipv4 ipv4-frag ipv4-tcp ipv4-udp ipv4-sctp ipv4-other
> ipv6 ipv6-frag ipv6-tcp ipv6-udp ipv6-sctp ipv6-other
> l2-payload ipv6-ex ipv6-tcp-ex ipv6-udp-ex port vxlan
> geneve nvgre mpls
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
> ---
> app/test-pmd/config.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 47de5b6d9458..5496ccd7f8ad 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -805,6 +805,7 @@ port_infos_display(portid_t port_id)
> else {
> uint64_t rss_types = dev_info.flow_type_rss_offloads;
> uint16_t i;
> + uint16_t len = 0;
>
> printf("Supported RSS offload flow types:\n");
> for (i = 0; rss_types != 0; i++) {
> @@ -813,12 +814,21 @@ port_infos_display(portid_t port_id)
> const char *p = rsstype_to_str(rss_type);
>
> if (p)
> - printf(" %s\n", p);
> + printf(" %s", p);
> else
> - printf(" user defined 0x%"PRIx64"\n", rss_type);
> + printf(" user defined 0x%"PRIx64, rss_type);
> +
> + len++;
> + /* wrap on every 6 items */
> + if (len == 6) {
Variable name 'len' is misleading here since the first idea is
like length, not a number of items.
Also 6 sounds to be to much in the worst case scenario with 6
user defined types. Each uses at least 18 chars. So, 18 * 6 = 108.
May be printing algorithm should be a bit more sophisticated and
really track line length.
> + printf("\n");
> + len = 0;
> + }
> }
> rss_types >>= 1;
> }
> + if (len)
> + printf("\n");
> }
>
> printf("Minimum size of RX buffer: %u\n", dev_info.min_rx_bufsize);
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] app/testpmd: fix displaying RSS info
2022-05-31 2:07 ` lihuisong (C)
@ 2022-05-31 16:35 ` Andrew Rybchenko
0 siblings, 0 replies; 14+ messages in thread
From: Andrew Rybchenko @ 2022-05-31 16:35 UTC (permalink / raw)
To: lihuisong (C),
Ferruh Yigit, Xiaoyun Li, Aman Singh, Yuying Zhang, Helin Zhang,
Jingjing Wu, Thomas Monjalon
Cc: dev, Thomas Monjalon, stable
On 5/31/22 05:07, lihuisong (C) wrote:
>
> 在 2022/5/30 21:02, Ferruh Yigit 写道:
>> On 5/30/2022 1:32 PM, lihuisong (C) wrote:
>>> CAUTION: This message has originated from an External Source. Please
>>> use proper judgment and caution when opening attachments, clicking
>>> links, or responding to this email.
>>>
>>>
>>> 在 2022/5/30 18:43, Ferruh Yigit 写道:
>>>> On 5/27/2022 3:30 AM, lihuisong (C) wrote:
>>>>> CAUTION: This message has originated from an External Source. Please
>>>>> use proper judgment and caution when opening attachments, clicking
>>>>> links, or responding to this email.
>>>>>
>>>>>
>>>>> 在 2022/5/26 1:37, Ferruh Yigit 写道:
>>>>>> When supported RSS offload flow types are printed via 'show port
>>>>>> info #'
>>>>>> command, flow names are get from flow type array which is wrong and
>>>>>> causing some RSS flow types not being displayed.
>>>>>>
>>>>>> Instead RSS flow type array should be used. Also helper functions
>>>>>> added
>>>>>> and existing code updated to use helpers.
>>>>>>
>>>>>> Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
>>>>>> Cc: stable@dpdk.org
>>>>>>
>>>>>> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
>>>>>> ---
>>>>>> Cc: helin.zhang@intel.com
>>>>>>
>>>>>> Note:
>>>>>> In ethdev, flow type macros 'RTE_ETH_FLOW_*' and RSS type macros
>>>>>> 'RTE_ETH_RSS_*' are related, buy they seems diverged a little, may
>>>>>> need
>>>>>> to check that too.
>>>>>> ---
>>>>>> app/test-pmd/config.c | 92
>>>>>> ++++++++++++++++++++++++++++++-------------
>>>>>> 1 file changed, 64 insertions(+), 28 deletions(-)
>>>>>>
>>>>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>>>>>> index 72d2606d19d5..c353d224ef06 100644
>>>>>> --- a/app/test-pmd/config.c
>>>>>> +++ b/app/test-pmd/config.c
>>>>>> @@ -147,6 +147,32 @@ const struct rss_type_info rss_type_table[] = {
>>>>>> { NULL, 0 },
>>>>>> };
>>>>>>
>>>>>> +static const char *
>>>>>> +rsstype_to_str(uint64_t rss_type)
>>>>>> +{
>>>>>> + int i;
>>>>>> +
>>>>>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>>>>>> + if (rss_type_table[i].rss_type == rss_type)
>>>>>> + return rss_type_table[i].str;
>>>>>> + }
>>>>>> +
>>>>>> + return NULL;
>>>>>> +}
>>>>>> +
>>>>>> +static uint64_t
>>>>>> +str_to_rsstype(const char *str)
>>>>>> +{
>>>>>> + int i;
>>>>>> +
>>>>>> + for (i = 0; rss_type_table[i].str != NULL; i++) {
>>>>>> + if (!strcmp(rss_type_table[i].str, str))
>>>>>> + return rss_type_table[i].rss_type;
>>>>>> + }
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> static const struct {
>>>>>> enum rte_eth_fec_mode mode;
>>>>>> const char *name;
>>>>>> @@ -779,19 +805,21 @@ port_infos_display(portid_t port_id)
>>>>>> if (!dev_info.flow_type_rss_offloads)
>>>>>> printf("No RSS offload flow type is supported.\n");
>>>>>> else {
>>>>>> + uint64_t rss_types = dev_info.flow_type_rss_offloads;
>>>>>> uint16_t i;
>>>>>> - char *p;
>>>>>>
>>>>>> printf("Supported RSS offload flow types:\n");
>>>>>> - for (i = RTE_ETH_FLOW_UNKNOWN + 1;
>>>>>> - i < sizeof(dev_info.flow_type_rss_offloads) *
>>>>>> CHAR_BIT; i++) {
>>>>>> - if (!(dev_info.flow_type_rss_offloads & (1ULL
>>>>>> << i)))
>>>>>> - continue;
>>>>>> - p = flowtype_to_str(i);
>>>>>> - if (p)
>>>>>> - printf(" %s\n", p);
>>>>>> - else
>>>>>> - printf(" user defined %d\n", i);
>>>>>> + for (i = 0; rss_types != 0; i++) {
>>>>>> + if (rss_types & 1) {
>>>>>> + uint64_t rss_type = 1ULL << i;
>>>>>> + const char *p =
>>>>>> rsstype_to_str(rss_type);
>>>>>> +
>>>>>> + if (p)
>>>>>> + printf(" %s\n", p);
>>>>>> + else
>>>>>> + printf(" user defined
>>>>>> 0x%"PRIx64"\n", rss_type);
>>> Maybe we need to add a mapping table between RSS offload and name for
>>> 'flow_type_rss_offloads'.
>>>>>> + }
>>>>>> + rss_types >>= 1;
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> @@ -1547,6 +1575,7 @@ port_flow_complain(struct rte_flow_error
>>>>>> *error)
>>>>>> static void
>>>>>> rss_config_display(struct rte_flow_action_rss *rss_conf)
>>>>>> {
>>>>>> + uint64_t rss_types;
>>>>>> uint8_t i;
>>>>>>
>>>>>> if (rss_conf == NULL) {
>>>>>> @@ -1582,16 +1611,23 @@ rss_config_display(struct
>>>>>> rte_flow_action_rss *rss_conf)
>>>>>> }
>>>>>>
>>>>>> printf(" types:\n");
>>>>>> - if (rss_conf->types == 0) {
>>>>>> + rss_types = rss_conf->types;
>>>>>> + if (rss_types == 0) {
>>>>>> printf(" none\n");
>>>>>> return;
>>>>>> }
>>>>>> - for (i = 0; rss_type_table[i].str; i++) {
>>>>>> - if ((rss_conf->types &
>>>>>> - rss_type_table[i].rss_type) ==
>>>>>> - rss_type_table[i].rss_type &&
>>>>>> - rss_type_table[i].rss_type != 0)
>>>>>> - printf(" %s\n", rss_type_table[i].str);
>>>>>> +
>>>>>> + for (i = 0; rss_types != 0; i++) {
>>>>>> + if (rss_types & 1) {
>>>>>> + uint64_t rss_type = 1ULL << i;
>>>>>> + const char *p = rsstype_to_str(rss_type);
>>>>> It seems that we can't use one bit to get rss type name.
>>>>> Because part of name in rss_type_table[] consist of multiple bits.
>>>>> Maybe it's better to use the original method to display RSS type name.
>>>>>
>>>>
>>>> Right, it doesn't cover 'all' case, but thinking twice how useful
>>>> 'all' case is, it doesn't really cover all options, and for user it is
>>>> hard to know which functions are set when it displays 'all'.
>>>> What about to remove 'all' item from 'rss_type_table[]'?
>>> It seems that remove 'all' item isn't the way to resolve this issue.
>>> There are other iterms in 'rss_type_table[], such as, 'ip', 'udp',
>>> 'tcp' and so on. The 'rss_type_table[]' should primarily serve
>>> the "show port rss xxx" and "port confg rss xxx" commands.
>>> They all need these.
>> >
>>
>> You are right, it is not just 'all'.
>>
>>>>
>>>>> On the other hand, RSS type name are printed in many places as this
>>>>> patch modified.
>>>>> It is recommended that you encapsulate a funcion to display RSS type.
>>>>
>>>> Yes but each are formatting slightly different, so to let various
>>>> formatting possible, common function returns string instead of
>>>> printing them. As 3/3 of this set does some formatting.
>>>
>>> We can unify this display if we add the mapping table mentioned above.
>>>
>>
>> If it can be unified, agree that this simplifies the code [2].
> Ack.
>>
>> Both above two changes already exists in your set [1], if you don't
>> mind to adapt 2/3 & 3/3 of this set into yours, we can continue with
>> your set, is this OK for you?
> ok. It's my pleasure to work with you.
>>
>>
>>
>> [1]
>> https://patches.dpdk.org/project/dpdk/list/?series=22735
>>
>> [2]
>> In your 'show_rss_types()', it can get additional parameter for
>> formatting, like after how many items to break to next line, this can
>> cover existing formatting options, but not sure how elegant a solution
>> it is.
So, I'm waiting for a new version from lihuisong.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] app/testpmd: compact RSS flow type output in port info
2022-05-31 16:35 ` Andrew Rybchenko
@ 2022-06-01 7:08 ` lihuisong (C)
0 siblings, 0 replies; 14+ messages in thread
From: lihuisong (C) @ 2022-06-01 7:08 UTC (permalink / raw)
To: Andrew Rybchenko, Ferruh Yigit, Xiaoyun Li, Aman Singh, Yuying Zhang
Cc: dev, Thomas Monjalon
在 2022/6/1 0:35, Andrew Rybchenko 写道:
> On 5/25/22 20:37, Ferruh Yigit wrote:
>> In port info command output, 'show port info all', supported RSS flow
>> types printed one type per line, and although this information is not
>> most important part of the command it takes big part of the command
>> output.
>>
>> Compacting the supported RSS flow type output by printing 6 (hardcoded
>> value) items per line, instead of one per line. Output becomes as
>> following:
>>
>> Supported RSS offload flow types:
>> ipv4 ipv4-frag ipv4-tcp ipv4-udp ipv4-sctp ipv4-other
>> ipv6 ipv6-frag ipv6-tcp ipv6-udp ipv6-sctp ipv6-other
>> l2-payload ipv6-ex ipv6-tcp-ex ipv6-udp-ex port vxlan
>> geneve nvgre mpls
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@xilinx.com>
>> ---
>> app/test-pmd/config.c | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 47de5b6d9458..5496ccd7f8ad 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -805,6 +805,7 @@ port_infos_display(portid_t port_id)
>> else {
>> uint64_t rss_types = dev_info.flow_type_rss_offloads;
>> uint16_t i;
>> + uint16_t len = 0;
>> printf("Supported RSS offload flow types:\n");
>> for (i = 0; rss_types != 0; i++) {
>> @@ -813,12 +814,21 @@ port_infos_display(portid_t port_id)
>> const char *p = rsstype_to_str(rss_type);
>> if (p)
>> - printf(" %s\n", p);
>> + printf(" %s", p);
>> else
>> - printf(" user defined 0x%"PRIx64"\n", rss_type);
>> + printf(" user defined 0x%"PRIx64, rss_type);
>> +
>> + len++;
>> + /* wrap on every 6 items */
>> + if (len == 6) {
>
> Variable name 'len' is misleading here since the first idea is
> like length, not a number of items.
>
> Also 6 sounds to be to much in the worst case scenario with 6
> user defined types. Each uses at least 18 chars. So, 18 * 6 = 108.
> May be printing algorithm should be a bit more sophisticated and
> really track line length.
ok. I will fix it in my set.
>
>> + printf("\n");
>> + len = 0;
>> + }
>> }
>> rss_types >>= 1;
>> }
>> + if (len)
>> + printf("\n");
>> }
>> printf("Minimum size of RX buffer: %u\n",
>> dev_info.min_rx_bufsize);
>
> .
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-06-01 7:08 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-25 17:37 [PATCH 1/3] app/testpmd: fix displaying RSS info Ferruh Yigit
2022-05-25 17:37 ` [PATCH 2/3] app/testpmd: remove duplicated flow type to string table Ferruh Yigit
2022-05-25 17:37 ` [PATCH 3/3] app/testpmd: compact RSS flow type output in port info Ferruh Yigit
2022-05-31 16:35 ` Andrew Rybchenko
2022-06-01 7:08 ` lihuisong (C)
2022-05-26 3:52 ` [PATCH 1/3] app/testpmd: fix displaying RSS info lihuisong (C)
2022-05-26 8:56 ` Ferruh Yigit
2022-05-27 2:48 ` lihuisong (C)
2022-05-27 2:30 ` lihuisong (C)
2022-05-30 10:43 ` Ferruh Yigit
2022-05-30 12:32 ` lihuisong (C)
2022-05-30 13:02 ` Ferruh Yigit
2022-05-31 2:07 ` lihuisong (C)
2022-05-31 16:35 ` Andrew Rybchenko
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).