March 2025 - dev

[PATCH 0/9] Improve handling of MTU limits
by David Gibson 12 Mar '25

12 Mar '25

After considerable lead up, this corrects the handling of the --mtu option so that it will respect limits imposed by the backend tap layers, correctly accounting for L2 headers. This incorporates the earlier mode setting patches, on which the rest depend. There's been no change in those patches, there just included here for self-containedness. David Gibson (9): conf: Use the same optstring for passt and pasta modes conf: Move mode detection into helper function conf: Detect vhost-user mode earlier packet: Give explicit name to maximum packet size packet: Remove redundant TAP_BUF_BYTES define tap: Use explicit defines for maximum length of L2 frame Simplify sizing of pkt_buf pcap: Correctly set snaplen based on tap backend type conf: Limit maximum MTU based on backend frame size conf.c | 98 +++++++++++++++++++++++++++++++++++++++++--------------- conf.h | 1 + packet.c | 4 +-- packet.h | 3 ++ passt.c | 16 ++------- passt.h | 7 ++-- pcap.c | 46 +++++++++++++------------- tap.c | 39 +++++++++++++++++++--- tap.h | 26 +++++++++++++++ util.h | 3 -- 10 files changed, 168 insertions(+), 75 deletions(-) -- 2.48.1

2 13

[PATCH] passt-repair: Add directory watch
by Stefano Brivio 12 Mar '25

12 Mar '25

