- dev - passt

[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

[PATCH] 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). 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> --- conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/conf.c b/conf.c index 5e15b66..2341007 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 external routable interface for IPv4"); 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 external routable interface for IPv6"); return 0; } -- 2.39.2

2 2

[PATCH] conf, passt.1: Exit if we can't bind a forwarded port, except for -[tu] all
by Stefano Brivio 15 Feb '24

15 Feb '24

...or similar, that is, if only excluded ranges are given (implying we'll forward any other available port). In that case, we'll usually forward large sets of ports, and it might be inconvenient for the user to skip excluding single ports that are already taken. The existing behaviour, that is, exiting only if we fail to bind all the ports for one given forwarding option, turns out to be problematic for several aspects raised by Paul: - Podman merges ranges anyway, so we might fail to bind all the ports from a specific range given by the user, but we'll not fail anyway because Podman merges it with another one where we succeed to bind at least one port. At the same time, there should be no semantic difference between multiple ranges given by a single option and multiple ranges given as multiple options: it's unexpected and not documented - the user might actually rely on a given port to be forwarded to a given container or a virtual machine, and if connections are forwarded to an unrelated process, this might raise security concerns - given that we can try and fail to bind multiple ports before exiting (in case we can't bind any), we don't have a specific error code we can return to the user, so we don't give the user helpful indication as to why we couldn't bind ports. Exit as soon as we fail to create or bind a socket for a given forwarded port, and report the actual error. Keep the current behaviour, however, in case the user wants to forward all the (available) ports for a given protocol, or all the ports with excluded ranges only. There, it's more reasonable that the user is expecting partial failures, and it's probably convenient that we continue with the ports we could forward. Update the manual page to reflect the new behaviour, and the old behaviour too in the cases where we keep it. Suggested-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> --- conf.c | 36 +++++++++++------------------------- passt.1 | 15 ++++++++++++--- 2 files changed, 23 insertions(+), 28 deletions(-) diff --git a/conf.c b/conf.c index 5e15b66..f68f5d3 100644 --- a/conf.c +++ b/conf.c @@ -119,6 +119,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, bool exclude_only = true, bound_one = false; uint8_t exclude[PORT_BITMAP_SIZE] = { 0 }; sa_family_t af = AF_UNSPEC; + unsigned i; int ret; if (!strcmp(optarg, "none")) { @@ -141,8 +142,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, } if (!strcmp(optarg, "all")) { - unsigned i; - if (fwd->mode) goto mode_conflict; @@ -171,7 +170,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, } if (!bound_one) - goto bind_fail; + goto bind_all_fail; return; } @@ -221,7 +220,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, p = spec; do { struct port_range xrange; - unsigned i; if (*p != '~') { /* Not an exclude range, parse later */ @@ -244,8 +242,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, } while ((p = next_chunk(p, ','))); if (exclude_only) { - unsigned i; - for (i = 0; i < PORT_EPHEMERAL_MIN; i++) { if (bitmap_isset(exclude, i)) continue; @@ -271,7 +267,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, } if (!bound_one) - goto bind_fail; + goto bind_all_fail; return; } @@ -280,7 +276,6 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, p = spec; do { struct port_range orig_range, mapped_range; - unsigned i; if (*p == '~') /* Exclude range, already parsed */ @@ -314,28 +309,16 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, fwd->delta[i] = mapped_range.first - orig_range.first; - if (optname == 't') { + ret = 0; + if (optname == 't') ret = tcp_sock_init(c, af, addr, ifname, i); - if (ret == -ENFILE || ret == -EMFILE) - goto enfile; - if (!ret) - bound_one = true; - } else if (optname == 'u') { + else if (optname == 'u') ret = udp_sock_init(c, 0, af, addr, ifname, i); - if (ret == -ENFILE || ret == -EMFILE) - goto enfile; - if (!ret) - bound_one = true; - } else { - /* No way to check in advance for -T and -U */ - bound_one = true; - } + if (ret) + goto bind_fail; } } while ((p = next_chunk(p, ','))); - if (!bound_one) - goto bind_fail; - return; enfile: die("Can't open enough sockets for port specifier: %s", optarg); @@ -344,6 +327,9 @@ bad: mode_conflict: die("Port forwarding mode '%s' conflicts with previous mode", optarg); bind_fail: + die("Failed to bind port %u (%s) for option '-%c %s', exiting", + i, strerror(-ret), optname, optarg); +bind_all_fail: die("Failed to bind any port for '-%c %s', exiting", optname, optarg); } diff --git a/passt.1 b/passt.1 index cc678ed..dc2d719 100644 --- a/passt.1 +++ b/passt.1 @@ -355,7 +355,8 @@ Don't forward any ports .TP .BR all Forward all unbound, non-ephemeral ports, as permitted by current capabilities. -For low (< 1024) ports, see \fBNOTES\fR. +For low (< 1024) ports, see \fBNOTES\fR. No failures are reported for +unavailable ports, unless no ports could be forwarded at all. .TP .BR ports @@ -364,7 +365,11 @@ optionally, with target ports after \fI:\fR, if they differ. Specific addresses can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports and ranges can be excluded by an additional specification prefixed by \fI~\fR. -Specifying excluded ranges only implies that all other ports are forwarded. + +Specifying excluded ranges only implies that all other ports are forwarded. In +this case, no failures are reported for unavailable ports, unless no ports could +be forwarded at all. + Examples: .RS .TP @@ -447,7 +452,11 @@ optionally, with target ports after \fI:\fR, if they differ. Specific addresses can be bound as well, separated by \fI/\fR, and also, since Linux 5.7, limited to specific interfaces, prefixed by \fI%\fR. Within given ranges, selected ports and ranges can be excluded by an additional specification prefixed by \fI~\fR. -Specifying excluded ranges only implies that all other ports are forwarded. + +Specifying excluded ranges only implies that all other ports are forwarded. In +this case, no failures are reported for unavailable ports, unless no ports could +be forwarded at all. + Examples: .RS .TP -- 2.39.2

