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

[Larysa Zaremba <larysa.zaremba@intel.com>]

On Mon, Jul 03, 2023 at 02:06:46PM -0700, John Fastabend wrote:
> Larysa Zaremba wrote:
> > From: Aleksander Lobakin <aleksander.lobakin@intel.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@intel.com>
> > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.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.