[V5,2/7] app/testpmd: unify the name of L2 payload offload

Message ID 20220624072401.21839-3-lihuisong@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: fix RSS and flow type |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

lihuisong (C) June 24, 2022, 7:23 a.m. UTC
  Currently, the "port config all rss xx" command uses 'ether' name to match
and to set 'RTE_ETH_RSS_L2_PAYLOAD' offload. However, others RSS command,
such as, "port config <port_id> rss-hash-key" and "show port <port_id>
rss-hash key", use 'l2-payload' to represent this offload. So this patch
unifies the name of 'RTE_ETH_RSS_L2_PAYLOAD' offload.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 app/test-pmd/cmdline.c                      | 6 +++---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)
  

Comments

Ferruh Yigit June 24, 2022, 1:53 p.m. UTC | #1
On 6/24/2022 8:23 AM, Huisong Li wrote:
> Currently, the "port config all rss xx" command uses 'ether' name to match
> and to set 'RTE_ETH_RSS_L2_PAYLOAD' offload. However, others RSS command,
> such as, "port config <port_id> rss-hash-key" and "show port <port_id>
> rss-hash key", use 'l2-payload' to represent this offload. So this patch
> unifies the name of 'RTE_ETH_RSS_L2_PAYLOAD' offload.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>

ack

But I wonder if we should continue to support 'ether' with an exception 
to not break the interface, at least for a while like to next LTS.

