February 2024 - dev

[PATCH v3] pasta: Don't try to watch namespaces in procfs with inotify, use timer instead
by Stefano Brivio 19 Feb '24

19 Feb '24

We watch network namespace entries to detect when we should quit (unless --no-netns-quit is passed), and these might stored in a tmpfs typically mounted at /run/user/UID or /var/run/user/UID, or found in procfs at /proc/PID/ns/. Currently, we try to use inotify for any possible location of those entries, but inotify, of course, doesn't work on pseudo-filesystems (see inotify(7)). The man page reflects this: the description of --no-netns-quit implies that we won't quit anyway if the namespace is not "bound to the filesystem". Well, we won't quit, but, since commit 9e0dbc894813 ("More deterministic detection of whether argument is a PID, PATH or NAME"), we try. And, indeed, this is harmless, as the caveat from that commit message states. Now, it turns out that Buildah, a tool to create container images, sharing its codebase with Podman, passes a procfs entry to pasta, and expects pasta to exit once the network namespace is not needed anymore, that is, once the original Buildah process terminates. Get this to work by using the timer fallback mechanism if the namespace name is passed as a path belonging to a pseudo-filesystem. This is expected to be procfs, but I covered sysfs and devpts pseudo-filesystems as well, because nothing actually prevents creating this kind of directory structure and links there. Note that fstatfs(), according to some versions of man pages, was apparently "deprecated" by the LSB. My reasoning for using it is essentially this: https://lore.kernel.org/linux-man/f54kudgblgk643u32tb6at4cd3kkzha6hslahv24s… ...that is, there was no such thing as an LSB deprecation, and anyway there's no other way to get the filesystem type. Also note that, while it might sound more obvious to detect the filesystem type using fstatfs() on the file descriptor itself (c->pasta_netns_fd), the reported filesystem type for it is nsfs, no matter what path was given to pasta. If we use the parent directory, we'll typically have either tmpfs or procfs reported. If the target naemsapce is given as a PID, or as a PID-based procfs entry, we don't risk races if this PID is recycled: our handle on /proc/PID/ns will always refer to the original namespace associated with that PID, and we don't re-open this entry from procfs to check it. Instead of directly monitoring the target namespace, we could have tried to monitor a process with a given PID, using pidfd_open() to get a handle on it, to decide when to terminate. But it's not guaranteed that the parent process is actually the one associated to the network namespace we operate on, and if we get a PID file descriptor for a PID (parent or not) or path that was given on our command line, this inherently causes a race condition as that PID might have been recycled by the time we call pidfd_open(). Even assuming the process we want to watch is the parent process, and a race-free usage of pidfd_open() would have been possible, I'm not very enthusiastic about enabling yet another system call in the seccomp profile just for this, while openat() is needed anyway. Update the man page to reflect that, even if the target network namespace is passed as a procfs path or a PID, we'll now quit when the procfs entry is gone. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://github.com/containers/podman/pull/21563#issuecomment-1948200214 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v3: Given that we now open c->netns_dir before checking the filesystem type, we could as well pass this file descriptor to fstatfs() to do the check, instead of statfs() on the path. Fix a couple of paragraphs in the commit message. v2: Coverity Scan isn't happy if we "check" (kind of) c->netns_dir with statfs() before opening it in a non-atomic way. Just to make things clear, false positive or not: open it, check it, close it if it wasn't needed: we don't rely on the check. passt.1 | 8 ++++++-- pasta.c | 24 +++++++++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/passt.1 b/passt.1 index dc2d719..de6e3bf 100644 --- a/passt.1 +++ b/passt.1 @@ -550,8 +550,12 @@ without \-\-userns. .TP .BR \-\-no-netns-quit -If the target network namespace is bound to the filesystem (that is, if PATH or -NAME are given as target), do not exit once the network namespace is deleted. +If the target network namespace is bound to the filesystem, do not exit once +that path is deleted. + +If the target network namespace is represented by a procfs entry, do not exit +once that entry is removed from procfs (representing the fact that a process +with the given PID terminated). .TP .BR \-\-config-net diff --git a/pasta.c b/pasta.c index 01d1511..61feaa9 100644 --- a/pasta.c +++ b/pasta.c @@ -33,6 +33,7 @@ #include <sys/timerfd.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/statfs.h> #include <fcntl.h> #include <sys/wait.h> #include <signal.h> @@ -41,6 +42,7 @@ #include <netinet/in.h> #include <net/ethernet.h> #include <sys/syscall.h> +#include <linux/magic.h> #include "util.h" #include "passt.h" @@ -390,12 +392,21 @@ void pasta_netns_quit_init(const struct ctx *c) union epoll_ref ref = { .type = EPOLL_TYPE_NSQUIT_INOTIFY }; struct epoll_event ev = { .events = EPOLLIN }; int flags = O_NONBLOCK | O_CLOEXEC; - int fd; + struct statfs s = { 0 }; + bool try_inotify = true; + int fd = -1, dir_fd; if (c->mode != MODE_PASTA || c->no_netns_quit || !*c->netns_base) return; - if ((fd = inotify_init1(flags)) < 0) + if ((dir_fd = open(c->netns_dir, O_CLOEXEC | O_RDONLY)) < 0) + die("netns dir open: %s, exiting", strerror(errno)); + + if (fstatfs(dir_fd, &s) || s.f_type == DEVPTS_SUPER_MAGIC || + s.f_type == PROC_SUPER_MAGIC || s.f_type == SYSFS_MAGIC) + try_inotify = false; + + if (try_inotify && (fd = inotify_init1(flags)) < 0) warn("inotify_init1(): %s, use a timer", strerror(errno)); if (fd >= 0 && inotify_add_watch(fd, c->netns_dir, IN_DELETE) < 0) { @@ -409,11 +420,11 @@ void pasta_netns_quit_init(const struct ctx *c) if ((fd = pasta_netns_quit_timer()) < 0) die("Failed to set up fallback netns timer, exiting"); - ref.nsdir_fd = open(c->netns_dir, O_CLOEXEC | O_RDONLY); - if (ref.nsdir_fd < 0) - die("netns dir open: %s, exiting", strerror(errno)); + ref.nsdir_fd = dir_fd; ref.type = EPOLL_TYPE_NSQUIT_TIMER; + } else { + close(dir_fd); } if (fd > FD_REF_MAX) @@ -463,6 +474,9 @@ void pasta_netns_quit_timer_handler(struct ctx *c, union epoll_ref ref) fd = openat(ref.nsdir_fd, c->netns_base, O_PATH | O_CLOEXEC); if (fd < 0) { + if (errno == EACCES) /* Expected for existing procfs entry */ + return; + info("Namespace %s is gone, exiting", c->netns_base); exit(EXIT_SUCCESS); } -- 2.39.2

3 6

[PATCH] pasta: Don't try to watch namespaces in procfs with inotify, use timer instead
by Stefano Brivio 19 Feb '24

19 Feb '24

