On 12/02/2025 18:29, Alexander Lobakin wrote:
> From: Gal Pressman <[email protected]>
> Date: Wed, 12 Feb 2025 16:09:53 +0200
>
>> Remove the hidden assumption that options are allocated at the end of
>> the struct, and teach the compiler about them using a flexible array.
>
> [...]
>
>> diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h
>> index 84c15402931c..4160731dcb6e 100644
>> --- a/include/net/dst_metadata.h
>> +++ b/include/net/dst_metadata.h
>> @@ -163,11 +163,8 @@ static inline struct metadata_dst
>> *tun_dst_unclone(struct sk_buff *skb)
>> if (!new_md)
>> return ERR_PTR(-ENOMEM);
>>
>> - unsafe_memcpy(&new_md->u.tun_info, &md_dst->u.tun_info,
>> - sizeof(struct ip_tunnel_info) + md_size,
>> - /* metadata_dst_alloc() reserves room (md_size bytes) for
>> - * options right after the ip_tunnel_info struct.
>> - */);
>> + memcpy(&new_md->u.tun_info, &md_dst->u.tun_info,
>> + sizeof(struct ip_tunnel_info) + md_size);
>> #ifdef CONFIG_DST_CACHE
>> /* Unclone the dst cache if there is one */
>> if (new_md->u.tun_info.dst_cache.cache) {
>> diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
>> index 1aa31bdb2b31..517f78070be0 100644
>> --- a/include/net/ip_tunnels.h
>> +++ b/include/net/ip_tunnels.h
>> @@ -93,12 +93,6 @@ struct ip_tunnel_encap {
>> GENMASK((sizeof_field(struct ip_tunnel_info, \
>> options_len) * BITS_PER_BYTE) - 1, 0)
>>
>> -#define ip_tunnel_info_opts(info) \
>> - _Generic(info, \
>> - const struct ip_tunnel_info * : ((const void *)((info) + 1)),\
>> - struct ip_tunnel_info * : ((void *)((info) + 1))\
>> - )
>
> You could leave this macro inplace and just change `(info) + 1` to
> `(info)->options` avoiding changes in lots of files and adding casts
> everywhere.
I'd rather not, having a macro to do 'info->options' doesn't help code
readability IMHO.
>
>> -
>> struct ip_tunnel_info {
>> struct ip_tunnel_key key;
>> struct ip_tunnel_encap encap;
>> @@ -107,6 +101,7 @@ struct ip_tunnel_info {
>> #endif
>> u8 options_len;
>> u8 mode;
>> + u8 options[] __aligned(sizeof(void *))
>> __counted_by(options_len);
>
> Since 96 % 16 == 0, I'd check if __aligned_largest would change the
> bytecode anyhow... Sometimes it does.
__aligned_largest aligns the field to 16 bytes, so the struct remains
the same (96 bytes).
When should __aligned_larget be used? What needs 16 bytes alignment?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