It might not be feasible for users to start passt-repair after passt is started, on a migration target, but before the migration process starts. For instance, with libvirt, the guest domain (and, hence, passt) is started on the target as part of the migration process. At least for the moment being, there's no hook a libvirt user (including KubeVirt) can use to start passt-repair before the migration starts. Add a directory watch using inotify: if PATH is a directory, instead of connecting to it, we'll watch for a .repair socket file to appear in it, and then attempt to connect to that socket. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- contrib/selinux/passt-repair.te | 16 +++---- passt-repair.1 | 6 ++- passt-repair.c | 82 ++++++++++++++++++++++++++++++--- 3 files changed, 88 insertions(+), 16 deletions(-) diff --git a/contrib/selinux/passt-repair.te b/contrib/selinux/passt-repair.te index f171be6..7157dfb 100644 --- a/contrib/selinux/passt-repair.te +++ b/contrib/selinux/passt-repair.te @@ -61,11 +61,11 @@ allow passt_repair_t unconfined_t:unix_stream_socket { connectto read write }; allow passt_repair_t passt_t:unix_stream_socket { connectto read write }; allow passt_repair_t user_tmp_t:unix_stream_socket { connectto read write }; -allow passt_repair_t user_tmp_t:dir search; +allow passt_repair_t user_tmp_t:dir { getattr read search watch }; -allow passt_repair_t unconfined_t:sock_file { read write }; -allow passt_repair_t passt_t:sock_file { read write }; -allow passt_repair_t user_tmp_t:sock_file { read write }; +allow passt_repair_t unconfined_t:sock_file { getattr read write }; +allow passt_repair_t passt_t:sock_file { getattr read write }; +allow passt_repair_t user_tmp_t:sock_file { getattr read write }; allow passt_repair_t unconfined_t:tcp_socket { read setopt write }; allow passt_repair_t passt_t:tcp_socket { read setopt write }; @@ -80,8 +80,8 @@ allow passt_repair_t passt_t:tcp_socket { read setopt write }; allow passt_repair_t qemu_var_run_t:unix_stream_socket { connectto read write }; allow passt_repair_t virt_var_run_t:unix_stream_socket { connectto read write }; -allow passt_repair_t qemu_var_run_t:dir search; -allow passt_repair_t virt_var_run_t:dir search; +allow passt_repair_t qemu_var_run_t:dir { getattr read search watch }; +allow passt_repair_t virt_var_run_t:dir { getattr read search watch }; -allow passt_repair_t qemu_var_run_t:sock_file { read write }; -allow passt_repair_t virt_var_run_t:sock_file { read write }; +allow passt_repair_t qemu_var_run_t:sock_file { getattr read write }; +allow passt_repair_t virt_var_run_t:sock_file { getattr read write }; diff --git a/passt-repair.1 b/passt-repair.1 index 7c1b140..e65aadd 100644 --- a/passt-repair.1 +++ b/passt-repair.1 @@ -16,13 +16,17 @@ .B passt-repair is a privileged helper setting and clearing repair mode on TCP sockets on behalf of \fBpasst\fR(1), as instructed via single-byte commands over a UNIX domain -socket, specified by \fIPATH\fR. +socket. It can be used to migrate TCP connections between guests without granting additional capabilities to \fBpasst\fR(1) itself: to migrate TCP connections, \fBpasst\fR(1) leverages repair mode, which needs the \fBCAP_NET_ADMIN\fR capability (see \fBcapabilities\fR(7)) to be set or cleared. +If \fIPATH\fR represents a UNIX domain socket, \fBpasst-repair\fR(1) attempts to +connect to it. If it is a directory, \fBpasst-repair\fR(1) waits until a file +ending with \fI.repair\fR appears in it, and then attempts to connect to it. + .SH PROTOCOL \fBpasst-repair\fR(1) connects to \fBpasst\fR(1) using the socket specified via diff --git a/passt-repair.c b/passt-repair.c index e0c366e..8bb3f00 100644 --- a/passt-repair.c +++ b/passt-repair.c @@ -16,11 +16,14 @@ * off. Reply by echoing the command. Exit on EOF. */ +#include <sys/inotify.h> #include <sys/prctl.h> #include <sys/types.h> #include <sys/socket.h> +#include <sys/stat.h> #include <sys/un.h> #include <errno.h> +#include <stdbool.h> #include <stddef.h> #include <stdio.h> #include <stdlib.h> @@ -39,6 +42,8 @@ #include "seccomp_repair.h" #define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in UAPI */ +#define REPAIR_EXT ".repair" +#define REPAIR_EXT_LEN strlen(REPAIR_EXT) /** * main() - Entry point and whole program with loop @@ -51,6 +56,9 @@ * #syscalls:repair socket s390x:socketcall i686:socketcall * #syscalls:repair recvfrom recvmsg arm:recv ppc64le:recv * #syscalls:repair sendto sendmsg arm:send ppc64le:send + * #syscalls:repair stat|statx stat64|statx statx + * #syscalls:repair fstat|fstat64 newfstatat|fstatat64 + * #syscalls:repair inotify_init1 inotify_add_watch */ int main(int argc, char **argv) { @@ -58,12 +66,14 @@ int main(int argc, char **argv) __attribute__ ((aligned(__alignof__(struct cmsghdr)))); struct sockaddr_un a = { AF_UNIX, "" }; int fds[SCM_MAX_FD], s, ret, i, n = 0; + bool inotify_dir = false; struct sock_fprog prog; int8_t cmd = INT8_MAX; struct cmsghdr *cmsg; struct msghdr msg; struct iovec iov; size_t cmsg_len; + struct stat sb; int op; prctl(PR_SET_DUMPABLE, 0); @@ -90,19 +100,77 @@ int main(int argc, char **argv) _exit(2); } - ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]); + if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { + fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno); + _exit(1); + } + + if ((stat(argv[1], &sb))) { + fprintf(stderr, "Can't stat() %s: %i\n", argv[1], errno); + _exit(1); + } + + if ((sb.st_mode & S_IFMT) == S_IFDIR) { + char buf[sizeof(struct inotify_event) + NAME_MAX + 1]; + const struct inotify_event *ev; + char path[PATH_MAX + 1]; + ssize_t n; + int fd; + + ev = (struct inotify_event *)buf; + + if ((fd = inotify_init1(IN_CLOEXEC)) < 0) { + fprintf(stderr, "inotify_init1: %i\n", errno); + _exit(1); + } + + if (inotify_add_watch(fd, argv[1], IN_CREATE) < 0) { + fprintf(stderr, "inotify_add_watch: %i\n", errno); + _exit(1); + } + + do { + n = read(fd, buf, sizeof(buf)); + if (n < 0) { + fprintf(stderr, "inotify read: %i", errno); + _exit(1); + } + + if (n < (ssize_t)sizeof(*ev)) { + fprintf(stderr, "Short inotify read: %zi", n); + _exit(1); + } + } while (ev->len < REPAIR_EXT_LEN || + memcmp(ev->name + strlen(ev->name) - REPAIR_EXT_LEN, + REPAIR_EXT, REPAIR_EXT_LEN)); + + snprintf(path, sizeof(path), "%s/%s", argv[1], ev->name); + if ((stat(path, &sb))) { + fprintf(stderr, "Can't stat() %s: %i\n", path, errno); + _exit(1); + } + + ret = snprintf(a.sun_path, sizeof(a.sun_path), path); + inotify_dir = true; + } else { + ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]); + } + if (ret <= 0 || ret >= (int)sizeof(a.sun_path)) { - fprintf(stderr, "Invalid socket path: %s\n", argv[1]); + fprintf(stderr, "Invalid socket path"); _exit(2); } - if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { - fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno); - _exit(1); + if ((sb.st_mode & S_IFMT) != S_IFSOCK) { + fprintf(stderr, "%s is not a socket\n", a.sun_path); + _exit(2); } - if (connect(s, (struct sockaddr *)&a, sizeof(a))) { - fprintf(stderr, "Failed to connect to %s: %s\n", argv[1], + while (connect(s, (struct sockaddr *)&a, sizeof(a))) { + if (inotify_dir && errno == ECONNREFUSED) + continue; + + fprintf(stderr, "Failed to connect to %s: %s\n", a.sun_path, strerror(errno)); _exit(1); } -- 2.43.0

