April 2024 - dev

[PATCH v2 00/14] Clean up checksum and header generation for inbound packets
by David Gibson 10 Oct '25

10 Oct '25

The main packet "fast paths" for UDP and TCP mostly just forward packets rather than generating them from scratch. However the control paths for ICMP and DHCP sometimes generate packets more or less from scratch. Because these are relatively rare, it's not performance critical. The paths for sending these packets have some duplication of the header generation. There's also some layering violation in tap_ip_send() which both generates IP headers and updates the L4 (UDP or UCMP) checksum. Finally that checksum generation is a little awkward: it temporarily generates the IP pseudo header (or something close enough to serve) in the place of the actual header, generates the checksum, then replaces it with the real IP header. This approach seems to be causing miscompiles with some LTO optimization, because the stores to the pseudo header are being moved or elided across the code calculating the checksum. This series addresses all of these. We consolidate and clarify the packet sending helpers, and use them in some places there was previously duplicated code. In the process we use new checksum generation helpers which take a different approach which should avoid the LTO problems (this aspect I haven't tested yet though). Changes since v1: * Numerous minor style changes * Rename header generation helpers to make their behaviour clearer * Added several missing function doc comments * Corrected some erroneous statements and terms in comments David Gibson (14): Add csum_icmp6() helper for calculating ICMPv6 checksums Add csum_icmp4() helper for calculating ICMP checksums Add csum_udp6() helper for calculating UDP over IPv6 checksums Add csum_udp4() helper for calculating UDP over IPv4 checksums Add csum_ip4_header() helper to calculate IPv4 header checksums Add helpers for normal inbound packet destination addresses Remove support for TCP packets from tap_ip_send() tap: Remove unhelpeful vnet_pre optimization from tap_send() Split tap_ip_send() into IPv4 and IPv6 specific functions tap: Split tap_ip6_send() into UDP and ICMP variants ndp: Remove unneeded eh_source parameter ndp: Use tap_icmp6_send() helper tap: Split tap_ip4_send() into UDP and ICMP variants dhcp: Use tap_udp4_send() helper in dhcp() arp.c | 2 +- checksum.c | 120 ++++++++++++++++----- checksum.h | 15 ++- dhcp.c | 19 +--- dhcpv6.c | 21 +--- icmp.c | 12 +-- ndp.c | 28 +---- ndp.h | 3 +- tap.c | 312 ++++++++++++++++++++++++++++++++++------------------- tap.h | 19 +++- 10 files changed, 345 insertions(+), 206 deletions(-) -- 2.37.3

2 17

[PATCH v2 0/4] mbuto: Assorted fixes and simplifications
by David Gibson 18 May '24

18 May '24

While debugging a problem Laurent Vivier was having with the passt test suite, I discovered mbuto's archivemount support is entirely broken: if archivemount is on the system, mbuto won't work. While fixing that, I discovered some slightly related simplfications that can be made. Here they all are. Changes since v1: * Updated commit message in 2/4 to clarify why the desired advantages of using archivemount can't be used in practice * Improve accuracy of comment on compress_select in 3/4 * Clarify removal of update existing archive functionality. David Gibson (4): ${wd} is always set, no need to test for it Remove stale archivemount support Split "auto" compression mode into its own path Remove unnecessary cpio_init function mbuto | 128 +++++++++++++++------------------------------------------- 1 file changed, 33 insertions(+), 95 deletions(-) -- 2.44.0

2 6

[PATCH 0/2] netlink: Don't duplicate routes with mismatching interfaces
by Stefano Brivio 01 May '24

01 May '24

Stefano Brivio (2): netlink: Fix iterations over nexthop objects netlink: Don't duplicate routes referring to unrelated host interfaces netlink.c | 53 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 9 deletions(-) -- 2.43.0

2 4

[PATCH 0/7] Rework UDP buffers
by David Gibson 01 May '24

01 May '24

Laurent recently reworked the TCP buffer handling to be split into various pieces tracked by iovecs. We'll want that for various future changes. This series makes a similar split for UDP buffers, which we'll want in order to allow dual-stack UDP sockets, amongst other things. This is based on my earlier series of cleanups for the TCP buffer handling. David Gibson (7): test: Allow sftp via vsock-ssh in tests udp: Split tap-bound UDP packets into multiple buffers using io vector udp: Combine initialisation of IPv4 and IPv6 iovs udp: Explicitly set checksum in guest-bound UDP headers udp: Share payload buffers between IPv4 and IPv6 udp: Use the same buffer for the L2 header for all frames udp: Single buffer for IPv4, IPv6 headers and metadata tap.h | 38 ------ test/passt.mbuto | 6 +- udp.c | 304 +++++++++++++++++++++++------------------------ 3 files changed, 153 insertions(+), 195 deletions(-) -- 2.44.0

2 13

[PATCH 0/7] Small improvements to IOV handling
by David Gibson 01 May '24

01 May '24

Laurent's changes to split TCP buffers into various components with IOVs is now merged. This series has a batch of small cleanups to make the handling of this slightly nicer. These are preliminaries to doing something similar with the UDP buffers. Note that patch 7/7 might interfere with the experiments to work out what is going wrong with the odd batching / performance issues we've seen. We can leave it off for the time being if that's a problem. David Gibson (7): checksum: Use proto_ipv6_header_psum() for ICMPv6 as well tap: Split tap specific and L2 (ethernet) headers treewide: Standardise variable names for various packet lengths tcp: Simplify packet length calculation when preparing headers tap, tcp: (Re-)abstract TAP specific header handling iov: Helper macro to construct iovs covering existing variables or fields tcp: Update tap specific header too in tcp_fill_headers[46]() checksum.c | 46 ++++++++--------- checksum.h | 10 ++-- icmp.c | 8 +-- iov.h | 3 ++ passt.h | 4 +- tap.c | 51 ++++++++++--------- tap.h | 46 +++++++++++++---- tcp.c | 144 +++++++++++++++++++++++------------------------------ udp.c | 50 ++++++++++--------- 9 files changed, 184 insertions(+), 178 deletions(-) -- 2.44.0

2 17

[PATCH 0/2] Support for SO_PEEK_OFF when a available
by Jon Maloy 29 Apr '24

29 Apr '24

First commit introduces support for SO_PEEK_OFF socket option. Second commit adds a workaround for a kernel TCP bug. Jon Maloy (2): tcp: leverage support of SO_PEEK_OFF socket option when available tcp: allow retransmit when peer receive window is zero tcp.c | 63 +++++++++++++++++++++++++++++++++++++++++++----------- tcp_conn.h | 2 ++ 2 files changed, 53 insertions(+), 12 deletions(-) -- 2.42.0

3 14

passt: new version 2024_04_26.d03c4e2 available
by Stefano Brivio 26 Apr '24

26 Apr '24

The new version with tag 2024_04_26.d03c4e2 includes the following changes: d03c4e2 netlink: Use IFA_F_NODAD also while duplicating addresses from the host bfc83b5 netlink: For IPv4, IFA_LOCAL is the interface address, not IFA_ADDRESS ff2ff2f test: Make log truncation test more robust 2681366 test: Slight simplification to pasta log tests 0804fdb udp: Correctly look up outbound socket with port remappings 9560123 tcp: Replace TCP buffer structure by an iovec array 27f1c76 conf: Don't fail if the template interface doesn't have a MAC address eca8baa conf: We're interested in the MAC address, not in the MAC itself ee338a2 pasta, util: Align stack area for clones to maximum natural alignment 5d5208b treewide: Compilers' name for armv6l and armv7l is "arm" https://passt.top/passt/log/?qt=range&q=2024_04_05.954589b..2024_04_26.d03c… Packages: - Alpine Linux: https://pkgs.alpinelinux.org/packages?name=passt - Arch Linux: https://www.archlinux.org/packages/extra/x86_64/passt/ https://archlinuxarm.org/packages/aarch64/passt https://archlinuxarm.org/packages/armv7h/passt - Chimera: https://pkgs.chimera-linux.org/packages?name=passt - Debian tracker: https://tracker.debian.org/pkg/passt - Copr (CentOS Stream, EPEL, Fedora, Mageia): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/7374013/ permanent mirror: https://passt.top/builds/copr/0^20240426.gd03c4e2/ - Fedora updates: https://bodhi.fedoraproject.org/updates/?packages=passt - Gentoo versions: https://packages.gentoo.org/packages/net-misc/passt - GNU Guix: https://packages.guix.gnu.org/packages/passt/ - openSUSE: https://build.opensuse.org/package/requests/Virtualization:containers/passt - Ubuntu tracker: https://launchpad.net/ubuntu/+source/passt - Void Linux: https://voidlinux.org/packages/?q=passt - Static builds: - Package for other RPM-based distributions, x86_64 only: https://passt.top/builds/latest/x86_64/passt-gd03c4e2-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_0d03c4e2-1_all.deb -- Stefano

1 0

[PATCH] netlink: For IPv4, IFA_LOCAL is the interface address, not IFA_ADDRESS
by Stefano Brivio 26 Apr '24

26 Apr '24

See the comment to the unnamed enum in linux/if_addr.h, which currently states: /* * Important comment: * IFA_ADDRESS is prefix address, rather than local interface address. * It makes no difference for normally configured broadcast interfaces, * but for point-to-point IFA_ADDRESS is DESTINATION address, * local address is supplied in IFA_LOCAL attribute. * * [...] */ if we fetch IFA_ADDRESS, and we have a point-to-point link with a peer address configured, we'll source the peer address as "our" address, and refuse to resolve it in arp(). This was reported with pasta and a tun upstream interface configured by OpenVPN in "p2p" topology: the target namespace will have similar addresses and routes as the host, which is fine, and will try to resolve the point-to-point peer address (because it's the default gateway). Given that we configure it as our address (only internally, not for visibly in the namespace), we'll fail to resolve that and traffic doesn't go anywhere. Note that this is not the case for IPv6: there, IFA_ADDRESS is the actual, local address of the interface, and IFA_LOCAL is not necessarily present, so the comment in linux/if_addr.h doesn't apply either. Link: https://github.com/containers/podman/issues/22320 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- netlink.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/netlink.c b/netlink.c index 89c0641..447fea2 100644 --- a/netlink.c +++ b/netlink.c @@ -668,7 +668,8 @@ int nl_addr_get(int s, unsigned int ifi, sa_family_t af, for (rta = IFA_RTA(ifa), na = IFA_PAYLOAD(nh); RTA_OK(rta, na); rta = RTA_NEXT(rta, na)) { - if (rta->rta_type != IFA_ADDRESS) + if ((af == AF_INET && rta->rta_type != IFA_LOCAL) || + (af == AF_INET6 && rta->rta_type != IFA_ADDRESS)) continue; if (af == AF_INET && ifa->ifa_prefixlen > prefix_max) { -- 2.43.0

