[PATCH v2] iproute2: build nsid-name cache only for commands that need it

The calling of netns_map_init() before command parsing introduced
a performance issue with large number of namespaces.

As commands such as add, del and exec do not need to iterate through
/var/run/netns it would be good not no build the cache before executing
these commands.

Example:
unpatched:
time seq 1 1000 | xargs -n 1 ip netns add

real    0m16.832s
user    0m1.350s
sys    0m15.029s

patched:
time seq 1 1000 | xargs -n 1 ip netns add

real    0m3.859s
user    0m0.132s
sys    0m3.205s

Signed-off-by: Anton Aksola <redacted>
---
 ip/ipnetns.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index af87065..564d034 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c

@@ -775,17 +775,21 @@ static int netns_monitor(int argc, char **argv)
 
 int do_netns(int argc, char **argv)
 {
-	netns_map_init();
-
-	if (argc < 1)
+	if (argc < 1) {
+		netns_map_init();
 		return netns_list(0, NULL);
+	}
 
 	if ((matches(*argv, "list") == 0) || (matches(*argv, "show") == 0) ||
-	    (matches(*argv, "lst") == 0))
+	    (matches(*argv, "lst") == 0)) {
+		netns_map_init();
 		return netns_list(argc-1, argv+1);
+	}
 
-	if ((matches(*argv, "list-id") == 0))
+	if ((matches(*argv, "list-id") == 0)) {
+		netns_map_init();
 		return netns_list_id(argc-1, argv+1);
+	}
 
 	if (matches(*argv, "help") == 0)
 		return usage();

-- 
1.8.3.1

Le 16/09/2016 à 09:22, Anton Aksola a écrit :

The calling of netns_map_init() before command parsing introduced
a performance issue with large number of namespaces.

As commands such as add, del and exec do not need to iterate through
/var/run/netns it would be good not no build the cache before executing
these commands.

Example:
unpatched:
time seq 1 1000 | xargs -n 1 ip netns add

real    0m16.832s
user    0m1.350s
sys    0m15.029s

patched:
time seq 1 1000 | xargs -n 1 ip netns add

real    0m3.859s
user    0m0.132s
sys    0m3.205s

Signed-off-by: Anton Aksola <redacted>
---

There is still some differences:
$ cat test.batch
netns add foo
netns set foo 1234
netns list-id

Before your patch:
$ ip -b test.batch
nsid 1234 (iproute2 netns name: foo)

After your patch:
$ ip -b test.batch
nsid 1234

On Fri, Sep 16, 2016 at 12:13 PM, Nicolas Dichtel
[off-list ref] wrote:

Le 16/09/2016 à 09:22, Anton Aksola a écrit :

quoted

The calling of netns_map_init() before command parsing introduced
a performance issue with large number of namespaces.

As commands such as add, del and exec do not need to iterate through
/var/run/netns it would be good not no build the cache before executing
these commands.

Example:
unpatched:
time seq 1 1000 | xargs -n 1 ip netns add

real    0m16.832s
user    0m1.350s
sys    0m15.029s

patched:
time seq 1 1000 | xargs -n 1 ip netns add

real    0m3.859s
user    0m0.132s
sys    0m3.205s

Signed-off-by: Anton Aksola <redacted>
---

There is still some differences:
$ cat test.batch
netns add foo
netns set foo 1234
netns list-id

Before your patch:
$ ip -b test.batch
nsid 1234 (iproute2 netns name: foo)

After your patch:
$ ip -b test.batch
nsid 1234

Would it be useful to add test for this case into testsuite/ ?

Regards,
Vadim Kochan

Le 16/09/2016 à 11:23, Vadim Kochan a écrit :
[snip]

Would it be useful to add test for this case into testsuite/ ?

Yes, it's a good idea.

Regards,
Nicolas

On Fri, Sep 16, 2016 at 11:13:11AM +0200, Nicolas Dichtel wrote:

There is still some differences:
$ cat test.batch
netns add foo
netns set foo 1234
netns list-id

Before your patch:
$ ip -b test.batch
nsid 1234 (iproute2 netns name: foo)

After your patch:
$ ip -b test.batch
nsid 1234

Nicolas,
This seems to be caused by netns_add calling unshare(CLONE_NEWNET).
If we initialize the socket for nsid after that it doesn't seem to work.

Unfortunately I'm not an expert in these details. Should we separate the
socket and cache initialization to different functions and call the
socket init in the beginning of do_netns() as before? What do you think?
I made a quick patch and it seems to work in batch mode now.

BR,
Anton

Le 16/09/2016 à 15:18, Anton Aksola a écrit :
[snip]

Nicolas,
This seems to be caused by netns_add calling unshare(CLONE_NEWNET).
If we initialize the socket for nsid after that it doesn't seem to work.

Unfortunately I'm not an expert in these details. Should we separate the
socket and cache initialization to different functions and call the
socket init in the beginning of do_netns() as before? What do you think?

Seems good.

I made a quick patch and it seems to work in batch mode now.

What is the result of that sequence?

$ ip netns add bar
$ ip netns set bar 5678
$ ip -b test.batch
nsid 1234 (iproute2 netns name: foo)
nsid 5678 (iproute2 netns name: bar)
$