| Message ID | 20250210133002.883422-6-shaw.leon@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | net: Improve netns handling in rtnetlink | expand |
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)
On Thu, Feb 13, 2025 at 2:20 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote: > > 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. > 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.
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h index 1aa31bdb2b31..ae1f2dda4533 100644 --- a/include/net/ip_tunnels.h +++ b/include/net/ip_tunnels.h @@ -406,8 +406,9 @@ int ip_tunnel_rcv(struct ip_tunnel *tunnel, struct sk_buff *skb, bool log_ecn_error); int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[], struct ip_tunnel_parm_kern *p, __u32 fwmark); -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); void ip_tunnel_setup(struct net_device *dev, unsigned int net_id); bool ip_tunnel_netlink_encap_parms(struct nlattr *data[], 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, + fwmark); } static int erspan_newlink(struct net_device *dev, @@ -1433,7 +1434,8 @@ static int erspan_newlink(struct net_device *dev, err = erspan_netlink_parms(dev, data, tb, &p, &fwmark); if (err) return err; - return ip_tunnel_newlink(dev, tb, &p, fwmark); + return ip_tunnel_newlink(params->link_net ? : dev_net(dev), dev, tb, &p, + fwmark); } static int ipgre_changelink(struct net_device *dev, struct nlattr *tb[], @@ -1701,7 +1703,7 @@ static struct rtnl_link_ops erspan_link_ops __read_mostly = { struct net_device *gretap_fb_dev_create(struct net *net, const char *name, u8 name_assign_type) { - struct rtnl_newlink_params params = { .net = net }; + struct rtnl_newlink_params params = { .src_net = net }; struct nlattr *tb[IFLA_MAX + 1]; struct net_device *dev; LIST_HEAD(list_kill); 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); + strscpy(tunnel->parms.name, dev->name); iph->version = 4; iph->ihl = 5; diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c index b901bee03e6d..159b4473290e 100644 --- a/net/ipv4/ip_vti.c +++ b/net/ipv4/ip_vti.c @@ -585,7 +585,8 @@ static int vti_newlink(struct net_device *dev, __u32 fwmark = 0; vti_netlink_parms(data, &parms, &fwmark); - return ip_tunnel_newlink(dev, tb, &parms, fwmark); + return ip_tunnel_newlink(params->link_net ? : dev_net(dev), dev, tb, + &parms, fwmark); } static int vti_changelink(struct net_device *dev, struct nlattr *tb[], diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c index a8b844bcfc64..bab0bf90c908 100644 --- a/net/ipv4/ipip.c +++ b/net/ipv4/ipip.c @@ -455,7 +455,8 @@ static int ipip_newlink(struct net_device *dev, } ipip_netlink_parms(data, &p, &t->collect_md, &fwmark); - return ip_tunnel_newlink(dev, tb, &p, fwmark); + return ip_tunnel_newlink(params->link_net ? : dev_net(dev), dev, tb, &p, + fwmark); } static int ipip_changelink(struct net_device *dev, struct nlattr *tb[],
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(). Signed-off-by: Xiao Liang <shaw.leon@gmail.com> --- include/net/ip_tunnels.h | 5 +++-- net/ipv4/ip_gre.c | 8 +++++--- net/ipv4/ip_tunnel.c | 10 ++++++---- net/ipv4/ip_vti.c | 3 ++- net/ipv4/ipip.c | 3 ++- 5 files changed, 18 insertions(+), 11 deletions(-)