Problem with iflink in netns

Problem with iflink in netns

[Stephen Hemminger]

There have been a couple of bugzilla reports already about cases where a macvlan
or vlan is moved into another namespace. In these cases the parent device ifindex (iflink)
is no longer valid.


Normally it is not a big issue, until another device is created using the ifindex
of the parent

Does anyone have a suggested fix? Marking iflink as 0 won't work then the devices no
longer appear as slaves. Another possibility would be to make block creation of device
where ifindex matches existing iflink of other devices; but this would slow down device
creation.
 

The bugs come in as ip command bugs, but obviously the issue is in the kernel.

https://bugzilla.kernel.org/show_bug.cgi?id=66691
https://bugzilla.kernel.org/show_bug.cgi?id=75911

Re: Problem with iflink in netns

[Cong Wang]

On Tue, May 13, 2014 at 8:39 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> There have been a couple of bugzilla reports already about cases where a macvlan
> or vlan is moved into another namespace. In these cases the parent device ifindex (iflink)
> is no longer valid.
>
>
> Normally it is not a big issue, until another device is created using the ifindex
> of the parent
>
> Does anyone have a suggested fix? Marking iflink as 0 won't work then the devices no
> longer appear as slaves. Another possibility would be to make block creation of device
> where ifindex matches existing iflink of other devices; but this would slow down device
> creation.
>

I have a pending fix which converts ->iflink to a pointer to struct net_device,
this is a suggested fix from Ben. And so, when the lower device is not
in the same
namespace with the upper device, we should not display the iflink information.

I will send it soon.

Re: Problem with iflink in netns

[Stephen Hemminger]

On Tue, 13 May 2014 11:20:58 -0700
Cong Wang <cwang@twopensource.com> wrote:

> On Tue, May 13, 2014 at 8:39 AM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > There have been a couple of bugzilla reports already about cases where a macvlan
> > or vlan is moved into another namespace. In these cases the parent device ifindex (iflink)
> > is no longer valid.
> >
> >
> > Normally it is not a big issue, until another device is created using the ifindex
> > of the parent
> >
> > Does anyone have a suggested fix? Marking iflink as 0 won't work then the devices no
> > longer appear as slaves. Another possibility would be to make block creation of device
> > where ifindex matches existing iflink of other devices; but this would slow down device
> > creation.
> >
> 
> I have a pending fix which converts ->iflink to a pointer to struct net_device,
> this is a suggested fix from Ben. And so, when the lower device is not
> in the same
> namespace with the upper device, we should not display the iflink information.
> 
> I will send it soon.

That won't solve the problem.
 1. a pointer increase size of netns and requires refcount
 2. iflink is still exposed in kernel user API and has to be set
    otherwise the device is falsely reported.

Re: Problem with iflink in netns

[Cong Wang]

On Tue, May 13, 2014 at 1:05 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> That won't solve the problem.
>  1. a pointer increase size of netns and requires refcount

Yes.

>  2. iflink is still exposed in kernel user API and has to be set
>     otherwise the device is falsely reported.

Then report 0 which is not a valid ifindex?

Re: Problem with iflink in netns

[Stephen Hemminger]

On Tue, 13 May 2014 13:44:24 -0700
Cong Wang <cwang@twopensource.com> wrote:

> On Tue, May 13, 2014 at 1:05 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > That won't solve the problem.
> >  1. a pointer increase size of netns and requires refcount
> 
> Yes.
> 
> >  2. iflink is still exposed in kernel user API and has to be set
> >     otherwise the device is falsely reported.
> 
> Then report 0 which is not a valid ifindex?

But that is what devices without a parent report.

Re: Problem with iflink in netns

[Nicolas Dichtel]

Le 13/05/2014 17:39, Stephen Hemminger a écrit :
> There have been a couple of bugzilla reports already about cases where a macvlan
> or vlan is moved into another namespace. In these cases the parent device ifindex (iflink)
> is no longer valid.
>
>
> Normally it is not a big issue, until another device is created using the ifindex
> of the parent
>
> Does anyone have a suggested fix? Marking iflink as 0 won't work then the devices no
> longer appear as slaves. Another possibility would be to make block creation of device
> where ifindex matches existing iflink of other devices; but this would slow down device
> creation.
The problem is that we to add an information about the netns where the ifindex
stands, we can currently use only a pid or a file descriptor, hence it's not
possible to broadcast this information. The advantage of the file descriptor is
that it's a local id not a global one.

