> -----Original Message-----
> From: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
> Sent: Tuesday, June 20, 2023 10:56 PM
>>
> Subject: Re: [PATCH v4 bpf-next 06/22] xsk: introduce wrappers and helpers
> for supporting multi-buffer in Tx path
>
> Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx> writes:
>
> > From: Tirthendu Sarkar <tirthendu.sarkar@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > ---
> > 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
>
Thanks for catching this. We will add cleanup during socket termination in v5.