2 1

[PATCH 0/4] mbuto: Assorted fixes and simplifications
by David Gibson 26 Apr '24

26 Apr '24

While debugging a problem Laurent Vivier was having with the passt test suite, I discovered mbuto's archivemount support is entirely broken: if archivemount is on the system, mbuto won't work. While fixing that, I discovered some slightly related simplfications that can be made. Here they all are. David Gibson (4): ${wd} is always set, no need to test for it Remove stale archivemount support Split "auto" compression mode into its own path Remove unnecessary cpio_init function mbuto | 120 +++++++++++++++------------------------------------------- 1 file changed, 30 insertions(+), 90 deletions(-) -- 2.44.0

2 15

[PATCH] netlink: Use IFA_F_NODAD also while duplicating addresses from the host
by Stefano Brivio 26 Apr '24

26 Apr '24

...not just for a single set address (legacy operation with --no-copy-addrs). I forgot to add this to nl_addr_dup(). Note that we can have two version of flags: the 8-bit ifa_flags in ifaddrmsg, and the newer 32-bit version as IFA_FLAGS attribute, which is given priority if present. Make sure IFA_F_NODAD is set in both. Without this, a Podman user reports, something on the lines of: pasta --config-net -- ping -c1 -6 passt.top would fail as the kernel would start Duplicate Address Detection once we configure the address, which can't really work (and doesn't make sense) in this case. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- netlink.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/netlink.c b/netlink.c index 447fea2..b3897e6 100644 --- a/netlink.c +++ b/netlink.c @@ -804,6 +804,8 @@ int nl_addr_dup(int s_src, unsigned int ifi_src, continue; ifa->ifa_index = ifi_dst; + /* Same as nl_addr_set(), but here it's more than a default */ + ifa->ifa_flags |= IFA_F_NODAD; for (rta = IFA_RTA(ifa), na = IFA_PAYLOAD(nh); RTA_OK(rta, na); rta = RTA_NEXT(rta, na)) { @@ -811,6 +813,10 @@ int nl_addr_dup(int s_src, unsigned int ifi_src, if (rta->rta_type == IFA_LABEL || rta->rta_type == IFA_CACHEINFO) rta->rta_type = IFA_UNSPEC; + + /* If 32-bit flags are used, add IFA_F_NODAD there */ + if (rta->rta_type == IFA_FLAGS) + *(uint32_t *)RTA_DATA(rta) |= IFA_F_NODAD; } rc = nl_do(s_dst, nh, RTM_NEWADDR, -- 2.43.0

2 1