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.yi...@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 <hpoth...@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
