[PATCH V1 0/2] fix RSS types display

[PATCH V1 0/2] fix RSS types display

[Huisong Li]

Fix RSS types display. 

Huisong Li (2):
  app/testpmd: fix supported RSS offload display
  app/testpmd: unify RSS types display

 app/test-pmd/config.c | 46 +++++++++++++++++--------------------------
 1 file changed, 18 insertions(+), 28 deletions(-)

-- 
2.33.0

[PATCH V1 1/2] app/testpmd: fix supported RSS offload display

[Huisong Li]

And rte_eth_dev_info.flow_type_rss_offloads is populated in terms of
RTE_ETH_RSS_* bits. If PMD sets RTE_ETH_RSS_L3_SRC_ONLY to
dev_info->flow_type_rss_offloads. testpmd will display "user defined 63"
when run 'show port info 0'. Because testpmd use flowtype_to_str()
to display the supported RSS offload of PMD. In fact, the function is
used to display flow type in FDIR commands. This patch uses the
RTE_ETH_RSS_* bits to display supported RSS offload of PMD.

Fixes: b12964f621dc ("ethdev: unification of RSS offload types")
Cc: stable@dpdk.org

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 app/test-pmd/config.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index cc8e7aa138..f58da01a3a 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -669,6 +669,20 @@ print_dev_capabilities(uint64_t capabilities)
 	}
 }
 
+static void
+show_rss_types(uint64_t rss_hf)
+{
+	uint16_t i;
+
+	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);
+	}
+	printf("\n");
+}
+
 void
 port_infos_display(portid_t port_id)
 {
@@ -773,20 +787,8 @@ port_infos_display(portid_t port_id)
 	if (!dev_info.flow_type_rss_offloads)
 		printf("No RSS offload flow type is supported.\n");
 	else {
-		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);
-		}
+		printf("Supported RSS offload:\n");
+		show_rss_types(dev_info.flow_type_rss_offloads);
 	}
 
 	printf("Minimum size of RX buffer: %u\n", dev_info.min_rx_bufsize);
-- 
2.33.0

Re: [PATCH V1 1/2] app/testpmd: fix supported RSS offload display

[David Marchand]

On Mon, Apr 25, 2022 at 11:26 AM Huisong Li <lihuisong@huawei.com> wrote:
> @@ -773,20 +787,8 @@ port_infos_display(portid_t port_id)
>         if (!dev_info.flow_type_rss_offloads)
>                 printf("No RSS offload flow type is supported.\n");
>         else {
> -               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);

This patch removes the only call to flowtype_to_str not under #if
defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE).

As reported by the CI, it breaks compilation when disabling net/i40e
and net/ixgbe drivers because it results in a static unused symbol
left behind.

FAILED: app/a172ced@@dpdk-testpmd@exe/test-pmd_config.c.o
ccache gcc -Iapp/a172ced@@dpdk-testpmd@exe -Iapp -I../app -Ilib/ethdev
-I../lib/ethdev -I. -I../ -Iconfig -I../config -Ilib/eal/include
-I../lib/eal/include -Ilib/eal/linux/include
-I../lib/eal/linux/include -Ilib/eal/x86/include
-I../lib/eal/x86/include -Ilib/eal/common -I../lib/eal/common
-Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs
-I../lib/telemetry/../metrics -Ilib/telemetry -I../lib/telemetry
-Ilib/net -I../lib/net -Ilib/mbuf -I../lib/mbuf -Ilib/mempool
-I../lib/mempool -Ilib/ring -I../lib/ring -Ilib/meter -I../lib/meter
-Ilib/cmdline -I../lib/cmdline -Idrivers/bus/pci -I../drivers/bus/pci
-I../drivers/bus/pci/linux -Ilib/pci -I../lib/pci -Ilib/bpf
-I../lib/bpf -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64
-Wall -Winvalid-pch -Wextra -Werror -O2 -g -include rte_config.h
-Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral
-Wformat-security -Wmissing-declarations -Wmissing-prototypes
-Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare
-Wstrict-prototypes -Wundef -Wwrite-strings
-Wno-missing-field-initializers -D_GNU_SOURCE -march=corei7
-DALLOW_EXPERIMENTAL_API -Wno-deprecated-declarations -MD -MQ
'app/a172ced@@dpdk-testpmd@exe/test-pmd_config.c.o' -MF
'app/a172ced@@dpdk-testpmd@exe/test-pmd_config.c.o.d' -o
'app/a172ced@@dpdk-testpmd@exe/test-pmd_config.c.o' -c
../app/test-pmd/config.c
../app/test-pmd/config.c:5506:1: error: ‘flowtype_to_str’ defined but
not used [-Werror=unused-function]
flowtype_to_str(uint16_t flow_type)
^~~~~~~~~~~~~~~
cc1: all warnings being treated as errors