3 2

pasta does not correctly handle bind errors with port ranges
by Paul Holzinger 14 Feb '24

14 Feb '24

Hi all, I found some issues with the pasta port binding logic, it does not correctly handle errors when trying to bind a port range. Let's first bind a port so we can force an error condition it: $ nc -l -p 8080 & $ pasta -t 8080 true Failed to bind any port for '-t 8080', exiting <-- fails as expected $ pasta -t 8081 -t 8080 true Failed to bind any port for '-t 8080', exiting <-- here it also fails correctly $ pasta -t 8080-8081 true <-- no error even though pasta could not bind 8080 Also besides this I find the error message less than ideal. It missing the errno from the bind syscall so important context gets lost (i.e. Address already in use vs Permission denied).

2 7

[PATCH 0/2] Fix some redundant and incorrect initialisation of UDP sockets
by David Gibson 14 Feb '24

14 Feb '24

This fixes bug 79 and makes another clean up in the same area. Link: https://bugs.passt.top/show_bug.cgi?id=79 David Gibson (2): udp: Don't prematurely (and incorrectly) set up automatic inbound forwards udp: udp_sock_init_ns() partially duplicats udp_port_rebind_outbound() udp.c | 90 +++++++++++++++++------------------------------------------ 1 file changed, 25 insertions(+), 65 deletions(-) -- 2.43.0

2 3

[PATCH] netlink: Use const rtnh pointer
by David Gibson 14 Feb '24

14 Feb '24

6c7623d07 ("netlink: Add support to fetch default gateway from multipath routes") inadvertently introduced a new cppcheck warning for a variable which could be a const pointer but isn't. This occurs with cppcheck-2.13.0-1.fc39.x86_64 in Fedora 39 at least. Fixes: 6c7623d07 ("netlink: Add support to fetch default gateway from multipath routes") Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netlink.c b/netlink.c index f0b04cb1..9e7cccb4 100644 --- a/netlink.c +++ b/netlink.c @@ -274,7 +274,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) if (rta->rta_type == RTA_OIF) { ifi = *(unsigned int *)RTA_DATA(rta); } else if (rta->rta_type == RTA_MULTIPATH) { - struct rtnexthop *rtnh; + const struct rtnexthop *rtnh; rtnh = (struct rtnexthop *)RTA_DATA(rta); ifi = rtnh->rtnh_ifindex; -- 2.43.0

