Re: [PATCH RESEND bpf-next 14/15] net, xdp: allow metadata > 32

[Jesper Dangaard Brouer]

On 16/05/2023 14.37, Alexander Lobakin wrote:
> From: Larysa Zaremba<larysa.zaremba@xxxxxxxxx>
> Date: Mon, 15 May 2023 19:08:39 +0200
>
> > On Mon, May 15, 2023 at 06:17:02PM +0200, Jesper Dangaard Brouer wrote:
> > > On 12/05/2023 17.26, Larysa Zaremba wrote:
> > > > From: Aleksander Lobakin<aleksander.lobakin@xxxxxxxxx>
> > > >
> > > > When using XDP hints, metadata sometimes has to be much bigger
> > > > than 32 bytes. Relax the restriction, allow metadata larger than 32 bytes
> > > > and make __skb_metadata_differs() work with bigger lengths.
> > > >
> > > > Now size of metadata is only limited by the fact it is stored as u8
> > > > in skb_shared_info, so maximum possible value is 255.
>>>
> > > I'm confused, IIRC the metadata area isn't stored "in skb_shared_info".
> > > The maximum possible size is limited by the XDP headroom, which is also
> > > shared/limited with/by xdp_frame.  I must be reading the sentence wrong,
> > > somehow.
>
> skb_shared_info::meta_size  is u8. Since metadata gets carried from
> xdp_buff to skb, this check is needed (it's compile-time constant anyway).
> Check for headroom is done separately already (two sentences below).

Damn, argh, for SKBs the "meta_len" is stored in skb_shared_info, which
is located on another cacheline.
That is a sure way to KILL performance! :-(

But only use for SKBs that gets created from xdp with metadata, right?



> > It's not 'metadata is stored as u8', it's 'metadata size is stored as u8' :)
> > Maybe I should rephrase it better in v2.

Yes, a rephrase will be good.

--Jesper



static inline u8 skb_metadata_len(const struct sk_buff *skb)
{
	return skb_shinfo(skb)->meta_len;
}