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

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 10/26/2021 4:58 PM, Thomas Monjalon wrote:
> 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.

+1

> Please Ivan what do you suggest?
> 
>