September 2024 - dev

[PATCH 0/4] Fix possible truncation of frames from /dev/net/tun
by David Gibson 06 Sep '24

06 Sep '24

These changes started off as part of the series re-introducing EPOLLET for tap event handling. That's now turned out to be of lower priority, but along the way we fixed a bug where we could truncate frames from the kernel tap interface. This is a respin of that patch, plus a few minor preliminary cleanups. Various minor tweaks based on feedback from the original posting as part of the tap EPOLLET series. David Gibson (4): tap: Split out handling of EPOLLIN events tap: Improve handling of EINTR in tap_passt_input() tap: Restructure in tap_pasta_input() tap: Don't risk truncating frames on full buffer in tap_pasta_input() tap.c | 98 +++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 58 insertions(+), 40 deletions(-) -- 2.46.0

2 5

[PATCH 0/6] RFC: Possible fixes for bug 94 and bug 95
by David Gibson 06 Sep '24

06 Sep '24

As discussed on IRC through the last day, here's a more polished version of possible fixes for bug 94 (EPOLLRDHUP storm) and bug 95 (EPOLLERR storm). Both of those were sub-problems arising while investigating podman bug 23686. We're pretty confident about the EPOLLRDHUP fix (bug 94, patch 1/6), based on Stefano's testing. I ended up rewriting Stefano's draft patch for the EPOLLERR case (bug 95, remaining patches), because I thought of a possibility we hadn't discussed yet: we weren't getting an error from the socket error queue, but we might be able to get one with the SO_ERROR getsockopt(). My examination of the kernel code suggests that's plausible, and that if that's the case using SO_ERROR should also clear that error condition. Link: https://bugs.passt.top/show_bug.cgi?id=94 Link: https://bugs.passt.top/show_bug.cgi?id=95 Link: https://github.com/containers/podman/issues/23686 David Gibson (6): flow: Fix incorrect hash probe in flowside_lookup() udp: Allow UDP flows to be prematurely closed flow: Helpers to log details of a flow udp: Split socket error handling out from udp_sock_recv() udp: Treat errors getting errors as unrecoverable udp: Handle more error conditions in udp_sock_errs() flow.c | 53 ++++++++++++++++++++------------ flow.h | 7 +++++ udp.c | 88 +++++++++++++++++++++++++++++++++++++++++++++--------- udp_flow.c | 18 ++++++++++- udp_flow.h | 4 +++ 5 files changed, 136 insertions(+), 34 deletions(-) -- 2.46.0

2 7

[PATCH v2] cppcheck: Work around some cppcheck 2.15.0 redundantInitialization warnings
by David Gibson 06 Sep '24

06 Sep '24

cppcheck-2.15.0 has apparently broadened when it throws a warning about redundant initialization to include some cases where we have an initializer for some fields, but then set other fields in the function body. This is arguably a false positive: although we are technically overwriting the zero-initialization the compiler supplies for fields not explicitly initialized, this sort of construct makes sense when there are some fields we know at the top of the function where the initializer is, but others that require more complex calculation. That said, in the two places this shows up, it's pretty easy to work around. The results are arguably slightly clearer than what we had, since they move the parts of the initialization closer together. So do that rather than having ugly suppressions or dealing with the tedious process of reporting a cppcheck false positive. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- pasta.c | 3 ++- udp.c | 10 ++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/pasta.c b/pasta.c index 1900693d..307fb4a7 100644 --- a/pasta.c +++ b/pasta.c @@ -427,12 +427,12 @@ static int pasta_netns_quit_timer(void) */ 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; struct statfs s = { 0 }; bool try_inotify = true; int fd = -1, dir_fd; + union epoll_ref ref; if (c->mode != MODE_PASTA || c->no_netns_quit || !*c->netns_base) return; @@ -463,6 +463,7 @@ void pasta_netns_quit_init(const struct ctx *c) ref.type = EPOLL_TYPE_NSQUIT_TIMER; } else { close(dir_fd); + ref.type = EPOLL_TYPE_NSQUIT_INOTIFY; } if (fd > FD_REF_MAX) diff --git a/udp.c b/udp.c index bd9051e1..aae6d142 100644 --- a/udp.c +++ b/udp.c @@ -713,16 +713,14 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, const void *addr, const char *ifname, in_port_t port) { - union udp_listen_epoll_ref uref = { .port = port }; + union udp_listen_epoll_ref uref = { + .pif = ns ? PIF_SPLICE : PIF_HOST, + .port = port, + }; int r4 = FD_REF_MAX + 1, r6 = FD_REF_MAX + 1; ASSERT(!c->no_udp); - if (ns) - uref.pif = PIF_SPLICE; - else - uref.pif = PIF_HOST; - if (af == AF_UNSPEC && c->ifi4 && c->ifi6) { int s; -- 2.46.0

