Re: [dpdk-dev] [PATCH v4] ethdev: add API to query proxy port to manage transfer flows

[Thomas Monjalon <thomas@monjalon.net>]

26/10/2021 17:47, Ivan Malov:
> Hi David,
> 
> On 26/10/2021 17:46, David Marchand wrote:
> > On Thu, Oct 14, 2021 at 5:22 AM Ivan Malov <ivan.malov@oktetlabs.ru> wrote:
> >>
> >> Not all DPDK ports in a given switching domain may have the
> >> privilege to manage "transfer" flows. Add an API to find a
> >> port with sufficient privileges by any port in the domain.
> >>
> >> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> >> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Acked-by: Ori Kam <orika@nvidia.com>
> >> ---
> >> Patch series [1] has reworked support for "transfer" flows.
> >> This allows to elaborate on the idea which first appeared
> >> in RFC [2]. Hence the patch in question.
> >>
> >> net/sfc driver is going to support the new API. The
> >> corresponding patch is already in progress and will
> >> be provided in the course of this release cycle.
> >>
> >> [1] https://patches.dpdk.org/project/dpdk/list/?series=19608
> >> [2] https://patches.dpdk.org/project/dpdk/list/?series=18737
> >>
> > 
> > [snip]
> > 
> >> @@ -568,6 +568,25 @@ eth_rx_metadata_negotiate_mp(uint16_t port_id)
> >>          }
> >>   }
> >>
> >> +static void
> >> +flow_pick_transfer_proxy_mp(uint16_t port_id)
> >> +{
> >> +       struct rte_port *port = &ports[port_id];
> >> +       int ret;
> >> +
> >> +       port->flow_transfer_proxy = port_id;
> >> +
> >> +       if (!is_proc_primary())
> >> +               return;
> >> +
> >> +       ret = rte_flow_pick_transfer_proxy(port_id, &port->flow_transfer_proxy,
> >> +                                          NULL);
> >> +       if (ret != 0) {
> >> +               fprintf(stderr, "Error picking flow transfer proxy for port %u: %s - ignore\n",
> >> +                       port_id, rte_strerror(-ret));
> >> +       }
> >> +}
> > 
> > I did not follow this subject, but I find it odd to get a warning
> > regardless of what is done in testpmd.
> > Like simply running a forwarding test with null pmd, I get:
> 
> The API is meant to avoid returning error when the PMD does not support
> the corresponding method. However, it has to return error when the PMD
> does not support flow API at all (rte_flow_ops_get() failure).
> 
> I guess, this is exactly what happens in your case. net/null does not
> support "flow_ops_get", and the API in question forwards error
> from rte_flow_ops_get().
> 
> Non-dummy PMDs should not trigger the error you see.
> 
> > 
> > $ ./devtools/test-null.sh
> > EAL: Detected CPU lcores: 8
> > EAL: Detected NUMA nodes: 1
> > EAL: Detected static linkage of DPDK
> > EAL: Multi-process socket /run/user/114840/dpdk/rte/mp_socket
> > EAL: Selected IOVA mode 'VA'
> > Interactive-mode selected
> > Auto-start selected
> > 
> > vvvv
> > Error picking flow transfer proxy for port 0: Function not implemented - ignore
> > Error picking flow transfer proxy for port 1: Function not implemented - ignore
> > ^^^^

We should avoid such meaningless error message.
Please Ivan what do you suggest?