March 2023 - 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

passt: new version 2023_03_29.b10b983 available
by Stefano Brivio 30 Mar '23

30 Mar '23

The new version with tag 2023_03_29.b10b983 includes the following changes: b10b983 fedora: Adjust path for SELinux policy and interface file to latest guidelines 387f4ac fedora: Don't install useless SELinux interface file for pasta dafd92d selinux: Drop useless interface file for pasta 98a9a7d conf: Allow binding to ports on an interface without a specific address 33d88f7 tcp: Clear ACK_FROM_TAP_DUE also on unchanged ACK sequence from peer 4e73e9b tcp: Don't special case the handling of the ack of a syn 085672f tcp: Clarify allowed state for tcp_data_from_tap() https://passt.top/passt/log/?qt=range&q=2023_03_21.1ee2f7c..2023_03_29.b10b… Packages: - Debian tracker: https://tracker.debian.org/pkg/passt - Copr (CentOS Stream, EPEL, Fedora, Mageia, openSUSE): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/5727640/ permanent mirror: https://passt.top/builds/copr/0^20230329.gb10b983/ - 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-g02130bb-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_02130bb-1_all.deb -- Stefano

1 0

[PATCH] fedora: Adjust path for SELinux policy and interface file to latest guidelines
by Stefano Brivio 29 Mar '23

29 Mar '23

Forget about: https://fedoraproject.org/wiki/SELinux_Policy_Modules_Packaging_Draft and: https://fedoraproject.org/wiki/PackagingDrafts/SELinux_Independent_Policy The guidelines to follow are: https://fedoraproject.org/wiki/SELinux/IndependentPolicy Start from fixing the most pressing issue, that is, a path conflict with policy-selinux-devel about passt.if, and, while at it, adjust the installation paths for policy files too. Reported-by: Xose Vazquez Perez <xose.vazquez(a)gmail.com> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2182476 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- contrib/fedora/passt.spec | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contrib/fedora/passt.spec b/contrib/fedora/passt.spec index da1accb..88aa3a7 100644 --- a/contrib/fedora/passt.spec +++ b/contrib/fedora/passt.spec @@ -62,17 +62,17 @@ ln -sr %{buildroot}%{_mandir}/man1/pasta.1 %{buildroot}%{_mandir}/man1/pasta.avx pushd contrib/selinux make -f %{_datadir}/selinux/devel/Makefile -install -p -m 644 -D passt.pp %{buildroot}%{_datadir}/selinux/packages/%{name}/passt.pp -install -p -m 644 -D passt.if %{buildroot}%{_datadir}/selinux/devel/include/contrib/passt.if -install -p -m 644 -D pasta.pp %{buildroot}%{_datadir}/selinux/packages/%{name}/pasta.pp +install -p -m 644 -D passt.pp %{buildroot}%{_datadir}/selinux/packages/%{selinuxtype}/passt.pp +install -p -m 644 -D passt.if %{buildroot}%{_datadir}/selinux/devel/include/distributed/passt.if +install -p -m 644 -D pasta.pp %{buildroot}%{_datadir}/selinux/packages/%{selinuxtype}/pasta.pp popd %pre selinux %selinux_relabel_pre -s %{selinuxtype} %post selinux -%selinux_modules_install -s %{selinuxtype} %{_datadir}/selinux/packages/%{name}/passt.pp -%selinux_modules_install -s %{selinuxtype} %{_datadir}/selinux/packages/%{name}/pasta.pp +%selinux_modules_install -s %{selinuxtype} %{_datadir}/selinux/packages/%{selinuxtype}/passt.pp +%selinux_modules_install -s %{selinuxtype} %{_datadir}/selinux/packages/%{selinuxtype}/pasta.pp %postun selinux if [ $1 -eq 0 ]; then -- 2.39.2

1 0

[PATCH 1/2] selinux: Drop useless interface file for pasta
by Stefano Brivio 27 Mar '23

27 Mar '23