2 1

[PATCH] util: Fix missing carry of one second, as nanoseconds, in timespec_diff_us()
by Stefano Brivio 06 Sep '24

06 Sep '24

If the nanoseconds of the minuend timestamp are less than the nanoseconds of the subtrahend timestamp, we need to carry one second in the subtraction. I subtracted this second from the minuend, but didn't actually carry it in the subtraction of nanoseconds, and logged timestamps would jump back whenever we switched to the first branch of timespec_diff_us() from the second one. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util.c b/util.c index 6e64279..eede4e5 100644 --- a/util.c +++ b/util.c @@ -249,7 +249,7 @@ void sock_probe_mem(struct ctx *c) int64_t timespec_diff_us(const struct timespec *a, const struct timespec *b) { if (a->tv_nsec < b->tv_nsec) { - return (b->tv_nsec - a->tv_nsec) / 1000 + + return (a->tv_nsec + 1000000000 - b->tv_nsec) / 1000 + (a->tv_sec - b->tv_sec - 1) * 1000000; } -- 2.43.0

2 3

[PATCH] tcp: Use EPOLLET for any state of not established connections
by Stefano Brivio 06 Sep '24

06 Sep '24

Currently, for not established connections, we monitor sockets with edge-triggered events (EPOLLET) if we are in the TAP_SYN_RCVD state (outbound connection being established) but not in the TAP_SYN_ACK_SENT case of it (socket is connected, and we sent SYN,ACK to the container/guest). While debugging https://bugs.passt.top/show_bug.cgi?id=94, I spotted another possibility for a short EPOLLRDHUP storm (10 seconds), which doesn't seem to happen in actual use cases, but I could reproduce it: start a connection from a container, while dropping (using netfilter) ACK segments coming out of the container itself. On the server side, outside the container, accept the connection and shutdown the writing side of it immediately. At this point, we're in the TAP_SYN_ACK_SENT case (not just a mere TAP_SYN_RCVD state), we get EPOLLRDHUP from the socket, but we don't have any reasonable way to handle it other than waiting for the tap side to complete the three-way handshake. So we'll just keep getting this EPOLLRDHUP until the SYN_TIMEOUT kicks in. Always enable EPOLLET when EPOLLRDHUP is the only epoll event we subscribe to: in this case, getting multiple EPOLLRDHUP reports is totally useless. In the only remaining non-established state, SOCK_ACCEPTED, for inbound connections, we're anyway discarding EPOLLRDHUP events until we established the conection, because we don't know what to do with them until we get an answer from the tap side, so it's safe to enable EPOLLET also in that case. Link: https://bugs.passt.top/show_bug.cgi?id=94 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- This is based on: [PATCH 0/6] RFC: Possible fixes for bug 94 and bug 95 tcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tcp.c b/tcp.c index 77c62f0..f9fe1b9 100644 --- a/tcp.c +++ b/tcp.c @@ -440,7 +440,7 @@ static uint32_t tcp_conn_epoll_events(uint8_t events, uint8_t conn_flags) if (events == TAP_SYN_RCVD) return EPOLLOUT | EPOLLET | EPOLLRDHUP; - return EPOLLRDHUP; + return EPOLLET | EPOLLRDHUP; } /** -- 2.43.0

