February 2023 - dev

passt: new version 2023_02_16.4663ccc available
by Stefano Brivio 17 Feb '23

17 Feb '23

The new version with tag 2023_02_16.4663ccc includes the following changes: 4663ccc conf: Fix typo and logic in conf_ports() check for port binding 36f0199 conf, tap: Silence two false positive invalidFunctionArg from cppcheck 89e0fbf tcp: Remove 'zero_len' goto from tcp_data_from_sock 83b2061 tcp: Remove 'recvmsg' goto from tcp_data_from_sock 42bfd21 tap: Eliminate goto from tap_handler() b62ed9c tap: Don't pcap frames that didn't get sent d957796 passt.1: Fix typo, improve wording in examples of port forwarding specifiers 77a6d97 dhcp: Fix netmask calculation for option 1 from prefix length 7564b58 tap: Use single counter for iov elements in tap_send_frames_pasta() 3d0de2c conf, tcp, udp: Exit if we fail to bind sockets for all given ports b4f13c2 log: Don't duplicate messages on stderr before daemonising c9af6f9 convert all remaining err() followed by exit() to die() a1ab1ca log a detailed error (not usage()) when there are extra non-option arguments 60bd93e make conf_netns_opt() exit immediately after logging error ead4a98 make conf_ugid() exit immediately after logging error fe2a54e make conf_pasta_ns() exit immediately after logging error b20fe11 make conf_ports() exit immediately after logging error c864c60 eliminate most calls to usage() in conf() dac4af8 add die() to log an error message and exit with a single call 193385b log to stderr until process is daemonized, even if a log file is set dccc2ff test: Fedora 32-35 have moved to the archives 64325a0 test: Update location for Debian ppc64 images 6ccab72 tcp: Improve handling of fallback if socket pool is empty on new splice dc467d5 tcp: Split pool lookup from creating new sockets in tcp_conn_new_sock() 912d37c tcp: Move socket pool declarations around c899347 tcp: Split init and ns cases for tcp_sock_refill() e456c1c tcp: Make a helper to refill each socket pool ee8353c Makefile: Explict int type in FALLOC_FL_COLLAPSE_RANGE probe 67920b8 test/pasta_options: Ignore failures on shell 'exit' a234407 pasta: propagate exit code from child command 04dfc5b pasta: correctly exit when execvp() fails 3487b5f pasta: do not leak netlink sock into child 7a8ed94 Make assertions actually useful cc6d828 tcp: Reset ACK_FROM_TAP_DUE flag only as needed, update timer ac15359 tap: Send frames after the first one in tap_send_frames_pasta() d8921da pasta: Wait for tap to be set up before spawning command 54502cc udp: Use tap_send_frames() 2d553b5 tap: Improve handling of partial frame sends 97d1ca2 udp: Use abstracted tap header 0fb7b2b tap: Use different io vector bases depending on tap type 2416310 tcp: Use abstracted tap header 4b3d38a tap: Add "tap headers" abstraction e4443ba tcp: Consolidate calculation of total frame size dcea260 tcp: Remove redundant and incorrect initialization from *_iov_init() 716a926 util: Parameterize ethernet header initializer macro 67afaab tcp, udp: Use named field initializers in iov_init functions f5a9501 util: Introduce hton*_constant() in place of #ifdefs 6d011c1 tap, tcp: Move tap send path to tap.c e21ee41 tcp: Combine two parts of pasta tap send path together a79e774 tcp: Improve interface to tcp_l2_buf_flush() f1d2d35 tcp: Don't compute total bytes in a message until we need it 99f0be3 tcp: Combine two parts of passt tap send path together d3089eb pcap: Replace pcapm() with pcap_multiple() cb3c1ce pcap: Introduce pcap_frame() helper b93d025 udp: Don't use separate sockets to listen for spliced packets 8d503e8 udp: Decide whether to "splice" per datagram rather than per socket 8a10f23 udp: Unify udp_sock_handler_splice() with udp_sock_handler() f1ed8db udp: Pre-populate msg_names with local address c9e193b udp: Don't handle tap receive batch size calculation within a #define 4eb54fd udp: Split receive from preparation and send in udp_sock_handler() 09c00f1 udp: Split sending to passt tap interface into separate function a6e919a udp: Move sending pasta tap frames to the end of udp_sock_handler() c196953 test/perf/pasta_tcp: Add host to namespace cases for traffic via tap 37f82cc tcp: Explicitly check option length field values in tcp_opt_get() 08c01f5 test/perf/pasta_udp: Add host to namespace cases for traffic via tap 310bdbd udp: Factor out control structure management from udp_sock_fill_data_v[46] https://passt.top/passt/log/?qt=range&q=2022_11_16.ace074c..2023_02_16.4663… Packages: - Debian tracker: https://tracker.debian.org/pkg/passt - Copr (CentOS Stream, EPEL, Fedora, Mageia, openSUSE): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/5536686/ permanent mirror: https://passt.top/builds/copr/0^20230216.g4663ccc/ - Debian package, from x86_64 static build: https://passt.top/builds/latest/x86_64/passt_4663ccc-1_all.deb - Fedora updates: https://bodhi.fedoraproject.org/updates/?packages=passt - Ubuntu tracker: https://packages.ubuntu.com/lunar/passt - Static builds: - Package for other RPM-based distributions, x86_64 only: https://passt.top/builds/latest/x86_64/passt-g4663ccc-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_4663ccc-1_all.deb -- Stefano