We watch network namespace entries to detect when we should quit (unless --no-netns-quit is passed), and these might stored in a tmpfs typically mounted at /run/user/UID or /var/run/user/UID, or found in procfs at /proc/PID/ns/. Currently, we try to use inotify for any possible location of those entries, but inotify, of course, doesn't work on pseudo-filesystems (see inotify(7)). The man page reflects this: the description of --no-netns-quit implies that we won't quit anyway if the namespace is not "bound to the filesystem". Well, we won't quit, but, since commit 9e0dbc894813 ("More deterministic detection of whether argument is a PID, PATH or NAME"), we try. And, indeed, this is harmless, as the caveat from that commit message states. Now, it turns out that Buildah, a tool to create container images, sharing its codebase with Podman, passes a procfs entry to pasta, and expects pasta to exit once the network namespace is not needed anymore, that is, once the original Buildah process terminates. Get this to work by using the timer fallback mechanism if the namespace name is passed as a path belonging to a pseudo-filesystem. This is expected to be procfs, but I covered sysfs and devpts pseudo-filesystems as well, because nothing actually prevents creating this kind of directory structure and links there. Note that statfs(), according to some versions of man pages, was apparently "deprecated" by the LSB. My reasoning for using it is essentially this: https://lore.kernel.org/linux-man/f54kudgblgk643u32tb6at4cd3kkzha6hslahv24s… ...that is, there was no such thing as an LSB deprecation, and anyway there's no other way to get the filesystem type. Also note that, while it might sound more robust to detect the filesystem type using fstatfs() on the file descriptor (c->pasta_netns_fd), the reported filesystem type for it is nsfs, no matter what path was given to pasta. If we use the parent directory, we'll typically have either tmpfs or procfs reported. The timer, however, still uses the file descriptor of the parent directory later, as it has no access to the filesystem, and that avoids possible races if the PID is recycled: if the original process terminates, the handle we have on /proc/PID/ns still refers to it, not to any other process with the same PID. We could have used pidfd_open() to get a handle on the parent process. But it's not guaranteed that the parent process is actually the one associated to the network namespace we operate on, and if we get a PID file descriptor for a PID (parent or not) or path that was given on our command line, this inherently causes a race condition as that PID might have been recycled by the time we call pidfd_open(). Even assuming the process we want to watch is the parent process, and a race-free usage of pidfd_open() would have been possible, I'm not very enthusiastic about enabling yet another system call in the seccomp profile just for this, while openat() is needed anyway. Update the man page to reflect that, even if the target network namespace is passed as a procfs path or a PID, we'll now quit when the procfs entry is gone. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://github.com/containers/podman/pull/21563#issuecomment-1948200214 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt.1 | 8 ++++++-- pasta.c | 15 +++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/passt.1 b/passt.1 index dc2d719..de6e3bf 100644 --- a/passt.1 +++ b/passt.1 @@ -550,8 +550,12 @@ without \-\-userns. .TP .BR \-\-no-netns-quit -If the target network namespace is bound to the filesystem (that is, if PATH or -NAME are given as target), do not exit once the network namespace is deleted. +If the target network namespace is bound to the filesystem, do not exit once +that path is deleted. + +If the target network namespace is represented by a procfs entry, do not exit +once that entry is removed from procfs (representing the fact that a process +with the given PID terminated). .TP .BR \-\-config-net diff --git a/pasta.c b/pasta.c index 01d1511..4110917 100644 --- a/pasta.c +++ b/pasta.c @@ -33,6 +33,7 @@ #include <sys/timerfd.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/statfs.h> #include <fcntl.h> #include <sys/wait.h> #include <signal.h> @@ -41,6 +42,7 @@ #include <netinet/in.h> #include <net/ethernet.h> #include <sys/syscall.h> +#include <linux/magic.h> #include "util.h" #include "passt.h" @@ -390,12 +392,18 @@ void pasta_netns_quit_init(const struct ctx *c) union epoll_ref ref = { .type = EPOLL_TYPE_NSQUIT_INOTIFY }; struct epoll_event ev = { .events = EPOLLIN }; int flags = O_NONBLOCK | O_CLOEXEC; - int fd; + struct statfs s = { 0 }; + bool try_inotify = true; + int fd = -1; if (c->mode != MODE_PASTA || c->no_netns_quit || !*c->netns_base) return; - if ((fd = inotify_init1(flags)) < 0) + if (statfs(c->netns_dir, &s) || s.f_type == DEVPTS_SUPER_MAGIC || + s.f_type == PROC_SUPER_MAGIC || s.f_type == SYSFS_MAGIC) + try_inotify = false; + + if (try_inotify && (fd = inotify_init1(flags)) < 0) warn("inotify_init1(): %s, use a timer", strerror(errno)); if (fd >= 0 && inotify_add_watch(fd, c->netns_dir, IN_DELETE) < 0) { @@ -463,6 +471,9 @@ void pasta_netns_quit_timer_handler(struct ctx *c, union epoll_ref ref) fd = openat(ref.nsdir_fd, c->netns_base, O_PATH | O_CLOEXEC); if (fd < 0) { + if (errno == EACCES) /* Expected for existing procfs entry */ + return; + info("Namespace %s is gone, exiting", c->netns_base); exit(EXIT_SUCCESS); } -- 2.39.2

2 4

[PATCH v2] pasta: Don't try to watch namespaces in procfs with inotify, use timer instead
by Stefano Brivio 19 Feb '24

19 Feb '24