2 3

[PATCH] flow, repair: Wait for a short while for passt-repair to connect
by Stefano Brivio 07 Mar '25

07 Mar '25

...and time out after that. This will be needed because of an upcoming change to passt-repair enabling it to start before passt is started, on both source and target, by means of an inotify watch. Once the inotify watch triggers, passt-repair will connect right away, but we have no guarantees that the connection completes before we start the migration process, so wait for it (for a reasonable amount of time). Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- I'm still working on the passt-repair change mentioned above. It's turning out to be a bit trickier than expected because to support typical libvirt migration usage, on the target, we also need to be able to wait for creation of a directory and, separately, for a socket file being added inside it. However, this part looks solid and it should do what we need. I tested it by starting passt-repair after sleep(1) in migrate/* tests, changing this timeout to a minute, on either source, target, or both. flow.c | 20 ++++++++++++++++++++ repair.c | 31 +++++++++++++++++++++++++++++++ repair.h | 1 + 3 files changed, 52 insertions(+) diff --git a/flow.c b/flow.c index 749c498..5e64b79 100644 --- a/flow.c +++ b/flow.c @@ -911,6 +911,21 @@ static int flow_migrate_source_rollback(struct ctx *c, unsigned bound, int ret) return ret; } +/** + * flow_migrate_need_repair() - Do we need to set repair mode for any flow? + * + * Return: true if repair mode is needed, false otherwise + */ +static bool flow_migrate_need_repair(void) +{ + union flow *flow; + + foreach_established_tcp_flow(flow) + return true; + + return false; +} + /** * flow_migrate_repair_all() - Turn repair mode on or off for all flows * @c: Execution context @@ -966,6 +981,9 @@ int flow_migrate_source_pre(struct ctx *c, const struct migrate_stage *stage, (void)stage; (void)fd; + if (flow_migrate_need_repair()) + repair_wait(c); + if ((rc = flow_migrate_repair_all(c, true))) return -rc; @@ -1083,6 +1101,8 @@ int flow_migrate_target(struct ctx *c, const struct migrate_stage *stage, if (!count) return 0; + repair_wait(c); + if ((rc = flow_migrate_repair_all(c, true))) return -rc; diff --git a/repair.c b/repair.c index 3ee089f..ce5a4a4 100644 --- a/repair.c +++ b/repair.c @@ -27,6 +27,11 @@ #define SCM_MAX_FD 253 /* From Linux kernel (include/net/scm.h), not in UAPI */ +/* Wait for a while for TCP_REPAIR helper to connect if it's not there yet */ +#define REPAIR_ACCEPT_TIMEOUT_MS 100 +#define REPAIR_ACCEPT_TIMEOUT_US (REPAIR_ACCEPT_TIMEOUT_MS * 1000) +static_assert(REPAIR_ACCEPT_TIMEOUT_US < 1000 * 1000); + /* Pending file descriptors for next repair_flush() call, or command change */ static int repair_fds[SCM_MAX_FD]; @@ -138,6 +143,32 @@ void repair_handler(struct ctx *c, uint32_t events) repair_close(c); } +/** + * repair_wait() - Wait (with timeout) for TCP_REPAIR helper to connect + * @c: Execution context + */ +void repair_wait(struct ctx *c) +{ + struct timeval tv = { .tv_sec = 0, + .tv_usec = (long)(REPAIR_ACCEPT_TIMEOUT_US) }; + + if (c->fd_repair >= 0 || c->fd_repair_listen == -1) + return; + + if (setsockopt(c->fd_repair_listen, SOL_SOCKET, SO_RCVTIMEO, + &tv, sizeof(tv))) { + err_perror("Set timeout on TCP_REPAIR listening socket"); + return; + } + + repair_listen_handler(c, EPOLLIN); + + tv.tv_usec = 0; + if (setsockopt(c->fd_repair_listen, SOL_SOCKET, SO_RCVTIMEO, + &tv, sizeof(tv))) + err_perror("Clear timeout on TCP_REPAIR listening socket"); +} + /** * repair_flush() - Flush current set of sockets to helper, with current command * @c: Execution context diff --git a/repair.h b/repair.h index de279d6..1d37922 100644 --- a/repair.h +++ b/repair.h @@ -10,6 +10,7 @@ void repair_sock_init(const struct ctx *c); void repair_listen_handler(struct ctx *c, uint32_t events); void repair_handler(struct ctx *c, uint32_t events); void repair_close(struct ctx *c); +void repair_wait(struct ctx *c); int repair_flush(struct ctx *c); int repair_set(struct ctx *c, int s, int cmd); -- 2.43.0

