All of lore.kernel.org
 help / color / mirror / Atom feed
* iproute2: potential upgrade regression with 58a3e827
@ 2013-11-08 18:03 Chris J Arges
  2013-11-08 21:36 ` Eric W. Biederman
  2013-11-09 17:00 ` Brian Haley
  0 siblings, 2 replies; 11+ messages in thread
From: Chris J Arges @ 2013-11-08 18:03 UTC (permalink / raw)
  To: shemminger, netdev

Hi,
The commit
https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=58a3e8270fe72f8ed92687d3a3132c2a708582dd
could be potentially introducing a regression on an upgrade.

I've noticed that upgrading iproute while there are active namespaces
could cause the following error:
seting the network namespace failed: Invalid argument

Here's a test case:
Build and install iproute2 with 4395d48c78a77a99c5a8618403211032356fe552

In one terminal run:
ip netns add netns_old
ip link add name if_old type veth peer name if_old_peer
ip link set dev if_old_peer netns netns_old
ip netns exec netns_old bash

Build and install iproute2 with 58a3e8270fe72f8ed92687d3a3132c2a708582dd

In the same terminal as you typed the original commands run:
ip netns add netns_one
ip link add name if_one type veth peer name if_one_peer
ip link set dev if_one_peer netns netns_one
ip netns exec netns_one bash
ip netns exec netns_old bash

You'll get:
seting the network namespace failed: Invalid argument

If you just run the above without transitioning to the code in 58a3e827,
then it works.

Thanks,
--chris j arges

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: iproute2: potential upgrade regression with 58a3e827
  2013-11-08 18:03 iproute2: potential upgrade regression with 58a3e827 Chris J Arges
@ 2013-11-08 21:36 ` Eric W. Biederman
  2013-11-08 22:30   ` Chris J Arges
  2013-11-09 17:00 ` Brian Haley
  1 sibling, 1 reply; 11+ messages in thread
From: Eric W. Biederman @ 2013-11-08 21:36 UTC (permalink / raw)
  To: Chris J Arges; +Cc: shemminger, netdev

Chris J Arges <chris.j.arges@canonical.com> writes:

> Hi,
> The commit
> https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=58a3e8270fe72f8ed92687d3a3132c2a708582dd
> could be potentially introducing a regression on an upgrade.
>
> I've noticed that upgrading iproute while there are active namespaces
> could cause the following error:
> seting the network namespace failed: Invalid argument
>
> Here's a test case:
> Build and install iproute2 with 4395d48c78a77a99c5a8618403211032356fe552
>
> In one terminal run:
> ip netns add netns_old
> ip link add name if_old type veth peer name if_old_peer
> ip link set dev if_old_peer netns netns_old
> ip netns exec netns_old bash
>
> Build and install iproute2 with 58a3e8270fe72f8ed92687d3a3132c2a708582dd
>
> In the same terminal as you typed the original commands run:
> ip netns add netns_one
> ip link add name if_one type veth peer name if_one_peer
> ip link set dev if_one_peer netns netns_one
> ip netns exec netns_one bash
> ip netns exec netns_old bash
>
> You'll get:
> seting the network namespace failed: Invalid argument
>
> If you just run the above without transitioning to the code in 58a3e827,
> then it works.

ip netns exec is fundamentally designed to run code that knows nothing
about network namespaces without modification.  If we can make network
namespace aware code work that is great but the entire point of ip netns
exec is to remove the need to convert every network aware program in
linux to be network namespace aware.

Shared subtree support that is added in 58a3e827 should make things a
little better, by letting changes that happen in the initial mount
namespace propogate into children running inside of ip netns exec.

It appears that somehow you wound up in a mount namespace where the
mount of netns_one did not propagate too.  Which is weird.  I expect
we are missing an error message somewhere.

So far nothing allows network namespaces added like:
ip netns exec ns1 ip netns add ns2
to propogate up to the initial mount namespace and that will definitely
cause problems, but not the problem you are seeing.

Shrug.  I will be happy to review patches to make this better but I
don't care enough to investigate.  If you aren't up to writing patches
to make ip netns work inside of ip netns exec I recommend you don't do
that then.

Eric

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: iproute2: potential upgrade regression with 58a3e827
  2013-11-08 21:36 ` Eric W. Biederman
@ 2013-11-08 22:30   ` Chris J Arges
  2013-11-08 22:42     ` Eric W. Biederman
  0 siblings, 1 reply; 11+ messages in thread
From: Chris J Arges @ 2013-11-08 22:30 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: shemminger, netdev

