Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix display capabilities routine

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

On 7/19/2019 6:07 AM, Slava Ovsiienko wrote:
> Hi, Ferruh
> 
> Please, see below
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Wednesday, July 17, 2019 17:56
>> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
>> Cc: bernard.iremonger@intel.com; stable@dpdk.org
>> Subject: Re: [dpdk-stable] [PATCH] app/testpmd: fix display capabilities
>> routine
>>
>> On 7/16/2019 12:39 PM, Viacheslav Ovsiienko wrote:
>>> The DEV_TX_OFFLOAD_MATCH_METADATA was introduced by [1], but it is
>> not
>>> displayed by "show port [id] tx_offloads capabilities"
>>> command in testpmd.
>>>
>>> [1]
>>>
>> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch
>>>
>> es.dpdk.org%2Fpatch%2F47265%2F&amp;data=02%7C01%7Cviacheslavo%4
>> 0mellan
>>>
>> ox.com%7C3fea893cbf43414d6f9a08d70ac6e78f%7Ca652971c7d2e4d9ba6a
>> 4d14925
>>>
>> 6f461b%7C0%7C0%7C636989721710321485&amp;sdata=bnKjCSFr%2FVX9k
>> Ds1TwijB0
>>> %2Bpe0xM4j2J9cZg872hBYA%3D&amp;reserved=0
>>>
>>> Fixes: 839b20be0e9b ("ethdev: support metadata as flow rule criteria")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
>>> ---
>>>  app/test-pmd/config.c | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
>>> ba43be5..8fb18be 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -830,6 +830,14 @@
>>>  			printf("off\n");
>>>  	}
>>>
>>> +	if (dev_info.tx_offload_capa &
>> DEV_TX_OFFLOAD_MATCH_METADATA) {
>>> +		printf("TX match Flow metafata:        ");
>>> +		if (ports[port_id].dev_conf.txmode.offloads &
>>> +		    DEV_TX_OFFLOAD_MATCH_METADATA)
>>> +			printf("on\n");
>>> +		else
>>> +			printf("off\n");
>>> +	}
>>>  }
>>>
>>>  int
>>>
>>
>> Hi Viacheslav,
>>
>> There is already another testpmd command that displays offloads, which is
>> more dynamic and I think better solution:
>> "show port <port_id> tx_offload capabilities"
>> "show port <port_id> tx_offload configuration"
> 
> Yes, it's implemented in dynamic approach and shows tx metadata offload cap/con correctly.
> It is OK, no need to update this one.
> 
>> "show port <port_id> rx_offload capabilities"
>> "show port <port_id> rx_offload configuration"
>>
>> As far as I can see 'metadata' already supported by these commands, can
>> you please confirm?
> 
> These commands are RX related, there is no any metadata offloads for RX.
> 
>>
>> And instead of improving it, what do you think dropping the duplicated
>> command "show port cap <port_id>|all" ?
> 
> I use this command, it is shorter than the "show port 0 tx_offload capabilities", even with autocompletion.
> Despite this, personally me is OK with drop. But someone else may use these commands also.

As long as we have another command that has this support it is not an issue I
think. And duplication is bad :)

Would you mind helping on this, and prepare a patch to remove this?