1 0

[PATCH] conf, tap: Silence two false positive invalidFunctionArg from cppcheck
by Stefano Brivio 17 Feb '23

17 Feb '23

The newly introduced die() calls exit(), but cppcheck doesn't see it and warns about possibly invalid arguments used after the check which triggers die(). Add return statements to silence the warnings. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 3 +++ tap.c | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/conf.c b/conf.c index 675d961..5426c9b 100644 --- a/conf.c +++ b/conf.c @@ -1036,6 +1036,9 @@ static void conf_ugid(char *runas, uid_t *uid, gid_t *gid) if ((fd = open("/proc/self/uid_map", O_RDONLY | O_CLOEXEC)) < 0) { die("Can't determine if we're in init namespace: %s", strerror(errno)); + + /* Silence cppcheck's invalidFunctionArg for 'fd' in read() */ + return; } if (read(fd, buf, BUFSIZ) != sizeof(root_uid_map) || diff --git a/tap.c b/tap.c index 88eed88..d6f962e 100644 --- a/tap.c +++ b/tap.c @@ -1037,9 +1037,13 @@ static void tap_sock_unix_init(struct ctx *c) snprintf(path, UNIX_PATH_MAX - 1, UNIX_SOCK_PATH, i); ex = socket(AF_UNIX, SOCK_STREAM | SOCK_NONBLOCK, 0); - if (ex < 0) + if (ex < 0) { die("UNIX domain socket check: %s", strerror(errno)); + /* Silence cppcheck's invalidFunctionArg for 'ex' */ + return; + } + ret = connect(ex, (const struct sockaddr *)&addr, sizeof(addr)); if (!ret || (errno != ENOENT && errno != ECONNREFUSED && errno != EACCES)) { -- 2.35.1

3 5

[PATCH v2 0/5] qemu_passt: Fix issues with PID file
by Michal Privoznik 17 Feb '23

17 Feb '23

This is a v2 of: https://listman.redhat.com/archives/libvir-list/2023-February/237731.html diff to v1: - Merged patches that were ACKed in v1, - Dropped 4/4 from the original series (the one that sets --foreground), and implemented a different approach Michal Prívozník (5): qemu_passt: Avoid double daemonizing passt qemu_passt: Report passt's error on failed start qemu_passt: Make passt report errors to stderr whenever possible qemu_passt: Deduplicate passt killing code qemu_passt: Let passt write the PID file src/qemu/qemu_passt.c | 61 +++++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 20 deletions(-) -- 2.39.1

7 24

[PATCH v2 0/4] Some cleanups to tap and tcp
by David Gibson 17 Feb '23

17 Feb '23

This is a handful of simple cleanups which I made while investigating https://bugs.passt.top/show_bug.cgi?id=41. Note that these don't themselves actually address that bug, they're just unrelated cleanups that happen to be in adjacent code. I've run basic tests on these, but not quite the complete test suite: I haven't yet had a chance to debug an unrelated problem with the IPv4 performance tests failing. Changes since v1: * Minor comment and style fixes David Gibson (4): tap: Don't pcap frames that didn't get sent tap: Eliminate goto from tap_handler() tcp: Remove 'recvmsg' goto from tcp_data_from_sock tcp: Remove 'zero_len' goto from tcp_data_from_sock tap.c | 49 +++++++++++++++++++++++++++---------------------- tcp.c | 37 +++++++++++++++++-------------------- 2 files changed, 44 insertions(+), 42 deletions(-) -- 2.39.1

2 7

[PATCH v4 0/9] error logging fixes
by Laine Stump 16 Feb '23

16 Feb '23

There are two topics covered here: 1) If a logFile is set, passt's behavior has been to send all error messages there, and *not* to stderr. This makes it difficult for another program that is exec'ing passt (and setting it to log to a file) to report useful error messages when passt fails - everything after the point that the logfile is opened is sent only to the logfile. The first patch makes a simple change to the logging functions that uses the value of the system logmask to decide if it should force writing messages to stderr even when a logfile has been specified. Change from "v2": I'm using Stefano's suggestion of "abusing" logmask, rather than adding a static bool to keep track. Change from "v3": tweak a commend to Stefano's liking. 2) All the rest of the patches eliminate use of the blanket usage() function when a commandline error is encountered (and add in specific/details error messages when previously usage() was all that was logged), and replace calls to err() followed by exit() with a single call to the new function die(). Change from "v2": I changed the name of the "log and exit" function from "errexit()" to "die()" at the suggestion of Dave Gibson (Stefano concurred). Although it seems a bit more violent, it does make moot many/most of Stefano's nits about needing to split lines to eliminate > 80 characters (I did address all the rest of the things he pointed out, though) NB: Yes, this says it is v3, and the previous version I sent was v2, and there *was no v1* - this is because I didn't realize that git-publish is automatically incrementing the version number every time I run it, and I had done a test-drive sending the patches to my personal address prior to sending them to the list - *that* was v1. Laine Stump (9): log to stderr until process is daemonized, even if a log file is set add die() to log an error message and exit with a single call eliminate most calls to usage() in conf() make conf_ports() exit immediately after logging error make conf_pasta_ns() exit immediately after logging error make conf_ugid() exit immediately after logging error make conf_netns_opt() exit immediately after logging error log a detailed error (not usage()) when there are extra non-option arguments convert all remaining err() followed by exit() to die() conf.c | 468 ++++++++++++++++++++-------------------------------- isolation.c | 67 +++----- log.c | 24 +-- log.h | 1 + netlink.c | 3 +- passt.c | 29 ++-- pasta.c | 20 +-- tap.c | 30 ++-- 8 files changed, 244 insertions(+), 398 deletions(-) -- 2.39.1

3 22

[PATCH v2] conf: Fix typo and logic in conf_ports() check for port binding
by Stefano Brivio 16 Feb '23

16 Feb '23

Ouch, I accidentally pushed the previous change without running the tests: - we need to check, in conf_ports(), that udp_sock_init() managed to bind at least a port, not the opposite - for -T and -U, we have no way to know if we'll manage to bind the port later, so never report an error for those Fixes: 3d0de2c1d727 ("conf, tcp, udp: Exit if we fail to bind sockets for all given ports") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Add the special case for -T and -U conf.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/conf.c b/conf.c index 5426c9b..4dc0660 100644 --- a/conf.c +++ b/conf.c @@ -306,6 +306,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, } else if (optname == 'u') { if (!udp_sock_init(c, 0, af, addr, ifname, i)) bound_one = true; + } else { + /* No way to check in advance for -T and -U */ + bound_one = true; } } @@ -356,8 +359,11 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, if (!tcp_sock_init(c, af, addr, ifname, i)) bound_one = true; } else if (optname == 'u') { - if (udp_sock_init(c, 0, af, addr, ifname, i)) + if (!udp_sock_init(c, 0, af, addr, ifname, i)) bound_one = true; + } else { + /* No way to check in advance for -T and -U */ + bound_one = true; } } } while ((p = next_chunk(p, ','))); -- 2.35.1

1 0

[PATCH] conf: Fix typo in conf_ports() check for UDP port binding
by Stefano Brivio 16 Feb '23

16 Feb '23

Ouch, I accidentally pushed the previous change without running the tests: we need to check, in conf_ports(), that udp_sock_init() managed to bind at least a port, not the opposite. Fixes: 3d0de2c1d727 ("conf, tcp, udp: Exit if we fail to bind sockets for all given ports") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conf.c b/conf.c index 5426c9b..60ca399 100644 --- a/conf.c +++ b/conf.c @@ -356,7 +356,7 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, if (!tcp_sock_init(c, af, addr, ifname, i)) bound_one = true; } else if (optname == 'u') { - if (udp_sock_init(c, 0, af, addr, ifname, i)) + if (!udp_sock_init(c, 0, af, addr, ifname, i)) bound_one = true; } } -- 2.35.1