On 11/08/2013 03:36 PM, Eric W. Biederman wrote:
> Chris J Arges <chris.j.arges@canonical.com> writes:
> 
>> Hi,
>> The commit
>> https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=58a3e8270fe72f8ed92687d3a3132c2a708582dd
>> could be potentially introducing a regression on an upgrade.
>>
>> I've noticed that upgrading iproute while there are active namespaces
>> could cause the following error:
>> seting the network namespace failed: Invalid argument
>>
>> Here's a test case:
>> Build and install iproute2 with 4395d48c78a77a99c5a8618403211032356fe552
>>
>> In one terminal run:
>> ip netns add netns_old
>> ip link add name if_old type veth peer name if_old_peer
>> ip link set dev if_old_peer netns netns_old
>> ip netns exec netns_old bash
>>
>> Build and install iproute2 with 58a3e8270fe72f8ed92687d3a3132c2a708582dd
>>
>> In the same terminal as you typed the original commands run:
>> ip netns add netns_one
>> ip link add name if_one type veth peer name if_one_peer
>> ip link set dev if_one_peer netns netns_one
>> ip netns exec netns_one bash
>> ip netns exec netns_old bash
>>
>> You'll get:
>> seting the network namespace failed: Invalid argument
>>
>> If you just run the above without transitioning to the code in 58a3e827,
>> then it works.
> 
> ip netns exec is fundamentally designed to run code that knows nothing
> about network namespaces without modification.  If we can make network
> namespace aware code work that is great but the entire point of ip netns
> exec is to remove the need to convert every network aware program in
> linux to be network namespace aware.
> 
> Shared subtree support that is added in 58a3e827 should make things a
> little better, by letting changes that happen in the initial mount
> namespace propogate into children running inside of ip netns exec.
> 
> It appears that somehow you wound up in a mount namespace where the
> mount of netns_one did not propagate too.  Which is weird.  I expect
> we are missing an error message somewhere.
> 
> So far nothing allows network namespaces added like:
> ip netns exec ns1 ip netns add ns2
> to propogate up to the initial mount namespace and that will definitely
> cause problems, but not the problem you are seeing.
> 
> Shrug.  I will be happy to review patches to make this better but I
> don't care enough to investigate.  If you aren't up to writing patches
> to make ip netns work inside of ip netns exec I recommend you don't do
> that then.
> 
> Eric

Fair enough. I really wanted to get a feel if this seemed like an actual
bug, or rather it's behaving like we'd expect. I don't mind writing
code, but I want to know if I'm hunting a bug, or adding a feature.

Thanks for the quick reply,
--chris j arges

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: iproute2: potential upgrade regression with 58a3e827
  2013-11-08 22:30   ` Chris J Arges
@ 2013-11-08 22:42     ` Eric W. Biederman
  0 siblings, 0 replies; 11+ messages in thread
From: Eric W. Biederman @ 2013-11-08 22:42 UTC (permalink / raw)
  To: Chris J Arges; +Cc: shemminger, netdev

Chris J Arges <chris.j.arges@canonical.com> writes:

> Fair enough. I really wanted to get a feel if this seemed like an actual
> bug, or rather it's behaving like we'd expect. I don't mind writing
> code, but I want to know if I'm hunting a bug, or adding a feature.

Making it ip netns add work reliably inside of ip netns exec and
handling all of the permutations is adding a feature.

Understanding why ip netns exec is failing in your specific case sounds
like there may be a missing error message.  Clearly you get into a
context where /var/run/netns/netns_old is no the bind mount we would
expect it to be.

I saw nothing that should unmake /var/run/netns/nnetns_old  as a mount
point.  So something strange is going on.

So you might want to go through and inspect to see what is happening.
Perhaps there is a missing error message somewhere.

Without understanding why that mountpoint fails to exist I can't say if
it is a real bug or if it just something weird caused by using an ip
netns exec it was not designed to be used and is known not to be 100%
robust in.