We watch network namespace entries to detect when we should quit (unless --no-netns-quit is passed), and these might stored in a tmpfs typically mounted at /run/user/UID or /var/run/user/UID, or found in procfs at /proc/PID/ns/. Currently, we try to use inotify for any possible location of those entries, but inotify, of course, doesn't work on pseudo-filesystems (see inotify(7)). The man page reflects this: the description of --no-netns-quit implies that we won't quit anyway if the namespace is not "bound to the filesystem". Well, we won't quit, but, since commit 9e0dbc894813 ("More deterministic detection of whether argument is a PID, PATH or NAME"), we try. And, indeed, this is harmless, as the caveat from that commit message states. Now, it turns out that Buildah, a tool to create container images, sharing its codebase with Podman, passes a procfs entry to pasta, and expects pasta to exit once the network namespace is not needed anymore, that is, once the original Buildah process terminates. Get this to work by using the timer fallback mechanism if the namespace name is passed as a path belonging to a pseudo-filesystem. This is expected to be procfs, but I covered sysfs and devpts pseudo-filesystems as well, because nothing actually prevents creating this kind of directory structure and links there. Note that statfs(), according to some versions of man pages, was apparently "deprecated" by the LSB. My reasoning for using it is essentially this: https://lore.kernel.org/linux-man/f54kudgblgk643u32tb6at4cd3kkzha6hslahv24s… ...that is, there was no such thing as an LSB deprecation, and anyway there's no other way to get the filesystem type. Also note that, while it might sound more robust to detect the filesystem type using fstatfs() on the file descriptor (c->pasta_netns_fd), the reported filesystem type for it is nsfs, no matter what path was given to pasta. If we use the parent directory, we'll typically have either tmpfs or procfs reported. The timer, however, still uses the file descriptor of the parent directory later, as it has no access to the filesystem, and that avoids possible races if the PID is recycled: if the original process terminates, the handle we have on /proc/PID/ns still refers to it, not to any other process with the same PID. We could have used pidfd_open() to get a handle on the parent process. But it's not guaranteed that the parent process is actually the one associated to the network namespace we operate on, and if we get a PID file descriptor for a PID (parent or not) or path that was given on our command line, this inherently causes a race condition as that PID might have been recycled by the time we call pidfd_open(). Even assuming the process we want to watch is the parent process, and a race-free usage of pidfd_open() would have been possible, I'm not very enthusiastic about enabling yet another system call in the seccomp profile just for this, while openat() is needed anyway. Update the man page to reflect that, even if the target network namespace is passed as a procfs path or a PID, we'll now quit when the procfs entry is gone. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://github.com/containers/podman/pull/21563#issuecomment-1948200214 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Coverity Scan isn't happy if we "check" (kind of) c->netns_dir with statfs() before opening it in a non-atomic way. Just to make things clear, false positive or not: open it, check it, close it if it wasn't needed: we don't rely on the check. passt.1 | 8 ++++++-- pasta.c | 24 +++++++++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/passt.1 b/passt.1 index dc2d719..de6e3bf 100644 --- a/passt.1 +++ b/passt.1 @@ -550,8 +550,12 @@ without \-\-userns. .TP .BR \-\-no-netns-quit -If the target network namespace is bound to the filesystem (that is, if PATH or -NAME are given as target), do not exit once the network namespace is deleted. +If the target network namespace is bound to the filesystem, do not exit once +that path is deleted. + +If the target network namespace is represented by a procfs entry, do not exit +once that entry is removed from procfs (representing the fact that a process +with the given PID terminated). .TP .BR \-\-config-net diff --git a/pasta.c b/pasta.c index 01d1511..465fe1a 100644 --- a/pasta.c +++ b/pasta.c @@ -33,6 +33,7 @@ #include <sys/timerfd.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/statfs.h> #include <fcntl.h> #include <sys/wait.h> #include <signal.h> @@ -41,6 +42,7 @@ #include <netinet/in.h> #include <net/ethernet.h> #include <sys/syscall.h> +#include <linux/magic.h> #include "util.h" #include "passt.h" @@ -390,12 +392,21 @@ void pasta_netns_quit_init(const struct ctx *c) union epoll_ref ref = { .type = EPOLL_TYPE_NSQUIT_INOTIFY }; struct epoll_event ev = { .events = EPOLLIN }; int flags = O_NONBLOCK | O_CLOEXEC; - int fd; + struct statfs s = { 0 }; + bool try_inotify = true; + int fd = -1, dir_fd; if (c->mode != MODE_PASTA || c->no_netns_quit || !*c->netns_base) return; - if ((fd = inotify_init1(flags)) < 0) + if ((dir_fd = open(c->netns_dir, O_CLOEXEC | O_RDONLY)) < 0) + die("netns dir open: %s, exiting", strerror(errno)); + + if (statfs(c->netns_dir, &s) || s.f_type == DEVPTS_SUPER_MAGIC || + s.f_type == PROC_SUPER_MAGIC || s.f_type == SYSFS_MAGIC) + try_inotify = false; + + if (try_inotify && (fd = inotify_init1(flags)) < 0) warn("inotify_init1(): %s, use a timer", strerror(errno)); if (fd >= 0 && inotify_add_watch(fd, c->netns_dir, IN_DELETE) < 0) { @@ -409,11 +420,11 @@ void pasta_netns_quit_init(const struct ctx *c) if ((fd = pasta_netns_quit_timer()) < 0) die("Failed to set up fallback netns timer, exiting"); - ref.nsdir_fd = open(c->netns_dir, O_CLOEXEC | O_RDONLY); - if (ref.nsdir_fd < 0) - die("netns dir open: %s, exiting", strerror(errno)); + ref.nsdir_fd = dir_fd; ref.type = EPOLL_TYPE_NSQUIT_TIMER; + } else { + close(dir_fd); } if (fd > FD_REF_MAX) @@ -463,6 +474,9 @@ void pasta_netns_quit_timer_handler(struct ctx *c, union epoll_ref ref) fd = openat(ref.nsdir_fd, c->netns_base, O_PATH | O_CLOEXEC); if (fd < 0) { + if (errno == EACCES) /* Expected for existing procfs entry */ + return; + info("Namespace %s is gone, exiting", c->netns_base); exit(EXIT_SUCCESS); } -- 2.39.2

2 1

[PATCH v2 0/8] Add vhost-user support to passt (part 1)
by Laurent Vivier 19 Feb '24

19 Feb '24

v2 comparing to vhost-user full part: - part 1 includes only preliminary patches (checksum, iovec, cleanup) - see detailed v2 history log in each patch. Full series v1 available at: [PATCH 00/24] Add vhost-user support to passt. https://url.corp.redhat.com/passt-vhost-user-v1 Thanks, Laurent Laurent Vivier (8): iov: add some functions to manage iovec pcap: add pcap_iov() checksum: align buffers checksum: add csum_iov() util: move IP stuff from util.[ch] to ip.[ch] checksum: use csum_ip4_header() in udp.c and tcp.c checksum: introduce functions to compute the header part checksum for TCP/UDP tap: make tap_update_mac() generic Makefile | 12 +-- checksum.c | 163 +++++++++++++++++++++------------------ checksum.h | 14 ++-- conf.c | 1 + dhcp.c | 1 + flow.c | 1 + icmp.c | 1 + iov.c | 212 +++++++++++++++++++++++++++++++++++++++++++++++++++ iov.h | 43 +++++++++++ ip.c | 72 +++++++++++++++++ ip.h | 86 +++++++++++++++++++++ ndp.c | 1 + pcap.c | 61 +++++++++++++-- pcap.h | 1 + port_fwd.c | 1 + qrap.c | 1 + tap.c | 32 ++++++-- tap.h | 2 +- tcp.c | 73 ++++++------------ tcp_splice.c | 1 + udp.c | 39 ++++------ util.c | 55 ------------- util.h | 76 ------------------ 23 files changed, 640 insertions(+), 309 deletions(-) create mode 100644 iov.c create mode 100644 iov.h create mode 100644 ip.c create mode 100644 ip.h -- 2.42.0

3 27

[PATCH v3] tcp: add support for SO_PEEK_OFF
by jmaloy＠redhat.com 19 Feb '24

19 Feb '24

