Re: [PATCH v4 bpf-next 06/22] xsk: introduce wrappers and helpers for supporting multi-buffer in Tx path

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Maciej Fijalkowski <maciej.fijalkowski@intel.com> writes:

> From: Tirthendu Sarkar <tirthendu.sarkar@intel.com>
>
> In Tx path, xsk core reserves space for each desc to be transmitted in
> the completion queue and it's address contained in it is stored in the
> skb destructor arg. After successful transmission the skb destructor
> submits the addr marking completion.
>
> To handle multiple descriptors per packet, now along with reserving
> space for each descriptor, the corresponding address is also stored in
> completion queue. The number of pending descriptors are stored in skb
> destructor arg and is used by the skb destructor to update completions.
>
> Introduce 'skb' in xdp_sock to store a partially built packet when
> __xsk_generic_xmit() must return before it sees the EOP descriptor for
> the current packet so that packet building can resume in next call of
> __xsk_generic_xmit().
>
> Helper functions are introduced to set and get the pending descriptors
> in the skb destructor arg. Also, wrappers are introduced for storing
> descriptor addresses, submitting and cancelling (for unsuccessful
> transmissions) the number of completions.
>
> Signed-off-by: Tirthendu Sarkar <tirthendu.sarkar@intel.com>
> ---
>  include/net/xdp_sock.h |  6 ++++
>  net/xdp/xsk.c          | 74 ++++++++++++++++++++++++++++++------------
>  net/xdp/xsk_queue.h    | 19 ++++-------
>  3 files changed, 67 insertions(+), 32 deletions(-)
>
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index 36b0411a0d1b..1617af380162 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -68,6 +68,12 @@ struct xdp_sock {
>  	u64 rx_dropped;
>  	u64 rx_queue_full;
>  
> +	/* When __xsk_generic_xmit() must return before it sees the EOP descriptor for the current
> +	 * packet, the partially built skb is saved here so that packet building can resume in next
> +	 * call of __xsk_generic_xmit().
> +	 */
> +	struct sk_buff *skb;

What ensures this doesn't leak? IIUC, when the loop in
__xsk_generic_xmit() gets to the end of a batch, userspace will get an
EAGAIN error and be expected to retry the call later, right? But if
userspace never retries, could the socket be torn down with this pointer
still populated? I looked for something that would prevent this in
subsequent patches, but couldn't find it; am I missing something?

-Toke