Eric

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: iproute2: potential upgrade regression with 58a3e827
  2013-11-08 18:03 iproute2: potential upgrade regression with 58a3e827 Chris J Arges
  2013-11-08 21:36 ` Eric W. Biederman
@ 2013-11-09 17:00 ` Brian Haley
  2013-11-11 21:26   ` Chris J Arges
  1 sibling, 1 reply; 11+ messages in thread
From: Brian Haley @ 2013-11-09 17:00 UTC (permalink / raw)
  To: Chris J Arges, shemminger, ebiederm, netdev

On 11/09/2013 02:03 AM, Chris J Arges wrote:
> Hi,
> The commit
> https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=58a3e8270fe72f8ed92687d3a3132c2a708582dd
> could be potentially introducing a regression on an upgrade.
>
> I've noticed that upgrading iproute while there are active namespaces
> could cause the following error:
> seting the network namespace failed: Invalid argument
>
> Here's a test case:
> Build and install iproute2 with 4395d48c78a77a99c5a8618403211032356fe552
>
> In one terminal run:
> ip netns add netns_old
> ip link add name if_old type veth peer name if_old_peer
> ip link set dev if_old_peer netns netns_old
> ip netns exec netns_old bash
>
> Build and install iproute2 with yypyye72f8ed92687d3a3132c2a708582dd
>
> In the same terminal as you typed the original commands run:
> ip netns add netns_one
> ip link add name if_one type veth peer name if_one_peer
> ip link set dev if_one_peer netns netns_one
> ip netns exec netns_one bash
> ip netns exec netns_old bash
>
> You'll get:
> seting the network namespace failed: Invalid argument
>
> If you just run the above without transitioning to the code in 58a3e827,
> then it works.

I've seen this error recently as well, and when it does happen if you go 
look in /var/run/netns you'll see that the permissions on your 
namespace(s) are most likely 000.  The only solution I found was to 
reboot, and then only use the newer iproute.

Maybe that info can help track down the cause?

-Brian

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: iproute2: potential upgrade regression with 58a3e827
  2013-11-09 17:00 ` Brian Haley
@ 2013-11-11 21:26   ` Chris J Arges
  2013-11-11 21:38     ` Dilip Daya
  0 siblings, 1 reply; 11+ messages in thread
From: Chris J Arges @ 2013-11-11 21:26 UTC (permalink / raw)
  To: Brian Haley, shemminger, ebiederm, netdev

On 11/09/2013 11:00 AM, Brian Haley wrote:
> On 11/09/2013 02:03 AM, Chris J Arges wrote:
>> Hi,
>> The commit
>> https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=58a3e8270fe72f8ed92687d3a3132c2a708582dd
>>
>> could be potentially introducing a regression on an upgrade.
>>
>> I've noticed that upgrading iproute while there are active namespaces
>> could cause the following error:
>> seting the network namespace failed: Invalid argument
>>
>> Here's a test case:
>> Build and install iproute2 with 4395d48c78a77a99c5a8618403211032356fe552
>>
>> In one terminal run:
>> ip netns add netns_old
>> ip link add name if_old type veth peer name if_old_peer
>> ip link set dev if_old_peer netns netns_old
>> ip netns exec netns_old bash
>>
>> Build and install iproute2 with yypyye72f8ed92687d3a3132c2a708582dd
>>
>> In the same terminal as you typed the original commands run:
>> ip netns add netns_one
>> ip link add name if_one type veth peer name if_one_peer
>> ip link set dev if_one_peer netns netns_one
>> ip netns exec netns_one bash
>> ip netns exec netns_old bash
>>
>> You'll get:
>> seting the network namespace failed: Invalid argument
>>
>> If you just run the above without transitioning to the code in 58a3e827,
>> then it works.
> 
> I've seen this error recently as well, and when it does happen if you go
> look in /var/run/netns you'll see that the permissions on your
> namespace(s) are most likely 000.  The only solution I found was to
> reboot, and then only use the newer iproute.
> 
> Maybe that info can help track down the cause?
> 
> -Brian
> 

Good suggestion,
So I'll use a more simple example now:

1)
ip netns add first
ip netns exec first bash

2)
ip netns add second
ip netns exec second bash

3)
ip netns exec first bash

If we do not upgrade the package, after we execute (2) we have:
# ls -l /var/run/netns
total 0
-r-------- 1 root root 0 Nov 11 20:38 first
-r-------- 1 root root 0 Nov 11 20:38 second

If we upgrade after (1), then run (2) we have:
# ls -l /var/run/netns
total 0
---------- 1 root root 0 Nov 11 20:56 first
-r-------- 1 root root 0 Nov 11 20:57 second

So looks like netns add is doing something different from 58a3e827 and on.