From: Jon Maloy <jmaloy(a)redhat.com> When reading received messages from a socket with MSG_PEEK, we may want to read the contents with an offset, like we can do with pread/preadv() when reading files. Currently, it is not possible to do that. In this commit, we add support for the SO_PEEK_OFF socket option for TCP, in a similar way it is done for Unix Domain sockets. In the iperf3 log examples shown below, we can observe a throughput improvement of 15-20 % in the direction host->namespace when using the protocol splicer 'pasta' (https://passt.top). This is a consistent result. pasta(1) and passt(1) implement user-mode networking for network namespaces (containers) and virtual machines by means of a translation layer between Layer-2 network interface and native Layer-4 sockets (TCP, UDP, ICMP/ICMPv6 echo). Received, pending TCP data to the container/guest is kept in kernel buffers until acknowledged, so the tool routinely needs to fetch new data from socket, skipping data that was already sent. At the moment this is implemented using a dummy buffer passed to recvmsg(). With this change, we don't need a dummy buffer and the related buffer copy (copy_to_user()) anymore. passt and pasta are supported in KubeVirt and libvirt/qemu. jmaloy@freyr:~/passt$ perf record -g ./pasta --config-net -f SO_PEEK_OFF not supported by kernel. jmaloy@freyr:~/passt# iperf3 -s ----------------------------------------------------------- Server listening on 5201 (test #1) ----------------------------------------------------------- Accepted connection from 192.168.122.1, port 44822 [ 5] local 192.168.122.180 port 5201 connected to 192.168.122.1 port 44832 [ ID] Interval Transfer Bitrate [ 5] 0.00-1.00 sec 1.02 GBytes 8.78 Gbits/sec [ 5] 1.00-2.00 sec 1.06 GBytes 9.08 Gbits/sec [ 5] 2.00-3.00 sec 1.07 GBytes 9.15 Gbits/sec [ 5] 3.00-4.00 sec 1.10 GBytes 9.46 Gbits/sec [ 5] 4.00-5.00 sec 1.03 GBytes 8.85 Gbits/sec [ 5] 5.00-6.00 sec 1.10 GBytes 9.44 Gbits/sec [ 5] 6.00-7.00 sec 1.11 GBytes 9.56 Gbits/sec [ 5] 7.00-8.00 sec 1.07 GBytes 9.20 Gbits/sec [ 5] 8.00-9.00 sec 667 MBytes 5.59 Gbits/sec [ 5] 9.00-10.00 sec 1.03 GBytes 8.83 Gbits/sec [ 5] 10.00-10.04 sec 30.1 MBytes 6.36 Gbits/sec - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bitrate [ 5] 0.00-10.04 sec 10.3 GBytes 8.78 Gbits/sec receiver ----------------------------------------------------------- Server listening on 5201 (test #2) ----------------------------------------------------------- ^Ciperf3: interrupt - the server has terminated jmaloy@freyr:~/passt# logout [ perf record: Woken up 23 times to write data ] [ perf record: Captured and wrote 5.696 MB perf.data (35580 samples) ] jmaloy@freyr:~/passt$ jmaloy@freyr:~/passt$ perf record -g ./pasta --config-net -f SO_PEEK_OFF supported by kernel. jmaloy@freyr:~/passt# iperf3 -s ----------------------------------------------------------- Server listening on 5201 (test #1) ----------------------------------------------------------- Accepted connection from 192.168.122.1, port 52084 [ 5] local 192.168.122.180 port 5201 connected to 192.168.122.1 port 52098 [ ID] Interval Transfer Bitrate [ 5] 0.00-1.00 sec 1.32 GBytes 11.3 Gbits/sec [ 5] 1.00-2.00 sec 1.19 GBytes 10.2 Gbits/sec [ 5] 2.00-3.00 sec 1.26 GBytes 10.8 Gbits/sec [ 5] 3.00-4.00 sec 1.36 GBytes 11.7 Gbits/sec [ 5] 4.00-5.00 sec 1.33 GBytes 11.4 Gbits/sec [ 5] 5.00-6.00 sec 1.21 GBytes 10.4 Gbits/sec [ 5] 6.00-7.00 sec 1.31 GBytes 11.2 Gbits/sec [ 5] 7.00-8.00 sec 1.25 GBytes 10.7 Gbits/sec [ 5] 8.00-9.00 sec 1.33 GBytes 11.5 Gbits/sec [ 5] 9.00-10.00 sec 1.24 GBytes 10.7 Gbits/sec [ 5] 10.00-10.04 sec 56.0 MBytes 12.1 Gbits/sec - - - - - - - - - - - - - - - - - - - - - - - - - [ ID] Interval Transfer Bitrate [ 5] 0.00-10.04 sec 12.9 GBytes 11.0 Gbits/sec receiver ----------------------------------------------------------- Server listening on 5201 (test #2) ----------------------------------------------------------- ^Ciperf3: interrupt - the server has terminated logout [ perf record: Woken up 20 times to write data ] [ perf record: Captured and wrote 5.040 MB perf.data (33411 samples) ] jmaloy@freyr:~/passt$ The perf record confirms this result. Below, we can observe that the CPU spends significantly less time in the function ____sys_recvmsg() when we have offset support. Without offset support: ---------------------- jmaloy@freyr:~/passt$ perf report -q --symbol-filter=do_syscall_64 \ -p ____sys_recvmsg -x --stdio -i perf.data | head -1 46.32% 0.00% passt.avx2 [kernel.vmlinux] [k] do_syscall_64 ____sys_recvmsg With offset support: ---------------------- jmaloy@freyr:~/passt$ perf report -q --symbol-filter=do_syscall_64 \ -p ____sys_recvmsg -x --stdio -i perf.data | head -1 28.12% 0.00% passt.avx2 [kernel.vmlinux] [k] do_syscall_64 ____sys_recvmsg Suggested-by: Paolo Abeni <pabeni(a)redhat.com> Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v3: - Applied changes suggested by Stefano Brivio and Paolo Abeni --- net/ipv4/af_inet.c | 1 + net/ipv4/tcp.c | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 4e635dd3d3c8..5f0e5d10c416 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -1071,6 +1071,7 @@ const struct proto_ops inet_stream_ops = { #endif .splice_eof = inet_splice_eof, .splice_read = tcp_splice_read, + .set_peek_off = sk_set_peek_off, .read_sock = tcp_read_sock, .read_skb = tcp_read_skb, .sendmsg_locked = tcp_sendmsg_locked, diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 7e2481b9eae1..1c8cab14a32c 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -1415,8 +1415,6 @@ static int tcp_peek_sndq(struct sock *sk, struct msghdr *msg, int len) struct sk_buff *skb; int copied = 0, err = 0; - /* XXX -- need to support SO_PEEK_OFF */ - skb_rbtree_walk(skb, &sk->tcp_rtx_queue) { err = skb_copy_datagram_msg(skb, 0, msg, skb->len); if (err) @@ -2327,6 +2325,7 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len, int target; /* Read at least this many bytes */ long timeo; struct sk_buff *skb, *last; + u32 peek_offset = 0; u32 urg_hole = 0; err = -ENOTCONN; @@ -2360,7 +2359,8 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len, seq = &tp->copied_seq; if (flags & MSG_PEEK) { - peek_seq = tp->copied_seq; + peek_offset = max(sk_peek_offset(sk, flags), 0); + peek_seq = tp->copied_seq + peek_offset; seq = &peek_seq; } @@ -2463,11 +2463,11 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len, } if ((flags & MSG_PEEK) && - (peek_seq - copied - urg_hole != tp->copied_seq)) { + (peek_seq - peek_offset - copied - urg_hole != tp->copied_seq)) { net_dbg_ratelimited("TCP(%s:%d): Application bug, race in MSG_PEEK\n", current->comm, task_pid_nr(current)); - peek_seq = tp->copied_seq; + peek_seq = tp->copied_seq + peek_offset; } continue; @@ -2508,7 +2508,10 @@ static int tcp_recvmsg_locked(struct sock *sk, struct msghdr *msg, size_t len, WRITE_ONCE(*seq, *seq + used); copied += used; len -= used; - + if (flags & MSG_PEEK) + sk_peek_offset_fwd(sk, used); + else + sk_peek_offset_bwd(sk, used); tcp_rcv_space_adjust(sk); skip_copy: @@ -3007,6 +3010,7 @@ int tcp_disconnect(struct sock *sk, int flags) __skb_queue_purge(&sk->sk_receive_queue); WRITE_ONCE(tp->copied_seq, tp->rcv_nxt); WRITE_ONCE(tp->urg_data, 0); + sk_set_peek_off(sk, -1); tcp_write_queue_purge(sk); tcp_fastopen_active_disable_ofo_check(sk); skb_rbtree_purge(&tp->out_of_order_queue); -- 2.42.0

6 23

[PATCH v2] conf: No routable interface for IPv4 or IPv6 is informational, not a warning
by Stefano Brivio 16 Feb '24

16 Feb '24

...Podman users might get confused by the fact that if we can't find a default route for a given IP version, we'll report that as a warning message and possibly just before actual error messages. However, a lack of routable interface for IPv4 or IPv6 can be a normal circumstance: don't warn about it, just state that as informational message, if those are displayed (they're not in non-error paths in Podman, for example). While at it, make it clear that we're disabling IPv4 or IPv6 if there's no routable interface for the corresponding IP version. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://github.com/containers/podman/pull/21563#issuecomment-1937024642 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Report that we're disabling IPv4 or IPv6 in the message conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conf.c b/conf.c index 5e15b66..3646700 100644 --- a/conf.c +++ b/conf.c @@ -579,7 +579,7 @@ static unsigned int conf_ip4(unsigned int ifi, ifi = nl_get_ext_if(nl_sock, AF_INET); if (!ifi) { - warn("No external routable interface for IPv4"); + info("No routable interface for IPv4: IPv4 is disabled"); return 0; } @@ -651,7 +651,7 @@ static unsigned int conf_ip6(unsigned int ifi, ifi = nl_get_ext_if(nl_sock, AF_INET6); if (!ifi) { - warn("No external routable interface for IPv6"); + info("No routable interface for IPv6: IPv6 is disabled"); return 0; } -- 2.39.2

