February 2025 - dev

[PATCH] passt-repair: Dodge "structurally unreachable code" warning from Coverity
by Stefano Brivio 07 Feb '25

07 Feb '25

While main() conventionally returns int, and we need a return at the end of the function to avoid compiler warnings, turning that return into _exit() to avoid exit handlers triggers a Coverity warning. It's unreachable code anyway, so switch that single occurence back to a plain return. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt-repair.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/passt-repair.c b/passt-repair.c index 5ad5c9c..322066a 100644 --- a/passt-repair.c +++ b/passt-repair.c @@ -173,5 +173,5 @@ loop: goto loop; - _exit(0); + return 0; } -- 2.43.0

2 1

[PATCH v11 0/9] Draft state migration
by Stefano Brivio 07 Feb '25

07 Feb '25

v11: - drop SO_REUSEADDR setting in target: sockets closed in repair mode don't actually go to TIME_WAIT it seems - bind sockets to address too, and to the right flowside depending on whether they are for inbound or outbound connections - debug messages in flow_migrate_source() and flow_migrate_target() - error messages in tcp_flow_repair_queues() - set the same socket options (TCP_NODELAY, TCP_MAXSEG) just like for "regular" sockets, in tcp_flow_repair_socket() - connect() to HOSTFLOW(conn) flowside, not to TGTSIDE, in tcp_flow_repair_connect() - in test setup: set up guest_2 context in subshell - add migrate/bidirectional test David Gibson (4): fixup: Fix errors in modes that don't support migration migrate: Migrate guest observed addresses migrate: Hack for late migration fixups fixup: Reset SO_PEEK_OFF value after incoming migration Stefano Brivio (5): migrate: Skeleton of live migration logic Add interfaces and configuration bits for passt-repair vhost_user: Make source quit after reporting migration state migrate: Migrate TCP flows test: Add migration tests Makefile | 14 +- conf.c | 44 +++- epoll_type.h | 6 +- flow.c | 212 ++++++++++++++++ flow.h | 6 + migrate.c | 291 ++++++++++++++++++++++ migrate.h | 57 +++++ passt.1 | 11 + passt.c | 15 +- passt.h | 15 ++ repair.c | 192 +++++++++++++++ repair.h | 16 ++ tap.c | 65 +---- tcp.c | 483 +++++++++++++++++++++++++++++++++++++ tcp_conn.h | 60 +++++ test/lib/layout | 55 ++++- test/lib/setup | 128 ++++++++++ test/lib/test | 3 + test/migrate/basic | 54 +++++ test/migrate/bidirectional | 59 +++++ test/run | 7 + util.c | 62 +++++ util.h | 30 +++ vhost_user.c | 70 ++---- virtio.h | 4 - vu_common.c | 49 +--- vu_common.h | 2 +- 27 files changed, 1830 insertions(+), 180 deletions(-) create mode 100644 migrate.c create mode 100644 migrate.h create mode 100644 repair.c create mode 100644 repair.h create mode 100644 test/migrate/basic create mode 100644 test/migrate/bidirectional -- 2.43.0

1 9

[PATCH] passt-repair: Fix calculation of payload length from cmsg_len
by Stefano Brivio 07 Feb '25

07 Feb '25

