Re: [PATCH net-next v2] net: Add options as a flexible array to struct ip_tunnel_info

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Gal Pressman <gal@nvidia.com>
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.

> -
>  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.

Thanks,
Olek