27/10/2021 11:55, Ivan Malov:
> Hi Thomas,
>
> On 27/10/2021 12:46, Thomas Monjalon wrote:
> > 27/10/2021 11:00, Ivan Malov:
> >> 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.ma...@oktetlabs.ru>
> >> ---
> >> --- 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) {
> >> *proxy_port_id = port_id;
> >> return 0;
> >> }
> >
> > I prefer this logic.
>
> Thank you.
>
> > You could add a comment to say that the current port is the default.
>
> As far as I remember, the comment ("note") is already in place (rte_flow.h).
I meant adding a comment in the implementation above.
> > There is also this logic in testpmd:
> >
> > port->flow_transfer_proxy = port_id;
> > if (!is_proc_primary())
> > return;
> >
> > Could we manage secondary process case inside the API?
>
> Shouldn't we manage secondary process in *all* flow APIs then?
Hmm, yes logically we should not care about secondary process at all in
rte_flow.
OK to leave it as is.
> > One more comment, for testpmd,
> > we are calling rte_flow_pick_transfer_proxy even if we do not config any
> > transfer flow.
> > It is called always in init_config_port_offloads().
> > It looks wrong. Can we call it only when needed?
>
> In which way does it look wrong? Does it inflict error(s), malfunction,
> performance drops? Please elaborate.
It is testing a function that we don't intend to test in a basic use case.
A driver can introduce a malfunction with this API while
we don't use rte_flow at all in the test scenario.