> ---
>   app/test-pmd/cmdline.c                      | 6 +++---
>   doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +-
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 9a7fd5fc35..a701bac953 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -694,7 +694,7 @@ static void cmd_help_long_parsed(void *parsed_result,
>   			"receive buffers available.\n\n"
>   
>   			"port config all rss (all|default|ip|tcp|udp|sctp|"
> -			"ether|port|vxlan|geneve|nvgre|vxlan-gpe|ecpri|mpls|ipv4-chksum|l2tpv2|"
> +			"l2-payload|port|vxlan|geneve|nvgre|vxlan-gpe|ecpri|mpls|ipv4-chksum|l2tpv2|"
>   			"none|level-default|level-outer|level-inner|<flowtype_id>)\n"
>   			"    Set the RSS mode.\n\n"
>   
> @@ -2080,7 +2080,7 @@ cmd_config_rss_parsed(void *parsed_result,
>   		rss_conf.rss_hf = RTE_ETH_RSS_TCP;
>   	else if (!strcmp(res->value, "sctp"))
>   		rss_conf.rss_hf = RTE_ETH_RSS_SCTP;
> -	else if (!strcmp(res->value, "ether"))
> +	else if (!strcmp(res->value, "l2_payload"))
>   		rss_conf.rss_hf = RTE_ETH_RSS_L2_PAYLOAD;
>   	else if (!strcmp(res->value, "port"))
>   		rss_conf.rss_hf = RTE_ETH_RSS_PORT;
> @@ -2203,7 +2203,7 @@ static cmdline_parse_inst_t cmd_config_rss = {
>   	.f = cmd_config_rss_parsed,
>   	.data = NULL,
>   	.help_str = "port config all rss "
> -		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
> +		"all|default|eth|vlan|ip|tcp|udp|sctp|l2-payload|port|vxlan|geneve|"
>   		"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|ipv4-chksum|l2tpv2|"
>   		"none|level-default|level-outer|level-inner|<flowtype_id>",
>   	.tokens = {
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index 0b7a53fdf1..cc299cff6c 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -2144,7 +2144,7 @@ port config - RSS
>   
>   Set the RSS (Receive Side Scaling) mode on or off::
>   
> -   testpmd> port config all rss (all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|l2tpv2|none)
> +   testpmd> port config all rss (all|default|eth|vlan|ip|tcp|udp|sctp|l2-payload|port|vxlan|geneve|nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|l2tpv2|none)
>   
>   RSS is on by default.
>
  
lihuisong (C) June 25, 2022, 2:12 a.m. UTC | #2
在 2022/6/24 21:53, Ferruh Yigit 写道:
> On 6/24/2022 8:23 AM, Huisong Li wrote:
>> Currently, the "port config all rss xx" command uses 'ether' name to 
>> match
>> and to set 'RTE_ETH_RSS_L2_PAYLOAD' offload. However, others RSS 
>> command,
>> such as, "port config <port_id> rss-hash-key" and "show port <port_id>
>> rss-hash key", use 'l2-payload' to represent this offload. So this patch
>> unifies the name of 'RTE_ETH_RSS_L2_PAYLOAD' offload.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>
> ack
>
> But I wonder if we should continue to support 'ether' with an 
> exception to not break the interface, at least for a while like to 
> next LTS.
It's supposed to have a very small impact, and it is just an optional input
of this command in testpmd. What's more, patch 3/7 has to use 
"l2-payload" to
match. This shouldn't affect the optimization progress of "port config 
all rss xx"
command, right?
>
>> ---
>>   app/test-pmd/cmdline.c                      | 6 +++---
>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +-
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 9a7fd5fc35..a701bac953 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -694,7 +694,7 @@ static void cmd_help_long_parsed(void 
>> *parsed_result,
>>               "receive buffers available.\n\n"
>>                 "port config all rss (all|default|ip|tcp|udp|sctp|"
>> - 
>> "ether|port|vxlan|geneve|nvgre|vxlan-gpe|ecpri|mpls|ipv4-chksum|l2tpv2|"
>> + 
>> "l2-payload|port|vxlan|geneve|nvgre|vxlan-gpe|ecpri|mpls|ipv4-chksum|l2tpv2|"
>> "none|level-default|level-outer|level-inner|<flowtype_id>)\n"
>>               "    Set the RSS mode.\n\n"
>>   @@ -2080,7 +2080,7 @@ cmd_config_rss_parsed(void *parsed_result,
>>           rss_conf.rss_hf = RTE_ETH_RSS_TCP;
>>       else if (!strcmp(res->value, "sctp"))
>>           rss_conf.rss_hf = RTE_ETH_RSS_SCTP;
>> -    else if (!strcmp(res->value, "ether"))
>> +    else if (!strcmp(res->value, "l2_payload"))
>>           rss_conf.rss_hf = RTE_ETH_RSS_L2_PAYLOAD;
>>       else if (!strcmp(res->value, "port"))
>>           rss_conf.rss_hf = RTE_ETH_RSS_PORT;
>> @@ -2203,7 +2203,7 @@ static cmdline_parse_inst_t cmd_config_rss = {
>>       .f = cmd_config_rss_parsed,
>>       .data = NULL,
>>       .help_str = "port config all rss "
>> - "all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
>> + "all|default|eth|vlan|ip|tcp|udp|sctp|l2-payload|port|vxlan|geneve|"
>> "nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|ipv4-chksum|l2tpv2|"
>> "none|level-default|level-outer|level-inner|<flowtype_id>",
>>       .tokens = {
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst 
>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index 0b7a53fdf1..cc299cff6c 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> @@ -2144,7 +2144,7 @@ port config - RSS
>>     Set the RSS (Receive Side Scaling) mode on or off::
>>   -   testpmd> port config all rss 
>> (all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|l2tpv2|none)
>> +   testpmd> port config all rss 
>> (all|default|eth|vlan|ip|tcp|udp|sctp|l2-payload|port|vxlan|geneve|nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|l2tpv2|none)
>>     RSS is on by default.
>
> .
  
Ferruh Yigit June 28, 2022, 1:17 p.m. UTC | #3
On 6/25/2022 3:12 AM, lihuisong (C) wrote:
> 
> 在 2022/6/24 21:53, Ferruh Yigit 写道:
>> On 6/24/2022 8:23 AM, Huisong Li wrote:
>>> Currently, the "port config all rss xx" command uses 'ether' name to
>>> match
>>> and to set 'RTE_ETH_RSS_L2_PAYLOAD' offload. However, others RSS
>>> command,
>>> such as, "port config <port_id> rss-hash-key" and "show port <port_id>
>>> rss-hash key", use 'l2-payload' to represent this offload. So this patch
>>> unifies the name of 'RTE_ETH_RSS_L2_PAYLOAD' offload.
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>
>> ack
>>
>> But I wonder if we should continue to support 'ether' with an
>> exception to not break the interface, at least for a while like to
>> next LTS.
> It's supposed to have a very small impact, and it is just an optional input
> of this command in testpmd. What's more, patch 3/7 has to use
> "l2-payload" to
> match. This shouldn't affect the optimization progress of "port config
> all rss xx"
> command, right?

Yes, impact is small, OK to rename. But can you please add a release 
note update to inform any possible user. A brief, one line update is 
good to say "testpmd RSS type 'ether' renamed to 'l2-payload'"

>>
>>> ---
>>>   app/test-pmd/cmdline.c                      | 6 +++---
>>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +-
>>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>> index 9a7fd5fc35..a701bac953 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -694,7 +694,7 @@ static void cmd_help_long_parsed(void
>>> *parsed_result,
>>>               "receive buffers available.\n\n"
>>>                 "port config all rss (all|default|ip|tcp|udp|sctp|"
>>> -
>>> "ether|port|vxlan|geneve|nvgre|vxlan-gpe|ecpri|mpls|ipv4-chksum|l2tpv2|"
>>> +
>>> "l2-payload|port|vxlan|geneve|nvgre|vxlan-gpe|ecpri|mpls|ipv4-chksum|l2tpv2|" 
>>>
>>> "none|level-default|level-outer|level-inner|<flowtype_id>)\n"
>>>               "    Set the RSS mode.\n\n"
>>>   @@ -2080,7 +2080,7 @@ cmd_config_rss_parsed(void *parsed_result,
>>>           rss_conf.rss_hf = RTE_ETH_RSS_TCP;
>>>       else if (!strcmp(res->value, "sctp"))
>>>           rss_conf.rss_hf = RTE_ETH_RSS_SCTP;
>>> -    else if (!strcmp(res->value, "ether"))
>>> +    else if (!strcmp(res->value, "l2_payload"))
>>>           rss_conf.rss_hf = RTE_ETH_RSS_L2_PAYLOAD;
>>>       else if (!strcmp(res->value, "port"))
>>>           rss_conf.rss_hf = RTE_ETH_RSS_PORT;
>>> @@ -2203,7 +2203,7 @@ static cmdline_parse_inst_t cmd_config_rss = {
>>>       .f = cmd_config_rss_parsed,
>>>       .data = NULL,
>>>       .help_str = "port config all rss "
>>> - "all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
>>> + "all|default|eth|vlan|ip|tcp|udp|sctp|l2-payload|port|vxlan|geneve|"
>>> "nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|ipv4-chksum|l2tpv2|"
>>> "none|level-default|level-outer|level-inner|<flowtype_id>",
>>>       .tokens = {
>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> index 0b7a53fdf1..cc299cff6c 100644
>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> @@ -2144,7 +2144,7 @@ port config - RSS
>>>     Set the RSS (Receive Side Scaling) mode on or off::
>>>   -   testpmd> port config all rss
>>> (all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|l2tpv2|none) 
>>>
>>> +   testpmd> port config all rss
>>> (all|default|eth|vlan|ip|tcp|udp|sctp|l2-payload|port|vxlan|geneve|nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|l2tpv2|none) 
>>>
>>>     RSS is on by default.
>>
>> .
  
lihuisong (C) June 29, 2022, 1:47 a.m. UTC | #4
在 2022/6/28 21:17, Ferruh Yigit 写道:
> On 6/25/2022 3:12 AM, lihuisong (C) wrote:
>>
>> 在 2022/6/24 21:53, Ferruh Yigit 写道:
>>> On 6/24/2022 8:23 AM, Huisong Li wrote:
>>>> Currently, the "port config all rss xx" command uses 'ether' name to
>>>> match
>>>> and to set 'RTE_ETH_RSS_L2_PAYLOAD' offload. However, others RSS
>>>> command,
>>>> such as, "port config <port_id> rss-hash-key" and "show port <port_id>
>>>> rss-hash key", use 'l2-payload' to represent this offload. So this 
>>>> patch
>>>> unifies the name of 'RTE_ETH_RSS_L2_PAYLOAD' offload.
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>
>>> ack
>>>
>>> But I wonder if we should continue to support 'ether' with an
>>> exception to not break the interface, at least for a while like to
>>> next LTS.
>> It's supposed to have a very small impact, and it is just an optional 
>> input
>> of this command in testpmd. What's more, patch 3/7 has to use
>> "l2-payload" to
>> match. This shouldn't affect the optimization progress of "port config
>> all rss xx"
>> command, right?
>
> Yes, impact is small, OK to rename. But can you please add a release 
> note update to inform any possible user. A brief, one line update is 
> good to say "testpmd RSS type 'ether' renamed to 'l2-payload'"
ok
>
>>>
>>>> ---
>>>>   app/test-pmd/cmdline.c                      | 6 +++---
>>>>   doc/guides/testpmd_app_ug/testpmd_funcs.rst | 2 +-
>>>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>>> index 9a7fd5fc35..a701bac953 100644
>>>> --- a/app/test-pmd/cmdline.c
>>>> +++ b/app/test-pmd/cmdline.c
>>>> @@ -694,7 +694,7 @@ static void cmd_help_long_parsed(void
>>>> *parsed_result,
>>>>               "receive buffers available.\n\n"
>>>>                 "port config all rss (all|default|ip|tcp|udp|sctp|"
>>>> -
>>>> "ether|port|vxlan|geneve|nvgre|vxlan-gpe|ecpri|mpls|ipv4-chksum|l2tpv2|" 
>>>>
>>>> +
>>>> "l2-payload|port|vxlan|geneve|nvgre|vxlan-gpe|ecpri|mpls|ipv4-chksum|l2tpv2|" 
>>>>
>>>> "none|level-default|level-outer|level-inner|<flowtype_id>)\n"
>>>>               "    Set the RSS mode.\n\n"
>>>>   @@ -2080,7 +2080,7 @@ cmd_config_rss_parsed(void *parsed_result,
>>>>           rss_conf.rss_hf = RTE_ETH_RSS_TCP;
>>>>       else if (!strcmp(res->value, "sctp"))
>>>>           rss_conf.rss_hf = RTE_ETH_RSS_SCTP;
>>>> -    else if (!strcmp(res->value, "ether"))
>>>> +    else if (!strcmp(res->value, "l2_payload"))
>>>>           rss_conf.rss_hf = RTE_ETH_RSS_L2_PAYLOAD;
>>>>       else if (!strcmp(res->value, "port"))
>>>>           rss_conf.rss_hf = RTE_ETH_RSS_PORT;
>>>> @@ -2203,7 +2203,7 @@ static cmdline_parse_inst_t cmd_config_rss = {
>>>>       .f = cmd_config_rss_parsed,
>>>>       .data = NULL,
>>>>       .help_str = "port config all rss "
>>>> - "all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
>>>> + "all|default|eth|vlan|ip|tcp|udp|sctp|l2-payload|port|vxlan|geneve|"
>>>> "nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|ipv4-chksum|l2tpv2|"
>>>> "none|level-default|level-outer|level-inner|<flowtype_id>",
>>>>       .tokens = {
>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> index 0b7a53fdf1..cc299cff6c 100644
>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> @@ -2144,7 +2144,7 @@ port config - RSS
>>>>     Set the RSS (Receive Side Scaling) mode on or off::
>>>>   -   testpmd> port config all rss
>>>> (all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|l2tpv2|none) 
>>>>
>>>> +   testpmd> port config all rss
>>>> (all|default|eth|vlan|ip|tcp|udp|sctp|l2-payload|port|vxlan|geneve|nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|l2tpv2|none) 
>>>>
>>>>     RSS is on by default.
>>>
>>> .
>
> .
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 9a7fd5fc35..a701bac953 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -694,7 +694,7 @@  static void cmd_help_long_parsed(void *parsed_result,
 			"receive buffers available.\n\n"
 
 			"port config all rss (all|default|ip|tcp|udp|sctp|"
-			"ether|port|vxlan|geneve|nvgre|vxlan-gpe|ecpri|mpls|ipv4-chksum|l2tpv2|"
+			"l2-payload|port|vxlan|geneve|nvgre|vxlan-gpe|ecpri|mpls|ipv4-chksum|l2tpv2|"
 			"none|level-default|level-outer|level-inner|<flowtype_id>)\n"
 			"    Set the RSS mode.\n\n"
 
@@ -2080,7 +2080,7 @@  cmd_config_rss_parsed(void *parsed_result,
 		rss_conf.rss_hf = RTE_ETH_RSS_TCP;
 	else if (!strcmp(res->value, "sctp"))
 		rss_conf.rss_hf = RTE_ETH_RSS_SCTP;
-	else if (!strcmp(res->value, "ether"))
+	else if (!strcmp(res->value, "l2_payload"))
 		rss_conf.rss_hf = RTE_ETH_RSS_L2_PAYLOAD;
 	else if (!strcmp(res->value, "port"))
 		rss_conf.rss_hf = RTE_ETH_RSS_PORT;
@@ -2203,7 +2203,7 @@  static cmdline_parse_inst_t cmd_config_rss = {
 	.f = cmd_config_rss_parsed,
 	.data = NULL,
 	.help_str = "port config all rss "
-		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
+		"all|default|eth|vlan|ip|tcp|udp|sctp|l2-payload|port|vxlan|geneve|"
 		"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|ipv4-chksum|l2tpv2|"
 		"none|level-default|level-outer|level-inner|<flowtype_id>",
 	.tokens = {
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 0b7a53fdf1..cc299cff6c 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -2144,7 +2144,7 @@  port config - RSS
 
 Set the RSS (Receive Side Scaling) mode on or off::
 
-   testpmd> port config all rss (all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|l2tpv2|none)
+   testpmd> port config all rss (all|default|eth|vlan|ip|tcp|udp|sctp|l2-payload|port|vxlan|geneve|nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|l2tpv2|none)
 
 RSS is on by default.