From: Larysa Zaremba <larysa.zaremba@xxxxxxxxx> Date: Thu, 6 Jul 2023 16:51:22 +0200 > On Mon, Jul 03, 2023 at 02:06:46PM -0700, John Fastabend wrote: >> 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. 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@xxxxxxxxx> >>> Signed-off-by: Larysa Zaremba <larysa.zaremba@xxxxxxxxx> >>> --- >>> 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. Sorry, I'm not always able to follow all the threads =\ As Larysa said, it's not a runtime `if`. Both conditions are always known at compilation time. And this looks a bit less ugly than with ifdefs to me :D Thanks, Olek