I'll have to spend more time to do further analysis.
--chris

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: iproute2: potential upgrade regression with 58a3e827
  2013-11-11 21:26   ` Chris J Arges
@ 2013-11-11 21:38     ` Dilip Daya
  2013-11-11 22:40       ` Eric W. Biederman
  0 siblings, 1 reply; 11+ messages in thread
From: Dilip Daya @ 2013-11-11 21:38 UTC (permalink / raw)
  To: Chris J Arges; +Cc: Brian Haley, shemminger, ebiederm, netdev

Hi Chris,

On Mon, 2013-11-11 at 15:26 -0600, Chris J Arges wrote:
> On 11/09/2013 11:00 AM, Brian Haley wrote:
> > On 11/09/2013 02:03 AM, Chris J Arges wrote:
> >> Hi,
> >> The commit
> >> https://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=58a3e8270fe72f8ed92687d3a3132c2a708582dd
> >>
> >> could be potentially introducing a regression on an upgrade.
> >>
> >> I've noticed that upgrading iproute while there are active namespaces
> >> could cause the following error:
> >> seting the network namespace failed: Invalid argument
> >>
> >> Here's a test case:
> >> Build and install iproute2 with 4395d48c78a77a99c5a8618403211032356fe552
> >>
> >> In one terminal run:
> >> ip netns add netns_old
> >> ip link add name if_old type veth peer name if_old_peer
> >> ip link set dev if_old_peer netns netns_old
> >> ip netns exec netns_old bash
> >>
> >> Build and install iproute2 with yypyye72f8ed92687d3a3132c2a708582dd
> >>
> >> In the same terminal as you typed the original commands run:
> >> ip netns add netns_one
> >> ip link add name if_one type veth peer name if_one_peer
> >> ip link set dev if_one_peer netns netns_one
> >> ip netns exec netns_one bash
> >> ip netns exec netns_old bash
> >>
> >> You'll get:
> >> seting the network namespace failed: Invalid argument
> >>
> >> If you just run the above without transitioning to the code in 58a3e827,
> >> then it works.
> > 
> > I've seen this error recently as well, and when it does happen if you go
> > look in /var/run/netns you'll see that the permissions on your
> > namespace(s) are most likely 000.  The only solution I found was to
> > reboot, and then only use the newer iproute.
> > 
> > Maybe that info can help track down the cause?
> > 
> > -Brian
> > 
> 
> Good suggestion,
> So I'll use a more simple example now:
> 
> 1)
> ip netns add first
> ip netns exec first bash
> 
> 2)
> ip netns add second
> ip netns exec second bash
> 
> 3)
> ip netns exec first bash
> 
> If we do not upgrade the package, after we execute (2) we have:
> # ls -l /var/run/netns
> total 0
> -r-------- 1 root root 0 Nov 11 20:38 first
> -r-------- 1 root root 0 Nov 11 20:38 second
> 
> If we upgrade after (1), then run (2) we have:
> # ls -l /var/run/netns
> total 0
> ---------- 1 root root 0 Nov 11 20:56 first
> -r-------- 1 root root 0 Nov 11 20:57 second
> 
> So looks like netns add is doing something different from 58a3e827 and on.

This could be related to:

"iproute2: Don't propagate mounts out of ip"
<http://git.kernel.org/cgit/linux/kernel/git/shemminger/iproute2.git/commit/?id=144e6ce1679a768e987230efb4afa402a5ab58ac>


Some systems are now following the advice in linux/Documentation/sharedsubtrees.txt
<https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/filesystems/sharedsubtree.txt?id=refs/tags/v3.12>
and running with all mount points shared between all mount namespaces by default.
After creating the mount namespace call mount on / with MS_SLAVE|MS_REC to modify
all mounts in the new mount namespace to slave mounts if they are shared or private
mounts otherwise. Guaranteeing that changes to the mount namespace created with
"ip netns exec" don't propagate to other namespaces.


-DilipD.


> 
> I'll have to spend more time to do further analysis.
> --chris
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
-DilipD.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: iproute2: potential upgrade regression with 58a3e827
  2013-11-11 21:38     ` Dilip Daya
@ 2013-11-11 22:40       ` Eric W. Biederman
  2013-11-12  0:36         ` Dilip Daya
  0 siblings, 1 reply; 11+ messages in thread
From: Eric W. Biederman @ 2013-11-11 22:40 UTC (permalink / raw)
  To: dilip.daya; +Cc: Chris J Arges, Brian Haley, shemminger, netdev

Dilip Daya <dilip.daya@hp.com> writes:

> Hi Chris,
>
> On Mon, 2013-11-11 at 15:26 -0600, Chris J Arges wrote:
>
>> Good suggestion,
>> So I'll use a more simple example now:
>> 
>> 1)
>> ip netns add first
>> ip netns exec first bash
>> 
>> 2)
>> ip netns add second
>> ip netns exec second bash
>> 
>> 3)
>> ip netns exec first bash
>> 
>> If we do not upgrade the package, after we execute (2) we have:
>> # ls -l /var/run/netns
>> total 0
>> -r-------- 1 root root 0 Nov 11 20:38 first
>> -r-------- 1 root root 0 Nov 11 20:38 second
>> 
>> If we upgrade after (1), then run (2) we have:
>> # ls -l /var/run/netns
>> total 0
>> ---------- 1 root root 0 Nov 11 20:56 first
>> -r-------- 1 root root 0 Nov 11 20:57 second
>> 
>> So looks like netns add is doing something different from 58a3e827 and on.

