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

[Ferruh Yigit <ferruh.yigit@xilinx.com>]

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