- dev - passt

[PATCH 0/6] tcp: Improve error handling around socket pools
by David Gibson 27 Feb '24

27 Feb '24

To reduce latency, the TCP code maintains several pools of ready to connect TCP sockets. This patch series contains a number of improvements to improve error handling and reporting when we're unable to refill these pools, or unable to obtain a socket from these pools. David Gibson (6): treewide: Use sa_family_t for address family variables tcp: Don't stop refilling socket pool if we find a filled entry tcp: Stop on first error when refilling socket pools tcp, tcp_splice: Issue warnings if unable to refill socket pool tcp, tcp_splice: Helpers for getting sockets from the pools tcp: Don't store errnos in socket pool icmp.c | 6 ++--- icmp.h | 4 +-- inany.h | 3 ++- tcp.c | 73 ++++++++++++++++++++++++++++++++++++++++------------ tcp.h | 2 +- tcp_conn.h | 4 +-- tcp_splice.c | 71 +++++++++++++++++++++++++++++++------------------- udp.c | 2 +- udp.h | 2 +- util.c | 2 +- util.h | 2 +- 11 files changed, 114 insertions(+), 57 deletions(-) -- 2.43.2

2 14

passt: new version 2024_02_20.1e6f92b available
by Stefano Brivio 20 Feb '24

20 Feb '24

The new version with tag 2024_02_20.1e6f92b includes the following changes: 1e6f92b udp: Fix 16-bit overflow in udp_invert_portmap() 8954c4a udp: Assertion in udp_invert_portmap() can be calculated at compile time https://passt.top/passt/log/?qt=range&q=2024_02_19.ff22a78..2024_02_20.1e6f… 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/7036203/ permanent mirror: https://passt.top/builds/copr/0^20240219.gff22a78/ - 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-g1e6f92b-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_1e6f92b-1_all.deb -- Stefano

1 0

[PATCH 0/2] Fix buffer overrun in UDP setup (bug 80)
by David Gibson 20 Feb '24

20 Feb '24

Laurent Jacquot pointed out another bug in UDP forwarding that turned out to also be a nasty buffer overrun. Fix that, and make a minor cleanup alongside while we're at it. Link: https://bugs.passt.top/show_bug.cgi?id=80 David Gibson (2): udp: Assertion in udp_invert_portmap() can be calculated at compile time udp: Fix 16-bit overflow in udp_invert_portmap() udp.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) -- 2.43.2

2 3

passt: new version 2024_02_19.ff22a78 available
by Stefano Brivio 19 Feb '24

19 Feb '24

The new version with tag 2024_02_19.ff22a78 includes the following change: ff22a78 pasta: Don't try to watch namespaces in procfs with inotify, use timer instead https://passt.top/passt/log/?qt=range&q=2024_02_16.08344da..2024_02_19.ff22… 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, openSUSE): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/7036203/ permanent mirror: https://passt.top/builds/copr/0^20240219.gff22a78/ - 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-gff22a78-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_0ff22a78-1_all.deb -- Stefano

1 0

[PATCH v4] 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 container process, also spawned by Buildah, 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 namespace 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. There's, however, a remaining race possibility if the parent process is not the one associated to the network namespace we operate on: in that case, the parent might pass a procfs entry associated to a PID that was recycled by the time we parse it. This can't happen if the namespace PID matches the one of the parent, because we detach from the controlling terminal after parsing the namespace reference. To avoid this type of race, if desired, we could add the option for the parent to pass a PID file descriptor, that the parent obtained via pidfd_open(). This is beyond the scope of this change. 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> --- I'm pushing this in a bit as Podman's CI needs it soon, but posting anyway to ease later reviews. v4: Fix some further paragraphs in the commit message. Make man page change hopefully easier to follow. 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..e97ef73 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. +Do not exit once the target namespace reference is removed. + +Without this option, \fBpasta\fR will terminate if the target network namespace +is bound to the filesystem, and the given path is deleted, or if the target +network namespace is represented by a procfs entry, and that entry is deleted, +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

1 0

[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