netdev - Re: [PATCH bpf-next v2 15/20] net, xdp: allow metadata

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 for Android: free password hash cracker in your pocket

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

Date: Thu, 6 Jul 2023 16:51:22 +0200
From: Larysa Zaremba <larysa.zaremba@...el.com>
To: John Fastabend <john.fastabend@...il.com>
CC: <bpf@...r.kernel.org>, <ast@...nel.org>, <daniel@...earbox.net>,
	<andrii@...nel.org>, <martin.lau@...ux.dev>, <song@...nel.org>, <yhs@...com>,
	<kpsingh@...nel.org>, <sdf@...gle.com>, <haoluo@...gle.com>,
	<jolsa@...nel.org>, David Ahern <dsahern@...il.com>, Jakub Kicinski
	<kuba@...nel.org>, Willem de Bruijn <willemb@...gle.com>, "Jesper Dangaard
 Brouer" <brouer@...hat.com>, Anatoly Burakov <anatoly.burakov@...el.com>,
	Alexander Lobakin <alexandr.lobakin@...el.com>, Magnus Karlsson
	<magnus.karlsson@...il.com>, Maryam Tahhan <mtahhan@...hat.com>,
	<xdp-hints@...-project.net>, <netdev@...r.kernel.org>, Aleksander Lobakin
	<aleksander.lobakin@...el.com>
Subject: Re: [PATCH bpf-next v2 15/20] net, xdp: allow metadata > 32

On Mon, Jul 03, 2023 at 02:06:46PM -0700, John Fastabend wrote:
> Larysa Zaremba wrote:
> > From: Aleksander Lobakin <aleksander.lobakin@...el.com>
> > 
> > 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. Other important
> > conditions, such as having enough space for xdp_frame building, are already
> > checked in bpf_xdp_adjust_meta().
> > 
> > The requirement of having its length aligned to 4 bytes is still
> > valid.
> > 
> > Signed-off-by: Aleksander Lobakin <aleksander.lobakin@...el.com>
> > Signed-off-by: Larysa Zaremba <larysa.zaremba@...el.com>
> > ---
> >  include/linux/skbuff.h | 13 ++++++++-----
> >  include/net/xdp.h      |  7 ++++++-
> >  2 files changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index 91ed66952580..cd49cdd71019 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -4209,10 +4209,13 @@ static inline bool __skb_metadata_differs(const struct sk_buff *skb_a,
> >  {
> >  	const void *a = skb_metadata_end(skb_a);
> >  	const void *b = skb_metadata_end(skb_b);
> > -	/* Using more efficient varaiant than plain call to memcmp(). */
> > -#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && BITS_PER_LONG == 64
> 
> Why are we removing the ifdef here? Its adding a runtime 'if' when its not
> necessary. I would keep the ifdef and simply add the default case
> in the switch.

Seems like Alex has missed your message, but we discussed this with him before, 
so I know the answer: Compiler will 100% convert it into a compile-time 'if' and 
this looks nicer than preprocessor condition.