I will just add that it is worth looking at /proc/mounts as well.

Although I have to admit that the difference in permissions is odd.

Eric

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: iproute2: potential upgrade regression with 58a3e827
  2013-11-11 22:40       ` Eric W. Biederman
@ 2013-11-12  0:36         ` Dilip Daya
  2013-12-13 18:46           ` [PATCH] " Chris J Arges
  0 siblings, 1 reply; 11+ messages in thread
From: Dilip Daya @ 2013-11-12  0:36 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Chris J Arges, Brian Haley, shemminger, netdev

Hi Eric,

On Mon, 2013-11-11 at 14:40 -0800, Eric W. Biederman wrote:
> Dilip Daya <dilip.daya@hp.com> writes:
> 
> > Hi Chris,
> >
> > On Mon, 2013-11-11 at 15:26 -0600, Chris J Arges wrote:
> >
> >> Good suggestion,
> >> So I'll use a more simple example now:
> >> 
> >> 1)
> >> ip netns add first
> >> ip netns exec first bash
> >> 
> >> 2)
> >> ip netns add second
> >> ip netns exec second bash
> >> 
> >> 3)
> >> ip netns exec first bash
> >> 
> >> If we do not upgrade the package, after we execute (2) we have:
> >> # ls -l /var/run/netns
> >> total 0
> >> -r-------- 1 root root 0 Nov 11 20:38 first
> >> -r-------- 1 root root 0 Nov 11 20:38 second
> >> 
> >> If we upgrade after (1), then run (2) we have:
> >> # ls -l /var/run/netns
> >> total 0
> >> ---------- 1 root root 0 Nov 11 20:56 first
> >> -r-------- 1 root root 0 Nov 11 20:57 second
> >> 
> >> So looks like netns add is doing something different from 58a3e827 and on.
> 
> I will just add that it is worth looking at /proc/mounts as well.
> 
> Although I have to admit that the difference in permissions is odd.


=> kernel v3.2.51 with iproute2-ss130903


Terminal #1--Add first netns
# ip netns add first


Terminal #1:
# tree --inodes /var/run/netns ; echo "=====" ; ls -li /var/run/netns ; echo "====="; cat /proc/self/mounts | grep first ; echo "=====" ; cat /proc/self/mountinfo | grep -e first
/var/run/netns
└── [   5204]  first

0 directories, 1 file
=====
total 0
5204 -r-------- 1 root root 0 Nov 11 17:17 first
=====
none /var/run/netns/first proc rw,nosuid,nodev,noexec,relatime 0 0
=====
23 22 0:3 /1935/ns/net /var/run/netns/first rw,nosuid,nodev,noexec,relatime shared:2 - proc none rw


Terminal #1:
# ip netns exec first /bin/bash


Terminal #1:
# tree --inodes /var/run/netns ; echo "=====" ; ls -li /var/run/netns ; echo "====="; cat /proc/self/mounts | grep first ; echo "=====" ; cat /proc/self/mountinfo | grep -e first
/var/run/netns
└── [   5204]  first

0 directories, 1 file
=====
total 0
5204 -r-------- 1 root root 0 Nov 11 17:17 first
=====
none /var/run/netns/first proc rw,nosuid,nodev,noexec,relatime 0 0
first /sys sysfs rw,relatime 0 0
=====
33 32 0:3 /1935/ns/net /var/run/netns/first rw,nosuid,nodev,noexec,relatime master:2 - proc none rw
29 25 0:17 / /sys rw,relatime - sysfs first rw


Terminal #1:
# ip netns add second


Terminal #1:
# tree --inodes /var/run/netns ; echo "=====" ; ls -li /var/run/netns ; echo "====="; cat /proc/self/mounts | grep first ; echo "=====" ; cat /proc/self/mountinfo | grep -e first -e second
/var/run/netns
├── [   5204]  first
└── [   5236]  second

0 directories, 2 files
=====
total 0
5204 -r-------- 1 root root 0 Nov 11 17:17 first
5236 -r-------- 1 root root 0 Nov 11 17:21 second   <<< observe this inode # and permissions
=====
none /var/run/netns/first proc rw,nosuid,nodev,noexec,relatime 0 0
first /sys sysfs rw,relatime 0 0
=====
33 32 0:3 /1935/ns/net /var/run/netns/first rw,nosuid,nodev,noexec,relatime shared:4 master:2 - proc none rw
29 25 0:17 / /sys rw,relatime - sysfs first rw
34 32 0:3 /1955/ns/net /var/run/netns/second rw,nosuid,nodev,noexec,relatime shared:5 - proc none rw



Terminal #2--in main (not in netns):
# tree --inodes /var/run/netns ; echo "=====" ; ls -li /var/run/netns ; echo "====="; cat /proc/self/mounts | grep first ; echo "=====" ; cat /proc/self/mountinfo | grep -e first -e second
/var/run/netns
├── [   5204]  first
└── [  51492]  second   <<< inode is different

0 directories, 2 files
=====
total 0
 5204 -r-------- 1 root root 0 Nov 11 17:17 first
51492 ---------- 1 root root 0 Nov 11 17:21 second   << inode different with NULL permissions
=====
none /var/run/netns/first proc rw,nosuid,nodev,noexec,relatime 0 0
=====
23 22 0:3 /1935/ns/net /var/run/netns/first rw,nosuid,nodev,noexec,relatime shared:2 - proc none rw

=> When in main (not in netns) "second" netns is not viewable.


Terminal #2--Enter first:
# ip netns exec first bash


Terminal #2:
# tree --inodes /var/run/netns ; echo "=====" ; ls -li /var/run/netns ; echo "====="; cat /proc/self/mounts | grep first ; echo "=====" ; cat /proc/self/mountinfo | grep -e first -e second
/var/run/netns
├── [   5204]  first
└── [  51492]  second   <<< inode different then when created from first in Terminal #1 above

0 directories, 2 files
=====
total 0
 5204 -r-------- 1 root root 0 Nov 11 17:17 first
51492 ---------- 1 root root 0 Nov 11 17:21 second   <<< inode with NULL permissions
=====
none /var/run/netns/first proc rw,nosuid,nodev,noexec,relatime 0 0
first /sys sysfs rw,relatime 0 0
=====
44 43 0:3 /1935/ns/net /var/run/netns/first rw,nosuid,nodev,noexec,relatime master:2 - proc none rw
40 36 0:17 / /sys rw,relatime - sysfs first rw

=> mounts and mountinfo does not show "second"


Terminal #2:
# ip netns exec second /bin/bash
seting the network namespace "second" failed: Invalid argument

=> "second" netns is now rendered unusable from "first" netns and from main.



Thanks,
-DilipD.



> 
> Eric

-- 
-DilipD.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] Re: iproute2: potential upgrade regression with 58a3e827
  2013-11-12  0:36         ` Dilip Daya