1 0

[PATCH 0/6] RFC: Clean up tap-side event handling
by David Gibson 05 Sep '24

05 Sep '24

This is a draft patch working towards adding EPOLLOUT handling to the tap code, which could then be used to "unstick" flows which have unsent data from the socket side. For now that's just a stub, but makes what I think are some worthwhile cleanups to the tap side event handling in the meantime. David Gibson (6): tap: Split out handling of EPOLLIN events tap: Improve handling of EINTR in tap_passt_input() tap: Restructure in tap_pasta_input() tap: Don't risk truncating frames on full buffer in tap_pasta_input() tap: Re-introduce EPOLLET for tap connections tap: Stub EPOLLOUT handling tap.c | 131 +++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 88 insertions(+), 43 deletions(-) -- 2.46.0

2 22

[PATCH v3 0/4] Add vhost-user support to passt. (part 3)
by Laurent Vivier 05 Sep '24

05 Sep '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. 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. Performance comparison between "-netdev stream" and "-netdev vhost-user": $ iperf3 -c localhost -p 10001 -t 60 -6 -u -b 50G socket: [ 5] 0.00-60.05 sec 95.6 GBytes 13.7 Gbits/sec 0.017 ms 6998988/10132413 (69%) receiver vhost-user: [ 5] 0.00-60.04 sec 237 GBytes 33.9 Gbits/sec 0.006 ms 53673/7813770 (0.69%) receiver $ iperf3 -c localhost -p 10001 -t 60 -4 -u -b 50G socket: [ 5] 0.00-60.05 sec 98.9 GBytes 14.1 Gbits/sec 0.018 ms 6260735/9501832 (66%) receiver vhost-user: [ 5] 0.00-60.05 sec 235 GBytes 33.7 Gbits/sec 0.008 ms 37581/7752699 (0.48%) receiver $ iperf3 -c localhost -p 10001 -t 60 -6 socket: [ 5] 0.00-60.00 sec 17.3 GBytes 2.48 Gbits/sec 0 sender [ 5] 0.00-60.06 sec 17.3 GBytes 2.48 Gbits/sec receiver vhost-user: [ 5] 0.00-60.00 sec 191 GBytes 27.4 Gbits/sec 0 sender [ 5] 0.00-60.05 sec 191 GBytes 27.3 Gbits/sec receiver $ iperf3 -c localhost -p 10001 -t 60 -4 socket: [ 5] 0.00-60.00 sec 15.6 GBytes 2.24 Gbits/sec 0 sender [ 5] 0.00-60.06 sec 15.6 GBytes 2.24 Gbits/sec receiver vhost-user: [ 5] 0.00-60.00 sec 189 GBytes 27.1 Gbits/sec 0 sender [ 5] 0.00-60.04 sec 189 GBytes 27.0 Gbits/sec receiver v3: - rebase on top of flow table - update tcp_vu.c to look like udp_vu.c (recv()/prepare()/send_frame()) - address comments from Stefano and David on version 2 v2: - remove PATCH 4 - rewrite PATCH 2 and 3 to follow passt coding style - move some code from PATCH 3 to PATCH 4 (previously PATCH 5) - partially addressed David's comment on PATCH 5 Laurent Vivier (4): packet: replace struct desc by struct iovec vhost-user: introduce virtio API vhost-user: introduce vhost-user API vhost-user: add vhost-user Makefile | 6 +- checksum.c | 1 - conf.c | 24 +- epoll_type.h | 4 + iov.c | 1 - isolation.c | 15 +- packet.c | 93 ++-- packet.h | 16 +- passt.c | 25 +- passt.h | 6 + pcap.c | 1 - tap.c | 106 +++- tap.h | 5 +- tcp.c | 33 +- tcp_buf.c | 6 +- tcp_internal.h | 3 +- tcp_vu.c | 593 ++++++++++++++++++++++ tcp_vu.h | 12 + udp.c | 71 +-- udp.h | 8 +- udp_internal.h | 34 ++ udp_vu.c | 338 +++++++++++++ udp_vu.h | 13 + util.h | 8 + vhost_user.c | 1277 ++++++++++++++++++++++++++++++++++++++++++++++++ vhost_user.h | 202 ++++++++ virtio.c | 656 +++++++++++++++++++++++++ virtio.h | 185 +++++++ vu_common.c | 27 + vu_common.h | 34 ++ 30 files changed, 3666 insertions(+), 137 deletions(-) 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 create mode 100644 vu_common.c create mode 100644 vu_common.h -- 2.45.2

