Re: [PATCH net-next v6 4/5] ip_tunnel: Preserve pointer const in ip_tunnel_info_opts

[Simon Horman]

On Thu, Mar 09, 2023 at 04:05:13PM +0100, Eric Dumazet wrote:
> On Thu, Mar 9, 2023 at 3:59 PM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
> >
> > On Thu, Mar 9, 2023 at 2:48 PM Gavin Li <gavinl@xxxxxxxxxx> wrote:
> > >
> > > Change ip_tunnel_info_opts( ) from static function to macro to cast return
> > > value and preserve the const-ness of the pointer.
> > >
> > > Signed-off-by: Gavin Li <gavinl@xxxxxxxxxx>
> > > ---
> > >  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.

Thanks Eric,

FWIIW, the feedback I gave earlier, which partly brought about this change,
was based around avoiding casts...

> >
> > What about using something cleaner instead ?
> >
> > (Not sure why we do not have an available generic helper for this kind
> > of repetitive things)
> >
> 
> Or more exactly :

... So, yes, this looks better to me.

> diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
> index fca3576798166416982ee6a9100b003810c49830..17fc6c8f7e0b9e5303c1fb9e5dad77c5310e01a9
> 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 * : (const void *)((info)
> + 1),    \
> +                struct ip_tunnel_info * : (void *)((info) + 1))        \
> +       )
> 
>  static inline void ip_tunnel_info_opts_get(void *to,
>                                            const struct ip_tunnel_info *info)
>