On idea I'm thinking is that each netns manages its own set of UID for peer
netns, this means that these UID will be valid only in a spcecified netns.
We may add a netlink message to help the user to associate an UID with a file
descriptor/pid (he gives the file descriptor/pid and the kernel returns the
UID).
These UID may be generated only the user requests them or when the kernel sends
an information about a peer netns.
These UID will be provided in existing netlink message in a separate netlink
attribute.

If the idea is ok, I can help to work on this topic.


Regards,
Nicolas
>
>
> The bugs come in as ip command bugs, but obviously the issue is in the kernel.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=66691
> https://bugzilla.kernel.org/show_bug.cgi?id=75911

Re: Problem with iflink in netns

[Cong Wang]

On Tue, May 13, 2014 at 5:51 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Tue, 13 May 2014 13:44:24 -0700
> Cong Wang <cwang@twopensource.com> wrote:
>
>> On Tue, May 13, 2014 at 1:05 PM, Stephen Hemminger
>> <stephen@networkplumber.org> wrote:
>> >
>> > That won't solve the problem.
>> >  1. a pointer increase size of netns and requires refcount
>>
>> Yes.
>>
>> >  2. iflink is still exposed in kernel user API and has to be set
>> >     otherwise the device is falsely reported.
>>
>> Then report 0 which is not a valid ifindex?
>
> But that is what devices without a parent report.

Its parent of out of this netns, so this is expected. This is no way
to express "it's in another netns" with just ifindex. Maybe a new flag
would help.

[RFC PATCH net-next 0/5] netns: allow to identify peer netns

[Nicolas Dichtel]

