- dev - passt

[PATCH] tcp: Suppress knownConditionTrueFalse cppcheck false positive
by Stefano Brivio 25 Feb '23

25 Feb '23

cppcheck 2.10 reports: tcp.c:1815:12: style: Condition 'wnd>prev_scaled' is always false [knownConditionTrueFalse] if ((wnd > prev_scaled && wnd * 99 / 100 < prev_scaled) || ^ tcp.c:1808:8: note: Assignment 'wnd=((1<<(16+8))<(wnd))?(1<<(16+8)):(wnd)', assigned value is less than 1 wnd = MIN(MAX_WINDOW, wnd); ^ tcp.c:1811:19: note: Assuming condition is false if (prev_scaled == wnd) ^ tcp.c:1815:12: note: Condition 'wnd>prev_scaled' is always false if ((wnd > prev_scaled && wnd * 99 / 100 < prev_scaled) || ^ but this is not actually the case: wnd is typically greater than 1, and might very well be greater than prev_scaled as well. I bisected this down to cppcheck commit b4d455df487c ("Fix 11349: FP negativeIndex for clamped array index (#4627)") and reported findings at https://github.com/danmar/cppcheck/pull/4627. Suppress the warning for the moment being. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tcp.c b/tcp.c index 803c2c4..21c319d 100644 --- a/tcp.c +++ b/tcp.c @@ -1812,6 +1812,15 @@ static void tcp_clamp_window(const struct ctx *c, struct tcp_tap_conn *conn, return; /* Discard +/- 1% updates to spare some syscalls. */ + /* TODO: cppcheck, starting from commit b4d455df487c ("Fix + * 11349: FP negativeIndex for clamped array index (#4627)"), + * reports wnd > prev_scaled as always being true, see also: + * + * https://github.com/danmar/cppcheck/pull/4627 + * + * drop this suppression once that's resolved. + */ + /* cppcheck-suppress knownConditionTrueFalse */ if ((wnd > prev_scaled && wnd * 99 / 100 < prev_scaled) || (wnd < prev_scaled && wnd * 101 / 100 > prev_scaled)) return; -- 2.39.1

1 0

[PATCH] qrap: Pass PCI device numbers to qemu in base 10, not in base 16
by Stefano Brivio 25 Feb '23

25 Feb '23

Alona reports that when libvirt starts qrap (KubeVirt integration) and the domain description leads to more than 10 devices, indices of PCI device descriptors are formatted as hexadecimal, so we end up with things like "pci.a" instead of "pci.10". Reported-by: Alona Paz <alkaplan(a)redhat.com> Fixes: 5307faa05997 ("qrap: Strip network devices from command line, set them up according to machine") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- qrap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qrap.c b/qrap.c index 287198e..ff99c89 100644 --- a/qrap.c +++ b/qrap.c @@ -281,11 +281,11 @@ int main(int argc, char **argv) qemu_argv[qemu_argc++] = "-device"; if (!has_json) { snprintf(dev_str, ARG_MAX, - "%s,%s%x%s,netdev=hostnet0,x-txburst=4096", + "%s,%s%i%s,netdev=hostnet0,x-txburst=4096", dev->name, dev->template, i, dev->template_post); } else { snprintf(dev_str, ARG_MAX, - "{\"driver\":\"%s\",%s%x\"%s,\"netdev\":\"hostnet0\",\"x-txburst\":4096}", + "{\"driver\":\"%s\",%s%i\"%s,\"netdev\":\"hostnet0\",\"x-txburst\":4096}", dev->name, dev->template_json, i, dev->template_json_post); } qemu_argv[qemu_argc++] = dev_str; -- 2.39.1

3 11

[PATCH] log: Send identifier string in log messages, openlog() won't work for us
by Stefano Brivio 24 Feb '23

24 Feb '23

openlog() can be used to set "ident" and have all the log messages prefixed by it, but only if we call syslog() -- this is implemented by C libraries. We don't log messages with syslog(), though, as we have a custom implementation to ensure we don't need dynamic memory allocation. This means that it's perfectly useless to call openlog(), and that we have to prefix every message we log by the identifier on our own. Reported-by: Andrea Bolognani <abologna(a)redhat.com> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- log.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/log.c b/log.c index f23e8ef..bfde6ce 100644 --- a/log.c +++ b/log.c @@ -106,7 +106,7 @@ void trace_init(int enable) } /** - * __openlog() - Non-optional openlog() wrapper, to allow custom vsyslog() + * __openlog() - Non-optional openlog() implementation, for custom vsyslog() * @ident: openlog() identity (program name) * @option: openlog() options * @facility: openlog() facility (LOG_DAEMON) @@ -136,8 +136,6 @@ void __openlog(const char *ident, int option, int facility) log_mask |= facility; strncpy(log_ident, ident, sizeof(log_ident) - 1); log_opt = option; - - openlog(ident, option, facility); } /** @@ -158,11 +156,11 @@ void __setlogmask(int mask) */ void passt_vsyslog(int pri, const char *format, va_list ap) { + int prefix_len, n; char buf[BUFSIZ]; - int n; - /* Send without name and timestamp, the system logger should add them */ - n = snprintf(buf, BUFSIZ, "<%i> ", pri); + /* Send without timestamp, the system logger should add it */ + n = prefix_len = snprintf(buf, BUFSIZ, "<%i> %s: ", pri, log_ident); n += vsnprintf(buf + n, BUFSIZ - n, format, ap); @@ -170,7 +168,7 @@ void passt_vsyslog(int pri, const char *format, va_list ap) n += snprintf(buf + n, BUFSIZ - n, "\n"); if (log_opt & LOG_PERROR) - fprintf(stderr, "%s", buf + sizeof("<0>")); + fprintf(stderr, "%s", buf + prefix_len); if (send(log_sock, buf, n, 0) != n) fprintf(stderr, "Failed to send %i bytes to syslog\n", n); -- 2.39.1

3 4

[libvirt PATCH 0/3] Support for restarting passt backend
by Laine Stump 22 Feb '23

22 Feb '23

If the passt process that implements the backend of a QEMU emulated network device terminates, QEMU itself is incapable of restarting a new process so that the networking can begin... working again. However, what QEMU can and does do is: 1) send the NETDEV_STREAM_DISCONNECTED event to libvirt when it sees that the socket has been closed (this event has been in QEMU for as long as support for "-netdev stream", which is used for connecting to passt) 2) as of QEMU 8.0.0, the qemu commandline for -netdev stream accepts the "reconnect" option, which tells QEMU to attempt reconnecting to the same socket it previously used, repeating the attempt every "n" seconds (the only argument to reconnect) until it is successful. If libvirt adds the reconnect option to the qemu commandline, and then responds to a NETDEV_STREAM_DISCONNECTED event by re-running the same passt command that it ran when the device was originally connected, then a guest will be able to recover in the (very unlikely, according to Stefano!) event that the original passt process unexpectedly exits, or is killed by some external entity. Patch 2/3 handles (2) above, while patch 3/3 handles (1). (patch 1/3 is a short guest appearance by pkrempa. Thanks pkrempa!). This resolves https://bugzilla.redhat.com/2172098 Along with Stefano's series fixing up selinux issues related to running the passt process, they make the passt backend very usable. Laine Stump (2): qemu: add reconnect=5 to passt qemu commandline options when available qemu: respond to NETDEV_STREAM_DISCONNECTED event Peter Krempa (1): qemu: capabilities: Introduce QEMU_CAPS_NETDEV_STREAM_RECONNECT src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 3 + src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 82 +++++++++++++++++++ src/qemu/qemu_monitor.c | 11 +++ src/qemu/qemu_monitor.h | 6 ++ src/qemu/qemu_monitor_json.c | 16 ++++ src/qemu/qemu_passt.c | 11 +++ src/qemu/qemu_process.c | 18 ++++ .../caps_8.0.0.x86_64.xml | 1 + .../net-user-passt.x86_64-7.2.0.args | 37 +++++++++ .../net-user-passt.x86_64-latest.args | 2 +- tests/qemuxml2argvtest.c | 1 + 14 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args -- 2.39.2

