netdev - Re: [PATCH net-next v9 05/11] net: ip_tunnel: Use link netns in newlink() of rtnl_link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABAhCOSqruMoMTg_=6Apo=gvnfe1j2fptADzoi=Gb8cdJqhgVw@mail.gmail.com>
Date: Thu, 13 Feb 2025 16:27:21 +0800
From: Xiao Liang <shaw.leon@...il.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: alex.aring@...il.com, andrew+netdev@...n.ch, 
	b.a.t.m.a.n@...ts.open-mesh.org, bpf@...r.kernel.org, bridge@...ts.linux.dev, 
	davem@...emloft.net, donald.hunter@...il.com, dsahern@...nel.org, 
	edumazet@...gle.com, herbert@...dor.apana.org.au, horms@...nel.org, 
	kuba@...nel.org, linux-can@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, linux-ppp@...r.kernel.org, 
	linux-rdma@...r.kernel.org, linux-wireless@...r.kernel.org, 
	linux-wpan@...r.kernel.org, miquel.raynal@...tlin.com, netdev@...r.kernel.org, 
	osmocom-net-gprs@...ts.osmocom.org, pabeni@...hat.com, shuah@...nel.org, 
	stefan@...enfreihafen.org, steffen.klassert@...unet.com, 
	wireguard@...ts.zx2c4.com
Subject: Re: [PATCH net-next v9 05/11] net: ip_tunnel: Use link netns in
 newlink() of rtnl_link_ops

On Thu, Feb 13, 2025 at 2:20 PM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
>
> From: Xiao Liang <shaw.leon@...il.com>
> Date: Mon, 10 Feb 2025 21:29:56 +0800
> > When link_net is set, use it as link netns instead of dev_net(). This
> > prepares for rtnetlink core to create device in target netns directly,
> > in which case the two namespaces may be different.
> >
> > Convert common ip_tunnel_newlink() to accept an extra link netns
> > argument. Don't overwrite ip_tunnel.net in ip_tunnel_init().
>
> Why... ?  see a comment below.
>
>
> [...]
> > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > index 1fe9b13d351c..26d15f907551 100644
> > --- a/net/ipv4/ip_gre.c
> > +++ b/net/ipv4/ip_gre.c
> > @@ -1413,7 +1413,8 @@ static int ipgre_newlink(struct net_device *dev,
> >       err = ipgre_netlink_parms(dev, data, tb, &p, &fwmark);
> >       if (err < 0)
> >               return err;
> > -     return ip_tunnel_newlink(dev, tb, &p, fwmark);
> > +     return ip_tunnel_newlink(params->link_net ? : dev_net(dev), dev, tb, &p,
>
> This is duplicate at all call sites, let's move it into
> ip_tunnel_newlink() by passing params.
>

Existing tunnels use `params->link_net ? : dev_net(dev)` for
backward compatibility. But I think we can leave the choice of netns
to future tunnel drivers because rtnl_newlink_link_net() is preferred
in general.

>
> > +                              fwmark);
> >  }
> >
> >  static int erspan_newlink(struct net_device *dev,
> >
> >
> > diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> > index 09b73acf037a..618a50d5c0c2 100644
> > --- a/net/ipv4/ip_tunnel.c
> > +++ b/net/ipv4/ip_tunnel.c
> > @@ -1213,11 +1213,11 @@ void ip_tunnel_delete_nets(struct list_head *net_list, unsigned int id,
> >  }
> >  EXPORT_SYMBOL_GPL(ip_tunnel_delete_nets);
> >
> > -int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
> > -                   struct ip_tunnel_parm_kern *p, __u32 fwmark)
> > +int ip_tunnel_newlink(struct net *net, struct net_device *dev,
> > +                   struct nlattr *tb[], struct ip_tunnel_parm_kern *p,
> > +                   __u32 fwmark)
> >  {
> >       struct ip_tunnel *nt;
> > -     struct net *net = dev_net(dev);
> >       struct ip_tunnel_net *itn;
> >       int mtu;
> >       int err;
> > @@ -1326,7 +1326,9 @@ int ip_tunnel_init(struct net_device *dev)
> >       }
> >
> >       tunnel->dev = dev;
> > -     tunnel->net = dev_net(dev);
> > +     if (!tunnel->net)
> > +             tunnel->net = dev_net(dev);
>
> Isn't tunnel->net always non-NULL ?
>
> ip_tunnel_newlink
> -> netdev_priv(dev)->net = net
> -> register_netdevice(dev)
>   -> dev->netdev_ops->ndo_init(dev)
>     -> ip_tunnel_init(dev)
>       -> netdev_priv(dev)->net = dev_net(dev)

Didn't find a path that can leave tunnel->net to NULL either.
I think we can remove this.

Thanks.