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

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Xiao Liang <shaw.leon@gmail.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.


> +				 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)