netns: switch netns in the child when executing commands

'ip netns exec' changes the current netns just before executing a child
process, and restores it after forking. This is needed if we're running
in batch or do_all mode.
Some cleanups must be done both in the parent and in the child: the
parent must restore the previous netns, while the child must reset any
VRF association.
Unfortunately, if do_all is set, the VRF are not reset in the child, and
the spawned processes are started with the wrong VRF context. This can
be triggered with this script:

	# ip -b - <<-'EOF'
		link add type vrf table 100
		link set vrf0 up
		link add type dummy
		link set dummy0 vrf vrf0 up
		netns add ns1
	EOF
	# ip -all -b - <<-'EOF'
		vrf exec vrf0 true
		netns exec setsid -f sleep 1h
	EOF
	# ip vrf pids vrf0
	  314  sleep
	# ps 314
	  PID TTY      STAT   TIME COMMAND
	  314 ?        Ss     0:00 sleep 1h

Refactor cmd_exec() and pass to it a function pointer which is called in
the child before the final exec. In the netns exec case the function just
resets the VRF and switches netns.

Doing it in the child is less error prone and safer, because the parent
environment is always kept unaltered.

After this refactor some utility functions became unused, so remove them.

Signed-off-by: Matteo Croce <mcroce@redhat.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

diff --git a/include/utils.h b/include/utils.h
index 8a9c302082b28a4ae96b98758cb047f37d46ba29..927fdc17e09dd6fd61d02d2bfa4a8fd04d7ef6ef 100644 (file)
--- a/include/utils.h
+++ b/include/utils.h
@@ -294,14 +294,12 @@ extern int cmdlineno;
 ssize_t getcmdline(char **line, size_t *len, FILE *in);
 int makeargs(char *line, char *argv[], int maxargs);
 
-int do_each_netns(int (*func)(char *nsname, void *arg), void *arg,
-               bool show_label);
-
 char *int_to_str(int val, char *buf);
 int get_guid(__u64 *guid, const char *arg);
 int get_real_family(int rtm_type, int rtm_family);
 
-int cmd_exec(const char *cmd, char **argv, bool do_fork);
+int cmd_exec(const char *cmd, char **argv, bool do_fork,
+            int (*setup)(void *), void *arg);
 int make_path(const char *path, mode_t mode);
 char *find_cgroup2_mount(void);
 int get_command_name(const char *pid, char *comm, size_t len);

diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index e4788e75c0771998a870de105a246d932e8eb681..e32e44bdbb4ed497a6e699a337ee21827b15bfde 100644 (file)
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -395,11 +395,24 @@ static int netns_list(int argc, char **argv)
        return 0;
 }
 
+static int do_switch(void *arg)
+{
+       char *netns = arg;
+
+       /* we just changed namespaces. clear any vrf association
+        * with prior namespace before exec'ing command
+        */
+       vrf_reset();
+
+       return netns_switch(netns);
+}
+
 static int on_netns_exec(char *nsname, void *arg)
 {
        char **argv = arg;
 
-       cmd_exec(argv[1], argv + 1, true);
+       printf("\nnetns: %s\n", nsname);
+       cmd_exec(argv[0], argv, true, do_switch, nsname);
        return 0;
 }
 
@@ -408,8 +421,6 @@ static int netns_exec(int argc, char **argv)
        /* Setup the proper environment for apps that are not netns
         * aware, and execute a program in that environment.
         */
-       const char *cmd;
-
        if (argc < 1 && !do_all) {
                fprintf(stderr, "No netns name specified\n");
                return -1;
@@ -420,22 +431,13 @@ static int netns_exec(int argc, char **argv)
        }
 
        if (do_all)
-               return do_each_netns(on_netns_exec, --argv, 1);
-
-       if (netns_switch(argv[0]))
-               return -1;
-
-       /* we just changed namespaces. clear any vrf association
-        * with prior namespace before exec'ing command
-        */
-       vrf_reset();
+               return netns_foreach(on_netns_exec, argv);
 
        /* ip must return the status of the child,
         * but do_cmd() will add a minus to this,
         * so let's add another one here to cancel it.
         */
-       cmd = argv[1];
-       return -cmd_exec(cmd, argv + 1, !!batch_mode);
+       return -cmd_exec(argv[1], argv + 1, !!batch_mode, do_switch, argv[0]);
 }
 
 static int is_pid(const char *str)

diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index 08a0d45b2570f47167b0e55bae55f1776334996a..aebcb2532addf92804de59592c47231aa4966bc7 100644 (file)
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -455,7 +455,7 @@ static int ipvrf_exec(int argc, char **argv)
        if (vrf_switch(argv[0]))
                return -1;
 
-       return -cmd_exec(argv[1], argv + 1, !!batch_mode);
+       return -cmd_exec(argv[1], argv + 1, !!batch_mode, NULL, NULL);
 }
 
 /* reset VRF association of current process to default VRF;

diff --git a/lib/exec.c b/lib/exec.c
index eb36b59dee7f494ace68636e439bced6670b052b..9b1c8f4a13960b10063f67959000f32bd1b7b1f1 100644 (file)
--- a/lib/exec.c
+++ b/lib/exec.c
@@ -5,8 +5,10 @@
 #include <unistd.h>
 
 #include "utils.h"
+#include "namespace.h"
 
-int cmd_exec(const char *cmd, char **argv, bool do_fork)
+int cmd_exec(const char *cmd, char **argv, bool do_fork,
+            int (*setup)(void *), void *arg)
 {
        fflush(stdout);
        if (do_fork) {
@@ -34,6 +36,9 @@ int cmd_exec(const char *cmd, char **argv, bool do_fork)
                }
        }
 
+       if (setup && setup(arg))
+               return -1;
+
        if (execvp(cmd, argv)  < 0)
                fprintf(stderr, "exec of \"%s\" failed: %s\n",
                                cmd, strerror(errno));

diff --git a/lib/utils.c b/lib/utils.c
index a81c070069fedecab440e57756aa018bc0eee321..be0f11b00280de813faf457672371eba9d5f5c6a 100644 (file)
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1418,33 +1418,6 @@ void print_nlmsg_timestamp(FILE *fp, const struct nlmsghdr *n)
        fprintf(fp, "Timestamp: %s %lu us\n", tstr, usecs);
 }
 
-static int on_netns(char *nsname, void *arg)
-{
-       struct netns_func *f = arg;
-
-       if (netns_switch(nsname))
-               return -1;
-
-       return f->func(nsname, f->arg);
-}
-
-static int on_netns_label(char *nsname, void *arg)
-{
-       printf("\nnetns: %s\n", nsname);
-       return on_netns(nsname, arg);
-}
-
-int do_each_netns(int (*func)(char *nsname, void *arg), void *arg,
-               bool show_label)
-{
-       struct netns_func nsf = { .func = func, .arg = arg };
-
-       if (show_label)
-               return netns_foreach(on_netns_label, &nsf);
-
-       return netns_foreach(on_netns, &nsf);
-}
-
 char *int_to_str(int val, char *buf)
 {
        sprintf(buf, "%d", val);