2 2

[PATCH 00/24] Add vhost-user support to passt.
by Laurent Vivier 14 Feb '24

14 Feb '24

This series of patches adds vhost-user support to passt and then allows passt to connect to QEMU network backend using virtqueue rather than a socket. We have with iperf3 a 10x speed improvement: $ iperf3 -c localhost -p 10001 -t 60 -6 -u -b 50G socket: [ 5] 0.00-60.04 sec 30.5 GBytes 4.36 Gbits/sec 0.065 ms 9127377/10125415 (90%) receiver vhost-user: [ 5] 0.00-60.05 sec 292 GBytes 41.8 Gbits/sec 0.007 ms 259805/9832736 (2.6%) receiver $ iperf3 -c localhost -p 10001 -t 60 -4 -u -b 50G socket: [ 5] 0.00-60.04 sec 36.4 GBytes 5.21 Gbits/sec 0.048 ms 7535735/8728101 (86%) receiver vhost-user: [ 5] 0.00-60.05 sec 259 GBytes 37.0 Gbits/sec 0.003 ms 142594/8616705 (1.7%) receiver $ iperf3 -c localhost -p 10001 -t 60 -6 socket: [ 5] 0.00-60.00 sec 16.3 GBytes 2.33 Gbits/sec 0 sender [ 5] 0.00-60.06 sec 16.3 GBytes 2.32 Gbits/sec receiver vhost-user: [ ID] Interval Transfer Bitrate Retr [ 5] 0.00-60.00 sec 205 GBytes 29.3 Gbits/sec 0 sender [ 5] 0.00-60.04 sec 205 GBytes 29.3 Gbits/sec receiver $ iperf3 -c localhost -p 10001 -t 60 -4 socket: [ 5] 0.00-60.00 sec 16.1 GBytes 2.31 Gbits/sec 0 sender [ 5] 0.00-60.07 sec 16.1 GBytes 2.31 Gbits/sec receiver vhost-user: [ 5] 0.00-60.00 sec 201 GBytes 28.7 Gbits/sec 0 sender [ 5] 0.00-60.04 sec 201 GBytes 28.7 Gbits/sec receiver With QEMU, rather than using to connect: -netdev stream,id=s,server=off,addr.type=unix,addr.path=/tmp/passt_1.socket we will use: -chardev socket,id=chr0,path=/tmp/passt_1.socket -netdev vhost-user,id=netdev0,chardev=chr0 -device virtio-net,netdev=netdev0 -object memory-backend-memfd,id=memfd0,share=on,size=$RAMSIZE -numa node,memdev=memfd0 The memory backend is needed to share data between passt and QEMU. The series starts to introduce new functions to manage iovec and to do checksum on unaligned memory (we cannot align buffers provided by the guest with the same value we use with internal passt buffers): iov: add some functions to manage iovec pcap: add pcap_iov() checksum: align buffers checksum: add csum_iov() We introduce new files ip.c and ip.h to provide IP generic functions: util: move IP stuff from util.[ch] to ip.[ch] ip: move duplicate IPv4 checksum function to ip.h Then we extract from existing TCP and UDP function the internal passt buffer management to be able to use them with the guest provided buffers: tcp: extract buffer management from tcp_send_flag() tcp: extract buffer management from tcp_conn_tap_mss() tcp: rename functions that manage buffers tcp: move buffers management functions to their own file tap: make tap_update_mac() generic tap: export pool_flush()/tapX_handler()/packet_add() udp: move udpX_l2_buf_t and udpX_l2_mh_sock out of udp_update_hdrX() udp: rename udp_sock_handler() to udp_buf_sock_handler() packet: replace struct desc by struct iovec As vhost-user is a variant of passt mode, modify the code to compare to (!MODE_PASTA) rather than (MODE_PASST || MODE_VU) vhost-user: compare mode MODE_PASTA and not MODE_PASST We introduce virtio and vhost-user management functions: vhost-user: introduce virtio API vhost-user: introduce vhost-user API And then a first version of vhost-user that copies data from the passt buffers to guest memory, and vice-versa, as it's done with the socket algorithm: vhost-user: add vhost-user And finaly remove the buffers copy on TX and RX (TCP/UDP): vhost-user: use guest buffer directly in vu_handle_tx() tcp: vhost-user RX nocopy udp: vhost-user RX nocopy vhost-user: remove tap_send_frames_vu() Thanks, Laurent Laurent Vivier (24): 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] ip: move duplicate IPv4 checksum function to ip.h ip: introduce functions to compute the header part checksum for TCP/UDP tcp: extract buffer management from tcp_send_flag() tcp: extract buffer management from tcp_conn_tap_mss() tcp: rename functions that manage buffers tcp: move buffers management functions to their own file tap: make tap_update_mac() generic tap: export pool_flush()/tapX_handler()/packet_add() udp: move udpX_l2_buf_t and udpX_l2_mh_sock out of udp_update_hdrX() udp: rename udp_sock_handler() to udp_buf_sock_handler() packet: replace struct desc by struct iovec vhost-user: compare mode MODE_PASTA and not MODE_PASST vhost-user: introduce virtio API vhost-user: introduce vhost-user API vhost-user: add vhost-user vhost-user: use guest buffer directly in vu_handle_tx() tcp: vhost-user RX nocopy udp: vhost-user RX nocopy vhost-user: remove tap_send_frames_vu() Makefile | 7 +- checksum.c | 51 ++- checksum.h | 1 + conf.c | 33 +- dhcp.c | 1 + flow.c | 1 + icmp.c | 1 + iov.c | 78 ++++ iov.h | 46 +++ ip.c | 72 ++++ ip.h | 124 ++++++ isolation.c | 10 +- ndp.c | 1 + packet.c | 81 ++-- packet.h | 16 +- passt.c | 18 +- passt.h | 10 + pcap.c | 32 ++ pcap.h | 1 + port_fwd.c | 1 + qrap.c | 1 + tap.c | 226 +++++++---- tap.h | 13 +- tcp.c | 789 ++++++------------------------------ tcp.h | 2 +- tcp_buf.c | 569 ++++++++++++++++++++++++++ tcp_buf.h | 17 + tcp_internal.h | 78 ++++ tcp_splice.c | 1 + tcp_vu.c | 447 +++++++++++++++++++++ tcp_vu.h | 10 + udp.c | 171 ++++---- udp.h | 4 +- udp_internal.h | 21 + udp_vu.c | 215 ++++++++++ udp_vu.h | 8 + util.c | 55 --- util.h | 83 +--- vhost_user.c | 1050 ++++++++++++++++++++++++++++++++++++++++++++++++ vhost_user.h | 137 +++++++ virtio.c | 484 ++++++++++++++++++++++ virtio.h | 121 ++++++ 42 files changed, 4041 insertions(+), 1046 deletions(-) create mode 100644 iov.c create mode 100644 iov.h create mode 100644 ip.c create mode 100644 ip.h create mode 100644 tcp_buf.c create mode 100644 tcp_buf.h create mode 100644 tcp_internal.h create mode 100644 tcp_vu.c create mode 100644 tcp_vu.h create mode 100644 udp_internal.h create mode 100644 udp_vu.c create mode 100644 udp_vu.h create mode 100644 vhost_user.c create mode 100644 vhost_user.h create mode 100644 virtio.c create mode 100644 virtio.h -- 2.42.0