2 3

passt: new version 2024_02_16.08344da available
by Stefano Brivio 16 Feb '24

16 Feb '24

The new version with tag 2024_02_16.08344da includes the following changes: 08344da selinux: Allow pasta to remount procfs 338b632 conf: No routable interface for IPv4 or IPv6 is informational, not a warning 8f3f8e1 pasta: Add fallback timer mechanism to check if namespace is gone f57a2fb conf, passt.1: Exit if we can't bind a forwarded port, except for -[tu] all 927cb84 udp: udp_sock_init_ns() partially duplicats udp_port_rebind_outbound() 96ad5c5 udp: Don't prematurely (and incorrectly) set up automatic inbound forwards 9f57983 netlink: Use const rtnh pointer 7ee4e17 log: setlogmask(0) can actually result in a system call, don't use it 78901ee tcp: Fix subtle bug in fast re-transmit path 6c7623d netlink: Add support to fetch default gateway from multipath routes 322660b icmp: Dedicated functions for starting and closing ping sequences b6a4e20 icmp: Validate packets received on ping sockets 6e86511 icmp: Warn on receive errors from ping sockets a325121 icmp: Consolidate icmp_sock_handler() with icmpv6_sock_handler() 70d43f9 icmp: Share more between IPv4 and IPv6 paths in icmp_tap_handler() 15be1bf icmp: Simplify socket expiry scanning 24badd0 icmp: Use -1 to represent "missing" sockets 43713af icmp: Don't attempt to match host IDs to guest IDs 8534cdb icmp: Don't attempt to handle "wrong direction" ping socket traffic 2cb2fe6 icmp: Remove redundant initialisation of sendto() address 5dffb99 icmp: Don't set "port" on destination sockaddr for ping sockets 8981a72 flow: Avoid moving flow entries to compact table 9c0881d flow: Enforce that freeing of closed flows must happen in deferred handlers 4a849e9 flow: Abstract allocation of new flows with helper function fb7c001 flow: Move flow_count from context structure to a global 7f37bf4 flow: Move flow_log_() to near top of flow.c 02e092b tcp, tcp_splice: Avoid double layered dispatch for connected TCP sockets 70121ca epoll: Better handling of number of epoll types 36dfa8b flow, tcp: Add handling for per-flow timers b43e448 flow, tcp: Add flow-centric dispatch for deferred flow handling c97bb52 tcp, tcp_splice: Move per-type cleanup logic into per-type helpers eebca11 tcp, tcp_splice: Remove redundant handling from tcp_timer() 8563e7c treewide: Standardise on 'now' for current timestamp variables 17bbab1 flow: Make flow_table.h #include the protocol specific headers it needs 00c6eb6 pif: Remove unused pif_name() function a179ca6 treewide: Make a bunch of pointer variables pointers to const f60c851 test: Fix passt.mbuto for cases where /usr/sbin doesn't exist https://passt.top/passt/log/?qt=range&q=2023_12_30.f091893..2024_02_16.0834… Packages: - Arch Linux: https://www.archlinux.org/packages/extra/x86_64/passt/ https://archlinuxarm.org/packages/aarch64/passt https://archlinuxarm.org/packages/armv7h/passt - Debian tracker: https://tracker.debian.org/pkg/passt - Copr (CentOS Stream, EPEL, Fedora, Mageia): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/7022413/ permanent mirror: https://passt.top/builds/copr/0^20240216.g08344da/ - Fedora updates: https://bodhi.fedoraproject.org/updates/?packages=passt - Gentoo versions: https://packages.gentoo.org/packages/net-misc/passt - Ubuntu tracker: https://launchpad.net/ubuntu/+source/passt - Void Linux: https://voidlinux.org/packages/?q=passt - Static builds: - Package for other RPM-based distributions, x86_64 only: https://passt.top/builds/latest/x86_64/passt-g08344da-1.x86_64.rpm - x86_64 static binaries: https://passt.top/builds/latest/x86_64/ - Debian package, from x86_64 static build: https://passt.top/builds/latest/x86_64/passt_08344da-1_all.deb -- Stefano