There's no inverse function for CMSG_LEN(), so we need to loop over SCM_MAX_FD (253) possible input values. The previous calculation is clearly wrong, as not every int takes CMSG_LEN(sizeof(int)) in cmsg data. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt-repair.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/passt-repair.c b/passt-repair.c index d137a18..5ad5c9c 100644 --- a/passt-repair.c +++ b/passt-repair.c @@ -57,7 +57,7 @@ int main(int argc, char **argv) char buf[CMSG_SPACE(sizeof(int) * SCM_MAX_FD)] __attribute__ ((aligned(__alignof__(struct cmsghdr)))); struct sockaddr_un a = { AF_UNIX, "" }; - int fds[SCM_MAX_FD], s, ret, i, n; + int fds[SCM_MAX_FD], s, ret, i, n = 0; struct sock_fprog prog; int8_t cmd = INT8_MAX; struct cmsghdr *cmsg; @@ -127,7 +127,21 @@ loop: _exit(1); } - n = cmsg->cmsg_len / CMSG_LEN(sizeof(int)); + /* No inverse formula for CMSG_LEN(x), and building one with CMSG_LEN(0) + * works but there's no guarantee it does. Search the whole domain. + */ + for (i = 1; i < SCM_MAX_FD; i++) { + if (CMSG_LEN(sizeof(int) * i) == cmsg->cmsg_len) { + n = i; + break; + } + } + if (!n) { + fprintf(stderr, "Invalid ancillary data length %zu from peer\n", + cmsg->cmsg_len); + _exit(1); + } + memcpy(fds, CMSG_DATA(cmsg), sizeof(int) * n); if (cmd != TCP_REPAIR_ON && cmd != TCP_REPAIR_OFF && -- 2.43.0

1 0

[PATCH] conf, passt.1: Un-deprecate --host-lo-to-ns-lo
by Stefano Brivio 06 Feb '25

06 Feb '25

It was established behaviour, and it's now the third report about it: users ask how to achieve the same functionality, and we don't have a better answer yet. The idea behind declaring it deprecated to start with, I guess, was that we would eventually replace it by more flexible and generic configuration options, which is still planned. But there's nothing preventing us to alias this in the future to a particular configuration. So, stop scaring users off, and un-deprecate this. Link: https://archives.passt.top/passt-dev/20240925102009.62b9a0ce@elisabeth/ Link: https://github.com/rootless-containers/rootlesskit/pull/482#issuecomment-25… Link: https://github.com/moby/moby/issues/48838 Link: https://github.com/containers/podman/discussions/25243 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 3 +-- passt.1 | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/conf.c b/conf.c index dcfc1d6..db7950e 100644 --- a/conf.c +++ b/conf.c @@ -966,8 +966,7 @@ pasta_opts: " -U, --udp-ns SPEC UDP port forwarding to init namespace\n" " SPEC is as described above\n" " default: auto\n" - " --host-lo-to-ns-lo DEPRECATED:\n" - " Translate host-loopback forwards to\n" + " --host-lo-to-ns-lo Translate host-loopback forwards to\n" " namespace loopback\n" " --userns NSPATH Target user namespace to join\n" " --netns PATH|NAME Target network namespace to join\n" diff --git a/passt.1 b/passt.1 index 63a3a01..395c0c6 100644 --- a/passt.1 +++ b/passt.1 @@ -633,7 +633,7 @@ Configure UDP port forwarding from target namespace to init namespace. Default is \fBauto\fR. .TP -.BR \-\-host-lo-to-ns-lo " " (DEPRECATED) +.BR \-\-host-lo-to-ns-lo If specified, connections forwarded with \fB\-t\fR and \fB\-u\fR from the host's loopback address will appear on the loopback address in the guest as well. Without this option such forwarded packets will appear -- 2.43.0

2 1

[PATCH v10 00/10] Draft state migration
by David Gibson 06 Feb '25

06 Feb '25

Changes from v9: * Assorted minor cleanups * cppcheck and clang-tidy fixes * Set SO_PEEK_OFF properly on the target * Migrate addr_seen and friends Minor changes, I've just folded into the existing patches. More major things are separate patches. Those marked "fixup" I think should be folded into the previous patch before merge, but are separate now to make it clearer what I'm doing. David Gibson (5): debug: Add tcpdump to mbuto.img fixup: Fix errors in modes that don't support migration migrate: Migrate guest observed addresses migrate: Hack for late migration fixups fixup: Reset SO_PEEK_OFF value after incoming migration Stefano Brivio (5): migrate: Skeleton of live migration logic Add interfaces and configuration bits for passt-repair vhost_user: Make source quit after reporting migration state migrate: Migrate TCP flows test: Add migrate/basic tests Makefile | 14 +- conf.c | 44 ++++- epoll_type.h | 6 +- flow.c | 201 +++++++++++++++++++ flow.h | 6 + migrate.c | 291 ++++++++++++++++++++++++++++ migrate.h | 57 ++++++ passt.1 | 11 ++ passt.c | 15 +- passt.h | 15 ++ repair.c | 192 +++++++++++++++++++ repair.h | 16 ++ tap.c | 65 +------ tcp.c | 466 +++++++++++++++++++++++++++++++++++++++++++++ tcp_conn.h | 60 ++++++ test/lib/layout | 55 +++++- test/lib/setup | 127 ++++++++++++ test/lib/test | 3 + test/migrate/basic | 54 ++++++ test/passt.mbuto | 3 +- test/run | 4 + util.c | 62 ++++++ util.h | 30 +++ vhost_user.c | 70 ++----- virtio.h | 4 - vu_common.c | 49 +---- vu_common.h | 2 +- 27 files changed, 1741 insertions(+), 181 deletions(-) create mode 100644 migrate.c create mode 100644 migrate.h create mode 100644 repair.c create mode 100644 repair.h create mode 100644 test/migrate/basic -- 2.48.1

2 11

Another migration bug
by David Gibson 06 Feb '25

06 Feb '25

I did some more cleaning up of the migration series, including handling SO_PEEK_OFF and some other fixes. I expect to post the latest series in an hour or two. However, then I hit another bug, and have made some, but not a lot of progress figuring it out. It occurs when after the migration I send data inbound from a peer before I send anything outbound from the guest. The data seems to get lost, and after that outbound things also don't seem to work, at least not reliably. Sometimes the data seems to eventually come through, but not always, and I haven't figured out a specific pattern yet. I had a look at packet captures, and discovered that what passt is recording isn't the same as what I see with a tcpdump running in the guest itself. This disagreement at L2 makes me suspect the problem is something in the guts of vhost-user. Do we need to be migrating some of the ring pointers from vu_virtq? Or are those supposed to be reset by the various config commands that come in to the target side with the migration? Laurent, any insight? I'm going to tackle something else for the remainder of today. I'm attaching the packet captures, in hopes that it helps one of you source.pcap - The packet capture recorded by the source side passt instance target.pcap - The packet capture recorded by the target side passt instance internal.pcap - The packet capture recored by tcpdump running within the guest The relevant stream is the one from port 55314 to port 5555. The data flow _should_ look like: guest to peer: "first\n" peer to guest: "before\n" -- migration occurs -- peer to guest: "after\n" guest to peer: "return\n" The "after" packet appears in both traces, but from passt's view it gets no reply and is retransmitted. From the guest's view it is acked promptly. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson

1 1

[PATCH v9 0/6] Draft state migration
by Stefano Brivio 06 Feb '25

06 Feb '25

1 6

[PATCH v8 0/6] Draft state migration
by Stefano Brivio 06 Feb '25

06 Feb '25

On top of v7: - call repair_close() from migrate_close(): shutdown passt-repair when we're done in the target - add tests! Run as PCAP=1 ./run migrate/basic - add save/restore of send and receive queues. Not tested with full queues. It doesn't break things with empty queues, at least David Gibson (1): migrate: Hack for late migration fixups Stefano Brivio (5): migrate: Skeleton of live migration logic Add interfaces and configuration bits for passt-repair vhost_user: Make source quit after reporting migration state migrate: Migrate TCP flows test: Add migrate/basic tests Makefile | 14 +- conf.c | 44 ++++- epoll_type.h | 6 +- flow.c | 226 +++++++++++++++++++++++ flow.h | 7 + migrate.c | 292 ++++++++++++++++++++++++++++++ migrate.h | 57 ++++++ passt.1 | 11 ++ passt.c | 14 +- passt.h | 15 ++ repair.c | 193 ++++++++++++++++++++ repair.h | 16 ++ tap.c | 65 +------ tcp.c | 462 ++++++++++++++++++++++++++++++++++++++++++++++++ tcp_conn.h | 61 +++++++ test/lib/layout | 55 +++++- test/lib/setup | 127 +++++++++++++ test/lib/test | 3 + test/run | 4 + util.c | 62 +++++++ util.h | 30 ++++ vhost_user.c | 69 +++----- virtio.h | 4 - vu_common.c | 49 +---- vu_common.h | 2 +- 25 files changed, 1708 insertions(+), 180 deletions(-) create mode 100644 migrate.c create mode 100644 migrate.h create mode 100644 repair.c create mode 100644 repair.h -- 2.43.0

1 6

[PATCH] passt-repair.1: Fix indication of TCP_REPAIR constants
by Stefano Brivio 05 Feb '25

05 Feb '25

...perhaps I should adopt the healthy habit of actually reading headers instead of using my mental copy. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt-repair.1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/passt-repair.1 b/passt-repair.1 index 8d07c97..7c1b140 100644 --- a/passt-repair.1 +++ b/passt-repair.1 @@ -31,7 +31,7 @@ same as the UNIX domain socket used for guest communication, suffixed by \fI.repair\fR. The messages consist of one 8-bit signed integer that can be \fITCP_REPAIR_ON\fR -(1), \fITCP_REPAIR_OFF\fR (2), or \fITCP_REPAIR_OFF_WP\fR (-1), as defined by +(1), \fITCP_REPAIR_OFF\fR (0), or \fITCP_REPAIR_OFF_NO_WP\fR (-1), as defined by the Linux kernel user API, and one to SCM_MAX_FD (253) sockets as SCM_RIGHTS (see \fBunix\fR(7)) ancillary message, sent by the server, \fBpasst\fR(1). -- 2.43.0

2 1

[PATCH] passt-repair: Build fixes for musl
by Stefano Brivio 05 Feb '25

05 Feb '25

When building against musl headers: - sizeof() needs sttdef.h, as it should be; - we can't initialise a struct msghdr by simply listing fields in order, as they contain explicit padding fields. Use field names instead. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt-repair.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/passt-repair.c b/passt-repair.c index 6f79423..3c3247b 100644 --- a/passt-repair.c +++ b/passt-repair.c @@ -21,6 +21,7 @@ #include <sys/socket.h> #include <sys/un.h> #include <errno.h> +#include <stddef.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -75,7 +76,11 @@ int main(int argc, char **argv) } iov = (struct iovec){ &cmd, sizeof(cmd) }; - msg = (struct msghdr){ NULL, 0, &iov, 1, buf, sizeof(buf), 0 }; + msg = (struct msghdr){ .msg_name = NULL, .msg_namelen = 0, + .msg_iov = &iov, .msg_iovlen = 1, + .msg_control = buf, + .msg_controllen = sizeof(buf), + .msg_flags = 0 }; cmsg = CMSG_FIRSTHDR(&msg); if (argc != 2) { -- 2.43.0

2 2