This was meant to be an example, but I managed to add syntax errors to it. Drop it altogether. Reported-by: Carl G. <carlg(a)fedoraproject.org> Link: https://bugzilla.redhat.com/show_bug.cgi?id=2182145 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- contrib/selinux/pasta.if | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 contrib/selinux/pasta.if diff --git a/contrib/selinux/pasta.if b/contrib/selinux/pasta.if deleted file mode 100644 index 149045a..0000000 --- a/contrib/selinux/pasta.if +++ /dev/null @@ -1,25 +0,0 @@ -# SPDX-License-Identifier: AGPL-3.0-or-later -# -# PASTA - Pack A Subtle Tap Abstraction -# for network namespace/tap device mode -# -# contrib/selinux/pasta.if - SELinux profile: Interface File for pasta -# -# Copyright (c) 2022 Red Hat GmbH -# Author: Stefano Brivio <sbrivio(a)redhat.com> - -interface('passt_read_data',' - gen_require(` - type passt_data_t; - ') - allow $1 passt_t:dir { search add_name }; - allow $1 passt_t:file { open read getattr }; -') - -interface('pasta_read_data',' - gen_require(` - type pasta_data_t; - ') - allow $1 pasta_t:dir { search add_name }; - allow $1 pasta_t:file { open read getattr }; -') -- 2.39.2

1 1

[PATCH] conf: Allow binding to ports on an interface without a specific address
by Stefano Brivio 27 Mar '23

27 Mar '23

Somebody might want to bind listening sockets to a specific interface, but not a specific address, and there isn't really a reason to prevent that. For example: -t %eth0/2022 Alternatively, we support options such as -t 0.0.0.0%eth0/2022 and -t ::%eth0/2022, but not together, for the same port. Enable this kind of syntax and add examples to the man page. Reported-by: Paul Holzinger <pholzing(a)redhat.com> Link: https://github.com/containers/podman/issues/14425#issuecomment-1485192195 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- conf.c | 4 +++- passt.1 | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/conf.c b/conf.c index 04e9956..f519a6a 100644 --- a/conf.c +++ b/conf.c @@ -263,7 +263,9 @@ static void conf_ports(const struct ctx *c, char optname, const char *optarg, ifname++; } - if (inet_pton(AF_INET, buf, addr)) + if (ifname == buf + 1) /* Interface without address */ + addr = NULL; + else if (inet_pton(AF_INET, buf, addr)) af = AF_INET; else if (inet_pton(AF_INET6, buf, addr)) af = AF_INET6; diff --git a/passt.1 b/passt.1 index 4c90634..cb7ae4e 100644 --- a/passt.1 +++ b/passt.1 @@ -385,6 +385,9 @@ Forward local port 22, bound to 192.0.2.1, to port 22 on the guest -t 192.0.2.1%eth0/22 Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22 .TP +-t %eth0/22 +Forward local port 22, bound to any address on interface eth0, to port 22 +.TP -t 2000-5000,~3000-3010 Forward local ports between 2000 and 5000, except for those between 3000 and 3010 @@ -467,6 +470,9 @@ Forward local port 22, bound to 192.0.2.1, to port 22 in the target namespace -t 192.0.2.1%eth0/22 Forward local port 22, bound to 192.0.2.1 and interface eth0, to port 22 .TP +-t %eth0/22 +Forward local port 22, bound to any address on interface eth0, to port 22 +.TP -t 2000-5000,~3000-3010 Forward local ports between 2000 and 5000, except for those between 3000 and 3010 -- 2.39.2

1 0

[PATCH 0/2] tcp: Correct handling of first ACK (or SYN-ACK) packet
by David Gibson 27 Mar '23

27 Mar '23

We have a subtle problem in the handling of the very first ack-flagged packet (either the SYN-ACK or ACK from the three way handshake). Stefano has posted a couple of versions of a patch addressing this, however I think this is a better approach. From the TCP logical point of view, that first ACK does advance the sequence number, and if we treat it as doing so, then the logic we already had in tcp_update_seqack_from_tap() is correct. David Gibson (2): tcp: Clarify allowed state for tcp_data_from_tap() tcp: Don't special case the handling of the ack of a syn tcp.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- 2.39.2

2 3

[PATCH] tcp: Clear and set ACK_FROM_TAP_DUE also on unchanged ACK sequence from peer
by Stefano Brivio 24 Mar '23

24 Mar '23

