DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] app/testpmd: fix testpmd failure due to RSS offload check
@ 2018-04-25  1:49 Qi Zhang
  2018-04-25  7:43 ` [dpdk-dev] [PATCH v2] " Qi Zhang
  2018-04-25 13:38 ` [dpdk-dev] [PATCH v3] " Qi Zhang
  0 siblings, 2 replies; 9+ messages in thread
From: Qi Zhang @ 2018-04-25  1:49 UTC (permalink / raw)
  To: ferruh.yigit, adrien.mazarguil; +Cc: thomas, dev, Qi Zhang

After add RSS hash offload check, default rss_hf will fail on
devices that not support all bits, the patch introduce RSS
negotiate flag, when it is set, RSS configuration will negotiate
with device capability. By default negotiate is turn on, so it
will not break exist PMD. It can be disabled by "--disable-rss-neg"
in start parameters or be enable/disable by testpmd command
"port config all rss neg|noneg".

Fixes: 527624c663f8 ("ethdev: add supported hash function check")
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---
 app/test-pmd/cmdline.c                      | 20 +++++++++++++++-----
 app/test-pmd/parameters.c                   |  5 +++++
 app/test-pmd/testpmd.c                      | 12 +++++++++++-
 app/test-pmd/testpmd.h                      |  1 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 +++-
 5 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 333852194..d6b30e20a 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -822,7 +822,7 @@ static void cmd_help_long_parsed(void *parsed_result,
 			" for ports.\n\n"
 
 			"port config all rss (all|default|ip|tcp|udp|sctp|"
-			"ether|port|vxlan|geneve|nvgre|none|<flowtype_id>)\n"
+			"ether|port|vxlan|geneve|nvgre|none|neg|noneg|<flowtype_id>)\n"
 			"    Set the RSS mode.\n\n"
 
 			"port config port-id rss reta (hash,queue)[,(hash,queue)]\n"
@@ -2029,7 +2029,13 @@ cmd_config_rss_parsed(void *parsed_result,
 	else if (isdigit(res->value[0]) && atoi(res->value) > 0 &&
 						atoi(res->value) < 64)
 		rss_conf.rss_hf = 1ULL << atoi(res->value);
-	else {
+	else if (!strcmp(res->value, "neg")) {
+		rss_hf_noneg = 0;
+		return;
+	} else if (!strcmp(res->value, "noneg")) {
+		rss_hf_noneg = 1;
+		return;
+	} else {
 		printf("Unknown parameter\n");
 		return;
 	}
@@ -2037,10 +2043,14 @@ cmd_config_rss_parsed(void *parsed_result,
 	/* Update global configuration for RSS types. */
 	rss_hf = rss_conf.rss_hf;
 	RTE_ETH_FOREACH_DEV(i) {
-		if (!strcmp(res->value, "default")) {
-			rte_eth_dev_info_get(i, &dev_info);
+		rte_eth_dev_info_get(i, &dev_info);
+		if (!strcmp(res->value, "default"))
 			rss_conf.rss_hf = dev_info.flow_type_rss_offloads;
-		}
+		else
+			rss_conf.rss_hf &= (rss_hf_noneg ?
+				rss_conf.rss_hf :
+				dev_info.flow_type_rss_offloads);
+
 		diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
 		if (diag < 0)
 			printf("Configuration of RSS hash at ethernet port %d "
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 394fa6d92..4758ec1d9 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -139,6 +139,7 @@ usage(char* progname)
 	printf("  --enable-hw-vlan-extend: enable hardware vlan extend.\n");
 	printf("  --enable-drop-en: enable per queue packet drop.\n");
 	printf("  --disable-rss: disable rss.\n");
+	printf("  --disable-rss-neg: disable rss negotiate with device capa.\n");
 	printf("  --port-topology=N: set port topology (N: paired (default) or "
 	       "chained).\n");
 	printf("  --forward-mode=N: set forwarding mode (N: %s).\n",
@@ -594,6 +595,7 @@ launch_args_parse(int argc, char** argv)
 		{ "enable-hw-vlan-extend",      0, 0, 0 },
 		{ "enable-drop-en",            0, 0, 0 },
 		{ "disable-rss",                0, 0, 0 },
+		{ "disable-rss-neg",            0, 0, 0 },
 		{ "port-topology",              1, 0, 0 },
 		{ "forward-mode",               1, 0, 0 },
 		{ "rss-ip",			0, 0, 0 },
@@ -909,6 +911,9 @@ launch_args_parse(int argc, char** argv)
 
 			if (!strcmp(lgopts[opt_idx].name, "disable-rss"))
 				rss_hf = 0;
+			if (!strcmp(lgopts[opt_idx].name, "disable-rss-neg")) {
+				rss_hf_noneg = 1;
+			}
 			if (!strcmp(lgopts[opt_idx].name, "port-topology")) {
 				if (!strcmp(optarg, "paired"))
 					port_topology = PORT_TOPOLOGY_PAIRED;
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index d6da41927..68214677c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -257,6 +257,12 @@ int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET;
 uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */
 
 /*
+ * Negotiate with device capability when config
+ * Receive Side Scaling (RSS).
+ */
+uint8_t rss_hf_noneg = 0; /* Negotiate by default */
+
+/*
  * Port topology configuration
  */
 uint16_t port_topology = PORT_TOPOLOGY_PAIRED; /* Ports are paired by default */
@@ -2265,13 +2271,17 @@ 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_noneg ? 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;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 070919822..c37c4ee70 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -385,6 +385,7 @@ extern struct rte_eth_rxmode rx_mode;
 extern struct rte_eth_txmode tx_mode;
 
 extern uint64_t rss_hf;
+extern uint8_t rss_hf_noneg;
 
 extern queueid_t nb_rxq;
 extern queueid_t nb_txq;
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 593b13a3d..8db9cf5c1 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1751,13 +1751,15 @@ port config - RSS
 
 Set the RSS (Receive Side Scaling) mode on or off::
 
-   testpmd> port config all rss (all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none)
+   testpmd> port config all rss (all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|neg|noneg)
 
 RSS is on by default.
 
 The ``all`` option is equivalent to ip|tcp|udp|sctp|ether.
 The ``default`` option enables all supported RSS types reported by device info.
 The ``none`` option is equivalent to the ``--disable-rss`` command-line option.
+The ``neg`` option enable the negotiation with device capability when configure RSS.
+The ``noneg`` option disable the negotiation and is equivalent to ``--disable-rss-neg`` command-line option.
 
 port config - RSS Reta
 ~~~~~~~~~~~~~~~~~~~~~~
-- 
2.13.6

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd failure due to RSS offload check
  2018-04-25  1:49 [dpdk-dev] [PATCH] app/testpmd: fix testpmd failure due to RSS offload check Qi Zhang
@ 2018-04-25  7:43 ` Qi Zhang
  2018-04-25 12:03   ` Ferruh Yigit
  2018-04-25 13:38 ` [dpdk-dev] [PATCH v3] " Qi Zhang
  1 sibling, 1 reply; 9+ messages in thread
From: Qi Zhang @ 2018-04-25  7:43 UTC (permalink / raw)
  To: ferruh.yigit, adrien.mazarguil; +Cc: thomas, dev, Qi Zhang

After add RSS hash offload check, default rss_hf will fail on
devices that not support all bits, the patch introduce RSS
negotiate flag, when it is set, RSS configuration will negotiate
with device capability. By default negotiate is turn on, so it
will not break exist PMD. It can be disabled by "--disable-rss-neg"
in start parameters or be enable/disable by testpmd command
"port config all rss neg|noneg".

Fixes: 527624c663f8 ("ethdev: add supported hash function check")
Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
---

v2:
- fix command help string.

 app/test-pmd/cmdline.c                      | 22 ++++++++++++++++------
 app/test-pmd/parameters.c                   |  5 +++++
 app/test-pmd/testpmd.c                      | 12 +++++++++++-
 app/test-pmd/testpmd.h                      |  1 +
 doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 +++-
 5 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 333852194..9390be741 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -822,7 +822,7 @@ static void cmd_help_long_parsed(void *parsed_result,
 			" for ports.\n\n"
 
 			"port config all rss (all|default|ip|tcp|udp|sctp|"
-			"ether|port|vxlan|geneve|nvgre|none|<flowtype_id>)\n"
+			"ether|port|vxlan|geneve|nvgre|none|neg|noneg|<flowtype_id>)\n"
 			"    Set the RSS mode.\n\n"
 
 			"port config port-id rss reta (hash,queue)[,(hash,queue)]\n"
@@ -2029,7 +2029,13 @@ cmd_config_rss_parsed(void *parsed_result,
 	else if (isdigit(res->value[0]) && atoi(res->value) > 0 &&
 						atoi(res->value) < 64)
 		rss_conf.rss_hf = 1ULL << atoi(res->value);
-	else {
+	else if (!strcmp(res->value, "neg")) {
+		rss_hf_noneg = 0;
+		return;
+	} else if (!strcmp(res->value, "noneg")) {
+		rss_hf_noneg = 1;
+		return;
+	} else {
 		printf("Unknown parameter\n");
 		return;
 	}
@@ -2037,10 +2043,14 @@ cmd_config_rss_parsed(void *parsed_result,
 	/* Update global configuration for RSS types. */
 	rss_hf = rss_conf.rss_hf;
 	RTE_ETH_FOREACH_DEV(i) {
-		if (!strcmp(res->value, "default")) {
-			rte_eth_dev_info_get(i, &dev_info);
+		rte_eth_dev_info_get(i, &dev_info);
+		if (!strcmp(res->value, "default"))
 			rss_conf.rss_hf = dev_info.flow_type_rss_offloads;
-		}
+		else
+			rss_conf.rss_hf &= (rss_hf_noneg ?
+				rss_conf.rss_hf :
+				dev_info.flow_type_rss_offloads);
+
 		diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
 		if (diag < 0)
 			printf("Configuration of RSS hash at ethernet port %d "
@@ -2064,7 +2074,7 @@ cmdline_parse_inst_t cmd_config_rss = {
 	.f = cmd_config_rss_parsed,
 	.data = NULL,
 	.help_str = "port config all rss "
-		"all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|<flowtype_id>",
+		"all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|neg|noneg|<flowtype_id>",
 	.tokens = {
 		(void *)&cmd_config_rss_port,
 		(void *)&cmd_config_rss_keyword,
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 394fa6d92..4758ec1d9 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -139,6 +139,7 @@ usage(char* progname)
 	printf("  --enable-hw-vlan-extend: enable hardware vlan extend.\n");
 	printf("  --enable-drop-en: enable per queue packet drop.\n");
 	printf("  --disable-rss: disable rss.\n");
+	printf("  --disable-rss-neg: disable rss negotiate with device capa.\n");
 	printf("  --port-topology=N: set port topology (N: paired (default) or "
 	       "chained).\n");
 	printf("  --forward-mode=N: set forwarding mode (N: %s).\n",
@@ -594,6 +595,7 @@ launch_args_parse(int argc, char** argv)
 		{ "enable-hw-vlan-extend",      0, 0, 0 },
 		{ "enable-drop-en",            0, 0, 0 },
 		{ "disable-rss",                0, 0, 0 },
+		{ "disable-rss-neg",            0, 0, 0 },
 		{ "port-topology",              1, 0, 0 },
 		{ "forward-mode",               1, 0, 0 },
 		{ "rss-ip",			0, 0, 0 },
@@ -909,6 +911,9 @@ launch_args_parse(int argc, char** argv)
 
 			if (!strcmp(lgopts[opt_idx].name, "disable-rss"))
 				rss_hf = 0;
+			if (!strcmp(lgopts[opt_idx].name, "disable-rss-neg")) {
+				rss_hf_noneg = 1;
+			}
 			if (!strcmp(lgopts[opt_idx].name, "port-topology")) {
 				if (!strcmp(optarg, "paired"))
 					port_topology = PORT_TOPOLOGY_PAIRED;
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index d6da41927..68214677c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -257,6 +257,12 @@ int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET;
 uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */
 
 /*
+ * Negotiate with device capability when config
+ * Receive Side Scaling (RSS).
+ */
+uint8_t rss_hf_noneg = 0; /* Negotiate by default */
+
+/*
  * Port topology configuration
  */
 uint16_t port_topology = PORT_TOPOLOGY_PAIRED; /* Ports are paired by default */
@@ -2265,13 +2271,17 @@ 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_noneg ? 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;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 070919822..c37c4ee70 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -385,6 +385,7 @@ extern struct rte_eth_rxmode rx_mode;
 extern struct rte_eth_txmode tx_mode;
 
 extern uint64_t rss_hf;
+extern uint8_t rss_hf_noneg;
 
 extern queueid_t nb_rxq;
 extern queueid_t nb_txq;
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 593b13a3d..8db9cf5c1 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -1751,13 +1751,15 @@ port config - RSS
 
 Set the RSS (Receive Side Scaling) mode on or off::
 
-   testpmd> port config all rss (all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none)
+   testpmd> port config all rss (all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|neg|noneg)
 
 RSS is on by default.
 
 The ``all`` option is equivalent to ip|tcp|udp|sctp|ether.
 The ``default`` option enables all supported RSS types reported by device info.
 The ``none`` option is equivalent to the ``--disable-rss`` command-line option.
+The ``neg`` option enable the negotiation with device capability when configure RSS.
+The ``noneg`` option disable the negotiation and is equivalent to ``--disable-rss-neg`` command-line option.
 
 port config - RSS Reta
 ~~~~~~~~~~~~~~~~~~~~~~
-- 
2.13.6

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd failure due to RSS offload check
  2018-04-25  7:43 ` [dpdk-dev] [PATCH v2] " Qi Zhang
@ 2018-04-25 12:03   ` Ferruh Yigit
  2018-04-25 13:34     ` Zhang, Qi Z
  0 siblings, 1 reply; 9+ messages in thread
From: Ferruh Yigit @ 2018-04-25 12:03 UTC (permalink / raw)
  To: Qi Zhang, adrien.mazarguil; +Cc: thomas, dev

On 4/25/2018 8:43 AM, Qi Zhang wrote:
> After add RSS hash offload check, default rss_hf will fail on
> devices that not support all bits, the patch introduce RSS
> negotiate flag, when it is set, RSS configuration will negotiate
> with device capability. By default negotiate is turn on, so it
> will not break exist PMD. It can be disabled by "--disable-rss-neg"
> in start parameters or be enable/disable by testpmd command
> "port config all rss neg|noneg".

Hi Qi,

Thanks for the patch, but I feel new neg|noneg is extra complexity, I am
wondering if we can fix this without this complexity.

"rss_hf" is rss hash function configuration for *all* ports. Default value is
ETH_RSS_IP.

Up until recently it was more like default value, Adrien's commit start updating
it in "port config all rss ..." and it become more like config value now.

And if it is config value, the question is what to do if PMD doesn't support
requested hash function:
- Fail
- Convert request what PMD supports.

init_port_config() called from many places and it sets rss_conf.rss_hf via
"rss_hf" and currently this is the problem some pmds hit.

Previously, before Adrien's update, when "port config all rss ..." failed it was
printing log and keep continue, not recording the failed value as config value.


What do you think:
- In init_port_config() use "rss_hf & dev_info.flow_type_rss_offloads" as you
suggested.
- in cmd_config_rss_parsed() set "rss_hf" only if all ethdev successfully called
 rte_eth_dev_rss_hash_update(), this will mean the config has supported by all
ports so it will be ok to use is as "rss_hf & dev_info.flow_type_rss_offloads"
in init_port_config()
- I think there is a defect in "port config all rss default", perhaps because of
me during merged, it shouldn't update "rss_hf"

After this changes answer to above question,
by default config behavior is "Convert request what PMD supports" so testpmd
don't fail, and when configuration updated only values supported by ports accepted.



> 
> Fixes: 527624c663f8 ("ethdev: add supported hash function check")
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> ---
> 
> v2:
> - fix command help string.
> 
>  app/test-pmd/cmdline.c                      | 22 ++++++++++++++++------
>  app/test-pmd/parameters.c                   |  5 +++++
>  app/test-pmd/testpmd.c                      | 12 +++++++++++-
>  app/test-pmd/testpmd.h                      |  1 +
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 +++-
>  5 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 333852194..9390be741 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -822,7 +822,7 @@ static void cmd_help_long_parsed(void *parsed_result,
>  			" for ports.\n\n"
>  
>  			"port config all rss (all|default|ip|tcp|udp|sctp|"
> -			"ether|port|vxlan|geneve|nvgre|none|<flowtype_id>)\n"
> +			"ether|port|vxlan|geneve|nvgre|none|neg|noneg|<flowtype_id>)\n"
>  			"    Set the RSS mode.\n\n"
>  
>  			"port config port-id rss reta (hash,queue)[,(hash,queue)]\n"
> @@ -2029,7 +2029,13 @@ cmd_config_rss_parsed(void *parsed_result,
>  	else if (isdigit(res->value[0]) && atoi(res->value) > 0 &&
>  						atoi(res->value) < 64)
>  		rss_conf.rss_hf = 1ULL << atoi(res->value);
> -	else {
> +	else if (!strcmp(res->value, "neg")) {
> +		rss_hf_noneg = 0;
> +		return;
> +	} else if (!strcmp(res->value, "noneg")) {
> +		rss_hf_noneg = 1;
> +		return;
> +	} else {
>  		printf("Unknown parameter\n");
>  		return;
>  	}
> @@ -2037,10 +2043,14 @@ cmd_config_rss_parsed(void *parsed_result,
>  	/* Update global configuration for RSS types. */
>  	rss_hf = rss_conf.rss_hf;
>  	RTE_ETH_FOREACH_DEV(i) {
> -		if (!strcmp(res->value, "default")) {
> -			rte_eth_dev_info_get(i, &dev_info);
> +		rte_eth_dev_info_get(i, &dev_info);
> +		if (!strcmp(res->value, "default"))
>  			rss_conf.rss_hf = dev_info.flow_type_rss_offloads;
> -		}
> +		else
> +			rss_conf.rss_hf &= (rss_hf_noneg ?
> +				rss_conf.rss_hf :
> +				dev_info.flow_type_rss_offloads);
> +
>  		diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
>  		if (diag < 0)
>  			printf("Configuration of RSS hash at ethernet port %d "
> @@ -2064,7 +2074,7 @@ cmdline_parse_inst_t cmd_config_rss = {
>  	.f = cmd_config_rss_parsed,
>  	.data = NULL,
>  	.help_str = "port config all rss "
> -		"all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|<flowtype_id>",
> +		"all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|neg|noneg|<flowtype_id>",
>  	.tokens = {
>  		(void *)&cmd_config_rss_port,
>  		(void *)&cmd_config_rss_keyword,
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index 394fa6d92..4758ec1d9 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -139,6 +139,7 @@ usage(char* progname)
>  	printf("  --enable-hw-vlan-extend: enable hardware vlan extend.\n");
>  	printf("  --enable-drop-en: enable per queue packet drop.\n");
>  	printf("  --disable-rss: disable rss.\n");
> +	printf("  --disable-rss-neg: disable rss negotiate with device capa.\n");
>  	printf("  --port-topology=N: set port topology (N: paired (default) or "
>  	       "chained).\n");
>  	printf("  --forward-mode=N: set forwarding mode (N: %s).\n",
> @@ -594,6 +595,7 @@ launch_args_parse(int argc, char** argv)
>  		{ "enable-hw-vlan-extend",      0, 0, 0 },
>  		{ "enable-drop-en",            0, 0, 0 },
>  		{ "disable-rss",                0, 0, 0 },
> +		{ "disable-rss-neg",            0, 0, 0 },
>  		{ "port-topology",              1, 0, 0 },
>  		{ "forward-mode",               1, 0, 0 },
>  		{ "rss-ip",			0, 0, 0 },
> @@ -909,6 +911,9 @@ launch_args_parse(int argc, char** argv)
>  
>  			if (!strcmp(lgopts[opt_idx].name, "disable-rss"))
>  				rss_hf = 0;
> +			if (!strcmp(lgopts[opt_idx].name, "disable-rss-neg")) {
> +				rss_hf_noneg = 1;
> +			}
>  			if (!strcmp(lgopts[opt_idx].name, "port-topology")) {
>  				if (!strcmp(optarg, "paired"))
>  					port_topology = PORT_TOPOLOGY_PAIRED;
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index d6da41927..68214677c 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -257,6 +257,12 @@ int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET;
>  uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */
>  
>  /*
> + * Negotiate with device capability when config
> + * Receive Side Scaling (RSS).
> + */
> +uint8_t rss_hf_noneg = 0; /* Negotiate by default */
> +
> +/*
>   * Port topology configuration
>   */
>  uint16_t port_topology = PORT_TOPOLOGY_PAIRED; /* Ports are paired by default */
> @@ -2265,13 +2271,17 @@ 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_noneg ? 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;
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 070919822..c37c4ee70 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -385,6 +385,7 @@ extern struct rte_eth_rxmode rx_mode;
>  extern struct rte_eth_txmode tx_mode;
>  
>  extern uint64_t rss_hf;
> +extern uint8_t rss_hf_noneg;
>  
>  extern queueid_t nb_rxq;
>  extern queueid_t nb_txq;
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 593b13a3d..8db9cf5c1 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -1751,13 +1751,15 @@ port config - RSS
>  
>  Set the RSS (Receive Side Scaling) mode on or off::
>  
> -   testpmd> port config all rss (all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none)
> +   testpmd> port config all rss (all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|neg|noneg)
>  
>  RSS is on by default.
>  
>  The ``all`` option is equivalent to ip|tcp|udp|sctp|ether.
>  The ``default`` option enables all supported RSS types reported by device info.
>  The ``none`` option is equivalent to the ``--disable-rss`` command-line option.
> +The ``neg`` option enable the negotiation with device capability when configure RSS.
> +The ``noneg`` option disable the negotiation and is equivalent to ``--disable-rss-neg`` command-line option.
>  
>  port config - RSS Reta
>  ~~~~~~~~~~~~~~~~~~~~~~
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v2] app/testpmd: fix testpmd failure due to RSS offload check
  2018-04-25 12:03   ` Ferruh Yigit
@ 2018-04-25 13:34     ` Zhang, Qi Z
  0 siblings, 0 replies; 9+ messages in thread
From: Zhang, Qi Z @ 2018-04-25 13:34 UTC (permalink / raw)
  To: Yigit, Ferruh, adrien.mazarguil; +Cc: thomas, dev



> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Wednesday, April 25, 2018 8:04 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; adrien.mazarguil@6wind.com
> Cc: thomas@monjalon.net; dev@dpdk.org
> Subject: Re: [PATCH v2] app/testpmd: fix testpmd failure due to RSS offload
> check
> 
> On 4/25/2018 8:43 AM, Qi Zhang wrote:
> > After add RSS hash offload check, default rss_hf will fail on devices
> > that not support all bits, the patch introduce RSS negotiate flag,
> > when it is set, RSS configuration will negotiate with device
> > capability. By default negotiate is turn on, so it will not break
> > exist PMD. It can be disabled by "--disable-rss-neg"
> > in start parameters or be enable/disable by testpmd command "port
> > config all rss neg|noneg".
> 
> Hi Qi,
> 
> Thanks for the patch, but I feel new neg|noneg is extra complexity, I am
> wondering if we can fix this without this complexity.
> 
> "rss_hf" is rss hash function configuration for *all* ports. Default value is
> ETH_RSS_IP.
> 
> Up until recently it was more like default value, Adrien's commit start
> updating it in "port config all rss ..." and it become more like config value
> now.
> 
> And if it is config value, the question is what to do if PMD doesn't support
> requested hash function:
> - Fail
> - Convert request what PMD supports.
> 
> init_port_config() called from many places and it sets rss_conf.rss_hf via
> "rss_hf" and currently this is the problem some pmds hit.
> 
> Previously, before Adrien's update, when "port config all rss ..." failed it was
> printing log and keep continue, not recording the failed value as config value.
> 
> 
> What do you think:
> - In init_port_config() use "rss_hf & dev_info.flow_type_rss_offloads" as you
> suggested.
> - in cmd_config_rss_parsed() set "rss_hf" only if all ethdev successfully called
> rte_eth_dev_rss_hash_update(), this will mean the config has supported by
> all ports so it will be ok to use is as "rss_hf &
> dev_info.flow_type_rss_offloads"
> in init_port_config()
> - I think there is a defect in "port config all rss default", perhaps because of
> me during merged, it shouldn't update "rss_hf"
> 
> After this changes answer to above question, by default config behavior is
> "Convert request what PMD supports" so testpmd don't fail, and when
> configuration updated only values supported by ports accepted.

Ok, thanks for share the background, I'm ok with your suggestion, please check v3.

Regards
Qi
> 
> 
> 
> >
> > Fixes: 527624c663f8 ("ethdev: add supported hash function check")
> > Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> > ---
> >
> > v2:
> > - fix command help string.
> >
> >  app/test-pmd/cmdline.c                      | 22
> ++++++++++++++++------
> >  app/test-pmd/parameters.c                   |  5 +++++
> >  app/test-pmd/testpmd.c                      | 12 +++++++++++-
> >  app/test-pmd/testpmd.h                      |  1 +
> >  doc/guides/testpmd_app_ug/testpmd_funcs.rst |  4 +++-
> >  5 files changed, 36 insertions(+), 8 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 333852194..9390be741 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -822,7 +822,7 @@ static void cmd_help_long_parsed(void
> *parsed_result,
> >  			" for ports.\n\n"
> >
> >  			"port config all rss (all|default|ip|tcp|udp|sctp|"
> > -			"ether|port|vxlan|geneve|nvgre|none|<flowtype_id>)\n"
> > +
> 	"ether|port|vxlan|geneve|nvgre|none|neg|noneg|<flowtype_id>)\n"
> >  			"    Set the RSS mode.\n\n"
> >
> >  			"port config port-id rss reta (hash,queue)[,(hash,queue)]\n"
> > @@ -2029,7 +2029,13 @@ cmd_config_rss_parsed(void *parsed_result,
> >  	else if (isdigit(res->value[0]) && atoi(res->value) > 0 &&
> >  						atoi(res->value) < 64)
> >  		rss_conf.rss_hf = 1ULL << atoi(res->value);
> > -	else {
> > +	else if (!strcmp(res->value, "neg")) {
> > +		rss_hf_noneg = 0;
> > +		return;
> > +	} else if (!strcmp(res->value, "noneg")) {
> > +		rss_hf_noneg = 1;
> > +		return;
> > +	} else {
> >  		printf("Unknown parameter\n");
> >  		return;
> >  	}
> > @@ -2037,10 +2043,14 @@ cmd_config_rss_parsed(void *parsed_result,
> >  	/* Update global configuration for RSS types. */
> >  	rss_hf = rss_conf.rss_hf;
> >  	RTE_ETH_FOREACH_DEV(i) {
> > -		if (!strcmp(res->value, "default")) {
> > -			rte_eth_dev_info_get(i, &dev_info);
> > +		rte_eth_dev_info_get(i, &dev_info);
> > +		if (!strcmp(res->value, "default"))
> >  			rss_conf.rss_hf = dev_info.flow_type_rss_offloads;
> > -		}
> > +		else
> > +			rss_conf.rss_hf &= (rss_hf_noneg ?
> > +				rss_conf.rss_hf :
> > +				dev_info.flow_type_rss_offloads);
> > +
> >  		diag = rte_eth_dev_rss_hash_update(i, &rss_conf);
> >  		if (diag < 0)
> >  			printf("Configuration of RSS hash at ethernet port %d "
> > @@ -2064,7 +2074,7 @@ cmdline_parse_inst_t cmd_config_rss = {
> >  	.f = cmd_config_rss_parsed,
> >  	.data = NULL,
> >  	.help_str = "port config all rss "
> > -
> 	"all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|<fl
> owtype_id>",
> > +
> >
> +"all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|neg|n
> > +oneg|<flowtype_id>",
> >  	.tokens = {
> >  		(void *)&cmd_config_rss_port,
> >  		(void *)&cmd_config_rss_keyword,
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> > index 394fa6d92..4758ec1d9 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -139,6 +139,7 @@ usage(char* progname)
> >  	printf("  --enable-hw-vlan-extend: enable hardware vlan extend.\n");
> >  	printf("  --enable-drop-en: enable per queue packet drop.\n");
> >  	printf("  --disable-rss: disable rss.\n");
> > +	printf("  --disable-rss-neg: disable rss negotiate with device
> > +capa.\n");
> >  	printf("  --port-topology=N: set port topology (N: paired (default) or "
> >  	       "chained).\n");
> >  	printf("  --forward-mode=N: set forwarding mode (N: %s).\n", @@
> > -594,6 +595,7 @@ launch_args_parse(int argc, char** argv)
> >  		{ "enable-hw-vlan-extend",      0, 0, 0 },
> >  		{ "enable-drop-en",            0, 0, 0 },
> >  		{ "disable-rss",                0, 0, 0 },
> > +		{ "disable-rss-neg",            0, 0, 0 },
> >  		{ "port-topology",              1, 0, 0 },
> >  		{ "forward-mode",               1, 0, 0 },
> >  		{ "rss-ip",			0, 0, 0 },
> > @@ -909,6 +911,9 @@ launch_args_parse(int argc, char** argv)
> >
> >  			if (!strcmp(lgopts[opt_idx].name, "disable-rss"))
> >  				rss_hf = 0;
> > +			if (!strcmp(lgopts[opt_idx].name, "disable-rss-neg")) {
> > +				rss_hf_noneg = 1;
> > +			}
> >  			if (!strcmp(lgopts[opt_idx].name, "port-topology")) {
> >  				if (!strcmp(optarg, "paired"))
> >  					port_topology = PORT_TOPOLOGY_PAIRED; diff --git
> > a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > d6da41927..68214677c 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -257,6 +257,12 @@ int16_t tx_rs_thresh = RTE_PMD_PARAM_UNSET;
> > uint64_t rss_hf = ETH_RSS_IP; /* RSS IP by default. */
> >
> >  /*
> > + * Negotiate with device capability when config
> > + * Receive Side Scaling (RSS).
> > + */
> > +uint8_t rss_hf_noneg = 0; /* Negotiate by default */
> > +
> > +/*
> >   * Port topology configuration
> >   */
> >  uint16_t port_topology = PORT_TOPOLOGY_PAIRED; /* Ports are paired
> by
> > default */ @@ -2265,13 +2271,17 @@ 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_noneg ? 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; diff --git
> > a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > 070919822..c37c4ee70 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -385,6 +385,7 @@ extern struct rte_eth_rxmode rx_mode;  extern
> > struct rte_eth_txmode tx_mode;
> >
> >  extern uint64_t rss_hf;
> > +extern uint8_t rss_hf_noneg;
> >
> >  extern queueid_t nb_rxq;
> >  extern queueid_t nb_txq;
> > diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > index 593b13a3d..8db9cf5c1 100644
> > --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> > @@ -1751,13 +1751,15 @@ port config - RSS
> >
> >  Set the RSS (Receive Side Scaling) mode on or off::
> >
> > -   testpmd> port config all rss
> (all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none)
> > +   testpmd> port config all rss
> > +
> (all|default|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|none|neg|
> > + noneg)
> >
> >  RSS is on by default.
> >
> >  The ``all`` option is equivalent to ip|tcp|udp|sctp|ether.
> >  The ``default`` option enables all supported RSS types reported by device
> info.
> >  The ``none`` option is equivalent to the ``--disable-rss`` command-line
> option.
> > +The ``neg`` option enable the negotiation with device capability when
> configure RSS.
> > +The ``noneg`` option disable the negotiation and is equivalent to
> ``--disable-rss-neg`` command-line option.
> >
> >  port config - RSS Reta
> >  ~~~~~~~~~~~~~~~~~~~~~~
> >


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dpdk-dev] [PATCH v3] app/testpmd: fix testpmd failure due to RSS offload check
  2018-04-25  1:49 [dpdk-dev] [PATCH] app/testpmd: fix testpmd failure due to RSS offload check Qi Zhang
  2018-04-25  7:43 ` [dpdk-dev] [PATCH v2] " Qi Zhang
@ 2018-04-25 13:38 ` Qi Zhang
  2018-04-25 14:02   ` Ferruh Yigit
  2018-04-25 16:27   ` Adrien Mazarguil
  1 sibling, 2 replies; 9+ messages in thread
From: Qi Zhang @ 2018-04-25 13:38 UTC (permalink / raw)
  To: ferruh.yigit, adrien.mazarguil; +Cc: thomas, dev, Qi Zhang

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(-)

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;
-- 
2.13.6

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v3] app/testpmd: fix testpmd failure due to RSS offload check
  2018-04-25 13:38 ` [dpdk-dev] [PATCH v3] " Qi Zhang
@ 2018-04-25 14:02   ` Ferruh Yigit
  2018-04-25 16:27   ` Adrien Mazarguil
  1 sibling, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2018-04-25 14:02 UTC (permalink / raw)
  To: Qi Zhang, adrien.mazarguil; +Cc: thomas, dev

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>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v3] app/testpmd: fix testpmd failure due to RSS offload check
  2018-04-25 13:38 ` [dpdk-dev] [PATCH v3] " Qi Zhang
  2018-04-25 14:02   ` Ferruh Yigit
@ 2018-04-25 16:27   ` Adrien Mazarguil
  2018-04-25 16:34     ` Ferruh Yigit
  2018-04-25 23:46     ` Ferruh Yigit
  1 sibling, 2 replies; 9+ messages in thread
From: Adrien Mazarguil @ 2018-04-25 16:27 UTC (permalink / raw)
  To: Qi Zhang; +Cc: ferruh.yigit, thomas, dev

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>

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v3] app/testpmd: fix testpmd failure due to RSS offload check
  2018-04-25 16:27   ` Adrien Mazarguil
@ 2018-04-25 16:34     ` Ferruh Yigit
  2018-04-25 23:46     ` Ferruh Yigit
  1 sibling, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2018-04-25 16:34 UTC (permalink / raw)
  To: Adrien Mazarguil, Qi Zhang; +Cc: thomas, dev

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>
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [PATCH v3] app/testpmd: fix testpmd failure due to RSS offload check
  2018-04-25 16:27   ` Adrien Mazarguil
  2018-04-25 16:34     ` Ferruh Yigit
@ 2018-04-25 23:46     ` Ferruh Yigit
  1 sibling, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2018-04-25 23:46 UTC (permalink / raw)
  To: Adrien Mazarguil, Qi Zhang; +Cc: thomas, dev

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-04-25 23:46 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-25  1:49 [dpdk-dev] [PATCH] app/testpmd: fix testpmd failure due to RSS offload check Qi Zhang
2018-04-25  7:43 ` [dpdk-dev] [PATCH v2] " Qi Zhang
2018-04-25 12:03   ` Ferruh Yigit
2018-04-25 13:34     ` Zhang, Qi Z
2018-04-25 13:38 ` [dpdk-dev] [PATCH v3] " Qi Zhang
2018-04-25 14:02   ` Ferruh Yigit
2018-04-25 16:27   ` Adrien Mazarguil
2018-04-25 16:34     ` Ferruh Yigit
2018-04-25 23:46     ` Ferruh Yigit

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).