diff mbox series

[v4,3/3] app/testpmd: add new types to RSS hash commands

Message ID 20200414174225.64120-4-jia.guo@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series add RSS configuration for iavf |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/travis-robot success Travis build: passed
ci/Intel-compilation fail Compilation issues

Commit Message

Guo, Jia April 14, 2020, 5:42 p.m. UTC 
  Add some new types, such as eth-src-only/eth-dst-only/svlan/cvlan/
l2tpv3/esp/ah/pfcp types into RSS hash commands, it could be used
to configure these rss input set by cmdline.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
v4->v3:
no change
---
 app/test-pmd/cmdline.c | 24 ++++++++++++++++++++++--
 app/test-pmd/config.c  |  8 ++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

Comments

Ori Kam April 14, 2020, 9:48 a.m. UTC | #1 
Hi Jeff,
PSB,

Thanks,
Ori

> -----Original Message-----
> From: Jeff Guo <jia.guo@intel.com>
> Sent: Tuesday, April 14, 2020 8:42 PM
> To: Ori Kam <orika@mellanox.com>; bernard.iremonger@intel.com;
> xiaolong.ye@intel.com; qi.z.zhang@intel.com
> Cc: dev@dpdk.org; jingjing.wu@intel.com; yahui.cao@intel.com;
> simei.su@intel.com; jia.guo@intel.com
> Subject: [dpdk-dev v4 3/3] app/testpmd: add new types to RSS hash commands
> 
> Add some new types, such as eth-src-only/eth-dst-only/svlan/cvlan/
> l2tpv3/esp/ah/pfcp types into RSS hash commands, it could be used
> to configure these rss input set by cmdline.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> ---
> v4->v3:
> no change
> ---
>  app/test-pmd/cmdline.c | 24 ++++++++++++++++++++++--
>  app/test-pmd/config.c  |  8 ++++++++
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 863b567c1..df7309f61 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2299,6 +2299,22 @@ cmd_config_rss_parsed(void *parsed_result,
>  		rss_conf.rss_hf = ETH_RSS_L4_SRC_ONLY;
>  	else if (!strcmp(res->value, "l4-dst-only"))
>  		rss_conf.rss_hf = ETH_RSS_L4_DST_ONLY;
> +	else if (!strcmp(res->value, "eth-src-only"))
> +		rss_conf.rss_hf = ETH_RSS_ETH_SRC_ONLY;
> +	else if (!strcmp(res->value, "eth-dst-only"))
> +		rss_conf.rss_hf = ETH_RSS_ETH_DST_ONLY;
> +	else if (!strcmp(res->value, "s-vlan"))
> +		rss_conf.rss_hf = ETH_RSS_S_VLAN;
> +	else if (!strcmp(res->value, "c-vlan"))
> +		rss_conf.rss_hf = ETH_RSS_C_VLAN;
> +	else if (!strcmp(res->value, "l2tpv3"))
> +		rss_conf.rss_hf = ETH_RSS_L2TPV3;
> +	else if (!strcmp(res->value, "esp"))
> +		rss_conf.rss_hf = ETH_RSS_ESP;
> +	else if (!strcmp(res->value, "ah"))
> +		rss_conf.rss_hf = ETH_RSS_AH;
> +	else if (!strcmp(res->value, "pfcp"))
> +		rss_conf.rss_hf = ETH_RSS_PFCP;


In patch ethdev: add new RSS offload types, you added a define for ETH_RSS_ETH
I don't see any usage for this enum in the code.
Should it be added? Or removed from the first patch?

>  	else if (!strcmp(res->value, "none"))
>  		rss_conf.rss_hf = 0;
>  	else if (!strcmp(res->value, "default"))
> @@ -2467,7 +2483,9 @@ cmdline_parse_token_string_t
> cmd_config_rss_hash_key_rss_type =
>  				 "ipv4-other#ipv6#ipv6-frag#ipv6-tcp#ipv6-
> udp#"
>  				 "ipv6-sctp#ipv6-other#l2-payload#ipv6-ex#"
>  				 "ipv6-tcp-ex#ipv6-udp-ex#"
> -				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-
> only");
> +				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-
> only#"
> +				 "eth-src-only#eth-dst-only#s-vlan#c-vlan#"
> +				 "l2tpv3#esp#ah#pfcp");
>  cmdline_parse_token_string_t cmd_config_rss_hash_key_value =
>  	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_key, key,
> NULL);
> 
> @@ -2478,7 +2496,9 @@ cmdline_parse_inst_t cmd_config_rss_hash_key = {
>  		"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|"
> -		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only "
> +		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only|"
> +		"eth-src-only|eth-dst-only|s-vlan|c-vlan|"
> +		"l2tpv3|esp|ah|pfcp "
>  		"<string of hex digits (variable length, NIC dependent)>",
>  	.tokens = {
>  		(void *)&cmd_config_rss_hash_key_port,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 71aeb5413..47256292b 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -79,6 +79,10 @@ const struct rss_type_info rss_type_table[] = {
>  			ETH_RSS_UDP | ETH_RSS_SCTP |
>  			ETH_RSS_L2_PAYLOAD },
>  	{ "none", 0 },
> +	{ "eth-src-only", ETH_RSS_ETH_SRC_ONLY },
> +	{ "eth-dst-only", ETH_RSS_ETH_DST_ONLY },
> +	{ "s-vlan", ETH_RSS_S_VLAN },
> +	{ "c-vlan", ETH_RSS_C_VLAN },
>  	{ "ipv4", ETH_RSS_IPV4 },
>  	{ "ipv4-frag", ETH_RSS_FRAG_IPV4 },
>  	{ "ipv4-tcp", ETH_RSS_NONFRAG_IPV4_TCP },
> @@ -108,6 +112,10 @@ const struct rss_type_info rss_type_table[] = {
>  	{ "l3-dst-only", ETH_RSS_L3_DST_ONLY },
>  	{ "l4-src-only", ETH_RSS_L4_SRC_ONLY },
>  	{ "l4-dst-only", ETH_RSS_L4_DST_ONLY },
> +	{ "l2tpv3", ETH_RSS_L2TPV3 },
> +	{ "esp", ETH_RSS_ESP },
> +	{ "ah", ETH_RSS_AH },
> +	{ "pfcp", ETH_RSS_PFCP },
>  	{ NULL, 0 },
>  };
> 
> --
> 2.20.1
Guo, Jia April 15, 2020, 8:08 a.m. UTC | #2 
hi, Ori


On 4/14/2020 5:48 PM, Ori Kam wrote:
> Hi Jeff,
> PSB,
>
> Thanks,
> Ori
>
>> -----Original Message-----
>> From: Jeff Guo <jia.guo@intel.com>
>> Sent: Tuesday, April 14, 2020 8:42 PM
>> To: Ori Kam <orika@mellanox.com>; bernard.iremonger@intel.com;
>> xiaolong.ye@intel.com; qi.z.zhang@intel.com
>> Cc: dev@dpdk.org; jingjing.wu@intel.com; yahui.cao@intel.com;
>> simei.su@intel.com; jia.guo@intel.com
>> Subject: [dpdk-dev v4 3/3] app/testpmd: add new types to RSS hash commands
>>
>> Add some new types, such as eth-src-only/eth-dst-only/svlan/cvlan/
>> l2tpv3/esp/ah/pfcp types into RSS hash commands, it could be used
>> to configure these rss input set by cmdline.
>>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>> ---
>> v4->v3:
>> no change
>> ---
>>   app/test-pmd/cmdline.c | 24 ++++++++++++++++++++++--
>>   app/test-pmd/config.c  |  8 ++++++++
>>   2 files changed, 30 insertions(+), 2 deletions(-)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 863b567c1..df7309f61 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -2299,6 +2299,22 @@ cmd_config_rss_parsed(void *parsed_result,
>>   		rss_conf.rss_hf = ETH_RSS_L4_SRC_ONLY;
>>   	else if (!strcmp(res->value, "l4-dst-only"))
>>   		rss_conf.rss_hf = ETH_RSS_L4_DST_ONLY;
>> +	else if (!strcmp(res->value, "eth-src-only"))
>> +		rss_conf.rss_hf = ETH_RSS_ETH_SRC_ONLY;
>> +	else if (!strcmp(res->value, "eth-dst-only"))
>> +		rss_conf.rss_hf = ETH_RSS_ETH_DST_ONLY;
>> +	else if (!strcmp(res->value, "s-vlan"))
>> +		rss_conf.rss_hf = ETH_RSS_S_VLAN;
>> +	else if (!strcmp(res->value, "c-vlan"))
>> +		rss_conf.rss_hf = ETH_RSS_C_VLAN;
>> +	else if (!strcmp(res->value, "l2tpv3"))
>> +		rss_conf.rss_hf = ETH_RSS_L2TPV3;
>> +	else if (!strcmp(res->value, "esp"))
>> +		rss_conf.rss_hf = ETH_RSS_ESP;
>> +	else if (!strcmp(res->value, "ah"))
>> +		rss_conf.rss_hf = ETH_RSS_AH;
>> +	else if (!strcmp(res->value, "pfcp"))
>> +		rss_conf.rss_hf = ETH_RSS_PFCP;
>
> In patch ethdev: add new RSS offload types, you added a define for ETH_RSS_ETH
> I don't see any usage for this enum in the code.
> Should it be added? Or removed from the first patch?


It should be added and please expect it in v5.


>>   	else if (!strcmp(res->value, "none"))
>>   		rss_conf.rss_hf = 0;
>>   	else if (!strcmp(res->value, "default"))
>> @@ -2467,7 +2483,9 @@ cmdline_parse_token_string_t
>> cmd_config_rss_hash_key_rss_type =
>>   				 "ipv4-other#ipv6#ipv6-frag#ipv6-tcp#ipv6-
>> udp#"
>>   				 "ipv6-sctp#ipv6-other#l2-payload#ipv6-ex#"
>>   				 "ipv6-tcp-ex#ipv6-udp-ex#"
>> -				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-
>> only");
>> +				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-
>> only#"
>> +				 "eth-src-only#eth-dst-only#s-vlan#c-vlan#"
>> +				 "l2tpv3#esp#ah#pfcp");
>>   cmdline_parse_token_string_t cmd_config_rss_hash_key_value =
>>   	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_key, key,
>> NULL);
>>
>> @@ -2478,7 +2496,9 @@ cmdline_parse_inst_t cmd_config_rss_hash_key = {
>>   		"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|"
>> -		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only "
>> +		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only|"
>> +		"eth-src-only|eth-dst-only|s-vlan|c-vlan|"
>> +		"l2tpv3|esp|ah|pfcp "
>>   		"<string of hex digits (variable length, NIC dependent)>",
>>   	.tokens = {
>>   		(void *)&cmd_config_rss_hash_key_port,
>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
>> index 71aeb5413..47256292b 100644
>> --- a/app/test-pmd/config.c
>> +++ b/app/test-pmd/config.c
>> @@ -79,6 +79,10 @@ const struct rss_type_info rss_type_table[] = {
>>   			ETH_RSS_UDP | ETH_RSS_SCTP |
>>   			ETH_RSS_L2_PAYLOAD },
>>   	{ "none", 0 },
>> +	{ "eth-src-only", ETH_RSS_ETH_SRC_ONLY },
>> +	{ "eth-dst-only", ETH_RSS_ETH_DST_ONLY },
>> +	{ "s-vlan", ETH_RSS_S_VLAN },
>> +	{ "c-vlan", ETH_RSS_C_VLAN },
>>   	{ "ipv4", ETH_RSS_IPV4 },
>>   	{ "ipv4-frag", ETH_RSS_FRAG_IPV4 },
>>   	{ "ipv4-tcp", ETH_RSS_NONFRAG_IPV4_TCP },
>> @@ -108,6 +112,10 @@ const struct rss_type_info rss_type_table[] = {
>>   	{ "l3-dst-only", ETH_RSS_L3_DST_ONLY },
>>   	{ "l4-src-only", ETH_RSS_L4_SRC_ONLY },
>>   	{ "l4-dst-only", ETH_RSS_L4_DST_ONLY },
>> +	{ "l2tpv3", ETH_RSS_L2TPV3 },
>> +	{ "esp", ETH_RSS_ESP },
>> +	{ "ah", ETH_RSS_AH },
>> +	{ "pfcp", ETH_RSS_PFCP },
>>   	{ NULL, 0 },
>>   };
>>
>> --
>> 2.20.1
Stephen Hemminger April 15, 2020, 4:31 p.m. UTC | #3 
On Tue, 14 Apr 2020 09:48:31 +0000
Ori Kam <orika@mellanox.com> wrote:

> > @@ -2299,6 +2299,22 @@ cmd_config_rss_parsed(void *parsed_result,
> >  		rss_conf.rss_hf = ETH_RSS_L4_SRC_ONLY;
> >  	else if (!strcmp(res->value, "l4-dst-only"))
> >  		rss_conf.rss_hf = ETH_RSS_L4_DST_ONLY;
> > +	else if (!strcmp(res->value, "eth-src-only"))
> > +		rss_conf.rss_hf = ETH_RSS_ETH_SRC_ONLY;
> > +	else if (!strcmp(res->value, "eth-dst-only"))
> > +		rss_conf.rss_hf = ETH_RSS_ETH_DST_ONLY;
> > +	else if (!strcmp(res->value, "s-vlan"))
> > +		rss_conf.rss_hf = ETH_RSS_S_VLAN;
> > +	else if (!strcmp(res->value, "c-vlan"))
> > +		rss_conf.rss_hf = ETH_RSS_C_VLAN;
> > +	else if (!strcmp(res->value, "l2tpv3"))
> > +		rss_conf.rss_hf = ETH_RSS_L2TPV3;
> > +	else if (!strcmp(res->value, "esp"))
> > +		rss_conf.rss_hf = ETH_RSS_ESP;
> > +	else if (!strcmp(res->value, "ah"))
> > +		rss_conf.rss_hf = ETH_RSS_AH;
> > +	else if (!strcmp(res->value, "pfcp"))
> > +		rss_conf.rss_hf = ETH_RSS_PFCP;  

Hasn't this grown big enough that it should be a table with lookup function,
versus a cascade of strcmp's.
Guo, Jia April 16, 2020, 8:55 a.m. UTC | #4 
hi, stephen


On 4/16/2020 12:31 AM, Stephen Hemminger wrote:
> On Tue, 14 Apr 2020 09:48:31 +0000
> Ori Kam <orika@mellanox.com> wrote:
>
>>> @@ -2299,6 +2299,22 @@ cmd_config_rss_parsed(void *parsed_result,
>>>   		rss_conf.rss_hf = ETH_RSS_L4_SRC_ONLY;
>>>   	else if (!strcmp(res->value, "l4-dst-only"))
>>>   		rss_conf.rss_hf = ETH_RSS_L4_DST_ONLY;
>>> +	else if (!strcmp(res->value, "eth-src-only"))
>>> +		rss_conf.rss_hf = ETH_RSS_ETH_SRC_ONLY;
>>> +	else if (!strcmp(res->value, "eth-dst-only"))
>>> +		rss_conf.rss_hf = ETH_RSS_ETH_DST_ONLY;
>>> +	else if (!strcmp(res->value, "s-vlan"))
>>> +		rss_conf.rss_hf = ETH_RSS_S_VLAN;
>>> +	else if (!strcmp(res->value, "c-vlan"))
>>> +		rss_conf.rss_hf = ETH_RSS_C_VLAN;
>>> +	else if (!strcmp(res->value, "l2tpv3"))
>>> +		rss_conf.rss_hf = ETH_RSS_L2TPV3;
>>> +	else if (!strcmp(res->value, "esp"))
>>> +		rss_conf.rss_hf = ETH_RSS_ESP;
>>> +	else if (!strcmp(res->value, "ah"))
>>> +		rss_conf.rss_hf = ETH_RSS_AH;
>>> +	else if (!strcmp(res->value, "pfcp"))
>>> +		rss_conf.rss_hf = ETH_RSS_PFCP;
> Hasn't this grown big enough that it should be a table with lookup function,
> versus a cascade of strcmp's.


I suppose what you said is make sense, but would you mind if a next 
coming specific patch to fix it but not involve in this feature patch set?
diff mbox series

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 863b567c1..df7309f61 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2299,6 +2299,22 @@  cmd_config_rss_parsed(void *parsed_result,
 		rss_conf.rss_hf = ETH_RSS_L4_SRC_ONLY;
 	else if (!strcmp(res->value, "l4-dst-only"))
 		rss_conf.rss_hf = ETH_RSS_L4_DST_ONLY;
+	else if (!strcmp(res->value, "eth-src-only"))
+		rss_conf.rss_hf = ETH_RSS_ETH_SRC_ONLY;
+	else if (!strcmp(res->value, "eth-dst-only"))
+		rss_conf.rss_hf = ETH_RSS_ETH_DST_ONLY;
+	else if (!strcmp(res->value, "s-vlan"))
+		rss_conf.rss_hf = ETH_RSS_S_VLAN;
+	else if (!strcmp(res->value, "c-vlan"))
+		rss_conf.rss_hf = ETH_RSS_C_VLAN;
+	else if (!strcmp(res->value, "l2tpv3"))
+		rss_conf.rss_hf = ETH_RSS_L2TPV3;
+	else if (!strcmp(res->value, "esp"))
+		rss_conf.rss_hf = ETH_RSS_ESP;
+	else if (!strcmp(res->value, "ah"))
+		rss_conf.rss_hf = ETH_RSS_AH;
+	else if (!strcmp(res->value, "pfcp"))
+		rss_conf.rss_hf = ETH_RSS_PFCP;
 	else if (!strcmp(res->value, "none"))
 		rss_conf.rss_hf = 0;
 	else if (!strcmp(res->value, "default"))
@@ -2467,7 +2483,9 @@  cmdline_parse_token_string_t cmd_config_rss_hash_key_rss_type =
 				 "ipv4-other#ipv6#ipv6-frag#ipv6-tcp#ipv6-udp#"
 				 "ipv6-sctp#ipv6-other#l2-payload#ipv6-ex#"
 				 "ipv6-tcp-ex#ipv6-udp-ex#"
-				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-only");
+				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-only#"
+				 "eth-src-only#eth-dst-only#s-vlan#c-vlan#"
+				 "l2tpv3#esp#ah#pfcp");
 cmdline_parse_token_string_t cmd_config_rss_hash_key_value =
 	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_key, key, NULL);
 
@@ -2478,7 +2496,9 @@  cmdline_parse_inst_t cmd_config_rss_hash_key = {
 		"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|"
-		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only "
+		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only|"
+		"eth-src-only|eth-dst-only|s-vlan|c-vlan|"
+		"l2tpv3|esp|ah|pfcp "
 		"<string of hex digits (variable length, NIC dependent)>",
 	.tokens = {
 		(void *)&cmd_config_rss_hash_key_port,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 71aeb5413..47256292b 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -79,6 +79,10 @@  const struct rss_type_info rss_type_table[] = {
 			ETH_RSS_UDP | ETH_RSS_SCTP |
 			ETH_RSS_L2_PAYLOAD },
 	{ "none", 0 },
+	{ "eth-src-only", ETH_RSS_ETH_SRC_ONLY },
+	{ "eth-dst-only", ETH_RSS_ETH_DST_ONLY },
+	{ "s-vlan", ETH_RSS_S_VLAN },
+	{ "c-vlan", ETH_RSS_C_VLAN },
 	{ "ipv4", ETH_RSS_IPV4 },
 	{ "ipv4-frag", ETH_RSS_FRAG_IPV4 },
 	{ "ipv4-tcp", ETH_RSS_NONFRAG_IPV4_TCP },
@@ -108,6 +112,10 @@  const struct rss_type_info rss_type_table[] = {
 	{ "l3-dst-only", ETH_RSS_L3_DST_ONLY },
 	{ "l4-src-only", ETH_RSS_L4_SRC_ONLY },
 	{ "l4-dst-only", ETH_RSS_L4_DST_ONLY },
+	{ "l2tpv3", ETH_RSS_L2TPV3 },
+	{ "esp", ETH_RSS_ESP },
+	{ "ah", ETH_RSS_AH },
+	{ "pfcp", ETH_RSS_PFCP },
 	{ NULL, 0 },
 };