1 0

[PATCH] selinux: Allow pasta to remount procfs
by Stefano Brivio 16 Feb '24

16 Feb '24

Partially equivalent to commit abf5ef6c22d2 ("apparmor: Allow pasta to remount /proc, access entries under its own copy"): we should allow pasta to remount /proc. It still works otherwise, but further UID remapping in nested user namespaces (e.g. pasta in pasta) won't. Reported-by: Laurent Jacquot <jk(a)lutty.net> Link: https://bugs.passt.top/show_bug.cgi?id=79#c3 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- contrib/selinux/pasta.te | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/selinux/pasta.te b/contrib/selinux/pasta.te index ed70c5f..0ceda06 100644 --- a/contrib/selinux/pasta.te +++ b/contrib/selinux/pasta.te @@ -201,6 +201,8 @@ allow pasta_t kernel_t:system module_request; allow pasta_t nsfs_t:file read; +allow pasta_t proc_t:dir mounton; +allow pasta_t proc_t:filesystem mount; allow pasta_t net_conf_t:lnk_file read; allow pasta_t proc_net_t:lnk_file read; -- 2.39.2

1 0

[PATCH v2] pasta: Add fallback timer mechanism to check if namespace is gone
by Stefano Brivio 16 Feb '24

16 Feb '24