@ 2013-12-13 18:46           ` Chris J Arges
  2013-12-13 18:55             ` Stephen Hemminger
  0 siblings, 1 reply; 11+ messages in thread
From: Chris J Arges @ 2013-12-13 18:46 UTC (permalink / raw)
  To: dilip.daya, Eric W. Biederman; +Cc: Brian Haley, shemminger, netdev

[-- Attachment #1: Type: text/plain, Size: 5689 bytes --]

On 11/11/2013 06:36 PM, Dilip Daya wrote:
> Hi Eric,
> 
> On Mon, 2013-11-11 at 14:40 -0800, Eric W. Biederman wrote:
>> Dilip Daya <dilip.daya@hp.com> writes:
>>
>>> Hi Chris,
>>>
>>> On Mon, 2013-11-11 at 15:26 -0600, Chris J Arges wrote:
>>>
>>>> Good suggestion,
>>>> So I'll use a more simple example now:
>>>>
>>>> 1)
>>>> ip netns add first
>>>> ip netns exec first bash
>>>>
>>>> 2)
>>>> ip netns add second
>>>> ip netns exec second bash
>>>>
>>>> 3)
>>>> ip netns exec first bash
>>>>
>>>> If we do not upgrade the package, after we execute (2) we have:
>>>> # ls -l /var/run/netns
>>>> total 0
>>>> -r-------- 1 root root 0 Nov 11 20:38 first
>>>> -r-------- 1 root root 0 Nov 11 20:38 second
>>>>
>>>> If we upgrade after (1), then run (2) we have:
>>>> # ls -l /var/run/netns
>>>> total 0
>>>> ---------- 1 root root 0 Nov 11 20:56 first
>>>> -r-------- 1 root root 0 Nov 11 20:57 second
>>>>
>>>> So looks like netns add is doing something different from 58a3e827 and on.
>>
>> I will just add that it is worth looking at /proc/mounts as well.
>>
>> Although I have to admit that the difference in permissions is odd.
> 
> 
> => kernel v3.2.51 with iproute2-ss130903
> 
> 
> Terminal #1--Add first netns
> # ip netns add first
> 
> 
> Terminal #1:
> # tree --inodes /var/run/netns ; echo "=====" ; ls -li /var/run/netns ; echo "====="; cat /proc/self/mounts | grep first ; echo "=====" ; cat /proc/self/mountinfo | grep -e first
> /var/run/netns
> └── [   5204]  first
> 
> 0 directories, 1 file
> =====
> total 0
> 5204 -r-------- 1 root root 0 Nov 11 17:17 first
> =====
> none /var/run/netns/first proc rw,nosuid,nodev,noexec,relatime 0 0
> =====
> 23 22 0:3 /1935/ns/net /var/run/netns/first rw,nosuid,nodev,noexec,relatime shared:2 - proc none rw
> 
> 
> Terminal #1:
> # ip netns exec first /bin/bash
> 
> 
> Terminal #1:
> # tree --inodes /var/run/netns ; echo "=====" ; ls -li /var/run/netns ; echo "====="; cat /proc/self/mounts | grep first ; echo "=====" ; cat /proc/self/mountinfo | grep -e first
> /var/run/netns
> └── [   5204]  first
> 
> 0 directories, 1 file
> =====
> total 0
> 5204 -r-------- 1 root root 0 Nov 11 17:17 first
> =====
> none /var/run/netns/first proc rw,nosuid,nodev,noexec,relatime 0 0
> first /sys sysfs rw,relatime 0 0
> =====
> 33 32 0:3 /1935/ns/net /var/run/netns/first rw,nosuid,nodev,noexec,relatime master:2 - proc none rw
> 29 25 0:17 / /sys rw,relatime - sysfs first rw
> 
> 
> Terminal #1:
> # ip netns add second
> 
> 
> Terminal #1:
> # tree --inodes /var/run/netns ; echo "=====" ; ls -li /var/run/netns ; echo "====="; cat /proc/self/mounts | grep first ; echo "=====" ; cat /proc/self/mountinfo | grep -e first -e second
> /var/run/netns
> ├── [   5204]  first
> └── [   5236]  second
> 
> 0 directories, 2 files
> =====
> total 0
> 5204 -r-------- 1 root root 0 Nov 11 17:17 first
> 5236 -r-------- 1 root root 0 Nov 11 17:21 second   <<< observe this inode # and permissions
> =====
> none /var/run/netns/first proc rw,nosuid,nodev,noexec,relatime 0 0
> first /sys sysfs rw,relatime 0 0
> =====
> 33 32 0:3 /1935/ns/net /var/run/netns/first rw,nosuid,nodev,noexec,relatime shared:4 master:2 - proc none rw
> 29 25 0:17 / /sys rw,relatime - sysfs first rw
> 34 32 0:3 /1955/ns/net /var/run/netns/second rw,nosuid,nodev,noexec,relatime shared:5 - proc none rw
> 
> 
> 
> Terminal #2--in main (not in netns):
> # tree --inodes /var/run/netns ; echo "=====" ; ls -li /var/run/netns ; echo "====="; cat /proc/self/mounts | grep first ; echo "=====" ; cat /proc/self/mountinfo | grep -e first -e second
> /var/run/netns
> ├── [   5204]  first
> └── [  51492]  second   <<< inode is different
> 
> 0 directories, 2 files
> =====
> total 0
>  5204 -r-------- 1 root root 0 Nov 11 17:17 first
> 51492 ---------- 1 root root 0 Nov 11 17:21 second   << inode different with NULL permissions
> =====
> none /var/run/netns/first proc rw,nosuid,nodev,noexec,relatime 0 0
> =====
> 23 22 0:3 /1935/ns/net /var/run/netns/first rw,nosuid,nodev,noexec,relatime shared:2 - proc none rw
> 
> => When in main (not in netns) "second" netns is not viewable.
> 
> 
> Terminal #2--Enter first:
> # ip netns exec first bash
> 
> 
> Terminal #2:
> # tree --inodes /var/run/netns ; echo "=====" ; ls -li /var/run/netns ; echo "====="; cat /proc/self/mounts | grep first ; echo "=====" ; cat /proc/self/mountinfo | grep -e first -e second
> /var/run/netns
> ├── [   5204]  first
> └── [  51492]  second   <<< inode different then when created from first in Terminal #1 above
> 
> 0 directories, 2 files
> =====
> total 0
>  5204 -r-------- 1 root root 0 Nov 11 17:17 first
> 51492 ---------- 1 root root 0 Nov 11 17:21 second   <<< inode with NULL permissions
> =====
> none /var/run/netns/first proc rw,nosuid,nodev,noexec,relatime 0 0
> first /sys sysfs rw,relatime 0 0
> =====
> 44 43 0:3 /1935/ns/net /var/run/netns/first rw,nosuid,nodev,noexec,relatime master:2 - proc none rw
> 40 36 0:17 / /sys rw,relatime - sysfs first rw
> 
> => mounts and mountinfo does not show "second"
> 
> 
> Terminal #2:
> # ip netns exec second /bin/bash
> seting the network namespace "second" failed: Invalid argument
> 
> => "second" netns is now rendered unusable from "first" netns and from main.
> 
> 
> 
> Thanks,
> -DilipD.
> 
> 
> 
>>
>> Eric
> 

