[EXT] Re: [PATCH v5 2/2] app/testpmd: add command to process Rx metadata negotiation

[Ferruh Yigit]

On 1/25/2023 12:55 PM, Thomas Monjalon wrote:
> 25/01/2023 10:30, Hanumanth Reddy Pothula:
>> ++ Ivan Malov and Andrew Rybchenko
>>
>> From: Ferruh Yigit <ferruh.yigit at amd.com>
>>> On 12/21/2022 2:07 AM, Hanumanth Pothula wrote:
>>>> Presently, Rx metadata is sent to PMD by default, leading to a
>>>> performance drop as processing for the same in Rx path takes extra
>>>> cycles.
>>>>
>>>> Hence, add new testpmd command,
>>>>   'enable port <port_id> nic_to_pmd_rx_metadata'
>>>>
>>>> This command helps in sending Rx metadata to PMD and thereby Rx
>>>> metadata flow command requests are processed.
>>>>
>>>> Signed-off-by: Hanumanth Pothula <hpothula at marvell.com>
>>>
>>> Hi Hanumanth,
>>>
>>> I agree with Thomas for the patch.
>>>
>>> 'eth_rx_metadata_negotiate_mp()' requests all Rx metadata offloads to be
>>> enabled, but at this stage if there is no flow rule for Rx metadata why it is
>>> consuming extra cycles?
>>>
>>> Can you update driver code to process Rx metadata when it is enabled by
>>> application (via 'rte_eth_rx_metadata_negotiate()') AND there is at least
>>> one flow rule for it?
>>
>> #1 What is the purpose of rte_eth_rx_metadata_negotiate() API if it is always called by testpmd.
>> We thought it was added so that when that metadata is not needed, application need not call this
>> thereby saving cycles/bandwidth.
> 
> testpmd is for testing all features. That's why all is negotiated.
> Cycles should be saved if you don't enable it until a flow rule requires it.
> 

Hi Thomas,

Not just for saving cycles, but from testing perspective too, do you
think does it work if a way to disable these Rx metadata added by
keeping default behavior as it is?

And new command can be in a consistent command syntax like:
"port config <port_id> ..."


>> #2 We use this API similar to Rx/Tx offload flags so that we can set things up before device is
>> configured. We thought that is the purpose of having this negotiate API and avoid depleting offload flags.
> 
> It is just a configuration negotiation specific to metadata.
> 
>> #3 Generally any new offloads added to DPDK would be in disabled state in testpmd and we would have
>> an option to enable it. In this case, testpmd is by default calling this negotiation.
> 
> Negotiating is not enabling.
> 
>> We can update the driver if the purpose of this API is clear.
> 
> Please do.
> 

Is following understanding correct?

     API        Flow Rule       Result
    -----    ------------     --------
    Enable    No Rule	       Feature Disabled
    Enable    Rule exist       Feature Enabled
    Disable     X              Feature Disabled