3 21

[PATCH] log: Don't prefix log file messages with time and severity if they're continuations
by Stefano Brivio 05 Sep '24

05 Sep '24

In fecb1b65b1ac ("log: Don't prefix message with timestamp on --debug if it's a continuation"), I fixed this for --debug on standard error, but not for log files: if messages are continuations, they shouldn't be prefixed by timestamp and severity. Otherwise, we'll print stuff like this: 0.0028: ERROR: Receive error on guest connection, reset0.0028: ERROR: : Bad file descriptor Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- log.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/log.c b/log.c index 433b552..a61468e 100644 --- a/log.c +++ b/log.c @@ -224,19 +224,23 @@ static int logfile_rotate(int fd, const struct timespec *now) /** * logfile_write() - Write entry to log file, trigger rotation if full * @newline: Append newline at the end of the message, if missing + * @cont: Continuation of a previous message, on the same line * @pri: Facility and level map, same as priority for vsyslog() * @now: Timestamp * @format: Same as vsyslog() format * @ap: Same as vsyslog() ap */ -static void logfile_write(bool newline, int pri, const struct timespec *now, +static void logfile_write(bool newline, bool cont, int pri, + const struct timespec *now, const char *format, va_list ap) { char buf[BUFSIZ]; - int n; + int n = 0; - n = logtime_fmt(buf, BUFSIZ, now); - n += snprintf(buf + n, BUFSIZ - n, ": %s", logfile_prefix[pri]); + if (!cont) { + n += logtime_fmt(buf, BUFSIZ, now); + n += snprintf(buf + n, BUFSIZ - n, ": %s", logfile_prefix[pri]); + } n += vsnprintf(buf + n, BUFSIZ - n, format, ap); @@ -278,7 +282,7 @@ void vlogmsg(bool newline, bool cont, int pri, const char *format, va_list ap) va_copy(ap2, ap); /* Don't clobber ap, we need it again */ if (log_file != -1) - logfile_write(newline, pri, now, format, ap2); + logfile_write(newline, cont, pri, now, format, ap2); else if (!(log_mask & LOG_MASK(LOG_DEBUG))) passt_vsyslog(newline, pri, format, ap2); -- 2.43.0

2 1

[PATCH 0/2] Adding SO_PEEK_OFF for TCPv6
by jmaloy＠redhat.com 01 Sep '24

01 Sep '24

From: Jon Maloy <jmaloy(a)redhat.com> Adding SO_PEEK_OFF for TCPv6 and selftest for both TCPv4 and TCPv6. Jon Maloy (2): tcp: add SO_PEEK_OFF socket option tor TCPv6 selftests: add selftest for tcp SO_PEEK_OFF support net/ipv6/af_inet6.c | 1 + tools/testing/selftests/net/Makefile | 1 + tools/testing/selftests/net/tcp_so_peek_off.c | 181 ++++++++++++++++++ 3 files changed, 183 insertions(+) create mode 100644 tools/testing/selftests/net/tcp_so_peek_off.c -- 2.45.2

4 8