[PATCH iproute2 v1] ip netns: allow negative nsid

[PATCH iproute2 v1] ip netns: allow negative nsid

[Christian Brauner]

If the kernel receives a negative nsid it will automatically assign the
next available nsid. In this case alloc_netid() will set min and max to
0 for ird_alloc(). And when max == 0 idr_alloc() will interpret this as
the maxium range, i.e. specific to nsids it will try to find an id in
the range [0,INT_MAX). This is intentionally supported in the kernel for
nsids. Commit acbe9118ce8086f765ffb0da15f80c7c01a8903a regressed ip
netns in that respect although previously the use-case was either
accidentally supported or opaquely supported such that it triggered the
original commit. From what I can gather it went as follows before:
atoi() was called with a string indicating a negative value which caused
it to return -1 which was passed to the kernel. Let's make it less
opaque by introducing the keyword "auto":

ip netns set <netns-name> auto

will cause nsid to be set to -1 and the kernel will select an available
nsid.

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
ChangeLog v0->v1:
* introduce "auto" keyword for ip netns to automatically allocate an
  available nsid
---
 ip/ipnetns.c        | 5 ++++-
 man/man8/ip-netns.8 | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 059a4220..631794b8 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -718,7 +718,10 @@ static int netns_set(int argc, char **argv)
 		return -1;
 	}
 	name = argv[0];
-	if (get_unsigned(&nsid, argv[1], 0))
+	/* If a negative nsid is specified the kernel will select the nsid. */
+	if (strcmp(argv[1], "auto") == 0)
+		nsid = -1;
+	else if (get_unsigned(&nsid, argv[1], 0))
 		invarg("Invalid \"netnsid\" value\n", argv[1]);
 
 	snprintf(netns_path, sizeof(netns_path), "%s/%s", NETNS_RUN_DIR, name);
diff --git a/man/man8/ip-netns.8 b/man/man8/ip-netns.8
index c5310e24..d539f18b 100644
--- a/man/man8/ip-netns.8
+++ b/man/man8/ip-netns.8
@@ -137,6 +137,7 @@ $ ip netns del net0
 .sp
 This command assigns a id to a peer network namespace. This id is valid
 only in the current network namespace.
+If the keyword "auto" is specified an available nsid will be chosen.
 This id will be used by the kernel in some netlink messages. If no id is
 assigned when the kernel needs it, it will be automatically assigned by
 the kernel.
-- 
2.14.1

Re: [PATCH iproute2 v1] ip netns: allow negative nsid

[Stephen Hemminger]

On Tue,  6 Feb 2018 19:39:31 +0100
Christian Brauner <christian.brauner@ubuntu.com> wrote:

> If the kernel receives a negative nsid it will automatically assign the
> next available nsid. In this case alloc_netid() will set min and max to
> 0 for ird_alloc(). And when max == 0 idr_alloc() will interpret this as
> the maxium range, i.e. specific to nsids it will try to find an id in
> the range [0,INT_MAX). This is intentionally supported in the kernel for
> nsids. Commit acbe9118ce8086f765ffb0da15f80c7c01a8903a regressed ip
> netns in that respect although previously the use-case was either
> accidentally supported or opaquely supported such that it triggered the
> original commit. From what I can gather it went as follows before:
> atoi() was called with a string indicating a negative value which caused
> it to return -1 which was passed to the kernel. Let's make it less
> opaque by introducing the keyword "auto":
> 
> ip netns set <netns-name> auto
> 
> will cause nsid to be set to -1 and the kernel will select an available
> nsid.
> 
> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

Applied thank you.
I did have to fix spelling and format of commit reference in
the commit description. If you run checkpatch on patches to
iproute you would have caught that.

Re: [PATCH iproute2 v1] ip netns: allow negative nsid

[Christian Brauner]

On Thu, Feb 8, 2018 at 5:01 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Tue,  6 Feb 2018 19:39:31 +0100
> Christian Brauner <christian.brauner@ubuntu.com> wrote:
>
>> If the kernel receives a negative nsid it will automatically assign the
>> next available nsid. In this case alloc_netid() will set min and max to
>> 0 for ird_alloc(). And when max == 0 idr_alloc() will interpret this as
>> the maxium range, i.e. specific to nsids it will try to find an id in
>> the range [0,INT_MAX). This is intentionally supported in the kernel for
>> nsids. Commit acbe9118ce8086f765ffb0da15f80c7c01a8903a regressed ip
>> netns in that respect although previously the use-case was either
>> accidentally supported or opaquely supported such that it triggered the
>> original commit. From what I can gather it went as follows before:
>> atoi() was called with a string indicating a negative value which caused
>> it to return -1 which was passed to the kernel. Let's make it less
>> opaque by introducing the keyword "auto":
>>
>> ip netns set <netns-name> auto
>>
>> will cause nsid to be set to -1 and the kernel will select an available
>> nsid.
>>
>> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
>
> Applied thank you.
> I did have to fix spelling and format of commit reference in
> the commit description. If you run checkpatch on patches to
> iproute you would have caught that.

Ah, sorry about that. Didn't think about applying checkpatch to iproute
patches as well. Won't happen again!

Thanks for applying.
Christian