2 3

[PATCH v4 0/1] Improve validation of --mtu option
by David Gibson 07 Mar '25

07 Mar '25

Here are some of the more straightforward parts of my patches making various MTU related fixes. These ones improve how we validate the --mtu option. Changes since v3: * None, just verified rebase and reposted. Changes since v2: * Don't disable a protocol because the MTU is too small, just warn. It's only an advertised MTU, not an enforced MTU David Gibson (1): conf: Be more precise about minimum MTUs conf.c | 18 +++++++++++++++--- ip.h | 7 +++++++ util.h | 6 ------ 3 files changed, 22 insertions(+), 9 deletions(-) -- 2.48.1

2 2

[PATCH 0/6] Deal with a bunch of staticFunction warnings from cppcheck 2.17
by David Gibson 07 Mar '25

07 Mar '25

The new cppcheck version in Fedora 41 is a bunch more aggressive with staticFunction warnings, about exposed functions that aren't used outside their .c file. Deal with these warnings. David Gibson (6): treewide: Mark assorted functions static log: Don't export passt_vsyslog() checksum: Don't export various functions tcp: Don't export tcp_update_csum() vhost_user: Don't export several functions cppcheck: Add suppressions for "logically" exported functions checksum.c | 34 ++++++++++++++++----------------- checksum.h | 3 --- iov.c | 1 + log.c | 51 +++++++++++++++++++++++++------------------------- log.h | 1 - netlink.c | 2 +- passt.c | 2 +- tcp.c | 9 +++++---- tcp_internal.h | 2 -- vhost_user.c | 2 +- vhost_user.h | 1 - virtio.c | 9 +++++---- virtio.h | 4 ---- 13 files changed, 57 insertions(+), 64 deletions(-) -- 2.48.1