1 0

[PATCH v2] passt.1: Fix typo, improve wording in examples of port forwarding specifiers
by Stefano Brivio 16 Feb '23

16 Feb '23

Based on a patch from Laine, and reports from Laine and Yalan: fix the "22-80:32-90" example, and improve wording for the other ones: instead of using "to" to denote the end of a range, use "between ... and", so that it's clear we're *not* referring to target ports. Reported-by: Laine Stump <laine(a)redhat.com> Reported-by: Yalan Zhang <yalzhang(a)redhat.com> Fixes: da20f57f19dc ("passt, qrap: Add man pages") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Adopted further changes proposed by David passt.1 | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/passt.1 b/passt.1 index 528763b..f317c33 100644 --- a/passt.1 +++ b/passt.1 @@ -338,7 +338,7 @@ Examples: .RS .TP -t 22 -Forward local port 22 to 22 on the guest +Forward local port 22 to port 22 on the guest .TP -t 22:23 Forward local port 22 to port 23 on the guest @@ -347,10 +347,10 @@ Forward local port 22 to port 23 on the guest Forward local ports 22 and 25 to ports 22 and 25 on the guest .TP -t 22-80 -Forward local ports 22 to 80 to corresponding ports on the guest +Forward local ports between 22 and 80 to corresponding ports on the guest .TP --t 22-80-32:90 -Forward local ports 22 to 80 to corresponding ports on the guest plus 10 +-t 22-80:32-90 +Forward local ports between 22 and 80 to ports between 32 and 90 on the guest .TP -t 192.0.2.1/22 Forward local port 22, bound to 192.0.2.1, to port 22 on the guest @@ -359,10 +359,12 @@ Forward local port 22, bound to 192.0.2.1, to port 22 on the guest Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22 .TP -t 2000-5000,~3000-3010 -Forward local ports 2000 to 5000, but not 3000 to 3010 +Forward local ports between 2000 and 5000, except for those between 3000 and +3010 .TP -t 192.0.2.1/20-30,~25 -Forward local ports 20 to 24, and 26 to 30, bound to 192.0.2.1 +For the local address 192.0.2.1, forward ports between 20 and 24 and between 26 +and 30 .TP -t ~20000-20010 Forward all ports to the guest, except for the range from 20000 to 20010 @@ -425,10 +427,11 @@ Forward local port 22 to port 23 in the target namespace Forward local ports 22 and 25 to ports 22 and 25 in the target namespace .TP -t 22-80 -Forward local ports 22 to 80 to corresponding ports in the target namespace +Forward local ports between 22 and 80 to corresponding ports in the target +namespace .TP --t 22-80-32:90 -Forward local ports 22 to 80 to corresponding ports plus 10 in the target +-t 22-80:32-90 +Forward local ports between 22 and 80 to ports between 32 and 90 in the target namespace .TP -t 192.0.2.1/22 @@ -438,13 +441,15 @@ Forward local port 22, bound to 192.0.2.1, to port 22 in the target namespace Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22 .TP -t 2000-5000,~3000-3010 -Forward local ports 2000 to 5000, but not 3000 to 3010 +Forward local ports between 2000 and 5000, except for those between 3000 and +3010 .TP -t 192.0.2.1/20-30,~25 -Forward local ports 20 to 24, and 26 to 30, bound to 192.0.2.1 +For the local address 192.0.2.1, forward ports between 20 and 24 and between 26 +and 30 .TP -t ~20000-20010 -Forward all ports to the namespace, except for the range from 20000 to 20010 +Forward all ports to the namespace, except for those between 20000 and 20010 .RE IPv6 bound ports are also forwarded for IPv4. -- 2.35.1

