Re: [dpdk-dev] [PATCH] ethdev: fine tune error reporting in pick transfer proxy API

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 11/2/21 6:45 PM, Thomas Monjalon wrote:
> 01/11/2021 10:41, Andrew Rybchenko:
>> On 10/27/21 12:00 PM, Ivan Malov wrote:
>>> There are PMDs which do not support flow offloads at all.
>>> In such cases, the API in question returns ENOTSUP. This
>>> is too loud. Restructure the code to avoid spamming logs.
>>>
>>> Fixes: 1179f05cc9a0 ("ethdev: query proxy port to manage transfer flows")
>>>
>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>>> ---
>>>    lib/ethdev/rte_flow.c | 5 +----
>>>    1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/lib/ethdev/rte_flow.c b/lib/ethdev/rte_flow.c
>>> index d268784532..9d98d2d716 100644
>>> --- a/lib/ethdev/rte_flow.c
>>> +++ b/lib/ethdev/rte_flow.c
>>> @@ -1335,10 +1335,7 @@ rte_flow_pick_transfer_proxy(uint16_t port_id, uint16_t *proxy_port_id,
>>>    	const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error);
>>>    	struct rte_eth_dev *dev;
>>>    
>>> -	if (unlikely(ops == NULL))
>>> -		return -rte_errno;
>>> -
>>> -	if (ops->pick_transfer_proxy == NULL) {
>>> +	if (ops == NULL || ops->pick_transfer_proxy == NULL) {
>>
>> First of all I think that the patch is wrong and origin code is better.
>> If flow API is not supported at all (ops == NULL), what's the point
>> to return some proxy port?
>>
>>>    		*proxy_port_id = port_id;
>>>    		return 0;
>>>    	}
>>>
>>
>> IMHO, spamming of testpmd logs in described case should be fixed
>> in testpmd itself to avoid logs in the case of ENOTSUP. That's it.
> 
> I think we should not call this API in testpmd
> if not doing rte_flow transfer rule.
> 

Since testpmd does not pursue top flow insertion rate etc,
I tend to agree. For debugging purposes (testpmd main goal)
the best way is to pick proxy just before usage without any
caching etc.