2 7

[PATCH v11 0/4] Reconstruct incoming ICMP headers for failed UDP connect and forward back
by Jon Maloy 07 Mar '25

07 Mar '25

v2: - Added patch breaking out udp header creation from function tap_udp4_send(). - Updated the ICMP creation by using the new function. - Added logics to find correct flow, depending on origin. - All done after feedback from David Gibson. v3: - More changes after feedback from David Gibson. v4: - Even more changes after feedback from D. Gibson v5: - Added corresponding patches for IPv6 v6: - Fixed some small nits after comments from D. Gibson. v7: - Added handling of all rejected ICMP messages - Returning correct user data amount if IPv6 as per RFC 4884. v8: - Added MTU to ICMPv4 ICMP_FRAG_NEEDED messages. - Added ASSERT() validation to message creation functions. v9: - Using real source address of ICMP to complement destination address for originial UDP message when needed. v10: -Fixed wrong l4len value given to tap_push_ip4h() in function udp_send_conn_fail_icmp4(), patch #2. v11: -Eliminated coverity warnings. -Fixed a couple of tab issues (My emacs settings weren't able to handle my combination of whitespace and tab settings correctly.) Jon Maloy (4): tap: break out building of udp header from tap_udp4_send function udp: create and send ICMPv4 to local peer when applicable tap: break out building of udp header from tap_udp6_send function udp: create and send ICMPv6 to local peer when applicable tap.c | 86 +++++++++++++++++++++++--------- tap.h | 15 ++++++ udp.c | 132 ++++++++++++++++++++++++++++++++++++++++++++----- udp_internal.h | 2 +- udp_vu.c | 4 +- 5 files changed, 200 insertions(+), 39 deletions(-) -- 2.48.1

2 5

Apparmor (and other) Issues
by Prafulla Giri 06 Mar '25

06 Mar '25