Attached is a patch that solves this issue for me. I traced through the
error values of mount and noticed the errno was being set to EINVAL (as
we'd expect per man 2 mount). However, this seemed to be causing issues
with later mount commands. I've reset the errno before the next mount
command in that loop.

Please review this patch,
Thanks,
--chris j arges




[-- Attachment #2: 0001-Fix-for-upgrade-regression-in-58a3e827.patch --]
[-- Type: text/x-patch, Size: 1637 bytes --]

>From 88494a34106df25827cb46820f6272915bee6e85 Mon Sep 17 00:00:00 2001
From: Chris J Arges <chris.j.arges@canonical.com>
Date: Fri, 13 Dec 2013 12:29:12 -0600
Subject: [PATCH] Fix for upgrade regression in 58a3e827

Commit 58a3e8270fe72f8ed92687d3a3132c2a708582dd introduces a regression
when upgrading code with existing active network namespaces.

Test case:
ip netns add first
ip netns exec first bash
  < upgrade to 58a3e827 version >
ip netns add second
ip netns exec second bash
ip netns exec first bash

This will work if you are using only versions before this commit, or
only versions after the commit. Otherwise you will get the following message:

seting the network namespace failed: Invalid argument

This also fixes the spelling error in that message.

Signed-off-by: Chris J Arges <chris.j.arges@canonical.com>
---
 ip/ipnetns.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 89dda3f..c5d7bde 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -158,7 +158,7 @@ static int netns_exec(int argc, char **argv)
 	}
 
 	if (setns(netns, CLONE_NEWNET) < 0) {
-		fprintf(stderr, "seting the network namespace \"%s\" failed: %s\n",
+		fprintf(stderr, "setting the network namespace \"%s\" failed: %s\n",
 			name, strerror(errno));
 		return -1;
 	}
@@ -424,6 +424,7 @@ static int netns_add(int argc, char **argv)
 		}
 
 		/* Upgrade NETNS_RUN_DIR to a mount point */
+		errno = 0;
 		if (mount(NETNS_RUN_DIR, NETNS_RUN_DIR, "none", MS_BIND, NULL)) {
 			fprintf(stderr, "mount --bind %s %s failed: %s\n",
 				NETNS_RUN_DIR, NETNS_RUN_DIR, strerror(errno));
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] Re: iproute2: potential upgrade regression with 58a3e827
  2013-12-13 18:46           ` [PATCH] " Chris J Arges
@ 2013-12-13 18:55             ` Stephen Hemminger
  0 siblings, 0 replies; 11+ messages in thread
From: Stephen Hemminger @ 2013-12-13 18:55 UTC (permalink / raw)
  To: Chris J Arges
  Cc: dilip.daya, Eric W. Biederman, Brian Haley, shemminger, netdev

On Fri, 13 Dec 2013 12:46:09 -0600
Chris J Arges <chris.j.arges@canonical.com> wrote:

> @@ -424,6 +424,7 @@ static int netns_add(int argc, char **argv)
>  		}
>  
>  		/* Upgrade NETNS_RUN_DIR to a mount point */
> +		errno = 0;
>  		if (mount(NETNS_RUN_DIR, NETNS_RUN_DIR, "none", MS_BIND, NULL)) {
>  			fprintf(stderr, "mount --bind %s %s failed: %s\n",
>  				NETNS_RUN_DIR, NETNS_RUN_DIR, strerror(errno));

It doesn't make sense that this changes anything.
errno is set by failed syscall. And the code here is smart enough
not to blindly check for (errno != 0)

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-12-13 18:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-08 18:03 iproute2: potential upgrade regression with 58a3e827 Chris J Arges
2013-11-08 21:36 ` Eric W. Biederman
2013-11-08 22:30   ` Chris J Arges
2013-11-08 22:42     ` Eric W. Biederman
2013-11-09 17:00 ` Brian Haley
2013-11-11 21:26   ` Chris J Arges
2013-11-11 21:38     ` Dilip Daya
2013-11-11 22:40       ` Eric W. Biederman
2013-11-12  0:36         ` Dilip Daya
2013-12-13 18:46           ` [PATCH] " Chris J Arges
2013-12-13 18:55             ` Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.