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).


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?


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.


Thanks


--
Ivan M

Reply via email to