Please fix by removing the unnecessary fwd declaration of this symbol,
then moving flowtype_to_str function under the #if
defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE) check.

I marked this patch as changes requested, thanks.


-- 
David Marchand

Re: [PATCH V1 1/2] app/testpmd: fix supported RSS offload display

[lihuisong (C)]

在 2022/4/29 15:27, David Marchand 写道:
> On Mon, Apr 25, 2022 at 11:26 AM Huisong Li <lihuisong@huawei.com> wrote:
>> @@ -773,20 +787,8 @@ port_infos_display(portid_t port_id)
>>          if (!dev_info.flow_type_rss_offloads)
>>                  printf("No RSS offload flow type is supported.\n");
>>          else {
>> -               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);
> This patch removes the only call to flowtype_to_str not under #if
> defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE).
>
> As reported by the CI, it breaks compilation when disabling net/i40e
> and net/ixgbe drivers because it results in a static unused symbol
> left behind.
>
> FAILED: app/a172ced@@dpdk-testpmd@exe/test-pmd_config.c.o
> ccache gcc -Iapp/a172ced@@dpdk-testpmd@exe -Iapp -I../app -Ilib/ethdev
> -I../lib/ethdev -I. -I../ -Iconfig -I../config -Ilib/eal/include
> -I../lib/eal/include -Ilib/eal/linux/include
> -I../lib/eal/linux/include -Ilib/eal/x86/include
> -I../lib/eal/x86/include -Ilib/eal/common -I../lib/eal/common
> -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs
> -I../lib/telemetry/../metrics -Ilib/telemetry -I../lib/telemetry
> -Ilib/net -I../lib/net -Ilib/mbuf -I../lib/mbuf -Ilib/mempool
> -I../lib/mempool -Ilib/ring -I../lib/ring -Ilib/meter -I../lib/meter
> -Ilib/cmdline -I../lib/cmdline -Idrivers/bus/pci -I../drivers/bus/pci
> -I../drivers/bus/pci/linux -Ilib/pci -I../lib/pci -Ilib/bpf
> -I../lib/bpf -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64
> -Wall -Winvalid-pch -Wextra -Werror -O2 -g -include rte_config.h
> -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral
> -Wformat-security -Wmissing-declarations -Wmissing-prototypes
> -Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare
> -Wstrict-prototypes -Wundef -Wwrite-strings
> -Wno-missing-field-initializers -D_GNU_SOURCE -march=corei7
> -DALLOW_EXPERIMENTAL_API -Wno-deprecated-declarations -MD -MQ
> 'app/a172ced@@dpdk-testpmd@exe/test-pmd_config.c.o' -MF
> 'app/a172ced@@dpdk-testpmd@exe/test-pmd_config.c.o.d' -o
> 'app/a172ced@@dpdk-testpmd@exe/test-pmd_config.c.o' -c
> ../app/test-pmd/config.c
> ../app/test-pmd/config.c:5506:1: error: ‘flowtype_to_str’ defined but
> not used [-Werror=unused-function]
> flowtype_to_str(uint16_t flow_type)
> ^~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
>
> Please fix by removing the unnecessary fwd declaration of this symbol,
> then moving flowtype_to_str function under the #if
> defined(RTE_NET_I40E) || defined(RTE_NET_IXGBE) check.
>
> I marked this patch as changes requested, thanks.
This is a little hidden. I'll fix it, thanks.
>
>

[PATCH V1 2/2] app/testpmd: unify RSS types display

[Huisong Li]

Unify RSS types display.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 app/test-pmd/config.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f58da01a3a..06f24d36ac 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1527,13 +1527,7 @@ rss_config_display(struct rte_flow_action_rss *rss_conf)
 		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);
-	}
+	show_rss_types(rss_conf->types);
 }
 
 static struct port_indirect_action *
@@ -3733,13 +3727,7 @@ 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);
-	}
-	printf("\n");
+	show_rss_types(rss_hf);
 	if (!show_rss_key)
 		return;
 	printf("RSS key:\n");
-- 
2.33.0