netdev - RE: [PATCH bpf-next 01/21] xsk: prepare 'options' in xdp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 24 May 2023 08:56:21 +0000
From: "Sarkar, Tirthendu" <tirthendu.sarkar@...el.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>, Stanislav Fomichev
	<sdf@...gle.com>
CC: "Fijalkowski, Maciej" <maciej.fijalkowski@...el.com>, bpf
	<bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann
	<daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, "Network
 Development" <netdev@...r.kernel.org>, "Karlsson, Magnus"
	<magnus.karlsson@...el.com>, Björn Töpel
	<bjorn@...nel.org>
Subject: RE: [PATCH bpf-next 01/21] xsk: prepare 'options' in xdp_desc for
 multi-buffer use

> -----Original Message-----
> From: Alexei Starovoitov <alexei.starovoitov@...il.com>
> Sent: Friday, May 19, 2023 10:44 PM
> To: Stanislav Fomichev <sdf@...gle.com>
> Cc: Fijalkowski, Maciej <maciej.fijalkowski@...el.com>; bpf
> <bpf@...r.kernel.org>; Alexei Starovoitov <ast@...nel.org>; Daniel
> Borkmann <daniel@...earbox.net>; Andrii Nakryiko <andrii@...nel.org>;
> Network Development <netdev@...r.kernel.org>; Karlsson, Magnus
> <magnus.karlsson@...el.com>; Sarkar, Tirthendu
> <tirthendu.sarkar@...el.com>; Björn Töpel <bjorn@...nel.org>
> 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@...gle.com>
> wrote:
> >
> > On 05/18, Maciej Fijalkowski wrote:
> > > From: Tirthendu Sarkar <tirthendu.sarkar@...el.com>
> > >
> > > 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@...el.com>
> > > ---
> > >  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." ?