- dev - passt

[PATCH v4 00/16] RFC: Unified flow table
by David Gibson 14 May '24

14 May '24

This is a fourth draft of the first steps in implementing more general "connection" tracking, as described at: https://pad.passt.top/p/NewForwardingModel This series changes the TCP connection table and hash table into a more general flow table that can track other protocols as well. Each flow uniformly keeps track of all the relevant addresses and ports, which will allow for more robust control of NAT and port forwarding. ICMP is converted to use the new flow table. This doesn't include UDP, but I'm working on it right now and making progress. I'm posting this to give a head start on the review :) Caveats: * We significantly increase the size of a connection/flow entry - Can probably be mitigated, but I haven't investigated much yet * We perform a number of extra getsockname() calls to know some of the socket endpoints - Haven't yet measured how much performance impact that has - Can be mitigated in at least some cases, but again, haven't tried yet Changes since v4: * Complex rebase on top of the many things that have happened upstream since v3. * Assorted other changes. Changes since v3: * Replace TAPFSIDE() and SOCKFSIDE() macros with local variables. Changes since v2: * Cosmetic fixes based on review * Extra doc comments for enum flow_type * Rename flowside to flowaddrs which turns out to make more sense in light of future changes * Fix bug where the socket flowaddrs for tap initiated connections wasn't initialised to match the socket address we were using in the case of map-gw NAT * New flowaddrs_from_sock() helper used in most cases which is cleaner and should avoid bugs like the above * Using newer centralised workarounds for clang-tidy issue 58992 * Remove duplicate definition of FLOW_MAX as maximum flow type and maximum number of tracked flows * Rebased on newer versions of preliminary work (ICMP, flow based dispatch and allocation, bind/address cleanups) * Unified hash table as well as base flow table * Integrated ICMP Changes since v1: * Terminology changes - "Endpoint" address/port instead of "correspondent" address/port - "flowside" instead of "demiflow" * Actually move the connection table to a new flow table structure in new files * Significant rearrangement of earlier patchs on top of that new table, to reduce churn David Gibson (16): flow: Common data structures for tracking flow addresses tcp: Maintain flowside information for "tap" connections tcp_splice: Maintain flowside information for spliced connections tcp: Obtain guest address from flowside tcp: Simplify endpoint validation using flowside information tcp, tcp_splice: Construct sockaddrs for connect() from flowside tcp_splice: Eliminate SPLICE_V6 flag tcp, flow: Replace TCP specific hash function with general flow hash flow, tcp: Generalise TCP hash table to general flow hash table tcp: Re-use flow hash for initial sequence number generation icmp: Populate flowside information icmp: Use flowsides as the source of truth wherever possible icmp: Look up ping flows using flow hash icmp: Eliminate icmp_id_map flow, tcp: flow based NAT and port forwarding for TCP flow, icmp: Use general flow forwarding rules for ICMP flow.c | 199 ++++++++++++++++++++- flow.h | 97 +++++++++++ fwd.c | 139 +++++++++++++++ fwd.h | 5 + icmp.c | 83 +++++---- icmp_flow.h | 1 - inany.h | 29 +++- passt.h | 3 + pif.h | 1 - tap.c | 11 -- tap.h | 1 - tcp.c | 475 +++++++++++++-------------------------------------- tcp_conn.h | 20 +-- tcp_splice.c | 93 ++-------- tcp_splice.h | 5 +- 15 files changed, 649 insertions(+), 513 deletions(-) -- 2.44.0

2 22

[PATCH] apparmor: allow netns paths on /tmp
by Paul Holzinger 13 May '24

13 May '24