2 6

new version 2023_02_22.4ddbcb9 available
by Stefano Brivio 22 Feb '23

22 Feb '23

The new version with tag 2023_02_22.4ddbcb9 includes the following changes: 4ddbcb9 tcp: Disable optimisations for tcp_hash() 933aa10 selinux/passt.te: Allow setting socket option on routing netlink socket 0c11355 selinux/passt.te: Allow /etc/resolv.conf symlinks to be followed 7d9150d selinux/passt.te: Allow setcap on the process itself 01801b1 selinux: Switch to a more reasonable model for PID and socket files 4902447 selinux: Define interfaces for libvirt and similar frameworks 9136f74 selinux/passt.if: Fix typo in passt_read_data interface definition https://passt.top/passt/log/?qt=range&q=2023_02_16.4663ccc..2023_02_22.4ddb… Packages: - Debian tracker: https://tracker.debian.org/pkg/passt - Copr (CentOS Stream, EPEL, Fedora, Mageia, openSUSE): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/5556988/ permanent mirror: https://passt.top/builds/copr/0^20230222.g4ddbcb9/ - 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] tcp: Disable optimisations for tcp_hash()
by Stefano Brivio 22 Feb '23

22 Feb '23

I'm not sure if we're breaking some aliasing rule here, but with gcc 12.2.1 on x86_64 and -flto, the siphash_20b() call in tcp_hash() doesn't see the connection address -- it gets all zeroes instead. Fix this temporarily by disabling optimisations for this tcp_hash(). Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tcp.c b/tcp.c index 81468d9..803c2c4 100644 --- a/tcp.c +++ b/tcp.c @@ -1185,6 +1185,9 @@ static int tcp_hash_match(const struct tcp_tap_conn *conn, #if TCP_HASH_NOINLINE __attribute__((__noinline__)) /* See comment in Makefile */ #endif +__attribute__((optimize("O0"))) /* TODO: with -O2 and -flto on gcc 12.2, + * siphash_20b() doesn't see 'addr', why? + */ static unsigned int tcp_hash(const struct ctx *c, const union inany_addr *addr, in_port_t tap_port, in_port_t sock_port) { -- 2.39.1

1 0

[PATCH 0/6] selinux: Assorted fixes, libvirt support
by Stefano Brivio 21 Feb '23

21 Feb '23

This series exports interfaces that are useful for libvirt, updates type enforcement rules to current needs, and fixes some issues. Stefano Brivio (6): selinux/passt.if: Fix typo in passt_read_data interface definition selinux: Define interfaces for libvirt and similar frameworks selinux: Switch to a more reasonable model for PID and socket files selinux/passt.te: Allow setcap on the process itself selinux/passt.te: Allow /etc/resolv.conf symlinks to be followed selinux/passt.te: Allow setting socket option on routing netlink socket contrib/selinux/passt.fc | 1 - contrib/selinux/passt.if | 28 +++++++++++++++++++++++++++- contrib/selinux/passt.te | 16 +++++++++++----- 3 files changed, 38 insertions(+), 7 deletions(-) -- 2.39.1

1 6

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