3 82

[PATCH v3] log: setlogmask(0) can actually result in a system call, don't use it
by Stefano Brivio 12 Feb '24

12 Feb '24

Before commit 32d07f5e59f2 ("passt, pasta: Completely avoid dynamic memory allocation"), we didn't store the current log mask in a variable, and we fetched it using setlogmask(0) wherever needed. But after that commit, we can use our log_mask copy instead. And we should: with recent glibc versions, setlogmask(0) actually results in a system call, which causes a substantial overhead with high transfer rates: we use setlogmask(0) even to decide we don't want to print debug messages. Now that we rely on log_mask in early stages, before setlogmask() is called, we need to initialise that variable to the special LOG_EMERG mask value right away: define LOG_EARLY to make this clearer, and, while at it, group conditions in vlogmsg() into something more terse. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v3: Include changes to passt.c I accidentally didn't commit in v2 v2: Initialise log_mask so that prints before the first __setlogmask() call still work. log.c | 18 ++++++++++-------- passt.c | 7 ++----- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/log.c b/log.c index 4a70e29..eafaca2 100644 --- a/log.c +++ b/log.c @@ -30,9 +30,12 @@ #include "util.h" #include "passt.h" +/* LOG_EARLY means we don't know yet: log everything. LOG_EMERG is unused */ +#define LOG_EARLY LOG_MASK(LOG_EMERG) + static int log_sock = -1; /* Optional socket to system logger */ static char log_ident[BUFSIZ]; /* Identifier string for openlog() */ -static int log_mask; /* Current log priority mask */ +static int log_mask = LOG_EARLY; /* Current log priority mask */ static int log_opt; /* Options for openlog() */ static int log_file = -1; /* Optional log file descriptor */ @@ -45,34 +48,33 @@ static time_t log_start; /* Start timestamp */ int log_trace; /* --trace mode enabled */ int log_to_stdout; /* Print to stdout instead of stderr */ -#define BEFORE_DAEMON (setlogmask(0) == LOG_MASK(LOG_EMERG)) - void vlogmsg(int pri, const char *format, va_list ap) { + bool debug_print = (log_mask & LOG_MASK(LOG_DEBUG)) && log_file == -1; + bool early_print = LOG_PRI(log_mask) == LOG_EARLY; FILE *out = log_to_stdout ? stdout : stderr; struct timespec tp; - if (setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) { + if (debug_print) { clock_gettime(CLOCK_REALTIME, &tp); fprintf(out, "%lli.%04lli: ", (long long int)tp.tv_sec - log_start, (long long int)tp.tv_nsec / (100L * 1000)); } - if ((LOG_MASK(LOG_PRI(pri)) & log_mask) || BEFORE_DAEMON) { + if ((log_mask & LOG_MASK(LOG_PRI(pri))) || early_print) { va_list ap2; va_copy(ap2, ap); /* Don't clobber ap, we need it again */ if (log_file != -1) logfile_write(pri, format, ap2); - else if (!(setlogmask(0) & LOG_MASK(LOG_DEBUG))) + else if (!(log_mask & LOG_MASK(LOG_DEBUG))) passt_vsyslog(pri, format, ap2); va_end(ap2); } - if ((setlogmask(0) & LOG_MASK(LOG_DEBUG) && log_file == -1) || - (BEFORE_DAEMON && !(log_opt & LOG_PERROR))) { + if (debug_print || (early_print && !(log_opt & LOG_PERROR))) { (void)vfprintf(out, format, ap); if (format[strlen(format)] != '\n') fprintf(out, "\n"); diff --git a/passt.c b/passt.c index 44d3a0b..aaa8e58 100644 --- a/passt.c +++ b/passt.c @@ -251,9 +251,6 @@ int main(int argc, char **argv) __openlog(log_name, 0, LOG_DAEMON); - /* Meaning we don't know yet: log everything. LOG_EMERG is unused */ - __setlogmask(LOG_MASK(LOG_EMERG)); - c.epollfd = epoll_create1(EPOLL_CLOEXEC); if (c.epollfd == -1) { perror("epoll_create1"); @@ -322,8 +319,8 @@ int main(int argc, char **argv) if (isolate_prefork(&c)) die("Failed to sandbox process, exiting"); - /* Once the log mask is not LOG_EMERG, we will no longer - * log to stderr if there was a log file specified. + /* Once the log mask is not LOG_EARLY, we will no longer log to stderr + * if there was a log file specified. */ if (c.debug) __setlogmask(LOG_UPTO(LOG_DEBUG)); -- 2.39.2

2 1