The goal of this serie is to be able to multicast netlink messages with an
attribute that identify a peer netns.
This is needed by the userland to interpret some informations contained in
netlink messages (like IFLA_LINK value, but also some other attributes in case
of x-netns netdevice (see also
http://thread.gmane.org/gmane.linux.network/315933/focus=316064)).

Each network namespaces allocates its own ids for other netns (including
itself). The user can retrieve these ids via a new netlink messages, but only
if he has a FD which points to this netns. Dump is not implemented so that a
user cannot get the whole netns list.

The goal of this RFC is mainly to validate the principle, ie patch 1/5 and 2/5.
Patch 3/5 and 4/5 shows an example of how to use these ids in rtnetlink
messages. And patch 5/5 shows that the netlink messages can be symetric between
a GET and a SET.

iproute2 patches are available, I can send them on demand.

 include/net/ip_tunnels.h     |   1 +
 include/net/net_namespace.h  |   6 ++
 include/net/rtnetlink.h      |   2 +
 include/uapi/linux/Kbuild    |   1 +
 include/uapi/linux/if_link.h |   1 +
 include/uapi/linux/netns.h   |  27 ++++++++
 net/core/net_namespace.c     | 143 +++++++++++++++++++++++++++++++++++++++++++
 net/core/rtnetlink.c         |  39 +++++++++++-
 net/ipv4/ip_gre.c            |   2 +
 net/ipv4/ip_tunnel.c         |   8 +++
 net/ipv4/ip_vti.c            |   1 +
 net/ipv4/ipip.c              |   1 +
 net/ipv6/sit.c               |   1 +
 net/netlink/genetlink.c      |   4 ++
 14 files changed, 234 insertions(+), 3 deletions(-)

Comments are welcome.

Regards,
Nicolas

[RFC PATCH net-next 1/5] netns: allocate netns ids

[Nicolas Dichtel]

With this patch, netns allocates ids for all netns. Each netns has its own list
of ids, it means that the id is valid only in the current netns.

For homogeneity, an id is also allocated for the netns owner itself.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/net/net_namespace.h |  5 +++++
 net/core/net_namespace.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 361d26077196..056d6eeee318 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -121,6 +121,8 @@ struct net {
 #endif
 	struct net_generic __rcu	*gen;
 
+	struct idr		netns_ids;
+
 	/* Note : following structs are cache line aligned */
 #ifdef CONFIG_XFRM
 	struct netns_xfrm	xfrm;
@@ -289,6 +291,9 @@ static inline struct net *read_pnet(struct net * const *pnet)
 #define __net_initconst	__initconst
 #endif
 
+int net2id(struct net *net, struct net *peer);
+struct net *get_net_from_netnsid(struct net *net, int id);
+
 struct pernet_operations {
 	struct list_head list;
 	int (*init)(struct net *net);
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 85b62691f4f2..2976864cef13 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -144,6 +144,30 @@ static void ops_free_list(const struct pernet_operations *ops,
 	}
 }
 
+static int net_eq_idr(int id, void *net, void *peer)
+{
+	return net_eq(net, peer) ? id : 0;
+}
+
+int net2id(struct net *net, struct net *peer)
+{
+	ASSERT_RTNL();
+	return idr_for_each(&(net)->netns_ids, net_eq_idr, (peer)) ? : -ENOENT;
+}
+
+struct net *get_net_from_netnsid(struct net *net, int id)
+{
+	struct net *peer;
+
+	rcu_read_lock();
+	peer = idr_find(&(net)->netns_ids, id);
+	if (peer)
+		get_net(peer);
+	rcu_read_unlock();
+
+	return peer;
+}
+
 /*
  * setup_net runs the initializers for the network namespace object.
  */
@@ -158,6 +182,7 @@ static __net_init int setup_net(struct net *net, struct user_namespace *user_ns)
 	atomic_set(&net->passive, 1);
 	net->dev_base_seq = 1;
 	net->user_ns = user_ns;
+	idr_init(&net->netns_ids);
 
 #ifdef NETNS_REFCNT_DEBUG
 	atomic_set(&net->use_count, 0);
@@ -253,7 +278,20 @@ struct net *copy_net_ns(unsigned long flags,
 	mutex_lock(&net_mutex);
 	rv = setup_net(net, user_ns);
 	if (rv == 0) {
+		struct net *tmp;
+
 		rtnl_lock();
+		idr_alloc_cyclic(&net->netns_ids, net, 1, 0, GFP_KERNEL);
+		for_each_net(tmp) {
+			/* Even if it fails, let's try to get ids for other
+			 * netns.
+			 */
+			idr_alloc_cyclic(&net->netns_ids, tmp, 1, 0,
+					 GFP_KERNEL);
+			idr_alloc_cyclic(&tmp->netns_ids, net, 1, 0,
+					 GFP_KERNEL);
+		}
+
 		list_add_tail_rcu(&net->list, &net_namespace_list);
 		rtnl_unlock();
 	}
@@ -288,6 +326,9 @@ static void cleanup_net(struct work_struct *work)
 	list_for_each_entry(net, &net_kill_list, cleanup_list) {
 		list_del_rcu(&net->list);
 		list_add_tail(&net->exit_list, &net_exit_list);
+		for_each_net(tmp)
+			idr_remove(&tmp->netns_ids, net2id(tmp, net));
+		idr_destroy(&net->netns_ids);
 	}
 	rtnl_unlock();
 
@@ -423,6 +464,7 @@ static int __init net_ns_init(void)
 		panic("Could not setup the initial network namespace");
 
 	rtnl_lock();
+	idr_alloc_cyclic(&init_net.netns_ids, &init_net, 1, 0, GFP_KERNEL);
 	list_add_tail_rcu(&init_net.list, &net_namespace_list);
 	rtnl_unlock();
 
-- 
1.9.0

[RFC PATCH net-next 2/5] netns: add genl cmd to get the id of a netns

[Nicolas Dichtel]

This patch allows a user to get an id of a peer netns. It will be usefull for
userland to be able to associate a netns file descriptor with a netns id.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/net/net_namespace.h |   1 +
 include/uapi/linux/Kbuild   |   1 +
 include/uapi/linux/netns.h  |  27 ++++++++++++
 net/core/net_namespace.c    | 101 ++++++++++++++++++++++++++++++++++++++++++++
 net/netlink/genetlink.c     |   4 ++
 5 files changed, 134 insertions(+)
 create mode 100644 include/uapi/linux/netns.h

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 056d6eeee318..3a83b3ec833c 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -293,6 +293,7 @@ static inline struct net *read_pnet(struct net * const *pnet)
 
 int net2id(struct net *net, struct net *peer);
 struct net *get_net_from_netnsid(struct net *net, int id);
+int netns_genl_register(void);
 
 struct pernet_operations {
 	struct list_head list;
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 24e9033f8b3f..5da0eef2771f 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -274,6 +274,7 @@ header-y += netfilter_decnet.h
 header-y += netfilter_ipv4.h
 header-y += netfilter_ipv6.h
 header-y += netlink.h
+header-y += netns.h
 header-y += netrom.h
 header-y += nfc.h
 header-y += nfs.h
diff --git a/include/uapi/linux/netns.h b/include/uapi/linux/netns.h
new file mode 100644
index 000000000000..2d69d183919f
--- /dev/null
+++ b/include/uapi/linux/netns.h
@@ -0,0 +1,27 @@
+#ifndef _UAPI_LINUX_NETNS_H_
+#define _UAPI_LINUX_NETNS_H_
+
+/* Generic netlink messages */
+
+#define NETNS_GENL_NAME			"netns"
+#define NETNS_GENL_VERSION		0x1
+
+/* Commands */
+enum {
+	NETNS_CMD_UNSPEC,
+	NETNS_CMD_GET,
+	__NETNS_CMD_MAX,
+};
+#define NETNS_CMD_MAX		(__NETNS_CMD_MAX - 1)
+
+/* Attributes */
+enum {
+	NETNSA_NONE,
+	NETNSA_NSINDEX,
+	NETNSA_PID,
+	NETNSA_FD,
+	__NETNSA_MAX,
+};
+#define NETNSA_MAX		(__NETNSA_MAX - 1)
+
+#endif /* _UAPI_LINUX_NETNS_H_ */
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 2976864cef13..20d6213c82bc 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -15,6 +15,8 @@
 #include <linux/file.h>
 #include <linux/export.h>
 #include <linux/user_namespace.h>
+#include <linux/netns.h>
+#include <net/genetlink.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 
@@ -438,6 +440,105 @@ static struct pernet_operations __net_initdata net_ns_ops = {
 	.exit = net_ns_net_exit,
 };
 
+static struct genl_family netns_nl_family = {
+	.id		= GENL_ID_GENERATE,
+	.name		= NETNS_GENL_NAME,
+	.version	= NETNS_GENL_VERSION,
+	.hdrsize	= 0,
+	.maxattr	= NETNSA_MAX,
+	.netnsok	= true,
+};
+
+static struct nla_policy netns_nl_policy[NETNSA_MAX + 1] = {
+	[NETNSA_NONE]		= { .type = NLA_UNSPEC, },
+	[NETNSA_NSINDEX]	= { .type = NLA_U32, },
+	[NETNSA_PID]		= { .type = NLA_U32 },
+	[NETNSA_FD]		= { .type = NLA_U32 },
+};
+
+static int netns_nl_get_size(void)
+{
+	return nla_total_size(sizeof(u32)) /* NETNSA_NSINDEX */
+	       ;
+}
+
+static int netns_nl_fill(struct sk_buff *skb, u32 portid, u32 seq, int flags,
+			 int cmd, struct net *net, struct net *peer)
+{
+	void *hdr;
+	int id;
+
+	hdr = genlmsg_put(skb, portid, seq, &netns_nl_family, flags, cmd);
+	if (!hdr)
+		return -EMSGSIZE;
+
+	rtnl_lock();
+	id = net2id(net, peer);
+	rtnl_unlock();
+	if (id < 0)
+		return id;
+	if (nla_put_u32(skb, NETNSA_NSINDEX, id))
+		goto nla_put_failure;
+
+	return genlmsg_end(skb, hdr);
+
+nla_put_failure:
+	genlmsg_cancel(skb, hdr);
+	return -EMSGSIZE;
+}
+
+static int netns_nl_cmd_getid(struct sk_buff *skb, struct genl_info *info)
+{
+	struct net *net = genl_info_net(info);
+	struct net *peer;
+	struct sk_buff *msg;
+	int err = -ENOBUFS;
+
+	if (info->attrs[NETNSA_PID])
+		peer = get_net_ns_by_pid(nla_get_u32(info->attrs[NETNSA_PID]));
+	else if (info->attrs[NETNSA_FD])
+		peer = get_net_ns_by_fd(nla_get_u32(info->attrs[NETNSA_FD]));
+	else
+		return -EINVAL;
+
+	if (IS_ERR(peer))
+		return PTR_ERR(peer);
+
+	msg = genlmsg_new(netns_nl_get_size(), GFP_KERNEL);
+	if (!msg) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	err = netns_nl_fill(msg, info->snd_portid, info->snd_seq,
+			    NLM_F_ACK, NETNS_CMD_GETID, net, peer);
+	if (err < 0)
+		goto err_out;
+
+	err = genlmsg_unicast(net, msg, info->snd_portid);
+	goto out;
+
+err_out:
+	nlmsg_free(msg);
+out:
+	put_net(peer);
+	return err;
+}
+
+static struct genl_ops netns_nl_ops[] = {
+	{
+		.cmd = NETNS_CMD_GETID,
+		.policy = netns_nl_policy,
+		.doit = netns_nl_cmd_getid,
+		.flags = GENL_ADMIN_PERM,
+	},
+};
+
+int netns_genl_register(void)
+{
+	return genl_register_family_with_ops(&netns_nl_family, netns_nl_ops);
+}
+
 static int __init net_ns_init(void)
 {
 	struct net_generic *ng;
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 76393f2f4b22..c6f39e40c9f3 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -1029,6 +1029,10 @@ static int __init genl_init(void)
 	if (err)
 		goto problem;
 
+	err = netns_genl_register();
+	if (err < 0)
+		goto problem;
+
 	return 0;
 
 problem:
-- 
1.9.0

[RFC PATCH net-next 3/5] rtnl: add link netns id to interface messages

[Nicolas Dichtel]

This patch adds a new attribute (IFLA_LINK_NETNSID) which contains the 'link'
netns id when this netns is different from the netns where the interface
stands (for example for x-net interfaces like ip tunnels).

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/net/rtnetlink.h      |  2 ++
 include/uapi/linux/if_link.h |  1 +
 net/core/rtnetlink.c         | 14 ++++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 72240e5ac2c4..d94b76dbc6bf 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -46,6 +46,7 @@ static inline int rtnl_msg_family(const struct nlmsghdr *nlh)
  *			    to create when creating a new device.
  *	@get_num_rx_queues: Function to determine number of receive queues
  *			    to create when creating a new device.
+ *	@get_link_net: Function to get the i/o netns of the device
  */
 struct rtnl_link_ops {
 	struct list_head	list;
@@ -93,6 +94,7 @@ struct rtnl_link_ops {
 	int			(*fill_slave_info)(struct sk_buff *skb,
 						   const struct net_device *dev,
 						   const struct net_device *slave_dev);
+	struct net		*(*get_link_net)(const struct net_device *dev);
 };
 
 int __rtnl_link_register(struct rtnl_link_ops *ops);
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index b38534895db5..a7433437da8d 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -145,6 +145,7 @@ enum {
 	IFLA_CARRIER,
 	IFLA_PHYS_PORT_ID,
 	IFLA_CARRIER_CHANGES,
+	IFLA_LINK_NETNSID,
 	__IFLA_MAX
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 27acaf7ff6d7..d99b98b41045 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -861,6 +861,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + nla_total_size(1) /* IFLA_OPERSTATE */
 	       + nla_total_size(1) /* IFLA_LINKMODE */
 	       + nla_total_size(4) /* IFLA_CARRIER_CHANGES */
+	       + nla_total_size(4) /* IFLA_LINK_NETNSID */
 	       + nla_total_size(ext_filter_mask
 			        & RTEXT_FILTER_VF ? 4 : 0) /* IFLA_NUM_VF */
 	       + rtnl_vfinfo_size(dev, ext_filter_mask) /* IFLA_VFINFO_LIST */
@@ -1133,6 +1134,19 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 			goto nla_put_failure;
 	}
 
+	if (dev->rtnl_link_ops &&
+	    dev->rtnl_link_ops->get_link_net) {
+		struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
+
+		if (!net_eq(dev_net(dev), link_net)) {
+			int id = net2id(dev_net(dev), link_net);
+
+			if (id > 0 &&
+			    nla_put_u32(skb, IFLA_LINK_NETNSID, id))
+				goto nla_put_failure;
+		}
+	}
+
 	if (!(af_spec = nla_nest_start(skb, IFLA_AF_SPEC)))
 		goto nla_put_failure;
 
-- 
1.9.0

[RFC PATCH net-next 4/5] iptunnels: advertise link netns via netlink

[Nicolas Dichtel]

Implement rtnl_link_ops->get_link_net() callback so that IFLA_LINK_NETNSID is
added to rtnetlink messages.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 include/net/ip_tunnels.h | 1 +
 net/ipv4/ip_gre.c        | 2 ++
 net/ipv4/ip_tunnel.c     | 8 ++++++++
 net/ipv4/ip_vti.c        | 1 +
 net/ipv4/ipip.c          | 1 +
 net/ipv6/sit.c           | 1 +
 6 files changed, 14 insertions(+)

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index a4daf9eb8562..2da3d9deb33f 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -105,6 +105,7 @@ struct ip_tunnel_net {
 int ip_tunnel_init(struct net_device *dev);
 void ip_tunnel_uninit(struct net_device *dev);
 void  ip_tunnel_dellink(struct net_device *dev, struct list_head *head);
+struct net *ip_tunnel_get_link_net(const struct net_device *dev);
 int ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
 		       struct rtnl_link_ops *ops, char *devname);
 
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 9b842544aea3..5a5034eb8367 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -745,6 +745,7 @@ static struct rtnl_link_ops ipgre_link_ops __read_mostly = {
 	.dellink	= ip_tunnel_dellink,
 	.get_size	= ipgre_get_size,
 	.fill_info	= ipgre_fill_info,
+	.get_link_net	= ip_tunnel_get_link_net,
 };
 
 static struct rtnl_link_ops ipgre_tap_ops __read_mostly = {
@@ -759,6 +760,7 @@ static struct rtnl_link_ops ipgre_tap_ops __read_mostly = {
 	.dellink	= ip_tunnel_dellink,
 	.get_size	= ipgre_get_size,
 	.fill_info	= ipgre_fill_info,
+	.get_link_net	= ip_tunnel_get_link_net,
 };
 
 static int __net_init ipgre_tap_init_net(struct net *net)
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 54b6731dab55..37efd17b4d08 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -854,6 +854,14 @@ void ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
 }
 EXPORT_SYMBOL_GPL(ip_tunnel_dellink);
 
+struct net *ip_tunnel_get_link_net(const struct net_device *dev)
+{
+	struct ip_tunnel *tunnel = netdev_priv(dev);
+
+	return tunnel->net;
+}
+EXPORT_SYMBOL_GPL(ip_tunnel_get_link_net);
+
 int ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
 				  struct rtnl_link_ops *ops, char *devname)
 {
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index b8960f3527f3..11ec1ed545df 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -530,6 +530,7 @@ static struct rtnl_link_ops vti_link_ops __read_mostly = {
 	.changelink	= vti_changelink,
 	.get_size	= vti_get_size,
 	.fill_info	= vti_fill_info,
+	.get_link_net	= ip_tunnel_get_link_net,
 };
 
 static int __init vti_init(void)
diff --git a/net/ipv4/ipip.c b/net/ipv4/ipip.c
index 62eaa005e146..aab4c231d2c5 100644
--- a/net/ipv4/ipip.c
+++ b/net/ipv4/ipip.c
@@ -420,6 +420,7 @@ static struct rtnl_link_ops ipip_link_ops __read_mostly = {
 	.dellink	= ip_tunnel_dellink,
 	.get_size	= ipip_get_size,
 	.fill_info	= ipip_fill_info,
+	.get_link_net	= ip_tunnel_get_link_net,
 };
 
 static struct xfrm_tunnel ipip_handler __read_mostly = {
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 4f408176dc64..1ccf4f9e7bb0 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -1673,6 +1673,7 @@ static struct rtnl_link_ops sit_link_ops __read_mostly = {
 	.get_size	= ipip6_get_size,
 	.fill_info	= ipip6_fill_info,
 	.dellink	= ipip6_dellink,
+	.get_link_net	= ip_tunnel_get_link_net,
 };
 
 static struct xfrm_tunnel sit_handler __read_mostly = {
-- 
1.9.0

[RFC PATCH net-next 5/5] rtnl: allow to create device with IFLA_LINK_NETNSID set

[Nicolas Dichtel]

This patch adds the ability to create a netdevice in a specified netns and
then move it into the final netns. In fact, it allows to have a symetry between
get and set rtnl messages.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 net/core/rtnetlink.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d99b98b41045..37fb96f51d2b 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1211,6 +1211,7 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_NUM_RX_QUEUES]	= { .type = NLA_U32 },
 	[IFLA_PHYS_PORT_ID]	= { .type = NLA_BINARY, .len = MAX_PHYS_PORT_ID_LEN },
 	[IFLA_CARRIER_CHANGES]	= { .type = NLA_U32 },  /* ignored */
+	[IFLA_LINK_NETNSID]	= { .type = NLA_U32 },
 };
 
 static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
@@ -1963,7 +1964,7 @@ replay:
 		struct nlattr *slave_attr[m_ops ? m_ops->slave_maxtype + 1 : 0];
 		struct nlattr **data = NULL;
 		struct nlattr **slave_data = NULL;
-		struct net *dest_net;
+		struct net *dest_net, *link_net = NULL;
 
 		if (ops) {
 			if (ops->maxtype && linkinfo[IFLA_INFO_DATA]) {
@@ -2067,7 +2068,18 @@ replay:
 		if (IS_ERR(dest_net))
 			return PTR_ERR(dest_net);
 
-		dev = rtnl_create_link(dest_net, ifname, ops, tb);
+		if (tb[IFLA_LINK_NETNSID]) {
+			link_net =
+				get_net_from_netnsid(dest_net,
+						     nla_get_u32(tb[IFLA_LINK_NETNSID]));
+
+			if (link_net &&
+			    !netlink_ns_capable(skb, link_net->user_ns,
+						CAP_NET_ADMIN))
+				return -EPERM;
+		}
+
+		dev = rtnl_create_link(link_net ? : dest_net, ifname, ops, tb);
 		if (IS_ERR(dev)) {
 			err = PTR_ERR(dev);
 			goto out;
@@ -2095,9 +2107,16 @@ replay:
 			}
 		}
 		err = rtnl_configure_link(dev, ifm);
-		if (err < 0)
+		if (err < 0) {
 			unregister_netdevice(dev);
+			goto out;
+		}
+
+		if (link_net)
+			err = dev_change_net_namespace(dev, net, ifname);
 out:
+		if (link_net)
+			put_net(link_net);
 		put_net(dest_net);
 		return err;
 	}
-- 
1.9.0

Re: [RFC PATCH net-next 1/5] netns: allocate netns ids

[Sergei Shtylyov]

Hello.

On 07/02/2014 03:59 PM, Nicolas Dichtel wrote:

> With this patch, netns allocates ids for all netns. Each netns has its own list
> of ids, it means that the id is valid only in the current netns.

> For homogeneity, an id is also allocated for the netns owner itself.

> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
[...]

> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index 85b62691f4f2..2976864cef13 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -144,6 +144,30 @@ static void ops_free_list(const struct pernet_operations *ops,
>   	}
>   }
>
> +static int net_eq_idr(int id, void *net, void *peer)
> +{
> +	return net_eq(net, peer) ? id : 0;
> +}
> +
> +int net2id(struct net *net, struct net *peer)
> +{
> +	ASSERT_RTNL();
> +	return idr_for_each(&(net)->netns_ids, net_eq_idr, (peer)) ? : -ENOENT;

    I don't think the inner parens are needed, this is not a macro body...

> +}
> +
> +struct net *get_net_from_netnsid(struct net *net, int id)
> +{
> +	struct net *peer;
> +
> +	rcu_read_lock();
> +	peer = idr_find(&(net)->netns_ids, id);

    Same here.

> +	if (peer)
> +		get_net(peer);
> +	rcu_read_unlock();
> +
> +	return peer;
> +}
> +
>   /*
>    * setup_net runs the initializers for the network namespace object.
>    */
[...]

WBR, Sergei

Re: [RFC PATCH net-next 1/5] netns: allocate netns ids

[Nicolas Dichtel]

Le 02/07/2014 15:33, Sergei Shtylyov a écrit :
> Hello.
>
> On 07/02/2014 03:59 PM, Nicolas Dichtel wrote:
>
>> With this patch, netns allocates ids for all netns. Each netns has its own list
>> of ids, it means that the id is valid only in the current netns.
>
>> For homogeneity, an id is also allocated for the netns owner itself.
>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> [...]
>
>> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
>> index 85b62691f4f2..2976864cef13 100644
>> --- a/net/core/net_namespace.c
>> +++ b/net/core/net_namespace.c
>> @@ -144,6 +144,30 @@ static void ops_free_list(const struct pernet_operations
>> *ops,
>>       }
>>   }
>>
>> +static int net_eq_idr(int id, void *net, void *peer)
>> +{
>> +    return net_eq(net, peer) ? id : 0;
>> +}
>> +
>> +int net2id(struct net *net, struct net *peer)
>> +{
>> +    ASSERT_RTNL();
>> +    return idr_for_each(&(net)->netns_ids, net_eq_idr, (peer)) ? : -ENOENT;
>
>     I don't think the inner parens are needed, this is not a macro body...
Right!
In fact, it was a macro at the beginning ;-)

>
>> +}
>> +
>> +struct net *get_net_from_netnsid(struct net *net, int id)
>> +{
>> +    struct net *peer;
>> +
>> +    rcu_read_lock();
>> +    peer = idr_find(&(net)->netns_ids, id);
>
>     Same here.
Same.

Thank you,
Nicolas

Re: [RFC PATCH net-next 0/5] netns: allow to identify peer netns

[Eric W. Biederman]

Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:

> The goal of this serie is to be able to multicast netlink messages with an
> attribute that identify a peer netns.
> This is needed by the userland to interpret some informations contained in
> netlink messages (like IFLA_LINK value, but also some other attributes in case
> of x-netns netdevice (see also
> http://thread.gmane.org/gmane.linux.network/315933/focus=316064)).
>
> Each network namespaces allocates its own ids for other netns (including
> itself). The user can retrieve these ids via a new netlink messages, but only
> if he has a FD which points to this netns. Dump is not implemented so that a
> user cannot get the whole netns list.
>
> The goal of this RFC is mainly to validate the principle, ie patch 1/5 and 2/5.
> Patch 3/5 and 4/5 shows an example of how to use these ids in rtnetlink
> messages. And patch 5/5 shows that the netlink messages can be symetric between
> a GET and a SET.

This approach fundamentally breaks process migration, and calls for a
namespace of namespaces.

Which means this is a major mess that really isn't going to work because
it generates more problems than it solves.

Eric

Re: [RFC PATCH net-next 0/5] netns: allow to identify peer netns

[Nicolas Dichtel]

Le 02/07/2014 22:09, Eric W. Biederman a écrit :
> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>
>> The goal of this serie is to be able to multicast netlink messages with an
>> attribute that identify a peer netns.
>> This is needed by the userland to interpret some informations contained in
>> netlink messages (like IFLA_LINK value, but also some other attributes in case
>> of x-netns netdevice (see also
>> http://thread.gmane.org/gmane.linux.network/315933/focus=316064)).
>>
>> Each network namespaces allocates its own ids for other netns (including
>> itself). The user can retrieve these ids via a new netlink messages, but only
>> if he has a FD which points to this netns. Dump is not implemented so that a
>> user cannot get the whole netns list.
>>
>> The goal of this RFC is mainly to validate the principle, ie patch 1/5 and 2/5.
>> Patch 3/5 and 4/5 shows an example of how to use these ids in rtnetlink
>> messages. And patch 5/5 shows that the netlink messages can be symetric between
>> a GET and a SET.
>
> This approach fundamentally breaks process migration, and calls for a
> namespace of namespaces.
Why does it call for a namespace of namespaces? Ids are different in each netns,
because each netns has its own list id.
Can you elaborate why it breaks the process migration?

Do you think that identifying a netns from another netns is fundamentally wrong?

Nicolas

Re: [RFC PATCH net-next 0/5] netns: allow to identify peer netns

[Nicolas Dichtel]

Le 02/07/2014 22:09, Eric W. Biederman a écrit :
> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>
>> The goal of this serie is to be able to multicast netlink messages with an
>> attribute that identify a peer netns.
>> This is needed by the userland to interpret some informations contained in
>> netlink messages (like IFLA_LINK value, but also some other attributes in case
>> of x-netns netdevice (see also
>> http://thread.gmane.org/gmane.linux.network/315933/focus=316064)).
>>
>> Each network namespaces allocates its own ids for other netns (including
>> itself). The user can retrieve these ids via a new netlink messages, but only
>> if he has a FD which points to this netns. Dump is not implemented so that a
>> user cannot get the whole netns list.
>>
>> The goal of this RFC is mainly to validate the principle, ie patch 1/5 and 2/5.
>> Patch 3/5 and 4/5 shows an example of how to use these ids in rtnetlink
>> messages. And patch 5/5 shows that the netlink messages can be symetric between
>> a GET and a SET.
>
> This approach fundamentally breaks process migration, and calls for a
> namespace of namespaces.
Maybe there is no other solution. Network people uses netns to implement
"virtual router", ie only network namespaces are used. Userland daemons
manage a set of netns. These daemons need to be able to identify a peer netns
when netlink messages from kernel are parsed. For now, these netlink messages
are incomplete and contain information that cannot be interpreted (like the
IFLA_LINK ifindex).

Can you give me more details about the "process migration"? How does it work
when you have a veth tunnel between two netns?

>
> Which means this is a major mess that really isn't going to work because
> it generates more problems than it solves.
It's just a netns inside another netns, or to be easier to figure out, one or
more virtual router inside a netns ;-)

Regards,
Nicolas