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