Esteemed maintainer, First and foremost, thank you very much for your hard work: passt is awesome and allows one to run more useful user-space VM-s. I have encountered 2 particular issues with the usage of passt with Debian, and wanted to bring them to your attention as I think you are probably the best person to deal with this. I do plan on sending a report to the Debian team afterwards. For reference, I tested these on Debian Testing Daily Image dated 28 January 2025, with updates, and the version of passt available with it is passt 0.0~git20250121.4f2c8e7-1 - Passt's default Apparmor config needs to allow writes to $XDG_RUNTIME_DIR (which is at /run/user/$UID). Currently it doesn't. Virt-manager, at least, tries to create the necessary sockets in the directory but apparmor prevents that from happening (and the error message Virt-Manager gives isn't helpful either: the first time around I falsely believed it was a segfault or similar issue). I managed to get passt working past this flaw (pun intended) by manually disabling apparmor for the binary. Passt works just fine in Fedora 41 as it doesn't use Apparmor but uses SELinux, and thus the configs don't affect it. - This second issue is perhaps a bit more Debian-specific, but I am going to mention it so that you might drop some hints for the Debian maintainers to debug this: Once Apparmor is disabled and a VM is configured to work with passt, DNS resolution doesn't work in the VM (IP Addresses work just fine) i.e. ping fsf.org doesn't work but `ping 209.51.188.174` does. The hypervisor details follow: $ virsh version # on Debian Testing a.k.a. 'Trixie' Compiled against library: libvirt 11.0.0 Using library: libvirt 11.0.0 Using API: QEMU 11.0.0Running hypervisor: QEMU 9.2.0 This, again, isn't an issue with Fedora 41, where everything just works. The hypervisor details for Fedora 41 are: $ virsh version # on Fedora 41 Compiled against library: libvirt 10.6.0 Using library: libvirt 10.6.0 Using API: QEMU 10.6.0 Running hypervisor: QEMU 9.1.2 Again, I will be making a report to the Debian maintainers, should they wish to chime in regarding Apparmor configs or the DNS resolution issue. Thank you once again for this awesome tool.

4 32

[PATCH v2 0/3] Tweaks to mode handling
by David Gibson 06 Mar '25

06 Mar '25

While working on something else, I got mildly irritated by the fact that there's no a consistent place after which we can rely on the mode. We detect passt vs. pasta early, but only split out vhost-user later, which forces some other things later still. v2: * No changes, just rebased and reposted David Gibson (3): conf: Use the same optstring for passt and pasta modes conf: Move mode detection into helper function conf: Detect vhost-user mode earlier conf.c | 87 ++++++++++++++++++++++++++++++++++++++++++--------------- conf.h | 1 + passt.c | 14 ++-------- 3 files changed, 68 insertions(+), 34 deletions(-) -- 2.48.1

1 3

[PATCH v2 0/3] Send RST for guest packets with no flow
by David Gibson 05 Mar '25

05 Mar '25

As we discussed on email, this adds support for sending an RST in response to packets from the guest which don't match an existing flow and are neither SYN (requesting a new connection) nor themselves RST. This is a sligjhtly larger patch than I'd like, but I can't really see a way to simplify it without making fairly extensive reworks to share more code with paths for RST where there is a known connection. That would end up being more churn. This doesn't (IMO) correctly handle IPv6 flow labels. Fixing that raises several additional issues regarding flow labels, so I've decided to defer that for now. v2: * Assorted cosmetic fixups * Use correct IPv6 flow label for packets * This required two preliminary patches * tcp_rst_no_conn() is now static David Gibson (3): ip: Helpers to access IPv6 flow label tap: Consider IPv6 flow label when building packet sequences tcp: Send RST in response to guest packets that match no connection ip.h | 24 ++++++++++++++++++ tap.c | 25 ++++++++++--------- tap.h | 6 +++++ tcp.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--- tcp.h | 2 +- 5 files changed, 118 insertions(+), 17 deletions(-) -- 2.48.1

2 4

[PATCH v9 0/4] Reconstruct incoming ICMP headers for failed UDP connect and forward back
by Jon Maloy 05 Mar '25

05 Mar '25

v2: - Added patch breaking out udp header creation from function tap_udp4_send(). - Updated the ICMP creation by using the new function. - Added logics to find correct flow, depending on origin. - All done after feedback from David Gibson. v3: - More changes after feedback from David Gibson. v4: - Even more changes after feedback from D. Gibson v5: - Added corresponding patches for IPv6 v6: - Fixed some small nits after comments from D. Gibson. v7: - Added handling of all rejected ICMP messages - Returning correct user data amount if IPv6 as per RFC 4884. v8: - Added MTU to ICMPv4 ICMP_FRAG_NEEDED messages. - Added ASSERT() validation to message creation functions. v9: - Using real source address of ICMP to complement destination address for originial UDP message when needed. Jon Maloy (4): tap: break out building of udp header from tap_udp4_send function udp: create and send ICMPv4 to local peer when applicable tap: break out building of udp header from tap_udp6_send function udp: create and send ICMPv6 to local peer when applicable tap.c | 86 +++++++++++++++++++++++--------- tap.h | 15 ++++++ udp.c | 132 ++++++++++++++++++++++++++++++++++++++++++++----- udp_internal.h | 2 +- udp_vu.c | 4 +- 5 files changed, 200 insertions(+), 39 deletions(-) -- 2.48.1

3 8