We don't know how frequently this happens, but hitting fs.inotify.max_user_watches or similar sysctl limits is definitely not out of question, and Paul mentioned that, for example, Podman's CI environments hit similar issues in the past. Introduce a fallback mechanism based on a timer file descriptor: we grab the directory handle at startup, and we can then use openat(), triggered periodically, to check if the (network) namespace directory still exists. If openat() fails at some point, exit. Link: https://github.com/containers/podman/pull/21563#issuecomment-1943505707 Reported-by: Paul Holzinger <pholzing(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Use new epoll reference type instead of abusing EPOLL_TYPE_NSQUIT, and check for errors or short counts from read() on the timerfd passt.c | 34 ++++++++++--------- passt.h | 6 +++- pasta.c | 101 ++++++++++++++++++++++++++++++++++++++++++++------------ pasta.h | 5 +-- 4 files changed, 107 insertions(+), 39 deletions(-) diff --git a/passt.c b/passt.c index aaa8e58..a90568f 100644 --- a/passt.c +++ b/passt.c @@ -61,17 +61,18 @@ char pkt_buf[PKT_BUF_BYTES] __attribute__ ((aligned(PAGE_SIZE))); char *epoll_type_str[] = { - [EPOLL_TYPE_TCP] = "connected TCP socket", - [EPOLL_TYPE_TCP_SPLICE] = "connected spliced TCP socket", - [EPOLL_TYPE_TCP_LISTEN] = "listening TCP socket", - [EPOLL_TYPE_TCP_TIMER] = "TCP timer", - [EPOLL_TYPE_UDP] = "UDP socket", - [EPOLL_TYPE_ICMP] = "ICMP socket", - [EPOLL_TYPE_ICMPV6] = "ICMPv6 socket", - [EPOLL_TYPE_NSQUIT] = "namespace inotify", - [EPOLL_TYPE_TAP_PASTA] = "/dev/net/tun device", - [EPOLL_TYPE_TAP_PASST] = "connected qemu socket", - [EPOLL_TYPE_TAP_LISTEN] = "listening qemu socket", + [EPOLL_TYPE_TCP] = "connected TCP socket", + [EPOLL_TYPE_TCP_SPLICE] = "connected spliced TCP socket", + [EPOLL_TYPE_TCP_LISTEN] = "listening TCP socket", + [EPOLL_TYPE_TCP_TIMER] = "TCP timer", + [EPOLL_TYPE_UDP] = "UDP socket", + [EPOLL_TYPE_ICMP] = "ICMP socket", + [EPOLL_TYPE_ICMPV6] = "ICMPv6 socket", + [EPOLL_TYPE_NSQUIT_INOTIFY] = "namespace inotify watch", + [EPOLL_TYPE_NSQUIT_TIMER] = "namespace timer watch", + [EPOLL_TYPE_TAP_PASTA] = "/dev/net/tun device", + [EPOLL_TYPE_TAP_PASST] = "connected qemu socket", + [EPOLL_TYPE_TAP_LISTEN] = "listening qemu socket", }; static_assert(ARRAY_SIZE(epoll_type_str) == EPOLL_NUM_TYPES, "epoll_type_str[] doesn't match enum epoll_type"); @@ -201,7 +202,7 @@ void exit_handler(int signal) */ int main(int argc, char **argv) { - int nfds, i, devnull_fd = -1, pidfile_fd = -1, quit_fd; + int nfds, i, devnull_fd = -1, pidfile_fd = -1; struct epoll_event events[EPOLL_EVENTS]; char *log_name, argv0[PATH_MAX], *name; struct ctx c = { 0 }; @@ -274,7 +275,7 @@ int main(int argc, char **argv) if (c.force_stderr || isatty(fileno(stdout))) __openlog(log_name, LOG_PERROR, LOG_DAEMON); - quit_fd = pasta_netns_quit_init(&c); + pasta_netns_quit_init(&c); tap_sock_init(&c); @@ -370,8 +371,11 @@ loop: case EPOLL_TYPE_TAP_LISTEN: tap_listen_handler(&c, eventmask); break; - case EPOLL_TYPE_NSQUIT: - pasta_netns_quit_handler(&c, quit_fd); + case EPOLL_TYPE_NSQUIT_INOTIFY: + pasta_netns_quit_inotify_handler(&c, ref.fd); + break; + case EPOLL_TYPE_NSQUIT_TIMER: + pasta_netns_quit_timer_handler(&c, ref); break; case EPOLL_TYPE_TCP: tcp_sock_handler(&c, ref, eventmask); diff --git a/passt.h b/passt.h index a9e8f15..fb729b6 100644 --- a/passt.h +++ b/passt.h @@ -64,7 +64,9 @@ enum epoll_type { /* ICMPv6 sockets */ EPOLL_TYPE_ICMPV6, /* inotify fd watching for end of netns (pasta) */ - EPOLL_TYPE_NSQUIT, + EPOLL_TYPE_NSQUIT_INOTIFY, + /* timer fd watching for end of netns, fallback for inotify (pasta) */ + EPOLL_TYPE_NSQUIT_TIMER, /* tuntap character device */ EPOLL_TYPE_TAP_PASTA, /* socket connected to qemu */ @@ -84,6 +86,7 @@ enum epoll_type { * @udp: UDP-specific reference part * @icmp: ICMP-specific reference part * @data: Data handled by protocol handlers + * @nsdir_fd: netns dirfd for fallback timer checking if namespace is gone * @u64: Opaque reference for epoll_ctl() and epoll_wait() */ union epoll_ref { @@ -99,6 +102,7 @@ union epoll_ref { union udp_epoll_ref udp; union icmp_epoll_ref icmp; uint32_t data; + int nsdir_fd; }; }; uint64_t u64; diff --git a/pasta.c b/pasta.c index 94807a3..01d1511 100644 --- a/pasta.c +++ b/pasta.c @@ -30,6 +30,7 @@ #include <sys/epoll.h> #include <sys/inotify.h> #include <sys/mount.h> +#include <sys/timerfd.h> #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> @@ -356,47 +357,79 @@ void pasta_ns_conf(struct ctx *c) proto_update_l2_buf(c->mac_guest, NULL); } +/** + * pasta_netns_quit_timer() - Set up fallback timer to monitor namespace + * + * Return: timerfd file descriptor, negative error code on failure + */ +static int pasta_netns_quit_timer(void) +{ + int fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); + struct itimerspec it = { { 1, 0 }, { 1, 0 } }; /* one-second interval */ + + if (fd == -1) { + err("timerfd_create(): %s", strerror(errno)); + return -errno; + } + + if (timerfd_settime(fd, 0, &it, NULL) < 0) { + err("timerfd_settime(): %s", strerror(errno)); + close(fd); + return -errno; + } + + return fd; +} + /** * pasta_netns_quit_init() - Watch network namespace to quit once it's gone * @c: Execution context - * - * Return: inotify file descriptor, -1 on failure or if not needed/applicable */ -int pasta_netns_quit_init(const struct ctx *c) +void pasta_netns_quit_init(const struct ctx *c) { + union epoll_ref ref = { .type = EPOLL_TYPE_NSQUIT_INOTIFY }; + struct epoll_event ev = { .events = EPOLLIN }; int flags = O_NONBLOCK | O_CLOEXEC; - union epoll_ref ref = { .type = EPOLL_TYPE_NSQUIT }; - struct epoll_event ev = { - .events = EPOLLIN - }; - int inotify_fd; + int fd; if (c->mode != MODE_PASTA || c->no_netns_quit || !*c->netns_base) - return -1; + return; - if ((inotify_fd = inotify_init1(flags)) < 0) { - perror("inotify_init(): won't quit once netns is gone"); - return -1; + if ((fd = inotify_init1(flags)) < 0) + warn("inotify_init1(): %s, use a timer", strerror(errno)); + + if (fd >= 0 && inotify_add_watch(fd, c->netns_dir, IN_DELETE) < 0) { + warn("inotify_add_watch(): %s, use a timer", + strerror(errno)); + close(fd); + fd = -1; } - if (inotify_add_watch(inotify_fd, c->netns_dir, IN_DELETE) < 0) { - perror("inotify_add_watch(): won't quit once netns is gone"); - return -1; + if (fd < 0) { + if ((fd = pasta_netns_quit_timer()) < 0) + die("Failed to set up fallback netns timer, exiting"); + + ref.nsdir_fd = open(c->netns_dir, O_CLOEXEC | O_RDONLY); + if (ref.nsdir_fd < 0) + die("netns dir open: %s, exiting", strerror(errno)); + + ref.type = EPOLL_TYPE_NSQUIT_TIMER; } - ref.fd = inotify_fd; - ev.data.u64 = ref.u64; - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, inotify_fd, &ev); + if (fd > FD_REF_MAX) + die("netns monitor file number %i too big, exiting", fd); - return inotify_fd; + ref.fd = fd; + ev.data.u64 = ref.u64; + epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev); } /** - * pasta_netns_quit_handler() - Handle ns directory events, exit if ns is gone + * pasta_netns_quit_inotify_handler() - Handle inotify watch, exit if ns is gone * @c: Execution context * @inotify_fd: inotify file descriptor with watch on namespace directory */ -void pasta_netns_quit_handler(struct ctx *c, int inotify_fd) +void pasta_netns_quit_inotify_handler(struct ctx *c, int inotify_fd) { char buf[sizeof(struct inotify_event) + NAME_MAX + 1]; const struct inotify_event *in_ev = (struct inotify_event *)buf; @@ -410,3 +443,29 @@ void pasta_netns_quit_handler(struct ctx *c, int inotify_fd) info("Namespace %s is gone, exiting", c->netns_base); exit(EXIT_SUCCESS); } + +/** + * pasta_netns_quit_timer_handler() - Handle timer, exit if ns is gone + * @c: Execution context + * @ref: epoll reference for timer descriptor + */ +void pasta_netns_quit_timer_handler(struct ctx *c, union epoll_ref ref) +{ + uint64_t expirations; + ssize_t n; + int fd; + + n = read(ref.fd, &expirations, sizeof(expirations)); + if (n < 0) + die("Namespace watch timer read() error: %s", strerror(errno)); + if ((size_t)n < sizeof(expirations)) + warn("Namespace watch timer: short read(): %zi", n); + + fd = openat(ref.nsdir_fd, c->netns_base, O_PATH | O_CLOEXEC); + if (fd < 0) { + info("Namespace %s is gone, exiting", c->netns_base); + exit(EXIT_SUCCESS); + } + + close(fd); +} diff --git a/pasta.h b/pasta.h index 5d20063..4b063d1 100644 --- a/pasta.h +++ b/pasta.h @@ -13,7 +13,8 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, int argc, char *argv[]); void pasta_ns_conf(struct ctx *c); void pasta_child_handler(int signal); -int pasta_netns_quit_init(const struct ctx *c); -void pasta_netns_quit_handler(struct ctx *c, int inotify_fd); +void pasta_netns_quit_init(const struct ctx *c); +void pasta_netns_quit_inotify_handler(struct ctx *c, int inotify_fd); +void pasta_netns_quit_timer_handler(struct ctx *c, union epoll_ref ref); #endif /* PASTA_H */ -- 2.39.2

1 0

[PATCH] pasta: Add fallback timer mechanism to check if namespace is gone
by Stefano Brivio 16 Feb '24

16 Feb '24

