MIME-Version: 1.0
References: <20230309134718.306570-1-gavinl@nvidia.com> <20230309134718.306570-5-gavinl@nvidia.com>
In-Reply-To: <20230309134718.306570-5-gavinl@nvidia.com>
From:   Eric Dumazet <edumazet@google.com>
Date:   Thu, 9 Mar 2023 15:59:51 +0100
Message-ID: <CANn89i+k3fcSw58owpr70eM_uSM5QUqEb_4y5wpXOKEz30+vvg@mail.gmail.com>
Subject: Re: [PATCH net-next v6 4/5] ip_tunnel: Preserve pointer const in ip_tunnel_info_opts
To:     Gavin Li <gavinl@nvidia.com>
Cc:     davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com,
        roopa@nvidia.com, eng.alaamohamedsoliman.am@gmail.com,
        bigeasy@linutronix.de, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, gavi@nvidia.com, roid@nvidia.com,
        maord@nvidia.com, saeedm@nvidia.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Mar 9, 2023 at 2:48=E2=80=AFPM Gavin Li <gavinl@nvidia.com> wrote:
>
> Change ip_tunnel_info_opts( ) from static function to macro to cast retur=
n
> value and preserve the const-ness of the pointer.
>
> Signed-off-by: Gavin Li <gavinl@nvidia.com>
> ---
>  include/net/ip_tunnels.h | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
> index fca357679816..3e5c102b841f 100644
> --- a/include/net/ip_tunnels.h
> +++ b/include/net/ip_tunnels.h
> @@ -67,6 +67,12 @@ struct ip_tunnel_key {
>         GENMASK((sizeof_field(struct ip_tunnel_info,            \
>                               options_len) * BITS_PER_BYTE) - 1, 0)
>
> +#define ip_tunnel_info_opts(info)                              \
> +       _Generic(info,                                          \
> +               const typeof(*(info)) * : ((const void *)((info) + 1)),\
> +               default : ((void *)((info) + 1))                \
> +       )
> +

Hmm...

This looks quite dangerous, we lost type safety with the 'default'
case, with all these casts.

What about using something cleaner instead ?

(Not sure why we do not have an available generic helper for this kind
of repetitive things)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index fca3576798166416982ee6a9100b003810c49830..7f26e07c5f3059d426b31529e4d=
1c3adec23d70f
100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -485,10 +485,11 @@ static inline void iptunnel_xmit_stats(struct
net_device *dev, int pkt_len)
        }
 }

-static inline void *ip_tunnel_info_opts(struct ip_tunnel_info *info)
-{
-       return info + 1;
-}
+#define ip_tunnel_info_opts(info)                              \
+       (_Generic(info,                                         \
+                const struct ip_tunnel_info * : ((info) + 1),  \
+                struct ip_tunnel_info * : ((info) + 1))        \
+       )

 static inline void ip_tunnel_info_opts_get(void *to,
                                           const struct ip_tunnel_info *inf=
o)