[PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Nicolas Dichtel]

When one of these attributes is set, the netdevice is created into the netns
pointed by IFLA_NET_NS_[PID|FD] (see the call to rtnl_create_link() in
rtnl_newlink()). Let's call this netns the dest_net. After this creation, if the
newlink handler exists, it is called with a netns argument that points to the
netns where the netlink message has been received (called src_net in the code)
which is the link netns.
Hence, with one of these attributes, it's possible to create a x-netns
netdevice.

Here is the result of my code review:
- all ip tunnels (sit, ipip, ip6_tunnels, gre[tap][v6], ip_vti[6]) does not
  really allows to use this feature: the netdevice is created in the dest_net
  and the src_net is completely ignored in the newlink handler.
- VLAN properly handles this x-netns creation.
- bridge ignores src_net, which seems fine (NETIF_F_NETNS_LOCAL is set).
- CAIF subsystem is not clear for me (I don't know how it works), but it seems
  to wrongly use src_net. Patch #1 tries to fix this, but it was done only by
  code review (and only compile-tested), so please carefully review it. I may
  miss something.
- HSR subsystem uses src_net to parse IFLA_HSR_SLAVE[1|2], but the netdevice has
  the flag NETIF_F_NETNS_LOCAL, so the question is: does this netdevice really
  supports x-netns? If not, the newlink handler should use the dest_net instead
  of src_net, I can provide the patch.
- ieee802154 uses also src_net and does not have NETIF_F_NETNS_LOCAL. Same
  question: does this netdevice really supports x-netns?
- bonding ignores src_net and flag NETIF_F_NETNS_LOCAL is set, ie x-netns is not
  supported. Fine.
- CAN does not support rtnl/newlink, ok.
- ipvlan uses src_net and does not have NETIF_F_NETNS_LOCAL. After looking at
  the code, it seems that this drivers support x-netns. Am I right?
- macvlan/macvtap uses src_net and seems to have x-netns support.
- team ignores src_net and has the flag NETIF_F_NETNS_LOCAL, ie x-netns is not
  supported. Ok.
- veth uses src_net and have x-netns support ;-) Ok.
- VXLAN didn't properly handle this. The link netns (vxlan->net) is the src_net
  and not dest_net (see patch #2). Note that it was already possible to create a
  x-netns vxlan before the commit f01ec1c017de ("vxlan: add x-netns support")
  but the nedevice remains broken.

To summarize:
 - CAIF patch must be carefully reviewed
 - for HSR, ieee802154, ipvlan: is x-netns supported?

 drivers/net/caif/caif_hsi.c |  1 -
 drivers/net/vxlan.c         | 10 +++++-----
 net/caif/chnl_net.c         |  1 -
 3 files changed, 5 insertions(+), 7 deletions(-)

Regards,
Nicolas

[PATCH net 1/2] caif: remove wrong dev_net_set() call

[Nicolas Dichtel]

src_net points to the netns where the netlink message has been received. This
netns may be different from the netns where the interface is created (because
the user may add IFLA_NET_NS_[PID|FD]). In this case, src_net is the link netns.

It seems wrong to override the netns in the newlink() handler because if it
was not already src_net, it means that the user explicitly asks to create the
netdevice in another netns.

CC: Sjur Brændeland <sjur.brandeland@stericsson.com>
CC: Dmitry Tarnyagin <dmitry.tarnyagin@lockless.no>
Fixes: 8391c4aab1aa ("caif: Bugfixes in CAIF netdevice for close and flow control")
Fixes: c41254006377 ("caif-hsi: Add rtnl support")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 drivers/net/caif/caif_hsi.c | 1 -
 net/caif/chnl_net.c         | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/net/caif/caif_hsi.c b/drivers/net/caif/caif_hsi.c
index 5e40a8b68cbe..b3b922adc0e4 100644
--- a/drivers/net/caif/caif_hsi.c
+++ b/drivers/net/caif/caif_hsi.c
@@ -1415,7 +1415,6 @@ static int caif_hsi_newlink(struct net *src_net, struct net_device *dev,
 
 	cfhsi = netdev_priv(dev);
 	cfhsi_netlink_parms(data, cfhsi);
-	dev_net_set(cfhsi->ndev, src_net);
 
 	get_ops = symbol_get(cfhsi_get_ops);
 	if (!get_ops) {
diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 4589ff67bfa9..67a4a36febd1 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -470,7 +470,6 @@ static int ipcaif_newlink(struct net *src_net, struct net_device *dev,
 	ASSERT_RTNL();
 	caifdev = netdev_priv(dev);
 	caif_netlink_parms(data, &caifdev->conn_req);
-	dev_net_set(caifdev->netdev, src_net);
 
 	ret = register_netdevice(dev);
 	if (ret)
-- 
2.2.2

[PATCH net 2/2] vxlan: setup the right link netns in newlink hdlr

[Nicolas Dichtel]

Rename the netns to src_net to avoid confusion with the netns where the
interface stands. The user may specify IFLA_NET_NS_[PID|FD] to create
a x-netns netndevice: IFLA_NET_NS_[PID|FD] points to the netns where the
netdevice stands and src_net to the link netns.

Note that before commit f01ec1c017de ("vxlan: add x-netns support"), it was
possible to create a x-netns vxlan netdevice, but the netdevice was not
operational.

Fixes: f01ec1c017de ("vxlan: add x-netns support")
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
 drivers/net/vxlan.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 7fbd89fbe107..a8c755dcab14 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2432,10 +2432,10 @@ static void vxlan_sock_work(struct work_struct *work)
 	dev_put(vxlan->dev);
 }
 
-static int vxlan_newlink(struct net *net, struct net_device *dev,
+static int vxlan_newlink(struct net *src_net, struct net_device *dev,
 			 struct nlattr *tb[], struct nlattr *data[])
 {
-	struct vxlan_net *vn = net_generic(net, vxlan_net_id);
+	struct vxlan_net *vn = net_generic(src_net, vxlan_net_id);
 	struct vxlan_dev *vxlan = netdev_priv(dev);
 	struct vxlan_rdst *dst = &vxlan->default_dst;
 	__u32 vni;
@@ -2445,7 +2445,7 @@ static int vxlan_newlink(struct net *net, struct net_device *dev,
 	if (!data[IFLA_VXLAN_ID])
 		return -EINVAL;
 
-	vxlan->net = dev_net(dev);
+	vxlan->net = src_net;
 
 	vni = nla_get_u32(data[IFLA_VXLAN_ID]);
 	dst->remote_vni = vni;
@@ -2481,7 +2481,7 @@ static int vxlan_newlink(struct net *net, struct net_device *dev,
 	if (data[IFLA_VXLAN_LINK] &&
 	    (dst->remote_ifindex = nla_get_u32(data[IFLA_VXLAN_LINK]))) {
 		struct net_device *lowerdev
-			 = __dev_get_by_index(net, dst->remote_ifindex);
+			 = __dev_get_by_index(src_net, dst->remote_ifindex);
 
 		if (!lowerdev) {
 			pr_info("ifindex %d does not exist\n", dst->remote_ifindex);
@@ -2557,7 +2557,7 @@ static int vxlan_newlink(struct net *net, struct net_device *dev,
 	    nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]))
 		vxlan->flags |= VXLAN_F_UDP_ZERO_CSUM6_RX;
 
-	if (vxlan_find_vni(net, vni, use_ipv6 ? AF_INET6 : AF_INET,
+	if (vxlan_find_vni(src_net, vni, use_ipv6 ? AF_INET6 : AF_INET,
 			   vxlan->dst_port)) {
 		pr_info("duplicate VNI %u\n", vni);
 		return -EEXIST;
-- 
2.2.2

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Alexander Aring]

Hi,

On Mon, Jan 26, 2015 at 10:28:12PM +0100, Nicolas Dichtel wrote:
> 
> When one of these attributes is set, the netdevice is created into the netns
> pointed by IFLA_NET_NS_[PID|FD] (see the call to rtnl_create_link() in
> rtnl_newlink()). Let's call this netns the dest_net. After this creation, if the
> newlink handler exists, it is called with a netns argument that points to the
> netns where the netlink message has been received (called src_net in the code)
> which is the link netns.
> Hence, with one of these attributes, it's possible to create a x-netns
> netdevice.
> 
> Here is the result of my code review:
> - all ip tunnels (sit, ipip, ip6_tunnels, gre[tap][v6], ip_vti[6]) does not
>   really allows to use this feature: the netdevice is created in the dest_net
>   and the src_net is completely ignored in the newlink handler.
> - VLAN properly handles this x-netns creation.
> - bridge ignores src_net, which seems fine (NETIF_F_NETNS_LOCAL is set).
> - CAIF subsystem is not clear for me (I don't know how it works), but it seems
>   to wrongly use src_net. Patch #1 tries to fix this, but it was done only by
>   code review (and only compile-tested), so please carefully review it. I may
>   miss something.
> - HSR subsystem uses src_net to parse IFLA_HSR_SLAVE[1|2], but the netdevice has
>   the flag NETIF_F_NETNS_LOCAL, so the question is: does this netdevice really
>   supports x-netns? If not, the newlink handler should use the dest_net instead
>   of src_net, I can provide the patch.
> - ieee802154 uses also src_net and does not have NETIF_F_NETNS_LOCAL. Same
>   question: does this netdevice really supports x-netns?

I am not sure if I understand exactly what you mean. First of all, I
didn't test anything about net namespaces for the ieee802154 branch.
In 802.15.4 branch we have two interfaces: wpan and 6LoWPAN.

After running "grep -r "src_net" net" I found this is used in:

net/ieee802154/6lowpan/core.c [0]

This file handles the IEEE 802.15.4 6LoWPAN interface to offering a
IPv6 interface with an IEEE 802.15.4 6LoWPAN adaption layer.

To the codeline "dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));".
By calling "ip link add link wpan0 name lowpan0 type lowpan" the
lowpan_newlink function will be called and we need to find the wpan interface
(returned as real_dev in this case).

Namespace setting in wpan interface:

Currently we don't use any net namespace settings there, also we don't
change the net namespace. The default net namespace for a wpan shoule be
"init_net".

So this line could be also written as (I found also some others code which search
the wpan interface in &init_net):

diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index 9dbe0d69..495c6ad 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -151,7 +151,7 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
        if (!tb[IFLA_LINK])
                return -EINVAL;
        /* find and hold real wpan device */
-       real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
+       real_dev = dev_get_by_index(&init_net, nla_get_u32(tb[IFLA_LINK]));
        if (!real_dev)
                return -ENODEV;
        if (real_dev->type != ARPHRD_IEEE802154) {



The above code is for finding the wpan interface (the real 802.15.4 L2 interface).
For the IEEE 802.15.4 6LoWPAN interface the whole IPv6 implementation is
used. This interface will be created inside function "newlink".

Running "grep -r "src_net" net/ipv6" reports me alot uses of "src_net".
Don't know if this information is really necessary.

Should I set now the NETIF_F_NETNS_LOCAL for both interface types?

- Alex

[0] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/net/ieee802154/6lowpan/core.c#n154

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Nicolas Dichtel]

Le 27/01/2015 10:34, Alexander Aring a écrit :
> Hi,
>
> On Mon, Jan 26, 2015 at 10:28:12PM +0100, Nicolas Dichtel wrote:
>>
[snip]
>> - ieee802154 uses also src_net and does not have NETIF_F_NETNS_LOCAL. Same
>>    question: does this netdevice really supports x-netns?
>
> I am not sure if I understand exactly what you mean. First of all, I
> didn't test anything about net namespaces for the ieee802154 branch.
> In 802.15.4 branch we have two interfaces: wpan and 6LoWPAN.
>
> After running "grep -r "src_net" net" I found this is used in:
>
> net/ieee802154/6lowpan/core.c [0]
Yes, I was talking about this.

>
> This file handles the IEEE 802.15.4 6LoWPAN interface to offering a
> IPv6 interface with an IEEE 802.15.4 6LoWPAN adaption layer.
>
> To the codeline "dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));".
> By calling "ip link add link wpan0 name lowpan0 type lowpan" the
> lowpan_newlink function will be called and we need to find the wpan interface
> (returned as real_dev in this case).
>
> Namespace setting in wpan interface:
>
> Currently we don't use any net namespace settings there, also we don't
> change the net namespace. The default net namespace for a wpan shoule be
> "init_net".
Ok. After grepping for init_net, it seems to be used a lot in net/ieee802154/.

>
> So this line could be also written as (I found also some others code which search
> the wpan interface in &init_net):
>
> diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
> index 9dbe0d69..495c6ad 100644
> --- a/net/ieee802154/6lowpan/core.c
> +++ b/net/ieee802154/6lowpan/core.c
> @@ -151,7 +151,7 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
>          if (!tb[IFLA_LINK])
>                  return -EINVAL;
>          /* find and hold real wpan device */
> -       real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
> +       real_dev = dev_get_by_index(&init_net, nla_get_u32(tb[IFLA_LINK]));
>          if (!real_dev)
>                  return -ENODEV;
>          if (real_dev->type != ARPHRD_IEEE802154) {
>
>
>
> The above code is for finding the wpan interface (the real 802.15.4 L2 interface).
> For the IEEE 802.15.4 6LoWPAN interface the whole IPv6 implementation is
> used. This interface will be created inside function "newlink".
>
> Running "grep -r "src_net" net/ipv6" reports me alot uses of "src_net".
> Don't know if this information is really necessary.
>
> Should I set now the NETIF_F_NETNS_LOCAL for both interface types?
I think yes. If it's not set, a user may do:
$ ip link add link wpan0 name lowpan0 type lowpan
$ ip netns add foo
$ ip link set lowpan0 netns foo

The flag forbids the last command.

Instead of your patch, what about this one:

 From d9a9cd22d5e1db1417b3ffb53cc020481dc761b2 Mon Sep 17 00:00:00 2001
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue, 27 Jan 2015 11:26:20 +0100
Subject: [PATCH] ieee802154: forbid to create an iface in a netns != init_net

6LoWPAN currently doesn't supports netns.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
  net/ieee802154/6lowpan/core.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index 055fbb71ba6f..fe8fd022042e 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -126,6 +126,7 @@ static void lowpan_setup(struct net_device *dev)
  	dev->header_ops		= &lowpan_header_ops;
  	dev->ml_priv		= &lowpan_mlme;
  	dev->destructor		= free_netdev;
+	dev->features		|= NETIF_F_NETNS_LOCAL;
  }

  static int lowpan_validate(struct nlattr *tb[], struct nlattr *data[])
@@ -148,7 +149,9 @@ static int lowpan_newlink(struct net *src_net, struct 
net_device *dev,

  	pr_debug("adding new link\n");

-	if (!tb[IFLA_LINK])
+	if (!tb[IFLA_LINK] ||
+	    !net_eq(src_net, &init_net) ||
+	    !net_eq(dev_net(dev), &init_net))
  		return -EINVAL;
  	/* find and hold real wpan device */
  	real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
-- 
2.2.2

Re: [PATCH net 1/2] caif: remove wrong dev_net_set() call

[Nicolas Dichtel]

Le 26/01/2015 22:28, Nicolas Dichtel a écrit :
> src_net points to the netns where the netlink message has been received. This
> netns may be different from the netns where the interface is created (because
> the user may add IFLA_NET_NS_[PID|FD]). In this case, src_net is the link netns.
>
> It seems wrong to override the netns in the newlink() handler because if it
> was not already src_net, it means that the user explicitly asks to create the
> netdevice in another netns.
>
> CC: Sjur Brændeland <sjur.brandeland@stericsson.com>
> CC: Dmitry Tarnyagin <dmitry.tarnyagin@lockless.no>

I got a "Delivery Status Notification (Failure)" for Dmitry Tarnyagin (I took 
the email address from the MAINTAINERS file):

   Delivery to the following recipient failed permanently:

        dmitry.tarnyagin@lockless.no

   Technical details of permanent failure:
   Google tried to deliver your message, but it was rejected by the server for
   the recipient domain lockless.no by mail.lockless.no. [46.29.221.38].

   The error that the other server returned was:
   550 5.1.1 <dmitry.tarnyagin@lockless.no>: Recipient address rejected: User
   unknown in virtual mailbox table

Any suggestions?

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Alexander Aring]

Hi,

(removing the bounced mail address).

On Tue, Jan 27, 2015 at 11:32:44AM +0100, Nicolas Dichtel wrote:
> Le 27/01/2015 10:34, Alexander Aring a écrit :
> >Hi,
> >
> >On Mon, Jan 26, 2015 at 10:28:12PM +0100, Nicolas Dichtel wrote:
> >>
> [snip]
> >>- ieee802154 uses also src_net and does not have NETIF_F_NETNS_LOCAL. Same
> >>   question: does this netdevice really supports x-netns?
> >
> >I am not sure if I understand exactly what you mean. First of all, I
> >didn't test anything about net namespaces for the ieee802154 branch.
> >In 802.15.4 branch we have two interfaces: wpan and 6LoWPAN.
> >
> >After running "grep -r "src_net" net" I found this is used in:
> >
> >net/ieee802154/6lowpan/core.c [0]
> Yes, I was talking about this.
> 

ok.

> >
> >This file handles the IEEE 802.15.4 6LoWPAN interface to offering a
> >IPv6 interface with an IEEE 802.15.4 6LoWPAN adaption layer.
> >
> >To the codeline "dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));".
> >By calling "ip link add link wpan0 name lowpan0 type lowpan" the
> >lowpan_newlink function will be called and we need to find the wpan interface
> >(returned as real_dev in this case).
> >
> >Namespace setting in wpan interface:
> >
> >Currently we don't use any net namespace settings there, also we don't
> >change the net namespace. The default net namespace for a wpan shoule be
> >"init_net".
> Ok. After grepping for init_net, it seems to be used a lot in net/ieee802154/.
> 

Yes, but the code in net/ieee802154 (except net/ieee802154/6lowpan) is only for
the WPAN interface. Currently the WPAN interface is created in mac802154
implementation only [0].

> >
> >So this line could be also written as (I found also some others code which search
> >the wpan interface in &init_net):
> >
> >diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
> >index 9dbe0d69..495c6ad 100644
> >--- a/net/ieee802154/6lowpan/core.c
> >+++ b/net/ieee802154/6lowpan/core.c
> >@@ -151,7 +151,7 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
> >         if (!tb[IFLA_LINK])
> >                 return -EINVAL;
> >         /* find and hold real wpan device */
> >-       real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
> >+       real_dev = dev_get_by_index(&init_net, nla_get_u32(tb[IFLA_LINK]));
> >         if (!real_dev)
> >                 return -ENODEV;
> >         if (real_dev->type != ARPHRD_IEEE802154) {
> >
> >
> >
> >The above code is for finding the wpan interface (the real 802.15.4 L2 interface).
> >For the IEEE 802.15.4 6LoWPAN interface the whole IPv6 implementation is
> >used. This interface will be created inside function "newlink".
> >
> >Running "grep -r "src_net" net/ipv6" reports me alot uses of "src_net".
> >Don't know if this information is really necessary.
> >
> >Should I set now the NETIF_F_NETNS_LOCAL for both interface types?
> I think yes. If it's not set, a user may do:
> $ ip link add link wpan0 name lowpan0 type lowpan
> $ ip netns add foo
> $ ip link set lowpan0 netns foo
> 

We should forbid that for the wpan interface. The code line:

real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));

searches for the "wpan0" interface which is given by:
$ ip link add link wpan0 name lowpan0 type lowpan

The returned real_dev netdevice pointer is the wpan interface. The given
"lowpan0" interface is a virtual interface for making IPv6 stuff.



For 6LoWPAN:

This interface is created in 6lowpan/core.c file and is used in the
whole IPv6 stack, because we set the skb->protocol to htons(ETH_P_IPV6). [1]

The IPv6 stack uses alot of "src_net".

> The flag forbids the last command.
> 
> Instead of your patch, what about this one:
> 
> From d9a9cd22d5e1db1417b3ffb53cc020481dc761b2 Mon Sep 17 00:00:00 2001
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Tue, 27 Jan 2015 11:26:20 +0100
> Subject: [PATCH] ieee802154: forbid to create an iface in a netns != init_net
> 
> 6LoWPAN currently doesn't supports netns.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>  net/ieee802154/6lowpan/core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
> index 055fbb71ba6f..fe8fd022042e 100644
> --- a/net/ieee802154/6lowpan/core.c
> +++ b/net/ieee802154/6lowpan/core.c
> @@ -126,6 +126,7 @@ static void lowpan_setup(struct net_device *dev)
>  	dev->header_ops		= &lowpan_header_ops;
>  	dev->ml_priv		= &lowpan_mlme;
>  	dev->destructor		= free_netdev;
> +	dev->features		|= NETIF_F_NETNS_LOCAL;
>  }
> 
>  static int lowpan_validate(struct nlattr *tb[], struct nlattr *data[])
> @@ -148,7 +149,9 @@ static int lowpan_newlink(struct net *src_net, struct
> net_device *dev,
> 
>  	pr_debug("adding new link\n");
> 
> -	if (!tb[IFLA_LINK])
> +	if (!tb[IFLA_LINK] ||
> +	    !net_eq(src_net, &init_net) ||
> +	    !net_eq(dev_net(dev), &init_net))
>  		return -EINVAL;
>  	/* find and hold real wpan device */
>  	real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));

With the check of "!net_eq(src_net, &init_net)" we need to be sure
that the wpan interface is always in "init_net". This means we need
definitely a dev->features |= NETIF_F_NETNS_LOCAL; somewhere in [0].

To adding "dev->features |= NETIF_F_NETNS_LOCAL;" for a 6LoWPAN interface,
I am not sure about this. I didn't test it yet and it will not break
anything, but we will lost the support for making net namespaces stuff
inside the IPv6/(netfilter) stack.


Summarize:

I would add the dev->features |= NETIF_F_NETNS_LOCAL; while wpan
interface generation and add only the !net_eq(src_net, &init_net) check
above. I suppose that src_net is the net namespace from "underlaying"
interface wpan by calling:

$ ip link add link wpan0 name lowpan0 type lowpan

- Alex

[0] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/net/mac802154/iface.c#n532
[1] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/net/ieee802154/6lowpan/rx.c#n25

Re: [PATCH net 1/2] caif: remove wrong dev_net_set() call

[Bjørn Mork]

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

>> CC: Sjur Brændeland <sjur.brandeland@stericsson.com>
>> CC: Dmitry Tarnyagin <dmitry.tarnyagin@lockless.no>
>
> I got a "Delivery Status Notification (Failure)" for Dmitry Tarnyagin
> (I took the email address from the MAINTAINERS file):
>
>   Delivery to the following recipient failed permanently:
>
>        dmitry.tarnyagin@lockless.no
>
>   Technical details of permanent failure:
>   Google tried to deliver your message, but it was rejected by the server for
>   the recipient domain lockless.no by mail.lockless.no. [46.29.221.38].
>
>   The error that the other server returned was:
>   550 5.1.1 <dmitry.tarnyagin@lockless.no>: Recipient address rejected: User
>   unknown in virtual mailbox table
>
> Any suggestions?


He used a gmail address in commit dd9dfb9f95e2 ("cfg80211: merge in
beacon ies of hidden bss.").  Probably by accident, but worth trying
anyway...

I guess the stericsson.com addresses could be failing silently now? Ref
http://stericsson.com/ . Or are they still routed to the named recepients?


Bjørn

Re: [PATCH net 1/2] caif: remove wrong dev_net_set() call

[Nicolas Dichtel]

Le 27/01/2015 13:41, Bjørn Mork a écrit :
> Nicolas Dichtel <nicolas.dichtel@6wind.com> writes:
>
>>> CC: Sjur Brændeland <sjur.brandeland@stericsson.com>
>>> CC: Dmitry Tarnyagin <dmitry.tarnyagin@lockless.no>
>>
>> I got a "Delivery Status Notification (Failure)" for Dmitry Tarnyagin
>> (I took the email address from the MAINTAINERS file):
>>
>>    Delivery to the following recipient failed permanently:
>>
>>         dmitry.tarnyagin@lockless.no
>>
>>    Technical details of permanent failure:
>>    Google tried to deliver your message, but it was rejected by the server for
>>    the recipient domain lockless.no by mail.lockless.no. [46.29.221.38].
>>
>>    The error that the other server returned was:
>>    550 5.1.1 <dmitry.tarnyagin@lockless.no>: Recipient address rejected: User
>>    unknown in virtual mailbox table
>>
>> Any suggestions?
>
>
> He used a gmail address in commit dd9dfb9f95e2 ("cfg80211: merge in
> beacon ies of hidden bss.").  Probably by accident, but worth trying
> anyway...
Ok, I will wait a bit before trying this address.

>
> I guess the stericsson.com addresses could be failing silently now? Ref
> http://stericsson.com/ . Or are they still routed to the named recepients?
I got also a delivery failure for sjur.brandeland@stericsson.com


Thank you,
Nicolas

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Alexander Aring]

On Tue, Jan 27, 2015 at 01:23:40PM +0100, Alexander Aring wrote:
...
> Summarize:
> 
> I would add the dev->features |= NETIF_F_NETNS_LOCAL; while wpan
> interface generation and add only the !net_eq(src_net, &init_net) check
> above. I suppose that src_net is the net namespace from "underlaying"
> interface wpan by calling:
> 
> $ ip link add link wpan0 name lowpan0 type lowpan
> 

should look something like:

diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index 055fbb7..a44963c 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -148,10 +148,11 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
 
        pr_debug("adding new link\n");
 
-       if (!tb[IFLA_LINK])
+       if (!tb[IFLA_LINK] |
+           !net_eq(src_net, &init_net))
                return -EINVAL;
        /* find and hold real wpan device */
-       real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
+       real_dev = dev_get_by_index(&init_net, nla_get_u32(tb[IFLA_LINK]));
        if (!real_dev)
                return -ENODEV;
        if (real_dev->type != ARPHRD_IEEE802154) {
diff --git a/net/ieee802154/core.c b/net/ieee802154/core.c
index 18bc7e7..161f0e5 100644
--- a/net/ieee802154/core.c
+++ b/net/ieee802154/core.c
@@ -229,6 +229,8 @@ static int cfg802154_netdev_notifier_call(struct notifier_block *nb,
                list_add_rcu(&wpan_dev->list, &rdev->wpan_dev_list);
                rdev->devlist_generation++;
 
+               /* can only change netns with wpan_phy */
+               dev->features |= NETIF_F_NETNS_LOCAL;
                wpan_dev->netdev = dev;
                break;
        case NETDEV_DOWN:
--


In ieee802154/core.c we set (like wireless it also does) the
dev->features |= NETIF_F_NETNS_LOCAL; for wpan interface.


In net/ieee802154/6lowpan/core.c, we only check if the wpan interface
belongs to !net_eq(src_net, &init_net).


On 6LoWPAN 802.15.4 interfaces it should be still possible to change
the net namespace.

- Alex

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Nicolas Dichtel]

Le 27/01/2015 13:23, Alexander Aring a écrit :
> Hi,
>
> (removing the bounced mail address).
>
> On Tue, Jan 27, 2015 at 11:32:44AM +0100, Nicolas Dichtel wrote:
>> Le 27/01/2015 10:34, Alexander Aring a écrit :
>>> Hi,
>>>
>>> On Mon, Jan 26, 2015 at 10:28:12PM +0100, Nicolas Dichtel wrote:
>>>>
[snip]
>>>
>>> This file handles the IEEE 802.15.4 6LoWPAN interface to offering a
>>> IPv6 interface with an IEEE 802.15.4 6LoWPAN adaption layer.
>>>
>>> To the codeline "dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));".
>>> By calling "ip link add link wpan0 name lowpan0 type lowpan" the
>>> lowpan_newlink function will be called and we need to find the wpan interface
>>> (returned as real_dev in this case).
>>>
>>> Namespace setting in wpan interface:
>>>
>>> Currently we don't use any net namespace settings there, also we don't
>>> change the net namespace. The default net namespace for a wpan shoule be
>>> "init_net".
>> Ok. After grepping for init_net, it seems to be used a lot in net/ieee802154/.
>>
>
> Yes, but the code in net/ieee802154 (except net/ieee802154/6lowpan) is only for
> the WPAN interface. Currently the WPAN interface is created in mac802154
> implementation only [0].
Ok. How is this interface created?

>
>>>
>>> So this line could be also written as (I found also some others code which search
>>> the wpan interface in &init_net):
>>>
>>> diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
>>> index 9dbe0d69..495c6ad 100644
>>> --- a/net/ieee802154/6lowpan/core.c
>>> +++ b/net/ieee802154/6lowpan/core.c
>>> @@ -151,7 +151,7 @@ static int lowpan_newlink(struct net *src_net, struct net_device *dev,
>>>          if (!tb[IFLA_LINK])
>>>                  return -EINVAL;
>>>          /* find and hold real wpan device */
>>> -       real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
>>> +       real_dev = dev_get_by_index(&init_net, nla_get_u32(tb[IFLA_LINK]));
>>>          if (!real_dev)
>>>                  return -ENODEV;
>>>          if (real_dev->type != ARPHRD_IEEE802154) {
>>>
>>>
>>>
>>> The above code is for finding the wpan interface (the real 802.15.4 L2 interface).
>>> For the IEEE 802.15.4 6LoWPAN interface the whole IPv6 implementation is
>>> used. This interface will be created inside function "newlink".
>>>
>>> Running "grep -r "src_net" net/ipv6" reports me alot uses of "src_net".
>>> Don't know if this information is really necessary.
>>>
>>> Should I set now the NETIF_F_NETNS_LOCAL for both interface types?
>> I think yes. If it's not set, a user may do:
>> $ ip link add link wpan0 name lowpan0 type lowpan
>> $ ip netns add foo
>> $ ip link set lowpan0 netns foo
>>
>
> We should forbid that for the wpan interface. The code line:
>
> real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
>
> searches for the "wpan0" interface which is given by:
> $ ip link add link wpan0 name lowpan0 type lowpan
>
> The returned real_dev netdevice pointer is the wpan interface. The given
> "lowpan0" interface is a virtual interface for making IPv6 stuff.
Yes. But with the current code, it seems that you can also do (not tested):
$ ip link add link wpan0 netns foo name lowpan0 type lowpan
With this cmd, src_net points to the netns where the ip cmd is launched (let's
say init_net in this case) but the interface lowpan0 will be created in the
netns foo. To summarize: wpan0 is in init_net and lowpan0 in foo.

Now, if you use this line:
real_dev = dev_get_by_index(dev_net(dev), nla_get_u32(tb[IFLA_LINK]));
With the same command, dev_net(dev) will point to foo.

>
>
>
> For 6LoWPAN:
>
> This interface is created in 6lowpan/core.c file and is used in the
> whole IPv6 stack, because we set the skb->protocol to htons(ETH_P_IPV6). [1]
>
> The IPv6 stack uses alot of "src_net".
In fact, src_net is used by IP tunnels (sit, ip6gre, ip6_tunnels, ip6_vti). But
these interfaces ignore this parameter.

The IPv6 stack is fully netns aware, thus it will use correctly the netns from
the netdevice and/or from the socket.

>
>> The flag forbids the last command.
>>
>> Instead of your patch, what about this one:
>>
>>  From d9a9cd22d5e1db1417b3ffb53cc020481dc761b2 Mon Sep 17 00:00:00 2001
>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> Date: Tue, 27 Jan 2015 11:26:20 +0100
>> Subject: [PATCH] ieee802154: forbid to create an iface in a netns != init_net
>>
>> 6LoWPAN currently doesn't supports netns.
>>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> ---
>>   net/ieee802154/6lowpan/core.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
>> index 055fbb71ba6f..fe8fd022042e 100644
>> --- a/net/ieee802154/6lowpan/core.c
>> +++ b/net/ieee802154/6lowpan/core.c
>> @@ -126,6 +126,7 @@ static void lowpan_setup(struct net_device *dev)
>>   	dev->header_ops		= &lowpan_header_ops;
>>   	dev->ml_priv		= &lowpan_mlme;
>>   	dev->destructor		= free_netdev;
>> +	dev->features		|= NETIF_F_NETNS_LOCAL;
>>   }
>>
>>   static int lowpan_validate(struct nlattr *tb[], struct nlattr *data[])
>> @@ -148,7 +149,9 @@ static int lowpan_newlink(struct net *src_net, struct
>> net_device *dev,
>>
>>   	pr_debug("adding new link\n");
>>
>> -	if (!tb[IFLA_LINK])
>> +	if (!tb[IFLA_LINK] ||
>> +	    !net_eq(src_net, &init_net) ||
>> +	    !net_eq(dev_net(dev), &init_net))
>>   		return -EINVAL;
>>   	/* find and hold real wpan device */
>>   	real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
>
> With the check of "!net_eq(src_net, &init_net)" we need to be sure
> that the wpan interface is always in "init_net". This means we need
> definitely a dev->features |= NETIF_F_NETNS_LOCAL; somewhere in [0].
>
> To adding "dev->features |= NETIF_F_NETNS_LOCAL;" for a 6LoWPAN interface,
> I am not sure about this. I didn't test it yet and it will not break
> anything, but we will lost the support for making net namespaces stuff
> inside the IPv6/(netfilter) stack.
Adding NETIF_F_NETNS_LOCAL does not mean that the netdevice can be used only
in init_net, this flag means that the netdevice cannot be moved to another
netns. You can still create a netdevice in another netns (if wpan0 is in netns
foo):

$ ip netns exec foo ip link add link wpan0 name lowpan0 type lowpan

I don't know how wpan0 is created and if this interface can be created directly
in another netns than init_net.

>
>
> Summarize:
>
> I would add the dev->features |= NETIF_F_NETNS_LOCAL; while wpan
> interface generation and add only the !net_eq(src_net, &init_net) check
> above. I suppose that src_net is the net namespace from "underlaying"
> interface wpan by calling:
>
> $ ip link add link wpan0 name lowpan0 type lowpan
No. src_net is the netns where the ip command is launched. With this patch, my
previous cmd (ip link add link wpan0 netns foo name lowpan0 type lowpan) will
still work and lowpan0 will be in netns foo.
Note: I just saw your patch proposal in another reply [0], with it, my sentence
is still true I think ;-)

But:
$ git grep "net_eq.*init_net" net/ieee802154/' | wc -l
7
So I wonder if it's really possible to use lowpan0 in another netns than
init_net (my proposal was based on this).

Maybe this can help you to understand my point.
A virtual interface can be across two netns:
  - the link netns (the interface receives and sends packets to/from the link
    layer into this netns)
  - the netns where the interface is visible (the user receives and sends
    packets to/from the interface into this netns)

[0] http://patchwork.ozlabs.org/patch/433467/

Regards,
Nicolas

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Alexander Aring]

Hi,

On Tue, Jan 27, 2015 at 02:28:47PM +0100, Nicolas Dichtel wrote:
...
> >With the check of "!net_eq(src_net, &init_net)" we need to be sure
> >that the wpan interface is always in "init_net". This means we need
> >definitely a dev->features |= NETIF_F_NETNS_LOCAL; somewhere in [0].
> >
> >To adding "dev->features |= NETIF_F_NETNS_LOCAL;" for a 6LoWPAN interface,
> >I am not sure about this. I didn't test it yet and it will not break
> >anything, but we will lost the support for making net namespaces stuff
> >inside the IPv6/(netfilter) stack.
> Adding NETIF_F_NETNS_LOCAL does not mean that the netdevice can be used only
> in init_net, this flag means that the netdevice cannot be moved to another
> netns. You can still create a netdevice in another netns (if wpan0 is in netns
> foo):
> 
> $ ip netns exec foo ip link add link wpan0 name lowpan0 type lowpan
> 
> I don't know how wpan0 is created and if this interface can be created directly
> in another netns than init_net.
> 

no it can't. The wpan0 interface can be created via the 802.15.4
userspace tools and we don't have such option for namespaces. It
should be always to init_net while creation.

> >
> >
> >Summarize:
> >
> >I would add the dev->features |= NETIF_F_NETNS_LOCAL; while wpan
> >interface generation and add only the !net_eq(src_net, &init_net) check
> >above. I suppose that src_net is the net namespace from "underlaying"
> >interface wpan by calling:
> >
> >$ ip link add link wpan0 name lowpan0 type lowpan
> No. src_net is the netns where the ip command is launched. With this patch, my

ah, and when no "ip netns" is given it's default to init_net?


Okay, then I agree with that both interfaces should be set

dev->features |= NETIF_F_NETNS_LOCAL

because both interfaces should started with "init_net" as default
namespace. For wpan interface this should always be in "init_net",
because we don't set anything while creation.

For 6LoWPAN interface this should also always in the same namespace like
the wpan interface and not diffrent namespace between link (wpan) and
virtual (6LoWPAN) interface.

Do you agree with that?

- Alex

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Nicolas Dichtel]

Le 27/01/2015 15:06, Alexander Aring a écrit :
> Hi,
>
> On Tue, Jan 27, 2015 at 02:28:47PM +0100, Nicolas Dichtel wrote:
> ...
[snip]
>>
>> I don't know how wpan0 is created and if this interface can be created directly
>> in another netns than init_net.
>>
>
> no it can't. The wpan0 interface can be created via the 802.15.4
> userspace tools and we don't have such option for namespaces. It
> should be always to init_net while creation.
Even with 'ip netns exec foo iwpan ...'?

>
>>>
>>>
>>> Summarize:
>>>
>>> I would add the dev->features |= NETIF_F_NETNS_LOCAL; while wpan
>>> interface generation and add only the !net_eq(src_net, &init_net) check
>>> above. I suppose that src_net is the net namespace from "underlaying"
>>> interface wpan by calling:
>>>
>>> $ ip link add link wpan0 name lowpan0 type lowpan
>> No. src_net is the netns where the ip command is launched. With this patch, my
>
> ah, and when no "ip netns" is given it's default to init_net?
The default netns is the netns where your shell is running :)
It may be different from init_net when you are playing on a virtual machine. On
a physical machine, it's usually init_net.

>
>
> Okay, then I agree with that both interfaces should be set
>
> dev->features |= NETIF_F_NETNS_LOCAL
Ok.

>
> because both interfaces should started with "init_net" as default
> namespace. For wpan interface this should always be in "init_net",
> because we don't set anything while creation.
Not sure this is true. It's probably possible to create it directly in another
netns (with 'ip netns exec' or because your system is a virtual machine that
runs over a namespaces construction (see docker [0], lxc [1], etc).

[0] https://www.docker.com/
[1] https://linuxcontainers.org/

>
> For 6LoWPAN interface this should also always in the same namespace like
> the wpan interface and not diffrent namespace between link (wpan) and
> virtual (6LoWPAN) interface.
>
> Do you agree with that?
Yes.

But I still wonder if we should add a check about dev_net(dev) != init_net in
net/ieee802154/6lowpan/core.c.
If my understanding is correct:
  - wpan can be created directly in a netns != init_net
  - 6lowpan must be in the same netns than wpan
  - code under net/ieee802154 only works in init_net, thus 6lowpan only works
    in init_net.

Do you agree?
What about this (based on net-next)?

 From 5ca1c46c68e4e4381b2f7e284f5dadeb28a53b2f Mon Sep 17 00:00:00 2001
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Tue, 27 Jan 2015 11:26:20 +0100
Subject: [PATCH] wpan/6lowpan: fix netns settings

6LoWPAN currently doesn't supports x-netns and works only in init_net.

With this patch, we ensure that:
  - the wpan interface cannot be moved to another netns;
  - the 6lowpan interface cannot be moved to another netns;
  - the wpan interface is in the same netns than the 6lowpan interface;
  - the 6lowpan interface is in init_net.

Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
  net/ieee802154/6lowpan/core.c | 6 ++++--
  net/mac802154/iface.c         | 1 +
  2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
index 055fbb71ba6f..dfd3c6007f60 100644
--- a/net/ieee802154/6lowpan/core.c
+++ b/net/ieee802154/6lowpan/core.c
@@ -126,6 +126,7 @@ static void lowpan_setup(struct net_device *dev)
  	dev->header_ops		= &lowpan_header_ops;
  	dev->ml_priv		= &lowpan_mlme;
  	dev->destructor		= free_netdev;
+	dev->features		|= NETIF_F_NETNS_LOCAL;
  }

  static int lowpan_validate(struct nlattr *tb[], struct nlattr *data[])
@@ -148,10 +149,11 @@ static int lowpan_newlink(struct net *src_net, struct 
net_device *dev,

  	pr_debug("adding new link\n");

-	if (!tb[IFLA_LINK])
+	if (!tb[IFLA_LINK] ||
+	    !net_eq(dev_net(dev), &init_net))
  		return -EINVAL;
  	/* find and hold real wpan device */
-	real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
+	real_dev = dev_get_by_index(dev_net(dev), nla_get_u32(tb[IFLA_LINK]));
  	if (!real_dev)
  		return -ENODEV;
  	if (real_dev->type != ARPHRD_IEEE802154) {
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 6fb6bdf9868c..b67da8d578b4 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -475,6 +475,7 @@ static void ieee802154_if_setup(struct net_device *dev)
  	dev->mtu		= IEEE802154_MTU;
  	dev->tx_queue_len	= 300;
  	dev->flags		= IFF_NOARP | IFF_BROADCAST;
+	dev->features		|= NETIF_F_NETNS_LOCAL;
  }

  static int
-- 
2.2.2

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Alexander Aring]

On Tue, Jan 27, 2015 at 03:50:31PM +0100, Nicolas Dichtel wrote:
> Le 27/01/2015 15:06, Alexander Aring a écrit :
> >Hi,
> >
> >On Tue, Jan 27, 2015 at 02:28:47PM +0100, Nicolas Dichtel wrote:
> >...
> [snip]
> >>
> >>I don't know how wpan0 is created and if this interface can be created directly
> >>in another netns than init_net.
> >>
> >
> >no it can't. The wpan0 interface can be created via the 802.15.4
> >userspace tools and we don't have such option for namespaces. It
> >should be always to init_net while creation.
> Even with 'ip netns exec foo iwpan ...'?
> 

I did a quick test:

diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index 6fb6bdf..df55b42 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -590,6 +590,11 @@ ieee802154_if_add(struct ieee802154_local *local, const char *name,
        list_add_tail_rcu(&sdata->list, &local->interfaces);
        mutex_unlock(&local->iflist_mtx);
 
+       if (net_eq(dev_net(ndev), &init_net))
+               printk(KERN_INFO "it's init_net\n");
+       else
+               printk(KERN_INFO "it's not init_net\n");

With this patch and running:

ip netns exec foo iwpan phy phy0 interface add bar%d type node

I got a:

[   52.032956] it's init_net

It's also init_net when I run with "ip netns exec foo iwpan ..."

> >
> >>>
> >>>
> >>>Summarize:
> >>>
> >>>I would add the dev->features |= NETIF_F_NETNS_LOCAL; while wpan
> >>>interface generation and add only the !net_eq(src_net, &init_net) check
> >>>above. I suppose that src_net is the net namespace from "underlaying"
> >>>interface wpan by calling:
> >>>
> >>>$ ip link add link wpan0 name lowpan0 type lowpan
> >>No. src_net is the netns where the ip command is launched. With this patch, my
> >
> >ah, and when no "ip netns" is given it's default to init_net?
> The default netns is the netns where your shell is running :)

Now, I understood how basically that works (I think).

> It may be different from init_net when you are playing on a virtual machine. On
> a physical machine, it's usually init_net.
> 

ok.

> >
> >
> >Okay, then I agree with that both interfaces should be set
> >
> >dev->features |= NETIF_F_NETNS_LOCAL
> Ok.
> 
> >
> >because both interfaces should started with "init_net" as default
> >namespace. For wpan interface this should always be in "init_net",
> >because we don't set anything while creation.
> Not sure this is true. It's probably possible to create it directly in another
> netns (with 'ip netns exec' or because your system is a virtual machine that
> runs over a namespaces construction (see docker [0], lxc [1], etc).
> 
> [0] https://www.docker.com/
> [1] https://linuxcontainers.org/
> 

ah, ok.

> >
> >For 6LoWPAN interface this should also always in the same namespace like
> >the wpan interface and not diffrent namespace between link (wpan) and
> >virtual (6LoWPAN) interface.
> >
> >Do you agree with that?
> Yes.
> 
> But I still wonder if we should add a check about dev_net(dev) != init_net in
> net/ieee802154/6lowpan/core.c.
> If my understanding is correct:
>  - wpan can be created directly in a netns != init_net
>  - 6lowpan must be in the same netns than wpan
>  - code under net/ieee802154 only works in init_net, thus 6lowpan only works
>    in init_net.
> 
> Do you agree?

yes. I will put the last item on my ToDo list if we can do more netns
stuff there.

> What about this (based on net-next)?
> 

There are some little issues, see below.

> From 5ca1c46c68e4e4381b2f7e284f5dadeb28a53b2f Mon Sep 17 00:00:00 2001
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Tue, 27 Jan 2015 11:26:20 +0100
> Subject: [PATCH] wpan/6lowpan: fix netns settings
> 

use "ieee802154:" instead "wpan/6lowpan:".

Can you rebase this patch on bluetooth-next?

> 6LoWPAN currently doesn't supports x-netns and works only in init_net.
> 
> With this patch, we ensure that:
>  - the wpan interface cannot be moved to another netns;
>  - the 6lowpan interface cannot be moved to another netns;
>  - the wpan interface is in the same netns than the 6lowpan interface;
>  - the 6lowpan interface is in init_net.
> 
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>  net/ieee802154/6lowpan/core.c | 6 ++++--
>  net/mac802154/iface.c         | 1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ieee802154/6lowpan/core.c b/net/ieee802154/6lowpan/core.c
> index 055fbb71ba6f..dfd3c6007f60 100644
> --- a/net/ieee802154/6lowpan/core.c
> +++ b/net/ieee802154/6lowpan/core.c
> @@ -126,6 +126,7 @@ static void lowpan_setup(struct net_device *dev)
>  	dev->header_ops		= &lowpan_header_ops;
>  	dev->ml_priv		= &lowpan_mlme;
>  	dev->destructor		= free_netdev;
> +	dev->features		|= NETIF_F_NETNS_LOCAL;
>  }
> 
>  static int lowpan_validate(struct nlattr *tb[], struct nlattr *data[])
> @@ -148,10 +149,11 @@ static int lowpan_newlink(struct net *src_net, struct
> net_device *dev,
> 
>  	pr_debug("adding new link\n");
> 
> -	if (!tb[IFLA_LINK])
> +	if (!tb[IFLA_LINK] ||
> +	    !net_eq(dev_net(dev), &init_net))
>  		return -EINVAL;
>  	/* find and hold real wpan device */
> -	real_dev = dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
> +	real_dev = dev_get_by_index(dev_net(dev), nla_get_u32(tb[IFLA_LINK]));
>  	if (!real_dev)
>  		return -ENODEV;
>  	if (real_dev->type != ARPHRD_IEEE802154) {
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 6fb6bdf9868c..b67da8d578b4 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -475,6 +475,7 @@ static void ieee802154_if_setup(struct net_device *dev)
>  	dev->mtu		= IEEE802154_MTU;
>  	dev->tx_queue_len	= 300;
>  	dev->flags		= IFF_NOARP | IFF_BROADCAST;
> +	dev->features		|= NETIF_F_NETNS_LOCAL;

We should set this inside the cfg802154_netdev_notifier_call function
and NETDEV_REGISTER case. This can be found in "net/ieee802154/core.c". [0]

The branch "net/mac802154" affects 802.15.4 SoftMAC interfaces only. We
currently support SoftMAC only, but further we support HardMAC drivers.
The HardMAC drivers doesn't use the "net/mac802154" branch and call
netdev_register in driver layer.

When we do it in cfg802154_netdev_notifier_call then this will be set
for SoftMAC and HardMAC drivers (when we have a HardMAC driver). The
same behaviour can also be found in wireless implementation. [1]

- Alex

[0] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/net/ieee802154/core.c#n227
[1] http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/net/wireless/core.c#n1000

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Nicolas Dichtel]

Le 27/01/2015 21:26, Alexander Aring a écrit :
> On Tue, Jan 27, 2015 at 03:50:31PM +0100, Nicolas Dichtel wrote:
>> Le 27/01/2015 15:06, Alexander Aring a écrit :
>>> Hi,
>>>
>>> On Tue, Jan 27, 2015 at 02:28:47PM +0100, Nicolas Dichtel wrote:
>>> ...
>> [snip]
>>>>
>>>> I don't know how wpan0 is created and if this interface can be created directly
>>>> in another netns than init_net.
>>>>
>>>
>>> no it can't. The wpan0 interface can be created via the 802.15.4
>>> userspace tools and we don't have such option for namespaces. It
>>> should be always to init_net while creation.
>> Even with 'ip netns exec foo iwpan ...'?
>>
>
> I did a quick test:
>
> diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
> index 6fb6bdf..df55b42 100644
> --- a/net/mac802154/iface.c
> +++ b/net/mac802154/iface.c
> @@ -590,6 +590,11 @@ ieee802154_if_add(struct ieee802154_local *local, const char *name,
>          list_add_tail_rcu(&sdata->list, &local->interfaces);
>          mutex_unlock(&local->iflist_mtx);
>
> +       if (net_eq(dev_net(ndev), &init_net))
> +               printk(KERN_INFO "it's init_net\n");
> +       else
> +               printk(KERN_INFO "it's not init_net\n");
>
> With this patch and running:
>
> ip netns exec foo iwpan phy phy0 interface add bar%d type node
>
> I got a:
>
> [   52.032956] it's init_net
>
> It's also init_net when I run with "ip netns exec foo iwpan ..."
Ok. After looking at the code, it's right that no netns is set in
ieee802154_if_add(), thus the interface is in the default netns (init_net).

[snip]
>>
>> But I still wonder if we should add a check about dev_net(dev) != init_net in
>> net/ieee802154/6lowpan/core.c.
>> If my understanding is correct:
>>   - wpan can be created directly in a netns != init_net
>>   - 6lowpan must be in the same netns than wpan
>>   - code under net/ieee802154 only works in init_net, thus 6lowpan only works
>>     in init_net.
>>
>> Do you agree?
>
> yes. I will put the last item on my ToDo list if we can do more netns
> stuff there.
Cool.

>
>> What about this (based on net-next)?
>>
>
> There are some little issues, see below.
>
>>  From 5ca1c46c68e4e4381b2f7e284f5dadeb28a53b2f Mon Sep 17 00:00:00 2001
>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> Date: Tue, 27 Jan 2015 11:26:20 +0100
>> Subject: [PATCH] wpan/6lowpan: fix netns settings
>>
>
> use "ieee802154:" instead "wpan/6lowpan:".
Ok.

>
> Can you rebase this patch on bluetooth-next?
Will do.

[snip]
>> --- a/net/mac802154/iface.c
>> +++ b/net/mac802154/iface.c
>> @@ -475,6 +475,7 @@ static void ieee802154_if_setup(struct net_device *dev)
>>   	dev->mtu		= IEEE802154_MTU;
>>   	dev->tx_queue_len	= 300;
>>   	dev->flags		= IFF_NOARP | IFF_BROADCAST;
>> +	dev->features		|= NETIF_F_NETNS_LOCAL;
>
> We should set this inside the cfg802154_netdev_notifier_call function
> and NETDEV_REGISTER case. This can be found in "net/ieee802154/core.c". [0]
>
> The branch "net/mac802154" affects 802.15.4 SoftMAC interfaces only. We
> currently support SoftMAC only, but further we support HardMAC drivers.
> The HardMAC drivers doesn't use the "net/mac802154" branch and call
> netdev_register in driver layer.
>
> When we do it in cfg802154_netdev_notifier_call then this will be set
> for SoftMAC and HardMAC drivers (when we have a HardMAC driver). The
> same behaviour can also be found in wireless implementation. [1]
Ok.


Regards,
Nicolas

Re: [PATCH net 1/2] caif: remove wrong dev_net_set() call

[Nicolas Dichtel]

Removing bouncing email addresses (dmitry.tarnyagin@lockless.no and
sjur.brandeland@stericsson.com)
CC: Dmitry Tarnyagin <abi.dmitryt@gmail.com>

Dmitry, please also have a look to
http://permalink.gmane.org/gmane.linux.network/347942


Thank you,
Nicolas

Le 26/01/2015 22:28, Nicolas Dichtel a écrit :
> src_net points to the netns where the netlink message has been received. This
> netns may be different from the netns where the interface is created (because
> the user may add IFLA_NET_NS_[PID|FD]). In this case, src_net is the link netns.
>
> It seems wrong to override the netns in the newlink() handler because if it
> was not already src_net, it means that the user explicitly asks to create the
> netdevice in another netns.
>
> CC: Sjur Brændeland <sjur.brandeland@stericsson.com>
> CC: Dmitry Tarnyagin <dmitry.tarnyagin@lockless.no>
> Fixes: 8391c4aab1aa ("caif: Bugfixes in CAIF netdevice for close and flow control")
> Fixes: c41254006377 ("caif-hsi: Add rtnl support")
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
>   drivers/net/caif/caif_hsi.c | 1 -
>   net/caif/chnl_net.c         | 1 -
>   2 files changed, 2 deletions(-)
>
> diff --git a/drivers/net/caif/caif_hsi.c b/drivers/net/caif/caif_hsi.c
> index 5e40a8b68cbe..b3b922adc0e4 100644
> --- a/drivers/net/caif/caif_hsi.c
> +++ b/drivers/net/caif/caif_hsi.c
> @@ -1415,7 +1415,6 @@ static int caif_hsi_newlink(struct net *src_net, struct net_device *dev,
>
>   	cfhsi = netdev_priv(dev);
>   	cfhsi_netlink_parms(data, cfhsi);
> -	dev_net_set(cfhsi->ndev, src_net);
>
>   	get_ops = symbol_get(cfhsi_get_ops);
>   	if (!get_ops) {
> diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
> index 4589ff67bfa9..67a4a36febd1 100644
> --- a/net/caif/chnl_net.c
> +++ b/net/caif/chnl_net.c
> @@ -470,7 +470,6 @@ static int ipcaif_newlink(struct net *src_net, struct net_device *dev,
>   	ASSERT_RTNL();
>   	caifdev = netdev_priv(dev);
>   	caif_netlink_parms(data, &caifdev->conn_req);
> -	dev_net_set(caifdev->netdev, src_net);
>
>   	ret = register_netdevice(dev);
>   	if (ret)
>

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[David Miller]

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Mon, 26 Jan 2015 22:28:12 +0100

> When one of these attributes is set, the netdevice is created into the netns
> pointed by IFLA_NET_NS_[PID|FD] (see the call to rtnl_create_link() in
> rtnl_newlink()). Let's call this netns the dest_net. After this creation, if the
> newlink handler exists, it is called with a netns argument that points to the
> netns where the netlink message has been received (called src_net in the code)
> which is the link netns.
> Hence, with one of these attributes, it's possible to create a x-netns
> netdevice.

Series applied, thanks.

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Arvid Brodin]

On 2015-01-26 22:28, Nicolas Dichtel wrote:
*snip*
> - HSR subsystem uses src_net to parse IFLA_HSR_SLAVE[1|2], but the netdevice has
>   the flag NETIF_F_NETNS_LOCAL, so the question is: does this netdevice really
>   supports x-netns? If not, the newlink handler should use the dest_net instead
>   of src_net, I can provide the patch.
*snip*

As the author of the HSR driver, I'd like to answer this question, but unfortunately 
I don't know what x-netns is. Neither Google nor Documentation/ has been particularly
helpful.

Care to elaborate? (Maybe this is a moot point now that the patch has been accepted,
but I'd still like to understand, if you have the time to explain.)

(Pretty much all I know about network namespaces is from this article: 
http://lwn.net/Articles/580893/.)


-- 
Arvid Brodin | Consultant (Linux)
ALTEN | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden
arvid.brodin@alten.se | www.alten.se/en/

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Arvid Brodin]

On 2015-01-26 22:28, Nicolas Dichtel wrote:
*snip*
> - HSR subsystem uses src_net to parse IFLA_HSR_SLAVE[1|2], but the netdevice has
>   the flag NETIF_F_NETNS_LOCAL, so the question is: does this netdevice really
>   supports x-netns? If not, the newlink handler should use the dest_net instead
>   of src_net, I can provide the patch.
*snip*

As the author of the HSR driver, I'd like to answer this question, but unfortunately 
I don't know what x-netns is. Neither Google nor Documentation/ has been particularly
helpful.

Care to elaborate? (Maybe this is a moot point now that the patch has been accepted,
but I'd still like to understand, if you have the time to explain.)

(Pretty much all I know about network namespaces is from this article: 
http://lwn.net/Articles/580893/.)


-- 
Arvid Brodin | Consultant (Linux)
ALTEN | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden
arvid.brodin@alten.se | www.alten.se/en/

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Nicolas Dichtel]

Le 30/01/2015 21:00, Arvid Brodin a écrit :
> On 2015-01-26 22:28, Nicolas Dichtel wrote:
> *snip*
>> - HSR subsystem uses src_net to parse IFLA_HSR_SLAVE[1|2], but the netdevice has
>>    the flag NETIF_F_NETNS_LOCAL, so the question is: does this netdevice really
>>    supports x-netns? If not, the newlink handler should use the dest_net instead
>>    of src_net, I can provide the patch.
> *snip*
>
> As the author of the HSR driver, I'd like to answer this question, but unfortunately
> I don't know what x-netns is. Neither Google nor Documentation/ has been particularly
> helpful.
>
> Care to elaborate? (Maybe this is a moot point now that the patch has been accepted,
> but I'd still like to understand, if you have the time to explain.)
Basically, network namespaces (netns) allow you to run several independant
instances of the linux networking stack.
Network interfaces are bound to one netns. By default, only one netns exists
(named init_net) when you boot your kernel.
For logical interfaces, they are usually bound to a link layer. For example, if
I understand well, hsr network interfaces receive and send their packets from
two physical interfaces (IFLA_HSR_SLAVE[1|2]).
Now imagine that these slaves are in a netns foo and the logical hsr interfaces
in netns bar. You have a x-netns interface, the link layer part of the interface
is not in the same netns than the upper part. A user will see the hsr interface
in netns bar, but this interface will send a receive packet in netns foo.
Usually, to configure an interface like this, you create it in netns foo and you
move it later to netns bar (ip link set hsr1 netns bar). The flag
NETIF_F_NETNS_LOCAL forbids this operation, you cannot move it to another netns.
But you still can create a x-netns interface:
ip netns add foo
ip link add hsr1 netns foo type hsr slave1 eth0 slave2 eth1
ip netns exec foo ip link ls hsr1

=> eth0 and eth1 are took from the current netns (because in the code, src_net
is the current netns) but hsr1 is built in netns foo.

Now, the question is: does HSR really work across netns? Why is the flag
NETIF_F_NETNS_LOCAL set?
dev_forward_skb() may be used to forward an skbuff to another netns.

Note, that I got a panic when playing with hsr:
ip link add hsr1 type hsr slave1 eth1 slave2 eth0
ip link del hsr1
=> panic

I dig a bit:
1/ hsr_netdev_notify() supposes that the port will always be available when the
notification is for an hsr interface. It's wrong. For example,
netdev_wait_allrefs() may resend NETDEV_UNREGISTER.
2/ with a patch that ignores the notification when the port is NULL, I got a
refcnt problem:
[  327.372099] unregister_netdevice: waiting for hsr1 to become free. Usage 
count = -1

Regards,
Nicolas

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Arvid Brodin]

On 2015-02-02 16:58, Nicolas Dichtel wrote:
> Le 30/01/2015 21:00, Arvid Brodin a écrit :
>> On 2015-01-26 22:28, Nicolas Dichtel wrote:
>> *snip*
>>> - HSR subsystem uses src_net to parse IFLA_HSR_SLAVE[1|2], but the netdevice has
>>>    the flag NETIF_F_NETNS_LOCAL, so the question is: does this netdevice really
>>>    supports x-netns? If not, the newlink handler should use the dest_net instead
>>>    of src_net, I can provide the patch.
>> *snip*
>>
>> As the author of the HSR driver, I'd like to answer this question, but unfortunately
>> I don't know what x-netns is. Neither Google nor Documentation/ has been particularly
>> helpful.
>>
>> Care to elaborate? (Maybe this is a moot point now that the patch has been accepted,
>> but I'd still like to understand, if you have the time to explain.)
> Basically, network namespaces (netns) allow you to run several independant
> instances of the linux networking stack.
> Network interfaces are bound to one netns. By default, only one netns exists
> (named init_net) when you boot your kernel.
> For logical interfaces, they are usually bound to a link layer. For example, if
> I understand well, hsr network interfaces receive and send their packets from
> two physical interfaces (IFLA_HSR_SLAVE[1|2]).
> Now imagine that these slaves are in a netns foo and the logical hsr interfaces
> in netns bar. You have a x-netns interface, the link layer part of the interface
> is not in the same netns than the upper part. A user will see the hsr interface
> in netns bar, but this interface will send a receive packet in netns foo.

Ok, so x-netns simply means cross-netns?

> Usually, to configure an interface like this, you create it in netns foo and you
> move it later to netns bar (ip link set hsr1 netns bar). The flag
> NETIF_F_NETNS_LOCAL forbids this operation, you cannot move it to another netns.
> But you still can create a x-netns interface:
> ip netns add foo
> ip link add hsr1 netns foo type hsr slave1 eth0 slave2 eth1
> ip netns exec foo ip link ls hsr1
> 
> => eth0 and eth1 are took from the current netns (because in the code, src_net
> is the current netns) but hsr1 is built in netns foo.
> 
> Now, the question is: does HSR really work across netns? Why is the flag
> NETIF_F_NETNS_LOCAL set?
> dev_forward_skb() may be used to forward an skbuff to another netns.

Here is the code snippet that sets NETIF_F_NETNS_LOCAL:
	/* Not sure about this. Taken from bridge code. netdev_features.h says
	 * it means "Does not change network namespaces".
	 */
	dev->features |= NETIF_F_NETNS_LOCAL;

HSR is a bit like a bridge since it forwards packets between interfaces on the 
same Ethernet network, and the bridge code sets NETIF_F_NETNS_LOCAL. And that's 
really all the reason for the inclusion of the flag - i.e. it should be removed
if it doesn't make sense.

So, does it make sense? I'm not sure exactly, but I don't think it makes sense
to have slaves that are in different namespaces - they are supposed to be part 
of the same ethernet network after all. But maybe having the HSR interface in a 
different namespace than the two slaves could make sense - this way you could 
force an application to only communicate using the HSR protocol, and not use any 
of the slave interfaces directly.

If you agree with the above, then I guess that means NETIF_F_NETNS_LOCAL should 
not be set?


> Note, that I got a panic when playing with hsr:
> ip link add hsr1 type hsr slave1 eth1 slave2 eth0
> ip link del hsr1
> => panic
> 
> I dig a bit:
> 1/ hsr_netdev_notify() supposes that the port will always be available when the
> notification is for an hsr interface. It's wrong. For example,
> netdev_wait_allrefs() may resend NETDEV_UNREGISTER.
> 2/ with a patch that ignores the notification when the port is NULL, I got a
> refcnt problem:
> [  327.372099] unregister_netdevice: waiting for hsr1 to become free. Usage count = -1

Thanks for the bug report! I'll take a look at it ASAP.


-- 
Arvid Brodin | Consultant (Linux)
ALTEN | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden
arvid.brodin@alten.se | www.alten.se/en/

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Arvid Brodin]

On 2015-02-02 16:58, Nicolas Dichtel wrote:
> Le 30/01/2015 21:00, Arvid Brodin a écrit :
>> On 2015-01-26 22:28, Nicolas Dichtel wrote:
>> *snip*
>>> - HSR subsystem uses src_net to parse IFLA_HSR_SLAVE[1|2], but the netdevice has
>>>    the flag NETIF_F_NETNS_LOCAL, so the question is: does this netdevice really
>>>    supports x-netns? If not, the newlink handler should use the dest_net instead
>>>    of src_net, I can provide the patch.
>> *snip*
>>
>> As the author of the HSR driver, I'd like to answer this question, but unfortunately
>> I don't know what x-netns is. Neither Google nor Documentation/ has been particularly
>> helpful.
>>
>> Care to elaborate? (Maybe this is a moot point now that the patch has been accepted,
>> but I'd still like to understand, if you have the time to explain.)
> Basically, network namespaces (netns) allow you to run several independant
> instances of the linux networking stack.
> Network interfaces are bound to one netns. By default, only one netns exists
> (named init_net) when you boot your kernel.
> For logical interfaces, they are usually bound to a link layer. For example, if
> I understand well, hsr network interfaces receive and send their packets from
> two physical interfaces (IFLA_HSR_SLAVE[1|2]).
> Now imagine that these slaves are in a netns foo and the logical hsr interfaces
> in netns bar. You have a x-netns interface, the link layer part of the interface
> is not in the same netns than the upper part. A user will see the hsr interface
> in netns bar, but this interface will send a receive packet in netns foo.

Ok, so x-netns simply means cross-netns?

> Usually, to configure an interface like this, you create it in netns foo and you
> move it later to netns bar (ip link set hsr1 netns bar). The flag
> NETIF_F_NETNS_LOCAL forbids this operation, you cannot move it to another netns.
> But you still can create a x-netns interface:
> ip netns add foo
> ip link add hsr1 netns foo type hsr slave1 eth0 slave2 eth1
> ip netns exec foo ip link ls hsr1
> 
> => eth0 and eth1 are took from the current netns (because in the code, src_net
> is the current netns) but hsr1 is built in netns foo.
> 
> Now, the question is: does HSR really work across netns? Why is the flag
> NETIF_F_NETNS_LOCAL set?
> dev_forward_skb() may be used to forward an skbuff to another netns.

Here is the code snippet that sets NETIF_F_NETNS_LOCAL:
	/* Not sure about this. Taken from bridge code. netdev_features.h says
	 * it means "Does not change network namespaces".
	 */
	dev->features |= NETIF_F_NETNS_LOCAL;

HSR is a bit like a bridge since it forwards packets between interfaces on the 
same Ethernet network, and the bridge code sets NETIF_F_NETNS_LOCAL. And that's 
really all the reason for the inclusion of the flag - i.e. it should be removed
if it doesn't make sense.

So, does it make sense? I'm not sure exactly, but I don't think it makes sense
to have slaves that are in different namespaces - they are supposed to be part 
of the same ethernet network after all. But maybe having the HSR interface in a 
different namespace than the two slaves could make sense - this way you could 
force an application to only communicate using the HSR protocol, and not use any 
of the slave interfaces directly.

If you agree with the above, then I guess that means NETIF_F_NETNS_LOCAL should 
not be set?


> Note, that I got a panic when playing with hsr:
> ip link add hsr1 type hsr slave1 eth1 slave2 eth0
> ip link del hsr1
> => panic
> 
> I dig a bit:
> 1/ hsr_netdev_notify() supposes that the port will always be available when the
> notification is for an hsr interface. It's wrong. For example,
> netdev_wait_allrefs() may resend NETDEV_UNREGISTER.
> 2/ with a patch that ignores the notification when the port is NULL, I got a
> refcnt problem:
> [  327.372099] unregister_netdevice: waiting for hsr1 to become free. Usage count = -1

Thanks for the bug report! I'll take a look at it ASAP.


-- 
Arvid Brodin | Consultant (Linux)
ALTEN | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden
arvid.brodin@alten.se | www.alten.se/en/

Re: [PATCH net 0/2] netns: audit netdevice creation with IFLA_NET_NS_[PID|FD]

[Nicolas Dichtel]

Le 04/02/2015 21:33, Arvid Brodin a écrit :
> On 2015-02-02 16:58, Nicolas Dichtel wrote:
>> Le 30/01/2015 21:00, Arvid Brodin a écrit :
>>> On 2015-01-26 22:28, Nicolas Dichtel wrote:
[snip]
> Ok, so x-netns simply means cross-netns?
Yes
>
[snip]
>> Now, the question is: does HSR really work across netns? Why is the flag
>> NETIF_F_NETNS_LOCAL set?
>> dev_forward_skb() may be used to forward an skbuff to another netns.
>
> Here is the code snippet that sets NETIF_F_NETNS_LOCAL:
> 	/* Not sure about this. Taken from bridge code. netdev_features.h says
> 	 * it means "Does not change network namespaces".
> 	 */
> 	dev->features |= NETIF_F_NETNS_LOCAL;
>
> HSR is a bit like a bridge since it forwards packets between interfaces on the
> same Ethernet network, and the bridge code sets NETIF_F_NETNS_LOCAL. And that's
> really all the reason for the inclusion of the flag - i.e. it should be removed
> if it doesn't make sense.
>
> So, does it make sense? I'm not sure exactly, but I don't think it makes sense
> to have slaves that are in different namespaces - they are supposed to be part
> of the same ethernet network after all. But maybe having the HSR interface in a
> different namespace than the two slaves could make sense - this way you could
> force an application to only communicate using the HSR protocol, and not use any
> of the slave interfaces directly.
>
> If you agree with the above, then I guess that means NETIF_F_NETNS_LOCAL should
> not be set?
It's ok for me. But I think some tests should be done. Usually,
dev_forward_skb() or skb_scrub_packet() are called to clean structures when a
skb crosses netns.