We don't know how frequently this happens, but hitting fs.inotify.max_user_watches or similar sysctl limits is definitely not out of question, and Paul mentioned that, for example, Podman's CI environments hit similar issues in the past. Introduce a fallback mechanism based on a timer file descriptor: we grab the directory handle at startup, and we can then use openat(), triggered periodically, to check if the (network) namespace directory still exists. If openat() fails at some point, exit. Link: https://github.com/containers/podman/pull/21563#issuecomment-1943505707 Reported-by: Paul Holzinger <pholzing(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt.c | 6 ++-- passt.h | 2 ++ pasta.c | 85 +++++++++++++++++++++++++++++++++++++++++++++------------ pasta.h | 2 +- 4 files changed, 73 insertions(+), 22 deletions(-) diff --git a/passt.c b/passt.c index aaa8e58..13670b9 100644 --- a/passt.c +++ b/passt.c @@ -201,7 +201,7 @@ void exit_handler(int signal) */ int main(int argc, char **argv) { - int nfds, i, devnull_fd = -1, pidfile_fd = -1, quit_fd; + int nfds, i, devnull_fd = -1, pidfile_fd = -1; struct epoll_event events[EPOLL_EVENTS]; char *log_name, argv0[PATH_MAX], *name; struct ctx c = { 0 }; @@ -274,7 +274,7 @@ int main(int argc, char **argv) if (c.force_stderr || isatty(fileno(stdout))) __openlog(log_name, LOG_PERROR, LOG_DAEMON); - quit_fd = pasta_netns_quit_init(&c); + pasta_netns_quit_init(&c); tap_sock_init(&c); @@ -371,7 +371,7 @@ loop: tap_listen_handler(&c, eventmask); break; case EPOLL_TYPE_NSQUIT: - pasta_netns_quit_handler(&c, quit_fd); + pasta_netns_quit_handler(&c, ref); break; case EPOLL_TYPE_TCP: tcp_sock_handler(&c, ref, eventmask); diff --git a/passt.h b/passt.h index a9e8f15..1c6bb13 100644 --- a/passt.h +++ b/passt.h @@ -84,6 +84,7 @@ enum epoll_type { * @udp: UDP-specific reference part * @icmp: ICMP-specific reference part * @data: Data handled by protocol handlers + * @nsdir_fd: netns dirfd for fallback timer checking if namespace is gone * @u64: Opaque reference for epoll_ctl() and epoll_wait() */ union epoll_ref { @@ -99,6 +100,7 @@ union epoll_ref { union udp_epoll_ref udp; union icmp_epoll_ref icmp; uint32_t data; + int nsdir_fd; }; }; uint64_t u64; diff --git a/pasta.c b/pasta.c index 94807a3..60b6223 100644 --- a/pasta.c +++ b/pasta.c @@ -30,6 +30,7 @@ #include <sys/epoll.h> #include <sys/inotify.h> #include <sys/mount.h> +#include <sys/timerfd.h> #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> @@ -356,57 +357,105 @@ void pasta_ns_conf(struct ctx *c) proto_update_l2_buf(c->mac_guest, NULL); } +/** + * pasta_netns_quit_timer() - Set up fallback timer to monitor namespace + * + * Return: timerfd file descriptor, negative error code on failure + */ +static int pasta_netns_quit_timer(void) +{ + int fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC); + struct itimerspec it = { { 1, 0 }, { 1, 0 } }; /* one-second interval */ + + if (fd == -1) { + err("timerfd_create(): %s", strerror(errno)); + return -errno; + } + + if (timerfd_settime(fd, 0, &it, NULL) < 0) { + err("timerfd_settime(): %s", strerror(errno)); + close(fd); + return -errno; + } + + return fd; +} + /** * pasta_netns_quit_init() - Watch network namespace to quit once it's gone * @c: Execution context * - * Return: inotify file descriptor, -1 on failure or if not needed/applicable + * Return: file descriptor (inotify or timerfd), -1 if not needed */ int pasta_netns_quit_init(const struct ctx *c) { + union epoll_ref ref = { .type = EPOLL_TYPE_NSQUIT, .nsdir_fd = -1 }; + struct epoll_event ev = { .events = EPOLLIN }; int flags = O_NONBLOCK | O_CLOEXEC; - union epoll_ref ref = { .type = EPOLL_TYPE_NSQUIT }; - struct epoll_event ev = { - .events = EPOLLIN - }; - int inotify_fd; + int fd = -1; if (c->mode != MODE_PASTA || c->no_netns_quit || !*c->netns_base) return -1; - if ((inotify_fd = inotify_init1(flags)) < 0) { - perror("inotify_init(): won't quit once netns is gone"); - return -1; + if ((fd = inotify_init1(flags)) < 0) + warn("inotify_init1(): %s, use a timer", strerror(errno)); + + if (fd >= 0 && inotify_add_watch(fd, c->netns_dir, IN_DELETE) < 0) { + warn("inotify_add_watch(): %s, use a timer", + strerror(errno)); + close(fd); + fd = -1; } - if (inotify_add_watch(inotify_fd, c->netns_dir, IN_DELETE) < 0) { - perror("inotify_add_watch(): won't quit once netns is gone"); - return -1; + if (fd < 0) { + if ((fd = pasta_netns_quit_timer()) < 0) + die("Failed to set up fallback netns timer, exiting"); + + ref.nsdir_fd = open(c->netns_dir, O_CLOEXEC | O_RDONLY); + if (ref.nsdir_fd < 0) + die("netns dir open: %s, exiting", strerror(errno)); } - ref.fd = inotify_fd; + if (fd > FD_REF_MAX) + die("netns monitor file number %i too big, exiting", fd); + + ref.fd = fd; ev.data.u64 = ref.u64; - epoll_ctl(c->epollfd, EPOLL_CTL_ADD, inotify_fd, &ev); + epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev); - return inotify_fd; + return fd; } /** * pasta_netns_quit_handler() - Handle ns directory events, exit if ns is gone * @c: Execution context - * @inotify_fd: inotify file descriptor with watch on namespace directory + * @ref: epoll reference for inotify or timer descriptor */ -void pasta_netns_quit_handler(struct ctx *c, int inotify_fd) +void pasta_netns_quit_handler(struct ctx *c, union epoll_ref ref) { char buf[sizeof(struct inotify_event) + NAME_MAX + 1]; const struct inotify_event *in_ev = (struct inotify_event *)buf; - if (read(inotify_fd, buf, sizeof(buf)) < (ssize_t)sizeof(*in_ev)) + if (ref.nsdir_fd != -1) { + uint64_t expirations; + int fd; + + read(ref.fd, &expirations, sizeof(expirations)); + + if ((fd = openat(ref.nsdir_fd, c->netns_base, O_PATH)) < 0) + goto gone; + + close(fd); + return; + } + + if (read(ref.fd, buf, sizeof(buf)) < (ssize_t)sizeof(*in_ev)) return; if (strncmp(in_ev->name, c->netns_base, sizeof(c->netns_base))) return; +gone: info("Namespace %s is gone, exiting", c->netns_base); exit(EXIT_SUCCESS); } diff --git a/pasta.h b/pasta.h index 5d20063..c120e94 100644 --- a/pasta.h +++ b/pasta.h @@ -14,6 +14,6 @@ void pasta_start_ns(struct ctx *c, uid_t uid, gid_t gid, void pasta_ns_conf(struct ctx *c); void pasta_child_handler(int signal); int pasta_netns_quit_init(const struct ctx *c); -void pasta_netns_quit_handler(struct ctx *c, int inotify_fd); +void pasta_netns_quit_handler(struct ctx *c, union epoll_ref ref); #endif /* PASTA_H */ -- 2.39.2

2 2