| Message ID | 20241023023146.372653-2-shaw.leon@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: Improve netns handling in RTNL and ip_tunnel | expand |
From: Xiao Liang <shaw.leon@gmail.com> Date: Wed, 23 Oct 2024 10:31:42 +0800 > When creating link, lookup for existing device in target net namespace > instead of current one. > For example, two links created by: > > # ip link add dummy1 type dummy > # ip link add netns ns1 dummy1 type dummy > > should have no conflict since they are in different namespaces. > > Signed-off-by: Xiao Liang <shaw.leon@gmail.com> > --- > net/core/rtnetlink.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index 194a81e5f608..ff8d25acfc00 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -3733,20 +3733,24 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, > { > struct nlattr ** const tb = tbs->tb; > struct net *net = sock_net(skb->sk); > + struct net *device_net; > struct net_device *dev; > struct ifinfomsg *ifm; > bool link_specified; > > + /* When creating, lookup for existing device in target net namespace */ > + device_net = nlh->nlmsg_flags & NLM_F_CREATE ? tgt_net : net; Technically, this changes uAPI behaviour. Let's say a user wants to 1) move the device X in the current netns to another if exists, otherwise 2) create a new device X in the target netns This can be achieved by setting NLM_F_CREATE and IFLA_NET_NS_PID, IFLA_NET_NS_FD, or IFLA_TARGET_NETNSID. But with this change, the device X in the current netns will not be moved, and a new device X is created in the target netns. > + > ifm = nlmsg_data(nlh); > if (ifm->ifi_index > 0) { > link_specified = true; > - dev = __dev_get_by_index(net, ifm->ifi_index); > + dev = __dev_get_by_index(device_net, ifm->ifi_index); > } else if (ifm->ifi_index < 0) { > NL_SET_ERR_MSG(extack, "ifindex can't be negative"); > return -EINVAL; > } else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME]) { > link_specified = true; > - dev = rtnl_dev_get(net, tb); > + dev = rtnl_dev_get(device_net, tb); > } else { > link_specified = false; > dev = NULL; > -- > 2.47.0
On Wed, Oct 23, 2024 at 11:49 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote: > > > --- a/net/core/rtnetlink.c > > +++ b/net/core/rtnetlink.c > > @@ -3733,20 +3733,24 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, > > { > > struct nlattr ** const tb = tbs->tb; > > struct net *net = sock_net(skb->sk); > > + struct net *device_net; > > struct net_device *dev; > > struct ifinfomsg *ifm; > > bool link_specified; > > > > + /* When creating, lookup for existing device in target net namespace */ > > + device_net = nlh->nlmsg_flags & NLM_F_CREATE ? tgt_net : net; > > Technically, this changes uAPI behaviour. > > Let's say a user wants to > > 1) move the device X in the current netns to another if exists, otherwise > 2) create a new device X in the target netns > > This can be achieved by setting NLM_F_CREATE and IFLA_NET_NS_PID, > IFLA_NET_NS_FD, or IFLA_TARGET_NETNSID. > > But with this change, the device X in the current netns will not be moved, > and a new device X is created in the target netns. You're right, what about testing for NLM_F_EXCL aslo?
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 194a81e5f608..ff8d25acfc00 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3733,20 +3733,24 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, { struct nlattr ** const tb = tbs->tb; struct net *net = sock_net(skb->sk); + struct net *device_net; struct net_device *dev; struct ifinfomsg *ifm; bool link_specified; + /* When creating, lookup for existing device in target net namespace */ + device_net = nlh->nlmsg_flags & NLM_F_CREATE ? tgt_net : net; + ifm = nlmsg_data(nlh); if (ifm->ifi_index > 0) { link_specified = true; - dev = __dev_get_by_index(net, ifm->ifi_index); + dev = __dev_get_by_index(device_net, ifm->ifi_index); } else if (ifm->ifi_index < 0) { NL_SET_ERR_MSG(extack, "ifindex can't be negative"); return -EINVAL; } else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME]) { link_specified = true; - dev = rtnl_dev_get(net, tb); + dev = rtnl_dev_get(device_net, tb); } else { link_specified = false; dev = NULL;
When creating link, lookup for existing device in target net namespace instead of current one. For example, two links created by: # ip link add dummy1 type dummy # ip link add netns ns1 dummy1 type dummy should have no conflict since they are in different namespaces. Signed-off-by: Xiao Liang <shaw.leon@gmail.com> --- net/core/rtnetlink.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)