2 3

[PATCH 0/4] qemu_passt: Don't let passt fork off
by Michal Privoznik 16 Feb '23

16 Feb '23

Here are some cleanup, as promised here: https://listman.redhat.com/archives/libvir-list/2023-February/237721.html Now, there are still some patches missing. Firstly, we still don't really capture error from passt. My suggestion was to wait for socket to show up with errfd open. But active wait is viewed as undesirable [1]. Secondly, Laine reported SELinux issues. Yeah, I don't see us setting SELinux label on nor socket that passt and QEMU talk to each other, nor on the log file. Speaking of which - we usually have per domain (or per helper daemon instance even) log file, while for passt we have a global one (/var/log/passt.log). I don't think that will fly if two or more SELinux enabled domains want to use passt. Thirdly, Stefano suggested a graceful shutdown for passt: have libvirt connect to the socket and close it. Since we pass --one-off, this should singal passt to exit. But I haven't implemented that because it's redundant. We can't rely on passt quitting itself and thus use the big gun (virPidFileForceCleanupPath()) at which point, the socket way is just an optimization. I might look into the first two, at some point. But not today. 1: https://listman.redhat.com/archives/libvir-list/2023-February/237663.html Michal Prívozník (4): Revert "qemu: allow passt to self-daemonize" qemu_extdevice: Make qemuExtDevicesHasDevice() check def->nets qemu_passt: Report error when getting passt PID failed qemu_passt: Don't let passt fork off src/qemu/qemu_extdevice.c | 11 +++++++++++ src/qemu/qemu_passt.c | 15 ++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) -- 2.39.1

5 18

[PATCH 0/4] RFC: Some cleanups to tap and tcp
by David Gibson 16 Feb '23

16 Feb '23

This is a handful of simple cleanups which I made while investigating https://bugs.passt.top/show_bug.cgi?id=41. Note that these don't themselves actually address that bug, they're just unrelated cleanups that happen to be in adjacent code. These are barely tested at all. I've had some crises right before going away that mean I haven't had a chance to plish these. I'm posting them so they're out there rather than waiting until I'm back in two weeks. These are based on my tap send unification series and the TCP socket pool cleanup series. David Gibson (4): tap: Don't pcap frames that didn't get sent tap: Eliminate goto from tap_handler() tcp: Remove 'recvmsg' goto from tcp_data_from_sock tcp: Remove 'zero_len' goto from tcp_data_from_sock tap.c | 49 +++++++++++++++++++++++++++---------------------- tcp.c | 37 +++++++++++++++++-------------------- 2 files changed, 44 insertions(+), 42 deletions(-) -- 2.39.1

2 9