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

[Ori Kam <orika@nvidia.com>]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Thomas Monjalon
> Sent: Tuesday, November 2, 2021 5:46 PM
> To: Ivan Malov <ivan.malov@oktetlabs.ru>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Cc: dev@dpdk.org; David Marchand <david.marchand@redhat.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Ori Kam <orika@nvidia.com>
> Subject: Re: [dpdk-dev] [PATCH] ethdev: fine tune error reporting in pick transfer proxy API
> 
> 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.
> 

+1 too the two points above.

Best,
Ori