diff mbox

[dpdk-dev,v3] app/testpmd: fix testpmd failure due to RSS offload check

Message ID 20180425133816.198161-1-qi.z.zhang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Qi Zhang April 25, 2018, 1:38 p.m. UTC 
  After add RSS hash offload check, default rss_hf  will fail on
devices that not support all bits, the patch take rss_hf as
a suggest value and only set bits that device supported base on
rte_eth_dev_get_info, also rss_hf will only be updated when new
rss offload is successfully updated on all ports by
"port config all rss [!default]" command.

Fixes: 586ac442be96 ("ethdev: add supported hash function check")
Fixes: 8c1f4aff92a6 ("app/testpmd: new parameter for port config all RSS command")
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 app/test-pmd/cmdline.c | 15 +++++++++++----
 app/test-pmd/testpmd.c |  5 ++++-
 2 files changed, 15 insertions(+), 5 deletions(-)

Comments

Ferruh Yigit April 25, 2018, 2:02 p.m. UTC | #1 
On 4/25/2018 2:38 PM, Qi Zhang wrote:
> After add RSS hash offload check, default rss_hf  will fail on
> devices that not support all bits, the patch take rss_hf as
> a suggest value and only set bits that device supported base on
> rte_eth_dev_get_info, also rss_hf will only be updated when new
> rss offload is successfully updated on all ports by
> "port config all rss [!default]" command.
> 
> Fixes: 586ac442be96 ("ethdev: add supported hash function check")
> Fixes: 8c1f4aff92a6 ("app/testpmd: new parameter for port config all RSS command")
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Adrien Mazarguil April 25, 2018, 4:27 p.m. UTC | #2 
On Wed, Apr 25, 2018 at 09:38:16PM +0800, Qi Zhang wrote:
> After add RSS hash offload check, default rss_hf  will fail on
> devices that not support all bits, the patch take rss_hf as
> a suggest value and only set bits that device supported base on
> rte_eth_dev_get_info, also rss_hf will only be updated when new
> rss offload is successfully updated on all ports by
> "port config all rss [!default]" command.
> 
> Fixes: 586ac442be96 ("ethdev: add supported hash function check")
> Fixes: 8c1f4aff92a6 ("app/testpmd: new parameter for port config all RSS command")
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

Although this approach prevents updating rss_hf if at least one port doesn't
support RSS configuration (i.e. it can't be used to update defaults for
subsequent flow rules on mlx4), I confirm it does fix the startup issue,
therefore:

Tested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Ferruh Yigit April 25, 2018, 4:34 p.m. UTC | #3 
On 4/25/2018 5:27 PM, Adrien Mazarguil wrote:
> On Wed, Apr 25, 2018 at 09:38:16PM +0800, Qi Zhang wrote:
>> After add RSS hash offload check, default rss_hf  will fail on
>> devices that not support all bits, the patch take rss_hf as
>> a suggest value and only set bits that device supported base on
>> rte_eth_dev_get_info, also rss_hf will only be updated when new
>> rss offload is successfully updated on all ports by
>> "port config all rss [!default]" command.
>>
>> Fixes: 586ac442be96 ("ethdev: add supported hash function check")
>> Fixes: 8c1f4aff92a6 ("app/testpmd: new parameter for port config all RSS command")
>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> 
> Although this approach prevents updating rss_hf if at least one port doesn't
> support RSS configuration (i.e. it can't be used to update defaults for
> subsequent flow rules on mlx4), 

Yes it does, and perhaps we need a value per port, instead of single value for all.

> I confirm it does fix the startup issue,
> therefore:
> 
> Tested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
>
Ferruh Yigit April 25, 2018, 11:46 p.m. UTC | #4 
On 4/25/2018 5:27 PM, Adrien Mazarguil wrote:
> On Wed, Apr 25, 2018 at 09:38:16PM +0800, Qi Zhang wrote:
>> After add RSS hash offload check, default rss_hf  will fail on
>> devices that not support all bits, the patch take rss_hf as
>> a suggest value and only set bits that device supported base on
>> rte_eth_dev_get_info, also rss_hf will only be updated when new
>> rss offload is successfully updated on all ports by
>> "port config all rss [!default]" command.
>>
>> Fixes: 586ac442be96 ("ethdev: add supported hash function check")
>> Fixes: 8c1f4aff92a6 ("app/testpmd: new parameter for port config all RSS command")
>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>

> Tested-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>

Applied to dpdk-next-net/master, thanks.
diff mbox

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index bdc2122a0..2d05fc91e 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2007,6 +2007,8 @@  cmd_config_rss_parsed(void *parsed_result,
 	struct cmd_config_rss *res = parsed_result;
 	struct rte_eth_rss_conf rss_conf = { .rss_key_len = 0, };
 	struct rte_eth_dev_info dev_info = { .flow_type_rss_offloads = 0, };
+	int use_default = 0;
+	int all_updated = 1;
 	int diag;
 	uint16_t i;
 
@@ -2032,8 +2034,10 @@  cmd_config_rss_parsed(void *parsed_result,
 		rss_conf.rss_hf = ETH_RSS_GENEVE;
 	else if (!strcmp(res->value, "nvgre"))
 		rss_conf.rss_hf = ETH_RSS_NVGRE;
-	else if (!strcmp(res->value, "none") || !strcmp(res->value, "default"))
+	else if (!strcmp(res->value, "none"))
 		rss_conf.rss_hf = 0;
+	else if (!strcmp(res->value, "default"))
+		use_default = 1;
 	else if (isdigit(res->value[0]) && atoi(res->value) > 0 &&
 						atoi(res->value) < 64)
 		rss_conf.rss_hf = 1ULL << atoi(res->value);
@@ -2043,18 +2047,21 @@  cmd_config_rss_parsed(void *parsed_result,
 	}
 	rss_conf.rss_key = NULL;
 	/* Update global configuration for RSS types. */
-	rss_hf = rss_conf.rss_hf;
 	RTE_ETH_FOREACH_DEV(i) {
-		if (!strcmp(res->value, "default")) {
+		if (use_default) {
 			rte_eth_dev_info_get(i, &dev_info);
 			rss_conf.rss_hf = dev_info.flow_type_rss_offloads;
 		}
 		diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
-		if (diag < 0)
+		if (diag < 0) {
+			all_updated = 0;
 			printf("Configuration of RSS hash at ethernet port %d "
 				"failed with error (%d): %s.\n",
 				i, -diag, strerror(-diag));
+		}
 	}
+	if (all_updated && !use_default)
+		rss_hf = rss_conf.rss_hf;
 }
 
 cmdline_parse_token_string_t cmd_config_rss_port =
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e757d8122..db23f23e5 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2290,13 +2290,16 @@  init_port_config(void)
 {
 	portid_t pid;
 	struct rte_port *port;
+	struct rte_eth_dev_info dev_info;
 
 	RTE_ETH_FOREACH_DEV(pid) {
 		port = &ports[pid];
 		port->dev_conf.fdir_conf = fdir_conf;
 		if (nb_rxq > 1) {
+			rte_eth_dev_info_get(pid, &dev_info);
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
-			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = rss_hf;
+			port->dev_conf.rx_adv_conf.rss_conf.rss_hf =
+				rss_hf & dev_info.flow_type_rss_offloads;
 		} else {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = 0;