* [PATCH iproute2 1/2] netns: switch netns in the child when executing commands
2019-06-10 22:16 [PATCH iproute2 0/2] refactor the 'ip netns exec' command Matteo Croce
@ 2019-06-10 22:16 ` Matteo Croce
2019-06-10 22:45 ` Stephen Hemminger
2019-06-10 22:16 ` [PATCH iproute2 2/2] netns: make netns_{save,restore} static Matteo Croce
1 sibling, 1 reply; 7+ messages in thread
From: Matteo Croce @ 2019-06-10 22:16 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Stephen Hemminger
'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, as well as other cleanup things like VRF associations.
Add an argument to cmd_exec() which allows to switch the current netns directly
in the child, so the parent environment is kept unaltered.
By doing so, some utility functions became unused, so remove them.
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
include/utils.h | 5 +----
ip/ip_common.h | 1 -
ip/ipnetns.c | 18 ++++--------------
ip/ipvrf.c | 16 +---------------
lib/exec.c | 6 +++++-
lib/utils.c | 27 ---------------------------
6 files changed, 11 insertions(+), 62 deletions(-)
diff --git a/include/utils.h b/include/utils.h
index 8a9c3020..c58a3886 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -294,14 +294,11 @@ 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, char *netns);
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/ip_common.h b/ip/ip_common.h
index b4aa34a7..38203aae 100644
--- a/ip/ip_common.h
+++ b/ip/ip_common.h
@@ -77,7 +77,6 @@ int do_tcp_metrics(int argc, char **argv);
int do_ipnetconf(int argc, char **argv);
int do_iptoken(int argc, char **argv);
int do_ipvrf(int argc, char **argv);
-void vrf_reset(void);
int netns_identify_pid(const char *pidstr, char *name, int len);
int do_seg6(int argc, char **argv);
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 8ead0c4c..9e414b55 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -400,7 +400,8 @@ 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, nsname);
return 0;
}
@@ -409,8 +410,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;
@@ -421,22 +420,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, argv[0]);
}
static int is_pid(const char *str)
diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index a13cf653..894d85fc 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -456,21 +456,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);
-}
-
-/* reset VRF association of current process to default VRF;
- * used by netns_exec
- */
-void vrf_reset(void)
-{
- char vrf[32];
-
- if (vrf_identify(getpid(), vrf, sizeof(vrf)) ||
- (vrf[0] == '\0'))
- return;
-
- vrf_switch("default");
+ return -cmd_exec(argv[1], argv + 1, !!batch_mode, NULL);
}
static int ipvrf_filter_req(struct nlmsghdr *nlh, int reqlen)
diff --git a/lib/exec.c b/lib/exec.c
index eb36b59d..3b07e908 100644
--- a/lib/exec.c
+++ b/lib/exec.c
@@ -5,8 +5,9 @@
#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, char *netns)
{
fflush(stdout);
if (do_fork) {
@@ -34,6 +35,9 @@ int cmd_exec(const char *cmd, char **argv, bool do_fork)
}
}
+ if (netns && netns_switch(netns))
+ 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 a81c0700..be0f11b0 100644
--- 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);
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH iproute2 2/2] netns: make netns_{save,restore} static
2019-06-10 22:16 [PATCH iproute2 0/2] refactor the 'ip netns exec' command Matteo Croce
2019-06-10 22:16 ` [PATCH iproute2 1/2] netns: switch netns in the child when executing commands Matteo Croce
@ 2019-06-10 22:16 ` Matteo Croce
1 sibling, 0 replies; 7+ messages in thread
From: Matteo Croce @ 2019-06-10 22:16 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, Stephen Hemminger
The netns_{save,restore} functions are only used in ipnetns.c now, since
the restore is not needed anymore after the netns exec command.
Move them in ipnetns.c, and make them static.
Signed-off-by: Matteo Croce <mcroce@redhat.com>
---
include/namespace.h | 2 --
ip/ip.c | 1 -
ip/ipnetns.c | 31 +++++++++++++++++++++++++++++++
lib/namespace.c | 31 -------------------------------
4 files changed, 31 insertions(+), 34 deletions(-)
diff --git a/include/namespace.h b/include/namespace.h
index 89cdda11..e47f9b5d 100644
--- a/include/namespace.h
+++ b/include/namespace.h
@@ -49,8 +49,6 @@ static inline int setns(int fd, int nstype)
}
#endif /* HAVE_SETNS */
-void netns_save(void);
-void netns_restore(void);
int netns_switch(char *netns);
int netns_get_fd(const char *netns);
int netns_foreach(int (*func)(char *nsname, void *arg), void *arg);
diff --git a/ip/ip.c b/ip/ip.c
index 49b3aa49..b71ae816 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -158,7 +158,6 @@ static int batch(const char *name)
if (!force)
break;
}
- netns_restore();
}
if (line)
free(line);
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 9e414b55..beb7fb8f 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -45,6 +45,7 @@ static int usage(void)
static struct rtnl_handle rtnsh = { .fd = -1 };
static int have_rtnl_getnsid = -1;
+static int saved_netns = -1;
static int ipnetns_accept_msg(struct rtnl_ctrl_data *ctrl,
struct nlmsghdr *n, void *arg)
@@ -623,6 +624,33 @@ static int create_netns_dir(void)
return 0;
}
+/* Obtain a FD for the current namespace, so we can reenter it later */
+static void netns_save(void)
+{
+ if (saved_netns != -1)
+ return;
+
+ saved_netns = open("/proc/self/ns/net", O_RDONLY | O_CLOEXEC);
+ if (saved_netns == -1) {
+ perror("Cannot open init namespace");
+ exit(1);
+ }
+}
+
+static void netns_restore(void)
+{
+ if (saved_netns == -1)
+ return;
+
+ if (setns(saved_netns, CLONE_NEWNET)) {
+ perror("setns");
+ exit(1);
+ }
+
+ close(saved_netns);
+ saved_netns = -1;
+}
+
static int netns_add(int argc, char **argv, bool create)
{
/* This function creates a new network namespace and
@@ -716,9 +744,12 @@ static int netns_add(int argc, char **argv, bool create)
proc_path, netns_path, strerror(errno));
goto out_delete;
}
+ netns_restore();
+
return 0;
out_delete:
if (create) {
+ netns_restore();
netns_delete(argc, argv);
} else if (unlink(netns_path) < 0) {
fprintf(stderr, "Cannot remove namespace file \"%s\": %s\n",
diff --git a/lib/namespace.c b/lib/namespace.c
index a2aea57a..06ae0a48 100644
--- a/lib/namespace.c
+++ b/lib/namespace.c
@@ -15,35 +15,6 @@
#include "utils.h"
#include "namespace.h"
-static int saved_netns = -1;
-
-/* Obtain a FD for the current namespace, so we can reenter it later */
-void netns_save(void)
-{
- if (saved_netns != -1)
- return;
-
- saved_netns = open("/proc/self/ns/net", O_RDONLY | O_CLOEXEC);
- if (saved_netns == -1) {
- perror("Cannot open init namespace");
- exit(1);
- }
-}
-
-void netns_restore(void)
-{
- if (saved_netns == -1)
- return;
-
- if (setns(saved_netns, CLONE_NEWNET)) {
- perror("setns");
- exit(1);
- }
-
- close(saved_netns);
- saved_netns = -1;
-}
-
static void bind_etc(const char *name)
{
char etc_netns_path[sizeof(NETNS_ETC_DIR) + NAME_MAX];
@@ -90,8 +61,6 @@ int netns_switch(char *name)
return -1;
}
- netns_save();
-
if (setns(netns, CLONE_NEWNET) < 0) {
fprintf(stderr, "setting the network namespace \"%s\" failed: %s\n",
name, strerror(errno));
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread