> -----Original Message----- > From: Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx> > Sent: Friday, May 19, 2023 10:44 PM > To: Stanislav Fomichev <sdf@xxxxxxxxxx> > Cc: Fijalkowski, Maciej <maciej.fijalkowski@xxxxxxxxx>; bpf > <bpf@xxxxxxxxxxxxxxx>; Alexei Starovoitov <ast@xxxxxxxxxx>; Daniel > Borkmann <daniel@xxxxxxxxxxxxx>; Andrii Nakryiko <andrii@xxxxxxxxxx>; > Network Development <netdev@xxxxxxxxxxxxxxx>; Karlsson, Magnus > <magnus.karlsson@xxxxxxxxx>; Sarkar, Tirthendu > <tirthendu.sarkar@xxxxxxxxx>; Björn Töpel <bjorn@xxxxxxxxxx> > Subject: Re: [PATCH bpf-next 01/21] xsk: prepare 'options' in xdp_desc for > multi-buffer use > > On Thu, May 18, 2023 at 12:22 PM Stanislav Fomichev <sdf@xxxxxxxxxx> > wrote: > > > > On 05/18, Maciej Fijalkowski wrote: > > > From: Tirthendu Sarkar <tirthendu.sarkar@xxxxxxxxx> > > > > > > Use the 'options' field in xdp_desc as a packet continuity marker. Since > > > 'options' field was unused till now and was expected to be set to 0, the > > > 'eop' descriptor will have it set to 0, while the non-eop descriptors > > > will have to set it to 1. This ensures legacy applications continue to > > > work without needing any change for single-buffer packets. > > > > > > Add helper functions and extend xskq_prod_reserve_desc() to use the > > > 'options' field. > > > > > > Signed-off-by: Tirthendu Sarkar <tirthendu.sarkar@xxxxxxxxx> > > > --- > > > include/uapi/linux/if_xdp.h | 16 ++++++++++++++++ > > > net/xdp/xsk.c | 8 ++++---- > > > net/xdp/xsk_queue.h | 12 +++++++++--- > > > 3 files changed, 29 insertions(+), 7 deletions(-) > > > > > > diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h > > > index a78a8096f4ce..4acc3a9430f3 100644 > > > --- a/include/uapi/linux/if_xdp.h > > > +++ b/include/uapi/linux/if_xdp.h > > > @@ -108,4 +108,20 @@ struct xdp_desc { > > > > > > /* UMEM descriptor is __u64 */ > > > > > > +/* Flag indicating that the packet continues with the buffer pointed out > by the > > > + * next frame in the ring. The end of the packet is signalled by setting > this > > > + * bit to zero. For single buffer packets, every descriptor has 'options' > set > > > + * to 0 and this maintains backward compatibility. > > > + */ > > > +#define XDP_PKT_CONTD (1 << 0) > > > + > > > +/* Maximum number of descriptors supported as frags for a packet. So > the total > > > + * number of descriptors supported for a packet is > XSK_DESC_MAX_FRAGS + 1. The > > > + * max frags supported by skb is 16 for page sizes greater than 4K and 17 > or > > > > This is now a config option CONFIG_MAX_SKB_FRAGS. Can we use it > > directly? > > Also it doesn't look right to expose kernel internal config in uapi > especially since XSK_DESC_MAX_FRAGS is not guaranteed to be 16. Ok, we have couple of options here: Option 1: We will define XSK_DESC_MAX_FRAGS to 17 now. This will ensure AF_XDP applications will work on any system without any change since the MAX_SKB_FRAGS is guaranteed to be at least 17. Option 2: Instead of defining a new macro, we say max frags supported is same as MAX_SKB_FRAGS as configured in your system. So use 17 or less frags if you want your app to work everywhere but you can go larger if you control the system. Any suggestions ? Also Alexei could you please clarify what you meant by ".. since XSK_DESC_MAX_FRAGS is not guaranteed to be 16." ?