Re: [PATCH v2] net/af_xdp: re-enable secondary process support

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

On 2/7/2022 11:39 AM, Loftus, Ciara wrote:
>>>>
>>>> On 2/4/2022 12:54 PM, Ciara Loftus wrote:
>>>>> Secondary process support had been disabled for the AF_XDP PMD
>>>>> because there was no logic in place to share the AF_XDP socket
>>>>> file descriptors between the processes. This commit introduces
>>>>> this logic using the IPC APIs.
>>>>>
>>>>> Since AF_XDP rings are single-producer single-consumer, rx/tx
>>>>> in the secondary process is disabled. However other operations
>>>>> including retrieval of stats are permitted.
>>>>>
>>>>> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
>>>>>
>>>>> ---
>>>>> v1 -> v2:
>>>>> * Rebase to next-net
>>>>>
>>>>> RFC -> v1:
>>>>> * Added newline to af_xdp.rst
>>>>> * Fixed spelling errors
>>>>> * Fixed potential NULL dereference in init_internals
>>>>> * Fixed potential free of address-of expression in
>> afxdp_mp_request_fds
>>>>> ---
>>>>>     doc/guides/nics/af_xdp.rst             |   9 ++
>>>>>     doc/guides/nics/features/af_xdp.ini    |   1 +
>>>>>     doc/guides/rel_notes/release_22_03.rst |   1 +
>>>>>     drivers/net/af_xdp/rte_eth_af_xdp.c    | 210
>>>> +++++++++++++++++++++++--
>>>>>     4 files changed, 207 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/doc/guides/nics/af_xdp.rst b/doc/guides/nics/af_xdp.rst
>>>>> index db02ea1984..eb4eab28a8 100644
>>>>> --- a/doc/guides/nics/af_xdp.rst
>>>>> +++ b/doc/guides/nics/af_xdp.rst
>>>>> @@ -141,4 +141,13 @@ Limitations
>>>>>       NAPI context from a watchdog timer instead of from softirqs. More
>>>> information
>>>>>       on this feature can be found at [1].
>>>>>
>>>>> +- **Secondary Processes**
>>>>> +
>>>>> +  Rx and Tx are not supported for secondary processes due to the
>> single-
>>>> producer
>>>>> +  single-consumer nature of the AF_XDP rings. However other
>> operations
>>>> including
>>>>> +  statistics retrieval are permitted.
>>>>
>>>> Hi Ciara,
>>>>
>>>> Isn't this limitation same for all PMDs, like not both primary & secondary
>> can
>>>> Rx/Tx
>>>> from same queue at the same time.
>>>> But primary can initiallize the PMD and secondary can do the datapath,
>>>> or isn't af_xdp supports multiple queue, if so some queues can be used
>> by
>>>> primary and some by secondary for datapath.
>>>>
>>>> Is there anyhing special for af_xdp that prevents it?
>>>
>>> Hi Ferruh,
>>>
>>> Thanks for the review.
>>> Each queue of the PMD corresponds to a new AF_XDP socket.
>>> Each socket has an RX and TX ring that is mmapped from the kernel to
>> userspace and this mapping is only valid for the primary process.
>>> I did not figure out a way to share that mapping with the secondary process
>> successfully. Can you think of anything that might work?
>>>
>>
>> Does the application knows the buffer address for the Rx/Tx, or is
>> abstracted to the 'fd'?
> 
> The application knows the buffer address of the Rx/Tx rings.
> We pass a pointer to these rings to the libbpf xsk_socket__create API, which sets up the mappings:
> http://code.dpdk.org/dpdk/v21.11/source/drivers/net/af_xdp/rte_eth_af_xdp.c#L1291
> Then later on in the datapath we operate directly on those rings:
> http://code.dpdk.org/dpdk/v21.11/source/drivers/net/af_xdp/rte_eth_af_xdp.c#L268
> The fd is used in the datapath, but just for the syscalls (recvfrom/poll/send).
> 

Got it, if the buffer address is explicitly required for datapath, fd conversion
is not enough.

Primary/secondary process works by mapping memory to same virtual address
on two different process. The same method can be used for af_xdp multi
process support, again @Anatoly can comment better.
But this method is fragile, not sure if we should implement it in more
places...

Anyway, agree to continue this patch without datapath support in secondary.

>> If only 'fd' is used, this patch already converts 'fd' between
>> processes.
>> cc'ed Anatoly, but what I understand is after MP fd conversion:
>> Primary process: FD=x
>> Secondary process: FD=y
>> And both x & y points to exact same socket in the kernel side.
>>
>> At least this is how it works for the 'tap' interface, and that is
>> why 'fs' are in the process_private area and converted between primary
>> and secondary, I thought it will be same for the xdp socket.
>>
>> Did you test the secondary Rx/Tx in the secondary after this patch?