Since commit cc6d8286d104 ("tcp: Reset ACK_FROM_TAP_DUE flag only as needed, update timer"), we don't clear ACK_FROM_TAP_DUE whenever we process an ACK segment, but, more correctly, only if we're really not waiting for a further ACK segment, that is, only if the acknowledged sequence matches what we sent. In the new function implementing this, tcp_update_seqack_from_tap(), we also reset the retransmission counter and store the updated ACK sequence. Both should be done iff forward progress is acknowledged, implied by the fact that the new ACK sequence is greater than the one we previously stored. At that point, it looked natural to also include the statements that clear and set the ACK_FROM_TAP_DUE flag inside the same conditional block: if we're not making forward progress, the need for an ACK, or lack thereof, should remain unchanged. There's a case where this isn't true, though: if a client initiates a connection, and the server doesn't send data, with the client also not sending any further data except for what's possibly sent along with the first ACK segment following SYN, ACK from the server, we'll never, in the established state of the connection, call tcp_update_seqack_from_tap() with reported forward progress. In that case, we'll never reset the initial ACK_FROM_TAP_DUE (used to trigger handshake timeouts), interpret the trigger as a the need for a retransmission (which won't actually retransmit anything), and eventually time out a perfectly healthy connection once we reach the maximum retransmission count. This is relatively simple to reproduce if we switch back to 30s iperf3 test runs, but it depends on the timing of the iperf3 client: sometimes the first ACK from the client (part of the three-way handshake) will come with data (and we'll hit the problem), sometimes data will be sent later (and we call to tcp_update_seqack_from_tap() from tcp_data_from_tap() later, avoiding the issue). A reliable reproducer is a simpler: $ strace -e accept,shutdown socat TCP-LISTEN:1111 STDIO & [2] 2202832 $ pasta --config-net -- sh -c '(sleep 30; echo a) | socat STDIN TCP:88.198.0.161:1111' accept(5, {sa_family=AF_INET, sin_port=htons(57200), sin_addr=inet_addr("127.0.0.1")}, [16]) = 6 shutdown(6, SHUT_RDWR) = 0 --- SIGTTOU {si_signo=SIGTTOU, si_code=SI_KERNEL} --- --- stopped by SIGTTOU --- 2023/03/23 16:05:06 socat[3] E write(5, 0x5645dbca9000, 2): Connection reset by peer where the socat client connects, and no data exchange happens for 30s in either direction. Here, I'm using the default gateway address to reach the socat server on the host. Fix this by clearing and setting the ACK_FROM_TAP_DUE flag regardless of reported forward progress. If we clear it when it's already unset, conn_flag() will do nothing with it. If it was set, it's always fine to reschedule the timer (as long as we're waiting for a further ACK), because we just received an ACK segment, no matter its sequence. Fixes: cc6d8286d104 ("tcp: Reset ACK_FROM_TAP_DUE flag only as needed, update timer") Reported-by: David Gibson <david(a)gibson.dropbear.id.au> Analysed-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tcp.c b/tcp.c index f156287..b225bbe 100644 --- a/tcp.c +++ b/tcp.c @@ -1610,12 +1610,12 @@ out: static void tcp_update_seqack_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, uint32_t seq) { - if (SEQ_GT(seq, conn->seq_ack_from_tap)) { - if (seq == conn->seq_to_tap) - conn_flag(c, conn, ~ACK_FROM_TAP_DUE); - else - conn_flag(c, conn, ACK_FROM_TAP_DUE); + if (seq == conn->seq_to_tap) + conn_flag(c, conn, ~ACK_FROM_TAP_DUE); + else + conn_flag(c, conn, ACK_FROM_TAP_DUE); + if (SEQ_GT(seq, conn->seq_ack_from_tap)) { conn->retrans = 0; conn->seq_ack_from_tap = seq; } -- 2.39.2

2 3

[PATCH v2] tcp: Clear ACK_FROM_TAP_DUE also on unchanged ACK sequence from peer
by Stefano Brivio 24 Mar '23

24 Mar '23

Since commit cc6d8286d104 ("tcp: Reset ACK_FROM_TAP_DUE flag only as needed, update timer"), we don't clear ACK_FROM_TAP_DUE whenever we process an ACK segment, but, more correctly, only if we're really not waiting for a further ACK segment, that is, only if the acknowledged sequence matches what we sent. In the new function implementing this, tcp_update_seqack_from_tap(), we also reset the retransmission counter and store the updated ACK sequence. Both should be done iff forward progress is acknowledged, implied by the fact that the new ACK sequence is greater than the one we previously stored. At that point, it looked natural to also include the statements that clear and set the ACK_FROM_TAP_DUE flag inside the same conditional block: if we're not making forward progress, the need for an ACK, or lack thereof, should remain unchanged. There's a case where this isn't true, though: if a tap-side client initiates a connection, and the server doesn't send any data, we'll never, in the established state of the connection, call tcp_update_seqack_from_tap() with reported forward progress. In that case, we'll never reset the initial ACK_FROM_TAP_DUE (used to trigger handshake timeouts), interpret the trigger as the need for a retransmission (which won't actually retransmit anything), and eventually time out a perfectly healthy connection once we reach the maximum retransmission count. This is relatively simple to reproduce if we switch back to 30s iperf3 test runs, but it depends on the timing of the iperf3 client: sometimes the first ACK from the client (part of the three-way handshake) will come with data (and we'll hit the problem), sometimes data will be sent later (and we call to tcp_update_seqack_from_tap() from tcp_data_from_tap() later, avoiding the issue). A reliable reproducer is a simpler: $ strace -e accept,shutdown socat TCP-LISTEN:1111 STDIO & [2] 2202832 $ pasta --config-net -- sh -c '(sleep 30; echo a) | socat STDIN TCP:88.198.0.161:1111' accept(5, {sa_family=AF_INET, sin_port=htons(57200), sin_addr=inet_addr("127.0.0.1")}, [16]) = 6 shutdown(6, SHUT_RDWR) = 0 --- SIGTTOU {si_signo=SIGTTOU, si_code=SI_KERNEL} --- --- stopped by SIGTTOU --- 2023/03/23 16:05:06 socat[3] E write(5, 0x5645dbca9000, 2): Connection reset by peer where the socat client connects, and no data exchange happens for 30s in either direction. Here, I'm using the default gateway address to reach the socat server on the host. Fix this by clearing the ACK_FROM_TAP_DUE flag regardless of reported forward progress. If we clear it when it's already unset, conn_flag() will do nothing with it. Fixes: cc6d8286d104 ("tcp: Reset ACK_FROM_TAP_DUE flag only as needed, update timer") Reported-by: David Gibson <david(a)gibson.dropbear.id.au> Analysed-by: David Gibson <david(a)gibson.dropbear.id.au> Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- v2: Don't reschedule the timer if no forward progress was made. Fix a couple of things in the commit message based on David's observations tcp.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tcp.c b/tcp.c index f156287..e640a4a 100644 --- a/tcp.c +++ b/tcp.c @@ -1610,10 +1610,12 @@ out: static void tcp_update_seqack_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, uint32_t seq) { + if (seq == conn->seq_to_tap) + conn_flag(c, conn, ~ACK_FROM_TAP_DUE); + if (SEQ_GT(seq, conn->seq_ack_from_tap)) { - if (seq == conn->seq_to_tap) - conn_flag(c, conn, ~ACK_FROM_TAP_DUE); - else + /* Forward progress, but more data to acknowledge: reschedule */ + if (SEQ_LT(seq, conn->seq_to_tap)) conn_flag(c, conn, ACK_FROM_TAP_DUE); conn->retrans = 0; -- 2.39.2

1 0

passt: new version 2023_03_21.1ee2f7c available
by Stefano Brivio 22 Mar '23

22 Mar '23

The new version with tag 2023_03_21.1ee2f7c includes the following changes: 1ee2f7c tcp: Don't reset ACK_TO_TAP_DUE on any ACK, reschedule timer as needed 9ffccf7 tcp: When a connection flag it set, don't negate it for debug print 89d1494 Fix false positive if cppcheck doesn't give a false positive 34ade90 Work around weird false positives with cppcheck-2.9.1 ccf6d2a udp: Actually bind detected namespace ports in init namespace 418f75a pasta: fix tcp port forwarding in auto mode https://passt.top/passt/log/?qt=range&q=2023_03_17.dd23496..2023_03_21.1ee2… Packages: - Debian tracker: https://tracker.debian.org/pkg/passt - Copr (CentOS Stream, EPEL, Fedora, Mageia, openSUSE): https://copr.fedorainfracloud.org/coprs/sbrivio/passt/build/5694155/ permanent mirror: https://passt.top/builds/copr/0^20230321.g1ee2f7c/ - 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-g1ee2f7c-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_1ee2f7c-1_all.deb -- Stefano

1 0

Everything Open 2023 passt presentation
by David Gibson 22 Mar '23

22 Mar '23

For anyone who might be interested, here's the presentation about passt and passt which I made at Everything Open 2023, a conference that was in Melbourne, Australia last week. Recording: watch on Youtube at: https://www.youtube.com/watch?v=QMUEtEt1i3I or download in webm format at https://mirror.linux.org.au/pub/everythingopen/2023/clarendon_room_a/Tuesda… Slides: as a Google Slides document https://docs.google.com/presentation/d/1P1lOy1hZtyCgDHltj2fwOEPDa2AGfN0vXQq… or as PDF https://drive.google.com/file/d/1b30UYxQ9F5q66ZWkO-CSRnJ-cvC1aO3S/view?usp=… Stefano, feel free to mirror those onto passt.top if you'd like. -- David Gibson | 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

2 1