For some unknown reason "owner" makes it impossible to open bind mounted netns references as apparmor denies it. In the kernel denied log entry we see ouid=0 but it is not clear why that is as the actual file is owned by the real (rootless) user id. In abstractions/pasta there is already `@{run}/user/@{uid}/**` without owner set for the same reason as this path contains the netns path by default when running under Podman. Fixes: 72884484b00d ("apparmor: allow read access on /tmp for pasta") Signed-off-by: Paul Holzinger <pholzing(a)redhat.com> --- contrib/apparmor/usr.bin.pasta | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/apparmor/usr.bin.pasta b/contrib/apparmor/usr.bin.pasta index 2a4d28c..bdfeb71 100644 --- a/contrib/apparmor/usr.bin.pasta +++ b/contrib/apparmor/usr.bin.pasta @@ -19,7 +19,7 @@ profile pasta /usr/bin/pasta{,.avx2} flags=(attach_disconnected) { include <abstractions/pasta> # Alternatively: include <abstractions/user-tmp> - owner /tmp/** rw, # tap_sock_unix_init(), pcap(), + /tmp/** rw, # tap_sock_unix_init(), pcap(), # write_pidfile(), # logfile_init(), # pasta_open_ns() -- 2.45.0

2 1

[PATCH v2 0/2] LLVM 18.1.1 / Fedora 40 clang-tidy fixes
by David Gibson 13 May '24

13 May '24

A new clang-tidy has introduce some new warnings. David Gibson (2): conf: Fix clang-tidy warning about using an undefined enum value clang-tidy: Suppress macro to enum conversion warnings Makefile | 9 ++++++++- conf.c | 4 ++-- fwd.h | 1 + 3 files changed, 11 insertions(+), 3 deletions(-) -- 2.45.0

2 3

[PATCH] passt.c: explicitly include libgen.h for basename
by lemmi 13 May '24

13 May '24

fixes implicit declaration warning on musl Signed-off-by: lemmi <lemmi(a)nerd2nerd.org> --- passt.c | 1 + 1 file changed, 1 insertion(+) diff --git a/passt.c b/passt.c index e93b6be..771b8a7 100644 --- a/passt.c +++ b/passt.c @@ -35,6 +35,7 @@ #include <syslog.h> #include <sys/prctl.h> #include <netinet/if_ether.h> +#include <libgen.h> #ifdef HAS_GETRANDOM #include <sys/random.h> #endif -- 2.45.0

3 2

Setting hostname while spawning pasta
by Danish Prakash 13 May '24

13 May '24

Hello I was having a discussion with a colleague of mine on how `pasta` when invoked independently drops you in a new user and network namespace and how that causes some folks to not realize what has happened especially because there's no cue as to what transpired. This is not uncommon though, because a lot of times, folks having issues with pasta while running it with podman, tend to invoke pasta directly to debug. I interacted with Stefano regarding this and he told me that pasta already detaches the UTS namespace and so in order to address this issue, I propose that we set the hostname as part of `pasta_spawn_cmd()`.Doing this would provide users with a clear visual indication that they are in a new user namespace (among other ns but that won't be indicative here) and avoid issues similar to the aforementioned. I'm looking to gather more opinions on how this change, if implemented, would affect the functionality. Looking forward to the discussion. Thank you danishpraka.sh :wq

3 4

Re: Setting hostname while spawning pasta
by Stefano Brivio 13 May '24

13 May '24

On Mon, 13 May 2024 13:45:51 +0530 Danish Prakash <danish.prakash(a)suse.com> wrote: > > But we could still retain the existing hostname and prefix with > > something like 'pasta-on-' (sure, we would need to truncate at 63 > > characters, but it shouldn't be a common case). What do you think? > > That sounds like a good idea. I didn't realize you'd be running > pasta-in-pasta.. but that's reasonable. Though this will be helpful, > I'm not entirely sure if this would override the prompts that many > have set. At least my rudimentary shell prompt doesn't recognize the > pasta user namespace and continues to show the same old prompt. I hope > I'm the only one having this issue. I currently have these behaviours with default prompts: - Alpine: host:~$ pasta host:~# - Debian: user@host:~$ pasta root@host:~# - Fedora: [user@host ~]$ pasta [root@host ~]# - openSUSE: user@host:~> pasta host:~ # so I generally have a hint of something going on, but in any case, changing the hostname is helpful for unfamiliar users or even just for the pasta-in-pasta case I mentioned. > > What I can't tell from the prompt, though, is at what "level" I'm at > > (at least pasta-in-pasta is something I try very commonly to reproduce > > pasta's behaviour with particular network setups). Having > > root@pasta-on-pasta-on-machine would be a nice plus. > > But in general, this idea sounds good, I'd like to take a shot at > this, and I can send over the patch for you to test once I'm done. > What do you think? Sure, thanks, looking forward to it! -- Stefano

