[PATCH 00/14] Improved tool for testing across multiple namespaces
This series extends the "nsholder" tool we use in the tests to "nstool" with some more features. In particular it allows entering an established namespace with capabilities intact, and it allows entering them with less verbose options than nsenter. For now this only gives a modest simplification of the tests, but it should enable more in future. I haven't managed to get all the way through the testsuite with this: it's wedging in the IPv4 UDP throughput tests, but that's happening for me with the main branch too. I'll debug that, but I don't want to delay this series on that for now. David Gibson (14): nstool: Rename nsholder to nstool nstool: Reverse parameters to nstool nstool: Move description of its operation modes from comment to usage nstool: Split some command line parsing and socket setup to subcommands nstool: Replace "pid" subcommand with "info" subcommand nstool: Detect what namespaces target is in nstool: Add magic number to advertized information nstool: Helpers to iterate through namespace types nstool: Add nstool exec command to execute commands in an nstool namespace nstool: Add --keep-caps option to nstool exec test: Initialise ${TRACE} properly test: Use "nstool exec" to slightly simplify tests nstool: Advertise the holder's cwd (in its mountns) across the socket nstool: Enter holder's cwd when changing mount ns with nstool exec test/.gitignore | 2 +- test/Makefile | 4 +- test/lib/context | 14 +- test/lib/setup | 49 ++--- test/nsholder.c | 139 ------------ test/nstool.c | 562 +++++++++++++++++++++++++++++++++++++++++++++++ test/run | 5 +- 7 files changed, 599 insertions(+), 176 deletions(-) delete mode 100644 test/nsholder.c create mode 100644 test/nstool.c -- 2.39.2
In preparation for extending what it does.
Signed-off-by: David Gibson
Having the "subcommand" first is more conventional and will make it more
natural for future extensions I have planned.
Signed-off-by: David Gibson
Easier to see it there.
Signed-off-by: David Gibson
On Tue, 4 Apr 2023 11:46:27 +1000
David Gibson
Easier to see it there.
Signed-off-by: David Gibson
--- test/nstool.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index cc6d617..7e069b6 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -4,20 +4,6 @@ * * Copyright Red Hat * Author: David Gibson
- * - * Can run in 3 modes: - * - * nstool hold <path> - * Designed to be run inside a namespace, opens a Unix domain - * control socket at <path> and waits until instructed to stop - * with "nstool stop <path>" - * nstool pid <path> - * Prints the PID of the nstool hold process with control socket - * <path>. This is given in the PID namespace where nstool pid - * is executed, not the one where nstool hold is running - * nstool stop <path> - * Instruct the nstool hold with control socket at <path> to - * exit. */ #define _GNU_SOURCE @@ -38,7 +24,17 @@
static void usage(void) { - die("Usage: nstool hold|pid|stop <socket path>\n"); + die("Usage:\n" + " nstool hold SOCK\n" + " Run within a set of namespaces, open a Unix domain control\n"
UNIX domain socket -- Stefano
On Wed, Apr 05, 2023 at 01:56:59PM +0200, Stefano Brivio wrote:
On Tue, 4 Apr 2023 11:46:27 +1000 David Gibson
wrote: Easier to see it there.
Signed-off-by: David Gibson
--- test/nstool.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index cc6d617..7e069b6 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -4,20 +4,6 @@ * * Copyright Red Hat * Author: David Gibson
- * - * Can run in 3 modes: - * - * nstool hold <path> - * Designed to be run inside a namespace, opens a Unix domain - * control socket at <path> and waits until instructed to stop - * with "nstool stop <path>" - * nstool pid <path> - * Prints the PID of the nstool hold process with control socket - * <path>. This is given in the PID namespace where nstool pid - * is executed, not the one where nstool hold is running - * nstool stop <path> - * Instruct the nstool hold with control socket at <path> to - * exit. */ #define _GNU_SOURCE @@ -38,7 +24,17 @@
static void usage(void) { - die("Usage: nstool hold|pid|stop <socket path>\n"); + die("Usage:\n" + " nstool hold SOCK\n" + " Run within a set of namespaces, open a Unix domain control\n"
UNIX domain socket
Adjusted. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
This will make it easier to differentiate the options to those commands
further in future.
Signed-off-by: David Gibson
On Tue, 4 Apr 2023 11:46:28 +1000
David Gibson
This will make it easier to differentiate the options to those commands further in future.
Signed-off-by: David Gibson
--- test/nstool.c | 102 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 34 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index 7e069b6..9ea7eeb 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -11,6 +11,7 @@ #include
#include #include +#include #include #include #include @@ -37,19 +38,55 @@ static void usage(void) " terminate.\n"); } -static void hold(int fd, const struct sockaddr_un *addr) +static int connect_ctl(const char * sockpath, bool wait) { + int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); + struct sockaddr_un addr = { + .sun_family = AF_UNIX, + }; int rc;
- rc = bind(fd, (struct sockaddr *)addr, sizeof(*addr)); + if (fd < 0) + die("socket(): %s\n", strerror(errno));
Unrelated: it would be nice if die() added newlines eventually.
+ + strncpy(addr.sun_path, sockpath, UNIX_PATH_MAX); + + do { + rc = connect(fd, (struct sockaddr *)&addr, sizeof(addr)); + if (rc < 0 && + (!wait || (errno != ENOENT && errno != ECONNREFUSED))) + die("connect() to %s: %s\n", sockpath, strerror(errno));
A (1ms?) delay would be nice to have here -- it's almost a busyloop, connect() fails fast.
+ } while (rc < 0); + + return fd; +} + +static void cmd_hold(int argc, char *argv[]) +{ + int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); + struct sockaddr_un addr = { + .sun_family = AF_UNIX, + }; + const char *sockpath = argv[1]; + int rc; + + if (argc != 2) + usage(); + + if (fd < 0) + die("socket(): %s\n", strerror(errno)); + + strncpy(addr.sun_path, sockpath, UNIX_PATH_MAX); + + rc = bind(fd, (struct sockaddr *)&addr, sizeof(addr)); if (rc < 0) - die("bind(): %s\n", strerror(errno)); + die("bind() to %s: %s\n", sockpath, strerror(errno));
rc = listen(fd, 0); if (rc < 0) - die("listen(): %s\n", strerror(errno)); + die("listen() on %s: %s\n", sockpath, strerror(errno));
- printf("nstool: local PID=%d local UID=%u local GID=%u\n", + printf("nstool hold: local PID=%d local UID=%u local GID=%u\n", getpid(), getuid(), getgid()); do { int afd = accept(fd, NULL, NULL); @@ -63,71 +100,68 @@ static void hold(int fd, const struct sockaddr_un *addr) die("read(): %s\n", strerror(errno)); } while (rc == 0);
- unlink(addr->sun_path); + unlink(sockpath); }
-static void pid(int fd, const struct sockaddr_un *addr) +static void cmd_pid(int argc, char *argv[]) { - int rc; + const char *sockpath = argv[1]; struct ucred peercred; socklen_t optlen = sizeof(peercred); + int fd, rc;
- do { - rc = connect(fd, (struct sockaddr *)addr, sizeof(*addr)); - if (rc < 0 && errno != ENOENT && errno != ECONNREFUSED) - die("connect(): %s\n", strerror(errno)); - } while (rc < 0); + if (argc != 2) + usage(); + + fd = connect_ctl(sockpath, true);
I didn't spot this earlier, but... does it really make sense to wait in cmd_pid(), also on ENOENT, rather than making 'hold' return only once the socket is ready? I don't think it would be outrageous to have 'nstool pid' failing if the holding process doesn't exist. Admittely, I'm biased by the few hundreds of times I needed to 'killall -9 nsholder' in the past months. :)
rc = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &peercred, &optlen); if (rc < 0) - die("getsockopet(SO_PEERCRED): %s\n", strerror(errno)); + die("getsockopet(SO_PEERCRED) %s: %s\n", + sockpath, strerror(errno));
close(fd);
printf("%d\n", peercred.pid); }
-static void stop(int fd, const struct sockaddr_un *addr) +static void cmd_stop(int argc, char *argv[]) { - int rc; + const char *sockpath = argv[1]; + int fd, rc; char buf = 'Q';
- rc = connect(fd, (struct sockaddr *)addr, sizeof(*addr)); - if (rc < 0) - die("connect(): %s\n", strerror(errno)); + if (argc != 2) + usage(); + + fd = connect_ctl(sockpath, false);
rc = write(fd, &buf, sizeof(buf));
Unrelated: a compound literal would make this more readable. -- Stefano
On Wed, Apr 05, 2023 at 01:58:00PM +0200, Stefano Brivio wrote:
On Tue, 4 Apr 2023 11:46:28 +1000 David Gibson
wrote: This will make it easier to differentiate the options to those commands further in future.
Signed-off-by: David Gibson
--- test/nstool.c | 102 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 68 insertions(+), 34 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index 7e069b6..9ea7eeb 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -11,6 +11,7 @@ #include
#include #include +#include #include #include #include @@ -37,19 +38,55 @@ static void usage(void) " terminate.\n"); } -static void hold(int fd, const struct sockaddr_un *addr) +static int connect_ctl(const char * sockpath, bool wait) { + int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); + struct sockaddr_un addr = { + .sun_family = AF_UNIX, + }; int rc;
- rc = bind(fd, (struct sockaddr *)addr, sizeof(*addr)); + if (fd < 0) + die("socket(): %s\n", strerror(errno));
Unrelated: it would be nice if die() added newlines eventually.
Sure, but as you say unrelated.
+ + strncpy(addr.sun_path, sockpath, UNIX_PATH_MAX); + + do { + rc = connect(fd, (struct sockaddr *)&addr, sizeof(addr)); + if (rc < 0 && + (!wait || (errno != ENOENT && errno != ECONNREFUSED))) + die("connect() to %s: %s\n", sockpath, strerror(errno));
A (1ms?) delay would be nice to have here -- it's almost a busyloop, connect() fails fast.
Yeah, I guess so. That's not new, it was already like that in "nsholder pid", so I think something to fix separately.
+ } while (rc < 0); + + return fd; +} + +static void cmd_hold(int argc, char *argv[]) +{ + int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); + struct sockaddr_un addr = { + .sun_family = AF_UNIX, + }; + const char *sockpath = argv[1]; + int rc; + + if (argc != 2) + usage(); + + if (fd < 0) + die("socket(): %s\n", strerror(errno)); + + strncpy(addr.sun_path, sockpath, UNIX_PATH_MAX); + + rc = bind(fd, (struct sockaddr *)&addr, sizeof(addr)); if (rc < 0) - die("bind(): %s\n", strerror(errno)); + die("bind() to %s: %s\n", sockpath, strerror(errno));
rc = listen(fd, 0); if (rc < 0) - die("listen(): %s\n", strerror(errno)); + die("listen() on %s: %s\n", sockpath, strerror(errno));
- printf("nstool: local PID=%d local UID=%u local GID=%u\n", + printf("nstool hold: local PID=%d local UID=%u local GID=%u\n", getpid(), getuid(), getgid()); do { int afd = accept(fd, NULL, NULL); @@ -63,71 +100,68 @@ static void hold(int fd, const struct sockaddr_un *addr) die("read(): %s\n", strerror(errno)); } while (rc == 0);
- unlink(addr->sun_path); + unlink(sockpath); }
-static void pid(int fd, const struct sockaddr_un *addr) +static void cmd_pid(int argc, char *argv[]) { - int rc; + const char *sockpath = argv[1]; struct ucred peercred; socklen_t optlen = sizeof(peercred); + int fd, rc;
- do { - rc = connect(fd, (struct sockaddr *)addr, sizeof(*addr)); - if (rc < 0 && errno != ENOENT && errno != ECONNREFUSED) - die("connect(): %s\n", strerror(errno)); - } while (rc < 0); + if (argc != 2) + usage(); + + fd = connect_ctl(sockpath, true);
I didn't spot this earlier, but... does it really make sense to wait in cmd_pid(), also on ENOENT, rather than making 'hold' return only once the socket is ready?
So, this is a consequence of the fact that the holder doesn't move into the background itself - it just sits in the foreground until terminated. That means that the typical usecase puts it into the background from the shell with &, which in turn means that when we reach the next shell command the socket may not be ready - or not even created. One of the things I had in mind for a hypothetical "nstool unshare" would be to avoid this and have it background itself once the socket is ready.
I don't think it would be outrageous to have 'nstool pid' failing if the holding process doesn't exist.
Admittely, I'm biased by the few hundreds of times I needed to 'killall -9 nsholder' in the past months. :)
So... I agree that's irritating, I've done it a similar number of times. However, I don't think that's really related to the question above - in my experience it's always been the holder process that's hung around, not something waiting on a holder.
rc = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &peercred, &optlen); if (rc < 0) - die("getsockopet(SO_PEERCRED): %s\n", strerror(errno)); + die("getsockopet(SO_PEERCRED) %s: %s\n", + sockpath, strerror(errno));
close(fd);
printf("%d\n", peercred.pid); }
-static void stop(int fd, const struct sockaddr_un *addr) +static void cmd_stop(int argc, char *argv[]) { - int rc; + const char *sockpath = argv[1]; + int fd, rc; char buf = 'Q';
- rc = connect(fd, (struct sockaddr *)addr, sizeof(*addr)); - if (rc < 0) - die("connect(): %s\n", strerror(errno)); + if (argc != 2) + usage(); + + fd = connect_ctl(sockpath, false);
rc = write(fd, &buf, sizeof(buf));
Unrelated: a compound literal would make this more readable.
Uh.. I don't see where a compound literal would even go here. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
On Thu, 6 Apr 2023 12:31:55 +1000
David Gibson
On Wed, Apr 05, 2023 at 01:58:00PM +0200, Stefano Brivio wrote:
[...]
I didn't spot this earlier, but... does it really make sense to wait in cmd_pid(), also on ENOENT, rather than making 'hold' return only once the socket is ready?
So, this is a consequence of the fact that the holder doesn't move into the background itself - it just sits in the foreground until terminated. That means that the typical usecase puts it into the background from the shell with &, which in turn means that when we reach the next shell command the socket may not be ready - or not even created.
One of the things I had in mind for a hypothetical "nstool unshare" would be to avoid this and have it background itself once the socket is ready.
Ah, sure, it makes sense now.
I don't think it would be outrageous to have 'nstool pid' failing if the holding process doesn't exist.
Admittely, I'm biased by the few hundreds of times I needed to 'killall -9 nsholder' in the past months. :)
So... I agree that's irritating, I've done it a similar number of times. However, I don't think that's really related to the question above - in my experience it's always been the holder process that's hung around, not something waiting on a holder.
Yes, same here, but it's something I file under the same category (I don't remember why nsholder would hang, you probably explained at some point...).
rc = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &peercred, &optlen); if (rc < 0) - die("getsockopet(SO_PEERCRED): %s\n", strerror(errno)); + die("getsockopet(SO_PEERCRED) %s: %s\n", + sockpath, strerror(errno));
close(fd);
printf("%d\n", peercred.pid); }
-static void stop(int fd, const struct sockaddr_un *addr) +static void cmd_stop(int argc, char *argv[]) { - int rc; + const char *sockpath = argv[1]; + int fd, rc; char buf = 'Q';
- rc = connect(fd, (struct sockaddr *)addr, sizeof(*addr)); - if (rc < 0) - die("connect(): %s\n", strerror(errno)); + if (argc != 2) + usage(); + + fd = connect_ctl(sockpath, false);
rc = write(fd, &buf, sizeof(buf));
Unrelated: a compound literal would make this more readable.
Uh.. I don't see where a compound literal would even go here.
I meant: rc = write(fd, &(char){ 'Q' }, 1); ...so that one doesn't need to look at 'buf'. nstool is C99 anyway. -- Stefano
On Thu, Apr 06, 2023 at 08:47:48AM +0200, Stefano Brivio wrote:
On Thu, 6 Apr 2023 12:31:55 +1000 David Gibson
wrote: On Wed, Apr 05, 2023 at 01:58:00PM +0200, Stefano Brivio wrote:
[...]
I didn't spot this earlier, but... does it really make sense to wait in cmd_pid(), also on ENOENT, rather than making 'hold' return only once the socket is ready?
So, this is a consequence of the fact that the holder doesn't move into the background itself - it just sits in the foreground until terminated. That means that the typical usecase puts it into the background from the shell with &, which in turn means that when we reach the next shell command the socket may not be ready - or not even created.
One of the things I had in mind for a hypothetical "nstool unshare" would be to avoid this and have it background itself once the socket is ready.
Ah, sure, it makes sense now.
I don't think it would be outrageous to have 'nstool pid' failing if the holding process doesn't exist.
Admittely, I'm biased by the few hundreds of times I needed to 'killall -9 nsholder' in the past months. :)
So... I agree that's irritating, I've done it a similar number of times. However, I don't think that's really related to the question above - in my experience it's always been the holder process that's hung around, not something waiting on a holder.
Yes, same here, but it's something I file under the same category (I don't remember why nsholder would hang, you probably explained at some point...).
I believe the main reason is because of the holders which are PID 1 within their pid namespaces. That means that if you interrupt the tests, the SIGINT or SIGHUP they'll get from tmux etc. shutting down won't be sufficient to kill them.
rc = getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &peercred, &optlen); if (rc < 0) - die("getsockopet(SO_PEERCRED): %s\n", strerror(errno)); + die("getsockopet(SO_PEERCRED) %s: %s\n", + sockpath, strerror(errno));
close(fd);
printf("%d\n", peercred.pid); }
-static void stop(int fd, const struct sockaddr_un *addr) +static void cmd_stop(int argc, char *argv[]) { - int rc; + const char *sockpath = argv[1]; + int fd, rc; char buf = 'Q';
- rc = connect(fd, (struct sockaddr *)addr, sizeof(*addr)); - if (rc < 0) - die("connect(): %s\n", strerror(errno)); + if (argc != 2) + usage(); + + fd = connect_ctl(sockpath, false);
rc = write(fd, &buf, sizeof(buf));
Unrelated: a compound literal would make this more readable.
Uh.. I don't see where a compound literal would even go here.
I meant:
rc = write(fd, &(char){ 'Q' }, 1);
...so that one doesn't need to look at 'buf'. nstool is C99 anyway.
Oh, ok. On the other hand it means not using sizeof() to get the length, which isn't ideal. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
The new subcommand gives more information about the holder process and its
namespace, and may be further extended in future. Add some options which
give the old behaviour for existing scripts.
Signed-off-by: David Gibson
On Tue, 4 Apr 2023 11:46:29 +1000
David Gibson
The new subcommand gives more information about the holder process and its namespace, and may be further extended in future. Add some options which give the old behaviour for existing scripts.
Signed-off-by: David Gibson
--- test/lib/setup | 12 +++++----- test/nstool.c | 60 +++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/test/lib/setup b/test/lib/setup index e6180b1..6d7644a 100755 --- a/test/lib/setup +++ b/test/lib/setup @@ -78,7 +78,7 @@ setup_pasta() { layout_pasta
context_run_bg unshare "unshare -rUnpf ${NSTOOL} hold ${STATESETUP}/ns.hold" - __target_pid=$(${NSTOOL} pid ${STATESETUP}/ns.hold) + __target_pid=$(${NSTOOL} info -pw ${STATESETUP}/ns.hold)
Either 'pid' or 'info -pw' needs to be typed a few times, and 'pid' is simpler. I would have a slight preference toward demultiplexing the different commands in nstool rather than in scripts (with, say, 'pid' passing 'pidonly' as true in cmd_info()). And you always pass '-w', right? -- Stefano
On Wed, Apr 05, 2023 at 01:58:15PM +0200, Stefano Brivio wrote:
On Tue, 4 Apr 2023 11:46:29 +1000 David Gibson
wrote: The new subcommand gives more information about the holder process and its namespace, and may be further extended in future. Add some options which give the old behaviour for existing scripts.
Signed-off-by: David Gibson
--- test/lib/setup | 12 +++++----- test/nstool.c | 60 +++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/test/lib/setup b/test/lib/setup index e6180b1..6d7644a 100755 --- a/test/lib/setup +++ b/test/lib/setup @@ -78,7 +78,7 @@ setup_pasta() { layout_pasta
context_run_bg unshare "unshare -rUnpf ${NSTOOL} hold ${STATESETUP}/ns.hold" - __target_pid=$(${NSTOOL} pid ${STATESETUP}/ns.hold) + __target_pid=$(${NSTOOL} info -pw ${STATESETUP}/ns.hold)
Either 'pid' or 'info -pw' needs to be typed a few times, and 'pid' is simpler. I would have a slight preference toward demultiplexing the different commands in nstool rather than in scripts (with, say, 'pid' passing 'pidonly' as true in cmd_info()).
And you always pass '-w', right?
As of this patch, that's true, but a lot of that goes away when nstool exec is introduced. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Give nstool the ability to detect what namespaces the target process is in,
relative to where it's called. That is, those namespace types for which
the target is not in the same namespace as the caller. For now, just
print this information with "info", which can be useful for debugging.
Signed-off-by: David Gibson
On Tue, 4 Apr 2023 11:46:30 +1000
David Gibson
Give nstool the ability to detect what namespaces the target process is in, relative to where it's called. That is, those namespace types for which the target is not in the same namespace as the caller. For now, just print this information with "info", which can be useful for debugging.
Signed-off-by: David Gibson
--- test/nstool.c | 154 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 140 insertions(+), 14 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index 2cb4fb3..428c9c4 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -15,8 +15,13 @@ #include
#include #include +#include #include #include +#include +#include + +#define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0]))) #define die(...) \ do { \ @@ -24,6 +29,28 @@ exit(1); \ } while (0)
+struct ns_type { + int flag; + const char *name; +}; + +const struct ns_type nstypes[] = { + { CLONE_NEWCGROUP, "cgroup" }, + { CLONE_NEWIPC, "ipc" }, + { CLONE_NEWNET, "net" }, + { CLONE_NEWNS, "mnt" }, + { CLONE_NEWPID, "pid" }, + { CLONE_NEWTIME, "time" }, + { CLONE_NEWUSER, "user" }, + { CLONE_NEWUTS, "uts" }, +}; + +struct holder_info { + pid_t pid; + uid_t uid; + gid_t gid; +}; + static void usage(void) { die("Usage:\n" @@ -41,12 +68,16 @@ static void usage(void) " terminate.\n"); }
-static int connect_ctl(const char * sockpath, bool wait) +static int connect_ctl(const char *sockpath, bool wait, + struct holder_info *info, + struct ucred *peercred) { int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); struct sockaddr_un addr = { .sun_family = AF_UNIX, }; + struct holder_info discard; + ssize_t len; int rc;
if (fd < 0) @@ -61,6 +92,25 @@ static int connect_ctl(const char * sockpath, bool wait) die("connect() to %s: %s\n", sockpath, strerror(errno)); } while (rc < 0);
+ if (!info) + info = &discard;
As you close the socket anyway moments later, I wonder if it wouldn't be doable (and more natural) to just do: if (info) { len = read(fd, info, sizeof(*info)); ... } if (peercred) { ... } ...but maybe it adds unnecessary complication to 7/14 (or perhaps the magic is not really part of 'info'?). -- Stefano
On Wed, Apr 05, 2023 at 01:58:25PM +0200, Stefano Brivio wrote:
On Tue, 4 Apr 2023 11:46:30 +1000 David Gibson
wrote: Give nstool the ability to detect what namespaces the target process is in, relative to where it's called. That is, those namespace types for which the target is not in the same namespace as the caller. For now, just print this information with "info", which can be useful for debugging.
Signed-off-by: David Gibson
--- test/nstool.c | 154 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 140 insertions(+), 14 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index 2cb4fb3..428c9c4 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -15,8 +15,13 @@ #include
#include #include +#include #include #include +#include +#include + +#define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0]))) #define die(...) \ do { \ @@ -24,6 +29,28 @@ exit(1); \ } while (0)
+struct ns_type { + int flag; + const char *name; +}; + +const struct ns_type nstypes[] = { + { CLONE_NEWCGROUP, "cgroup" }, + { CLONE_NEWIPC, "ipc" }, + { CLONE_NEWNET, "net" }, + { CLONE_NEWNS, "mnt" }, + { CLONE_NEWPID, "pid" }, + { CLONE_NEWTIME, "time" }, + { CLONE_NEWUSER, "user" }, + { CLONE_NEWUTS, "uts" }, +}; + +struct holder_info { + pid_t pid; + uid_t uid; + gid_t gid; +}; + static void usage(void) { die("Usage:\n" @@ -41,12 +68,16 @@ static void usage(void) " terminate.\n"); }
-static int connect_ctl(const char * sockpath, bool wait) +static int connect_ctl(const char *sockpath, bool wait, + struct holder_info *info, + struct ucred *peercred) { int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); struct sockaddr_un addr = { .sun_family = AF_UNIX, }; + struct holder_info discard; + ssize_t len; int rc;
if (fd < 0) @@ -61,6 +92,25 @@ static int connect_ctl(const char * sockpath, bool wait) die("connect() to %s: %s\n", sockpath, strerror(errno)); } while (rc < 0);
+ if (!info) + info = &discard;
As you close the socket anyway moments later, I wonder if it wouldn't be doable (and more natural) to just do:
if (info) { len = read(fd, info, sizeof(*info)); ... }
I had something like that initially, but that causes the holder to spit a broken pipe error, which is distracting.
if (peercred) { ... }
...but maybe it adds unnecessary complication to 7/14 (or perhaps the magic is not really part of 'info'?).
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
So that we'll probably give a better error if you point it at something
that's not an nstool hold control socket.
Signed-off-by: David Gibson
On Tue, 4 Apr 2023 11:46:31 +1000
David Gibson
So that we'll probably give a better error if you point it at something that's not an nstool hold control socket.
Signed-off-by: David Gibson
--- test/nstool.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/nstool.c b/test/nstool.c index 428c9c4..e995f3e 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -12,6 +12,7 @@ #include
#include #include +#include #include #include #include @@ -45,7 +46,10 @@ const struct ns_type nstypes[] = { { CLONE_NEWUTS, "uts" }, }; +#define NSTOOL_MAGIC 0x75601d75 + struct holder_info { + uint64_t magic;
Is uint64_t with just 32 bits "set" a trick to detect endianness problems (perhaps it's a common pattern I'm not aware of)? -- Stefano
On Wed, Apr 05, 2023 at 01:58:34PM +0200, Stefano Brivio wrote:
On Tue, 4 Apr 2023 11:46:31 +1000 David Gibson
wrote: So that we'll probably give a better error if you point it at something that's not an nstool hold control socket.
Signed-off-by: David Gibson
--- test/nstool.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/nstool.c b/test/nstool.c index 428c9c4..e995f3e 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -12,6 +12,7 @@ #include
#include #include +#include #include #include #include @@ -45,7 +46,10 @@ const struct ns_type nstypes[] = { { CLONE_NEWUTS, "uts" }, }; +#define NSTOOL_MAGIC 0x75601d75 + struct holder_info { + uint64_t magic;
Is uint64_t with just 32 bits "set" a trick to detect endianness problems (perhaps it's a common pattern I'm not aware of)?
Heh, no, that's just me having brainfade. I've extended the constant to 64-bits. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Will make things a bit less verbose in future.
Signed-off-by: David Gibson
On Tue, 4 Apr 2023 11:46:32 +1000
David Gibson
Will make things a bit less verbose in future.
Signed-off-by: David Gibson
--- test/nstool.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index e995f3e..5681ce8 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -46,6 +46,14 @@ const struct ns_type nstypes[] = { { CLONE_NEWUTS, "uts" }, };
+#define for_each_nst(_nst, _flags) \ + for ((_nst) = &nstypes[0]; \ + ((_nst) - nstypes) < ARRAY_SIZE(nstypes); \ + (_nst)++) \ + if ((_flags) & (_nst)->flag) + +#define for_every_nst(_nst) for_each_nst(_nst, 0x7fffffff)
Strictly speaking this should be 0x7fffff00 (minus CSIGNAL). -- Stefano
On Wed, Apr 05, 2023 at 01:58:44PM +0200, Stefano Brivio wrote:
On Tue, 4 Apr 2023 11:46:32 +1000 David Gibson
wrote: Will make things a bit less verbose in future.
Signed-off-by: David Gibson
--- test/nstool.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index e995f3e..5681ce8 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -46,6 +46,14 @@ const struct ns_type nstypes[] = { { CLONE_NEWUTS, "uts" }, };
+#define for_each_nst(_nst, _flags) \ + for ((_nst) = &nstypes[0]; \ + ((_nst) - nstypes) < ARRAY_SIZE(nstypes); \ + (_nst)++) \ + if ((_flags) & (_nst)->flag) + +#define for_every_nst(_nst) for_each_nst(_nst, 0x7fffffff)
Strictly speaking this should be 0x7fffff00 (minus CSIGNAL).
So, I'm here deliberately not trying to embed knowledge of which ns types actually exist (if I need that, we really need to probe the kernel to find out). So this is just intended to be every bit in a positive int. I've now replaced it with INT_MAX, which is probably a better choice here. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
This combines nstool info -pw <sock> with nsenter with various options for
a more convenient and less verbose of entering existing nstool managed
namespaces.
Signed-off-by: David Gibson
On Tue, 4 Apr 2023 11:46:33 +1000
David Gibson
This combines nstool info -pw <sock> with nsenter with various options for a more convenient and less verbose of entering existing nstool managed namespaces.
Signed-off-by: David Gibson
--- test/nstool.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 137 insertions(+), 2 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index 5681ce8..25079aa 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -17,7 +17,9 @@ #include
#include #include +#include #include +#include #include #include #include @@ -75,6 +77,9 @@ static void usage(void) " socket at SOCK\n" " -p Print just the holder's PID as seen by the caller\n" " -w Retry connecting to SOCK until it is ready\n" + " nstool exec SOCK [COMMAND [ARGS...]]\n" + " Execute command or shell in the namespaces of the nstool hold\n" + " with control socket at SOCK\n" " nstool stop SOCK\n" " Instruct the nstool hold with control socket at SOCK to\n" " terminate.\n"); @@ -84,7 +89,7 @@ static int connect_ctl(const char *sockpath, bool wait, struct holder_info *info, struct ucred *peercred) { - int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); + int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, PF_UNIX); struct sockaddr_un addr = { .sun_family = AF_UNIX, }; @@ -132,7 +137,7 @@ static int connect_ctl(const char *sockpath, bool wait, static void cmd_hold(int argc, char *argv[]) { - int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); + int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, PF_UNIX); struct sockaddr_un addr = { .sun_family = AF_UNIX, }; @@ -301,6 +306,134 @@ static void cmd_info(int argc, char *argv[]) } }
+static int openns(const char *fmt, ...) +{ + char nspath[PATH_MAX]; + va_list ap; + int fd; + + va_start(ap, fmt); + if (vsnprintf(nspath, sizeof(nspath), fmt, ap) >= PATH_MAX) + die("Truncated path \"%s\"\n", nspath); + va_end(ap); + + fd = open(nspath, O_RDONLY | O_CLOEXEC); + if (fd < 0) + die("open() %s: %s\n", nspath, strerror(errno)); + + return fd; +} + +static void wait_for_child(pid_t pid) +{ + int status; + + /* Match the child's exit status, if possible */ + for (;;) { + pid_t rc; + + rc = waitpid(pid, &status, WUNTRACED); + if (rc < 0) + die("waitpid() on %d: %s\n", pid, strerror(errno)); + if (rc != pid) + die("waitpid() on %d returned %d", pid, rc); + if (WIFSTOPPED(status)) { + /* Stop the parent to patch */ + kill(getpid(), SIGSTOP); + /* We must have resumed, resume the child */ + kill(pid, SIGCONT); + continue; + } + + break; + } + + if (WIFEXITED(status)) + exit(WEXITSTATUS(status)); + else if (WIFSIGNALED(status)) + kill(getpid(), WTERMSIG(status));
An alternative could be what pasta_child_handler() does on WIFSIGNALED(status) -- I don't actually have a preference between the two.
+ + die("Unexpected status for child %d\n", pid);
Weird indentation in this function. -- Stefano
On Wed, Apr 05, 2023 at 01:58:54PM +0200, Stefano Brivio wrote:
On Tue, 4 Apr 2023 11:46:33 +1000 David Gibson
wrote: This combines nstool info -pw <sock> with nsenter with various options for a more convenient and less verbose of entering existing nstool managed namespaces.
Signed-off-by: David Gibson
--- test/nstool.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 137 insertions(+), 2 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index 5681ce8..25079aa 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -17,7 +17,9 @@ #include
#include #include +#include #include +#include #include #include #include @@ -75,6 +77,9 @@ static void usage(void) " socket at SOCK\n" " -p Print just the holder's PID as seen by the caller\n" " -w Retry connecting to SOCK until it is ready\n" + " nstool exec SOCK [COMMAND [ARGS...]]\n" + " Execute command or shell in the namespaces of the nstool hold\n" + " with control socket at SOCK\n" " nstool stop SOCK\n" " Instruct the nstool hold with control socket at SOCK to\n" " terminate.\n"); @@ -84,7 +89,7 @@ static int connect_ctl(const char *sockpath, bool wait, struct holder_info *info, struct ucred *peercred) { - int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); + int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, PF_UNIX); struct sockaddr_un addr = { .sun_family = AF_UNIX, }; @@ -132,7 +137,7 @@ static int connect_ctl(const char *sockpath, bool wait, static void cmd_hold(int argc, char *argv[]) { - int fd = socket(AF_UNIX, SOCK_STREAM, PF_UNIX); + int fd = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, PF_UNIX); struct sockaddr_un addr = { .sun_family = AF_UNIX, }; @@ -301,6 +306,134 @@ static void cmd_info(int argc, char *argv[]) } }
+static int openns(const char *fmt, ...) +{ + char nspath[PATH_MAX]; + va_list ap; + int fd; + + va_start(ap, fmt); + if (vsnprintf(nspath, sizeof(nspath), fmt, ap) >= PATH_MAX) + die("Truncated path \"%s\"\n", nspath); + va_end(ap); + + fd = open(nspath, O_RDONLY | O_CLOEXEC); + if (fd < 0) + die("open() %s: %s\n", nspath, strerror(errno)); + + return fd; +} + +static void wait_for_child(pid_t pid) +{ + int status; + + /* Match the child's exit status, if possible */ + for (;;) { + pid_t rc; + + rc = waitpid(pid, &status, WUNTRACED); + if (rc < 0) + die("waitpid() on %d: %s\n", pid, strerror(errno)); + if (rc != pid) + die("waitpid() on %d returned %d", pid, rc); + if (WIFSTOPPED(status)) { + /* Stop the parent to patch */ + kill(getpid(), SIGSTOP); + /* We must have resumed, resume the child */ + kill(pid, SIGCONT); + continue; + } + + break; + } + + if (WIFEXITED(status)) + exit(WEXITSTATUS(status)); + else if (WIFSIGNALED(status)) + kill(getpid(), WTERMSIG(status));
An alternative could be what pasta_child_handler() does on WIFSIGNALED(status) -- I don't actually have a preference between the two.
Right. The idiom above I copied from unshare and nsenter in util-linux. Seems marginally more thorough than the 128+ trick.
+ + die("Unexpected status for child %d\n", pid);
Weird indentation in this function.
Ugh, yes. I think I did a copy paste at some point that introduced a bunch of spaces where there should be tabs. Fixing. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
This allows you to run commands within a user namespace with the
privilege that comes from owning that userns.
Signed-off-by: David Gibson
On Tue, 4 Apr 2023 11:46:34 +1000
David Gibson
This allows you to run commands within a user namespace with the privilege that comes from owning that userns.
Signed-off-by: David Gibson
--- test/nstool.c | 89 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 79 insertions(+), 10 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index 25079aa..3ecc456 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -18,11 +18,15 @@ #include
#include #include +#include +#include #include #include +#include +#include #include -#include #include +#include #define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0])))
@@ -75,11 +79,13 @@ static void usage(void) " nstool info [-pw] pid SOCK\n" " Print information about the nstool hold process with control\n" " socket at SOCK\n" - " -p Print just the holder's PID as seen by the caller\n" - " -w Retry connecting to SOCK until it is ready\n" - " nstool exec SOCK [COMMAND [ARGS...]]\n" + " -p Print just the holder's PID as seen by the caller\n" + " -w Retry connecting to SOCK until it is ready\n" + " nstool exec [--keep-caps] SOCK [COMMAND [ARGS...]]\n" " Execute command or shell in the namespaces of the nstool hold\n" " with control socket at SOCK\n" + " --keep-caps Give all possible capabilities to COMMAND via\n" + " the ambient capability mask\n" " nstool stop SOCK\n" " Instruct the nstool hold with control socket at SOCK to\n" " terminate.\n"); @@ -275,7 +281,6 @@ static void cmd_info(int argc, char *argv[]) } while (opt != -1);
if (optind != argc - 1) { - fprintf(stderr, "B\n"); usage(); }
@@ -356,21 +361,82 @@ static void wait_for_child(pid_t pid) die("Unexpected status for child %d\n", pid); }
+static void caps_to_ambient(void) +{ + /* Use raw system calls to avoid the overly complex caps + * libraries. */
Bad indentation here. -- Stefano
On Wed, Apr 05, 2023 at 01:59:02PM +0200, Stefano Brivio wrote:
On Tue, 4 Apr 2023 11:46:34 +1000 David Gibson
wrote: This allows you to run commands within a user namespace with the privilege that comes from owning that userns.
Signed-off-by: David Gibson
--- test/nstool.c | 89 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 79 insertions(+), 10 deletions(-) diff --git a/test/nstool.c b/test/nstool.c index 25079aa..3ecc456 100644 --- a/test/nstool.c +++ b/test/nstool.c @@ -18,11 +18,15 @@ #include
#include #include +#include +#include #include #include +#include +#include #include -#include #include +#include #define ARRAY_SIZE(a) ((int)(sizeof(a) / sizeof((a)[0])))
@@ -75,11 +79,13 @@ static void usage(void) " nstool info [-pw] pid SOCK\n" " Print information about the nstool hold process with control\n" " socket at SOCK\n" - " -p Print just the holder's PID as seen by the caller\n" - " -w Retry connecting to SOCK until it is ready\n" - " nstool exec SOCK [COMMAND [ARGS...]]\n" + " -p Print just the holder's PID as seen by the caller\n" + " -w Retry connecting to SOCK until it is ready\n" + " nstool exec [--keep-caps] SOCK [COMMAND [ARGS...]]\n" " Execute command or shell in the namespaces of the nstool hold\n" " with control socket at SOCK\n" + " --keep-caps Give all possible capabilities to COMMAND via\n" + " the ambient capability mask\n" " nstool stop SOCK\n" " Instruct the nstool hold with control socket at SOCK to\n" " terminate.\n"); @@ -275,7 +281,6 @@ static void cmd_info(int argc, char *argv[]) } while (opt != -1);
if (optind != argc - 1) { - fprintf(stderr, "B\n"); usage(); }
@@ -356,21 +361,82 @@ static void wait_for_child(pid_t pid) die("Unexpected status for child %d\n", pid); }
+static void caps_to_ambient(void) +{ + /* Use raw system calls to avoid the overly complex caps + * libraries. */
Bad indentation here.
More pasted spaces. Fixing. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Unlike ${DEBUG} we don't initialize ${TRACE} to 0 if not set, which cases
failures when testing it later. That failure acts as though it is false,
however it emits spurious errors in script.log, which can make it harder to
spot real errors.
Signed-off-by: David Gibson
Using this, rather than using "nstool info" to get the pid then manually
connecting with nsenter makes things a little simpler.
Signed-off-by: David Gibson
On Tue, 4 Apr 2023 11:46:36 +1000
David Gibson
Using this, rather than using "nstool info" to get the pid then manually connecting with nsenter makes things a little simpler.
Signed-off-by: David Gibson
--- test/lib/context | 14 +++++++++----- test/lib/setup | 33 +++++++++++++-------------------- test/run | 2 +- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/test/lib/context b/test/lib/context index ee6b683..d9d8260 100644 --- a/test/lib/context +++ b/test/lib/context @@ -13,6 +13,8 @@ # Copyright Red Hat # Author: David Gibson
+NSTOOL="${BASEPATH}/nstool" + # context_setup_common() - Create outline of a new context # $1: Context name context_setup_common() { @@ -30,15 +32,17 @@ context_setup_host() { echo sh -c > "${__enter}" }
-# context_setup_nsenter() - Create a new context for running commands in a namespace +# context_setup_nstool() - Create a new context for running commands with nstool exec # $1: Context name -# $2: Namespace PID -context_setup_nsenter() { +# $2: nstool control socket +context_setup_nstool() { __name="$1" - shift + __sock="$2" __enter="${STATESETUP}/context_${__name}.enter" + # Wait for the ns to be ready + ${NSTOOL} info -w "${__sock}" > /dev/null
Bad indentation here.
context_setup_common "${__name}" - echo "nsenter $@ sh -c" > "${__enter}" + echo "${NSTOOL} exec ${__sock} -- sh -c" > "${__enter}" }
# context_setup_guest() - Create a new context for running commands in a guest diff --git a/test/lib/setup b/test/lib/setup index 6d7644a..7b17336 100755 --- a/test/lib/setup +++ b/test/lib/setup @@ -17,7 +17,6 @@ INITRAMFS="${BASEPATH}/mbuto.img" VCPUS="$( [ $(nproc) -ge 8 ] && echo 6 || echo $(( $(nproc) / 2 + 1 )) )" __mem_kib="$(sed -n 's/MemTotal:[ ]*\([0-9]*\) kB/\1/p' /proc/meminfo)" VMEM="$((${__mem_kib} / 1024 / 4))" -NSTOOL="${BASEPATH}/nstool"
# setup_build() - Set up pane layout for build tests setup_build() { @@ -78,9 +77,8 @@ setup_pasta() { layout_pasta
context_run_bg unshare "unshare -rUnpf ${NSTOOL} hold ${STATESETUP}/ns.hold" - __target_pid=$(${NSTOOL} info -pw ${STATESETUP}/ns.hold)
- context_setup_nsenter ns -U -n -p --preserve-credentials -t ${__target_pid} + context_setup_nstool ns ${STATESETUP}/ns.hold
# Ports: # @@ -94,7 +92,7 @@ setup_pasta() { [ ${DEBUG} -eq 1 ] && __opts="${__opts} -d" [ ${TRACE} -eq 1 ] && __opts="${__opts} --trace"
- context_run_bg passt "./pasta ${__opts} -f -t 10002 -T 10003 -u 10002 -U 10003 -P ${STATESETUP}/passt.pid ${__target_pid}" + context_run_bg passt "./pasta ${__opts} -f -t 10002 -T 10003 -u 10002 -U 10003 -P ${STATESETUP}/passt.pid $(${NSTOOL} info -pw ${STATESETUP}/ns.hold)"
# pidfile isn't created until pasta is ready wait_for [ -f "${STATESETUP}/passt.pid" ] @@ -128,11 +126,10 @@ setup_passt_in_ns() {
context_run_bg pasta "./pasta ${__opts} -t 10001,10002,10011,10012 -T 10003,10013 -u 10001,10002,10011,10012 -U 10003,10013 -P ${STATESETUP}/pasta.pid --config-net ${NSTOOL} hold ${STATESETUP}/ns.hold" wait_for [ -f "${STATESETUP}/pasta.pid" ] - __ns_pid=$(${NSTOOL} info -pw ${STATESETUP}/ns.hold)
- context_setup_nsenter qemu "-t ${__ns_pid} -U -n -p --preserve-credentials" - context_setup_nsenter ns "-t ${__ns_pid} -U -n -p --preserve-credentials" - context_setup_nsenter passt "-t ${__ns_pid} -U -n -p --preserve-credentials" + context_setup_nstool qemu ${STATESETUP}/ns.hold + context_setup_nstool ns ${STATESETUP}/ns.hold + context_setup_nstool passt ${STATESETUP}/ns.hold
__opts= [ ${PCAP} -eq 1 ] && __opts="${__opts} -p ${LOGDIR}/passt_in_pasta.pcap" @@ -190,19 +187,17 @@ setup_two_guests() { [ ${DEBUG} -eq 1 ] && __opts="${__opts} -d" [ ${TRACE} -eq 1 ] && __opts="${__opts} --trace" context_run_bg pasta_1 "./pasta ${__opts} --trace -l /tmp/pasta1.log -P ${STATESETUP}/pasta_1.pid -t 10001,10002 -T 10003,10004 -u 10001,10002 -U 10003,10004 --config-net ${NSTOOL} hold ${STATESETUP}/ns1.hold" - __ns1_pid=$(${NSTOOL} info -pw ${STATESETUP}/ns1.hold) - context_setup_nsenter passt_1 -U -n -p --preserve-credentials -t ${__ns1_pid} + context_setup_nstool passt_1 ${STATESETUP}/ns1.hold
__opts= [ ${PCAP} -eq 1 ] && __opts="${__opts} -p ${LOGDIR}/pasta_2.pcap" [ ${DEBUG} -eq 1 ] && __opts="${__opts} -d" [ ${TRACE} -eq 1 ] && __opts="${__opts} --trace" context_run_bg pasta_2 "./pasta ${__opts} --trace -l /tmp/pasta2.log -P ${STATESETUP}/pasta_2.pid -t 10004,10005 -T 10003,10001 -u 10004,10005 -U 10003,10001 --config-net ${NSTOOL} hold ${STATESETUP}/ns2.hold" - __ns2_pid=$(${NSTOOL} info -pw ${STATESETUP}/ns2.hold) - context_setup_nsenter passt_2 -U -n -p --preserve-credentials -t ${__ns2_pid} + context_setup_nstool passt_2 ${STATESETUP}/ns2.hold
- context_setup_nsenter qemu_1 -U -n -p --preserve-credentials -t ${__ns1_pid} - context_setup_nsenter qemu_2 -U -n -p --preserve-credentials -t ${__ns2_pid} + context_setup_nstool qemu_1 ${STATESETUP}/ns1.hold + context_setup_nstool qemu_2 ${STATESETUP}/ns2.hold
__ifname="$(context_run qemu_1 "ip -j link show | jq -rM '.[] | select(.link_type == \"ether\").ifname'")"
@@ -310,15 +305,13 @@ teardown_passt_in_ns() {
# teardown_two_guests() - Exit namespaces, kill qemu processes, passt and pasta teardown_two_guests() { - __ns1_pid=$(${NSTOOL} info -pw "${STATESETUP}/ns1.hold") - __ns2_pid=$(${NSTOOL} info -pw "${STATESETUP}/ns2.hold") - nsenter -U -p --preserve-credentials -t ${__ns1_pid} kill $(cat "${STATESETUP}/qemu_1.pid") - nsenter -U -p --preserve-credentials -t ${__ns2_pid} kill $(cat "${STATESETUP}/qemu_2.pid") + ${NSTOOL} exec ${STATESETUP}/ns1.hold -- kill $(cat "${STATESETUP}/qemu_1.pid") + ${NSTOOL} exec ${STATESETUP}/ns2.hold -- kill $(cat "${STATESETUP}/qemu_2.pid") context_wait qemu_1 context_wait qemu_2
- nsenter -U -p --preserve-credentials -t ${__ns1_pid} kill $(cat "${STATESETUP}/passt_1.pid") - nsenter -U -p --preserve-credentials -t ${__ns2_pid} kill $(cat "${STATESETUP}/passt_2.pid") + ${NSTOOL} exec ${STATESETUP}/ns1.hold -- kill $(cat "${STATESETUP}/passt_1.pid") + ${NSTOOL} exec ${STATESETUP}/ns2.hold -- kill $(cat "${STATESETUP}/passt_2.pid")
...and here. -- Stefano
On Wed, Apr 05, 2023 at 01:59:12PM +0200, Stefano Brivio wrote:
On Tue, 4 Apr 2023 11:46:36 +1000 David Gibson
wrote: Using this, rather than using "nstool info" to get the pid then manually connecting with nsenter makes things a little simpler.
Signed-off-by: David Gibson
--- test/lib/context | 14 +++++++++----- test/lib/setup | 33 +++++++++++++-------------------- test/run | 2 +- 3 files changed, 23 insertions(+), 26 deletions(-) diff --git a/test/lib/context b/test/lib/context index ee6b683..d9d8260 100644 --- a/test/lib/context +++ b/test/lib/context @@ -13,6 +13,8 @@ # Copyright Red Hat # Author: David Gibson
+NSTOOL="${BASEPATH}/nstool" + # context_setup_common() - Create outline of a new context # $1: Context name context_setup_common() { @@ -30,15 +32,17 @@ context_setup_host() { echo sh -c > "${__enter}" }
-# context_setup_nsenter() - Create a new context for running commands in a namespace +# context_setup_nstool() - Create a new context for running commands with nstool exec # $1: Context name -# $2: Namespace PID -context_setup_nsenter() { +# $2: nstool control socket +context_setup_nstool() { __name="$1" - shift + __sock="$2" __enter="${STATESETUP}/context_${__name}.enter" + # Wait for the ns to be ready + ${NSTOOL} info -w "${__sock}" > /dev/null
Bad indentation here.
Fixed. Drat, emacs isn't doing what I expected here. [snip]
# teardown_two_guests() - Exit namespaces, kill qemu processes, passt and pasta teardown_two_guests() { - __ns1_pid=$(${NSTOOL} info -pw "${STATESETUP}/ns1.hold") - __ns2_pid=$(${NSTOOL} info -pw "${STATESETUP}/ns2.hold") - nsenter -U -p --preserve-credentials -t ${__ns1_pid} kill $(cat "${STATESETUP}/qemu_1.pid") - nsenter -U -p --preserve-credentials -t ${__ns2_pid} kill $(cat "${STATESETUP}/qemu_2.pid") + ${NSTOOL} exec ${STATESETUP}/ns1.hold -- kill $(cat "${STATESETUP}/qemu_1.pid") + ${NSTOOL} exec ${STATESETUP}/ns2.hold -- kill $(cat "${STATESETUP}/qemu_2.pid") context_wait qemu_1 context_wait qemu_2
- nsenter -U -p --preserve-credentials -t ${__ns1_pid} kill $(cat "${STATESETUP}/passt_1.pid") - nsenter -U -p --preserve-credentials -t ${__ns2_pid} kill $(cat "${STATESETUP}/passt_2.pid") + ${NSTOOL} exec ${STATESETUP}/ns1.hold -- kill $(cat "${STATESETUP}/passt_1.pid") + ${NSTOOL} exec ${STATESETUP}/ns2.hold -- kill $(cat "${STATESETUP}/passt_2.pid")
...and here.
Fixed here as well. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
This is possible useful in nstool info and has further uses for nstool
exec.
Signed-off-by: David Gibson
If we enter a mount namespace with nstool exec our working directory will
be changed to / in the new mount ns. This is surprising if we haven't
actually altered any mounts yet in the new ns. Instead, change the working
directory to match that of the holder process in this situation.
Signed-off-by: David Gibson
On Tue, 4 Apr 2023 11:46:24 +1000
David Gibson
This series extends the "nsholder" tool we use in the tests to "nstool" with some more features. In particular it allows entering an established namespace with capabilities intact, and it allows entering them with less verbose options than nsenter.
For now this only gives a modest simplification of the tests, but it should enable more in future.
I haven't managed to get all the way through the testsuite with this: it's wedging in the IPv4 UDP throughput tests, but that's happening for me with the main branch too. I'll debug that, but I don't want to delay this series on that for now.
David Gibson (14): nstool: Rename nsholder to nstool nstool: Reverse parameters to nstool nstool: Move description of its operation modes from comment to usage nstool: Split some command line parsing and socket setup to subcommands nstool: Replace "pid" subcommand with "info" subcommand nstool: Detect what namespaces target is in nstool: Add magic number to advertized information nstool: Helpers to iterate through namespace types nstool: Add nstool exec command to execute commands in an nstool namespace nstool: Add --keep-caps option to nstool exec test: Initialise ${TRACE} properly test: Use "nstool exec" to slightly simplify tests nstool: Advertise the holder's cwd (in its mountns) across the socket nstool: Enter holder's cwd when changing mount ns with nstool exec
In general this series looks good to me. None of the comments I have should actually impact functionality, so I can also apply as it is for the moment, I don't want to stand in the way of progress (and focus) -- as you prefer. -- Stefano
participants (2)
-
David Gibson
-
Stefano Brivio