netdev - Re: [RFC bpf-next 0/4] Add XDP rx hw hints support performing 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]

Message-ID: <Zu_eghZAEoYBkThM@lore-desk>
Date: Sun, 22 Sep 2024 11:08:18 +0200
From: Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: Lorenzo Bianconi <lorenzo@...nel.org>,
	Toke Høiland-Jørgensen <toke@...nel.org>,
	bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...nel.org,
	daniel@...earbox.net, davem@...emloft.net, kuba@...nel.org,
	hawk@...nel.org, john.fastabend@...il.com, edumazet@...gle.com,
	pabeni@...hat.com, toke@...e.dk, sdf@...ichev.me, tariqt@...dia.com,
	saeedm@...dia.com, anthony.l.nguyen@...el.com,
	przemyslaw.kitszel@...el.com, intel-wired-lan@...ts.osuosl.org,
	mst@...hat.com, jasowang@...hat.com, mcoquelin.stm32@...il.com,
	alexandre.torgue@...s.st.com
Subject: Re: [RFC bpf-next 0/4] Add XDP rx hw hints support performing
 XDP_REDIRECT

> From: Lorenzo Bianconi <lorenzo@...nel.org>
> Date: Sat, 21 Sep 2024 18:52:56 +0200
> 
> > This series introduces the xdp_rx_meta struct in the xdp_buff/xdp_frame
> 
> &xdp_buff is on the stack.
> &xdp_frame consumes headroom.

ack, right.

> 
> IOW they're size-sensitive and putting metadata directly there might
> play bad; if not now, then later.

I was thinking to use a TLV approach for it (so a variable struct), but then
I decided to implement the simplest solution for the moment since, using TLV,
we would need to add parsing logic and waste at least 2B for each meta info
to store the type and length. Moreover, with XDP we have 256B available for
headeroom and for xdp_frame we would use the same cacheline of the current
implementation:

struct xdp_frame {
	void *                     data;                 /*     0     8 */
	u16                        len;                  /*     8     2 */
	u16                        headroom;             /*    10     2 */
	u32                        metasize;             /*    12     4 */
	struct xdp_mem_info        mem;                  /*    16     8 */
	struct net_device *        dev_rx;               /*    24     8 */
	u32                        frame_sz;             /*    32     4 */
	u32                        flags;                /*    36     4 */
	struct xdp_rx_meta         rx_meta;              /*    40    12 */

	/* size: 56, cachelines: 1, members: 9 */
	/* padding: 4 */
	/* last cacheline: 56 bytes */
};

Anyway I do not have a strong opinion about it and I am fine to covert the
current implementation to a TLV one if we agree on it.

> 
> Our idea (me + Toke) was as follows:
> 
> - new BPF kfunc to build generic meta. If called, the driver builds a
>   generic meta with hash, csum etc., in the data_meta area.
>   Yes, this also consumes headroom, but only when the corresponding func
>   is called. Introducing new fields like you're doing will consume it
>   unconditionally;

ack, I am currently reusing the kfuncs added by Stanislav but I agree it is
better to add a new one to store the rx hw hints info, I will work on it.

> - when &xdp_frame gets converted to sk_buff, the function checks whether
>   data_meta contains a generic structure filled with hints.
> 
> We also thought about &skb_shared_info, but it's also size-sensitive as
> it consumes tailroom.

for rx_timestamp we can reuse the field available in the skb_shared_info.

Regards,
Lorenzo

> 
> > one as a container to store the already supported xdp rx hw hints (rx_hash
> > and rx_vlan, rx_timestamp will be stored in skb_shared_info area) when the
> > eBPF program running on the nic performs XDP_REDIRECT. Doing so, we are able
> > to set the skb metadata converting the xdp_buff/xdp_frame to a skb.
> 
> Thanks,
> Olek
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)