2 1

[PATCH] tcp: move seq_to_tap update to when frame is queued
by Jon Maloy 13 May '24

13 May '24

commit a469fc393fa1 ("tcp, tap: Don't increase tap-side sequence counter for dropped frames") delayed update of conn->seq_to_tap until the moment the corresponding frame has been successfully pushed out. This has the advantage that we immediately can retransmit a buffer that we fail to trasnmit, rather than waiting for the peer side to discover the loss and initiate fast retransmit. This approach has turned out to cause a problem with spurious sequence number updates during peer-initiated retransmits, and we have realized it may not be the best way to solve te above issue. We now restore the previous method, by updating the said field at the moment a frame is added to the outqueue. To retain the advantage of fast retansmit based on local failure detection, we now scan through the part of the outqueue that had do be dropped, and restore the sequence counter for each affected connection to the most appropriate value. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- tcp.c | 52 ++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/tcp.c b/tcp.c index 21d0af0..58fdbc9 100644 --- a/tcp.c +++ b/tcp.c @@ -412,11 +412,13 @@ static union inany_addr low_rtt_dst[LOW_RTT_TABLE_SIZE]; /** * tcp_buf_seq_update - Sequences to update with length of frames once sent - * @seq: Pointer to sequence number sent to tap-side, to be updated + * @conn: Pointer to connection corresponding to frame. May need update + * @seq: Sequence number of the corresponding frame * @len: TCP payload length */ struct tcp_buf_seq_update { - uint32_t *seq; + struct tcp_tap_conn *conn; + uint32_t seq; uint16_t len; }; @@ -1261,25 +1263,52 @@ static void tcp_flags_flush(const struct ctx *c) tcp4_flags_used = 0; } +/** + * tcp_revert_seq() - Revert affected conn->seq_to_tap after failed transmission + * @seq_update: Array with connection and sequence number data + * @s: Entry corresponding to first dropped frame + * @e: Entry corresponding to last dropped frame + */ +static void tcp_revert_seq(struct tcp_buf_seq_update *seq_update, int s, int e) +{ + struct tcp_tap_conn *conn; + uint32_t lowest_seq; + int i, ii; + + for (i = s; i < e; i++) { + conn = seq_update[i].conn; + lowest_seq = seq_update[i].seq; + + for (ii = i + 1; ii < e; ii++) { + if (seq_update[ii].conn != conn) + continue; + if (SEQ_GT(lowest_seq, seq_update[ii].seq)) + lowest_seq = seq_update[ii].seq; + } + + if (SEQ_GT(conn->seq_to_tap, lowest_seq)) + conn->seq_to_tap = lowest_seq; + } +} + /** * tcp_payload_flush() - Send out buffers for segments with data * @c: Execution context */ static void tcp_payload_flush(const struct ctx *c) { - unsigned i; size_t m; m = tap_send_frames(c, &tcp6_l2_iov[0][0], TCP_NUM_IOVS, tcp6_payload_used); - for (i = 0; i < m; i++) - *tcp6_seq_update[i].seq += tcp6_seq_update[i].len; + if (m != tcp6_payload_used) + tcp_revert_seq(tcp6_seq_update, m, tcp6_payload_used); tcp6_payload_used = 0; m = tap_send_frames(c, &tcp4_l2_iov[0][0], TCP_NUM_IOVS, tcp4_payload_used); - for (i = 0; i < m; i++) - *tcp4_seq_update[i].seq += tcp4_seq_update[i].len; + if (m != tcp4_payload_used) + tcp_revert_seq(tcp4_seq_update, m, tcp4_payload_used); tcp4_payload_used = 0; } @@ -2129,10 +2158,11 @@ static int tcp_sock_consume(const struct tcp_tap_conn *conn, uint32_t ack_seq) static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, ssize_t dlen, int no_csum, uint32_t seq) { - uint32_t *seq_update = &conn->seq_to_tap; struct iovec *iov; size_t l4len; + conn->seq_to_tap = seq; + if (CONN_V4(conn)) { struct iovec *iov_prev = tcp4_l2_iov[tcp4_payload_used - 1]; const uint16_t *check = NULL; @@ -2142,7 +2172,8 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, check = &iph->check; } - tcp4_seq_update[tcp4_payload_used].seq = seq_update; + tcp4_seq_update[tcp4_payload_used].conn = conn; + tcp4_seq_update[tcp4_payload_used].seq = seq; tcp4_seq_update[tcp4_payload_used].len = dlen; iov = tcp4_l2_iov[tcp4_payload_used++]; @@ -2151,7 +2182,8 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, if (tcp4_payload_used > TCP_FRAMES_MEM - 1) tcp_payload_flush(c); } else if (CONN_V6(conn)) { - tcp6_seq_update[tcp6_payload_used].seq = seq_update; + tcp6_seq_update[tcp6_payload_used].conn = conn; + tcp6_seq_update[tcp6_payload_used].seq = seq; tcp6_seq_update[tcp6_payload_used].len = dlen; iov = tcp6_l2_iov[tcp6_payload_used++]; -- 2.42.0

3 4

passt: new version 2024_05_10.7288448 available
by Stefano Brivio 10 May '24

10 May '24

The new version with tag 2024_05_10.7288448 includes the following changes: 7288448 apparmor: allow read access on /tmp for pasta 7e6a606 tcp_splice: Set OUT_WAIT_ flag whenever pipe isn't emptied 1ba76c9 udp: Single buffer for IPv4, IPv6 headers and metadata d4598e1 udp: Use the same buffer for the L2 header for all frames 6170688 udp: Share payload buffers between IPv4 and IPv6 2d16946 udp: Explicitly set checksum in guest-bound UDP headers 6c4d26a udp: Combine initialisation of IPv4 and IPv6 iovs 3f9bd86 udp: Split tap-bound UDP packets into multiple buffers using io vector fcd9308 test: Allow sftp via vsock-ssh in tests eea5d3e tcp: Update tap specific header too in tcp_fill_headers[46]() 3559899 iov: Helper macro to construct iovs covering existing variables or fields 40f8b29 tap, tcp: (Re-)abstract TAP specific header handling 68d1b0a tcp: Simplify packet length calculation when preparing headers 5566386 treewide: Standardise variable names for various packet lengths 9e22c53 checksum: Make csum_ip4_header() take a host endian length 1095a7b treewide: Remove misleading and redundant endianness notes 5d37dab tap: Remove unused structs tap_msg, tap_l4_msg 34fb381 tap: Split tap specific and L2 (ethernet) headers c27ca91 checksum: Use proto_ipv6_header_psum() for ICMPv6 as well 76e3202 netlink: Fix iterations over nexthop objects https://passt.top/passt/log/?qt=range&q=2024_04_26.d03c4e2..2024_05_10.7288… 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/7434838/ permanent mirror: https://passt.top/builds/copr/0^20240510.g7288448/ - 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-g7288448-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_7288448-1_all.deb -- Stefano

1 0

[PATCH] apparmor: allow read access on /tmp for pasta
by Paul Holzinger 10 May '24

10 May '24

The podman CI on debian runs tests based on /tmp but pasta is failing there because it is unable to open the netns path as the open for read access is denied. Signed-off-by: Paul Holzinger <pholzing(a)redhat.com> --- contrib/apparmor/usr.bin.pasta | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contrib/apparmor/usr.bin.pasta b/contrib/apparmor/usr.bin.pasta index e5ee4df..2a4d28c 100644 --- a/contrib/apparmor/usr.bin.pasta +++ b/contrib/apparmor/usr.bin.pasta @@ -19,9 +19,10 @@ profile pasta /usr/bin/pasta{,.avx2} flags=(attach_disconnected) { include <abstractions/pasta> # Alternatively: include <abstractions/user-tmp> - owner /tmp/** w, # tap_sock_unix_init(), pcap(), + owner /tmp/** rw, # tap_sock_unix_init(), pcap(), # write_pidfile(), - # logfile_init() + # logfile_init(), + # pasta_open_ns() owner @{HOME}/** w, # pcap(), write_pidfile() } -- 2.44.0

2 1

[PATCH] tcp_splice: Set OUT_WAIT_ flag whenever pipe isn't emptied
by Stefano Brivio 09 May '24

09 May '24

In tcp_splice_sock_handler(), if we get EAGAIN on the second splice(), from pipe to receiving socket, that doesn't necessarily mean that the pipe is empty: the receiver buffer might be full instead. Hence, we can't use the 'never_read' flag to decide that there's nothing to wait for: even if we didn't read anything from the sending side in a given iteration, we might still have data to send in the pipe. Use read/written counters, instead. This fixes an issue where large bulk transfers would occasionally hang. From a corresponding strace: 0.000061 epoll_wait(4, [{events=EPOLLOUT, data={u32=29442, u64=12884931330}}], 8, 1000) = 1 0.005003 epoll_ctl(4, EPOLL_CTL_MOD, 211, {events=EPOLLIN|EPOLLRDHUP, data={u32=54018, u64=8589988610}}) = 0 0.000089 epoll_ctl(4, EPOLL_CTL_MOD, 115, {events=EPOLLIN|EPOLLRDHUP, data={u32=29442, u64=12884931330}}) = 0 0.000081 splice(211, NULL, 151, NULL, 1048576, SPLICE_F_MOVE|SPLICE_F_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable) 0.000073 splice(150, NULL, 115, NULL, 1048576, SPLICE_F_MOVE|SPLICE_F_NONBLOCK) = 1048576 0.000087 splice(211, NULL, 151, NULL, 1048576, SPLICE_F_MOVE|SPLICE_F_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable) 0.000045 splice(150, NULL, 115, NULL, 1048576, SPLICE_F_MOVE|SPLICE_F_NONBLOCK) = 520415 0.000060 splice(211, NULL, 151, NULL, 1048576, SPLICE_F_MOVE|SPLICE_F_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable) 0.000044 splice(150, NULL, 115, NULL, 1048576, SPLICE_F_MOVE|SPLICE_F_NONBLOCK) = -1 EAGAIN (Resource temporarily unavailable) 0.000044 epoll_wait(4, [], 8, 1000) = 0 we're reading from socket 211 into to the pipe end numbered 151, which connects to pipe end 150, and from there we're writing into socket 115. We initially drop EPOLLOUT from the set of monitored flags for socket 115, because it already signaled it's ready for output. Then we read nothing from socket 211 (the sender had nothing to send), and we keep emptying the pipe into socket 115 (first 1048576 bytes, then 520415 bytes). This call of tcp_splice_sock_handler() ends with EAGAIN on the writing side, and we just exit this function without setting the OUT_WAIT_1 flag (and, in turn, EPOLLOUT for socket 115). However, it turns out, the pipe wasn't actually emptied, and while socket 211 had nothing more to send, we should have waited on socket 115 to be ready for output again. As a further step, we could consider not clearing EPOLLOUT at all, unless the read/written counters match, but I'm first trying to fix this ugly issue with a minimal patch. Link: https://github.com/containers/podman/issues/22575 Link: https://github.com/containers/podman/issues/22593 Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- tcp_splice.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tcp_splice.c b/tcp_splice.c index 42b7be0..4c36b72 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -616,7 +616,7 @@ eintr: if (errno != EAGAIN) goto close; - if (never_read) + if (conn->read[fromside] == conn->written[fromside]) break; conn_event(c, conn, -- 2.43.0

2 1