April 2025 - dev

[PATCH v5 00/29] Introduce discontiguous frames management
by Laurent Vivier 27 May '25

27 May '25

This series introduces iov_tail to convey frame information between functions. This is only an API change, for the moment the memory pool is only able to store contiguous buffer, so, except for vhost-user in a special case, we only play with iovec array with only one entry. v5: - store in the pool iovec array with several entries v4: Prepare to introduce iovec array in the pool: - passe iov_tail rather than pool to ndp,icmp, dhcp, dhcpv6 and arp - remove unused pool macros - add memory regions in the pool structure, this will allow us to use the buf pointer to store the iovec array for vhost-user v3: Address comments from David Laurent Vivier (29): arp: Don't mix incoming and outgoing buffers iov: Introduce iov_slice(), iov_tail_slice() and iov_tail_drop(). iov: Update IOV_REMOVE_HEADER() and IOV_PEEK_HEADER() tap: Use iov_tail with tap_add_packet() packet: Use iov_tail with packet_add() packet: Add packet_data() arp: Convert to iov_tail ndp: Convert to iov_tail icmp: Convert to iov_tail udp: Convert to iov_tail tcp: Convert tcp_tap_handler() to use iov_tail tcp: Convert tcp_data_from_tap() to use iov_tail dhcpv6: move offset initialization out of dhcpv6_opt() dhcpv6: Extract sending of NotOnLink status dhcpv6: Convert to iov_tail dhcpv6: Use iov_tail in dhcpv6_opt() dhcp: Convert to iov_tail ip: Use iov_tail in ipv6_l4hdr() tap: Convert tap4_handler() to iov_tail tap: Convert tap6_handler() to iov_tail arp: use iov_tail rather than pool dhcp: use iov_tail rather than pool dhcpv6: use iov_tail rather than pool icmp: use iov_tail rather than pool ndp: use iov_tail rather than pool packet: remove PACKET_POOL() and PACKET_POOL_P() packet: remove unused parameter from PACKET_POOL_DECL() packet: add memory regions information into pool packet: use buf to store iovec array arp.c | 86 +++++++++++++------- arp.h | 2 +- dhcp.c | 47 ++++++----- dhcp.h | 2 +- dhcpv6.c | 223 +++++++++++++++++++++++++++++++-------------------- dhcpv6.h | 2 +- icmp.c | 41 ++++++---- icmp.h | 2 +- iov.c | 142 +++++++++++++++++++++++++++++--- iov.h | 59 ++++++++++---- ip.c | 27 +++---- ip.h | 3 +- ndp.c | 13 ++- ndp.h | 4 +- packet.c | 223 ++++++++++++++++++++++++++++++++++++++------------- packet.h | 56 ++++++------- pcap.c | 1 + tap.c | 132 +++++++++++++++++------------- tap.h | 4 +- tcp.c | 61 +++++++++----- tcp_buf.c | 2 +- udp.c | 34 +++++--- vhost_user.c | 28 +++---- virtio.c | 4 +- virtio.h | 8 +- vu_common.c | 46 ++++------- 26 files changed, 823 insertions(+), 429 deletions(-) -- 2.49.0

2 50

[PATCH 00/12] Use connect()ed sockets for both sides of UDP flows
by David Gibson 05 May '25

05 May '25

As discussed, I've been working on using connect()ed sockets, rather than dups of the listening sockets for handling traffic on the initiating side of UDP flows. This improves consistency, avoids some problems (bug 103) and will allow for some useful future improvements. It has the nice side effect of allowing some more code to be shared between various paths, resulting in a pretty nice negative diffstat. David Gibson (12): udp: Use connect()ed sockets for initiating side udp: Make udp_sock_recv() take max number of frames as a parameter udp: Polish udp_vu_sock_info() and remove from vu specific code udp: Don't bother to batch datagrams from "listening" socket udp: Parameterize number of datagrams handled by udp_*_reply_sock_data() udp: Split spliced forwarding path from udp_buf_reply_sock_data() udp: Merge vhost-user and "buf" listening socket paths udp: Move UDP_MAX_FRAMES to udp.c udp_flow: Take pif and port as explicit parameters to udp_flow_from_sock() udp: Rework udp_listen_sock_data() into udp_sock_fwd() udp: Fold udp_splice_prepare and udp_splice_send into udp_sock_to_sock udp_flow: Don't discard packets that arrive between bind() and connect() epoll_type.h | 4 +- flow.c | 2 +- passt.c | 6 +- udp.c | 332 +++++++++++++++++++++++-------------------------- udp.h | 4 +- udp_flow.c | 120 ++++++++++-------- udp_flow.h | 8 +- udp_internal.h | 4 +- udp_vu.c | 93 +------------- udp_vu.h | 2 +- util.c | 2 +- 11 files changed, 242 insertions(+), 335 deletions(-) -- 2.49.0

2 25

[PATCH] flow: fix podman issue #25959
by Laurent Vivier 02 May '25

02 May '25

While running piHole using podman, traffic can trigger the following assert: ASSSERTION FAILED in flow_alloc (flow.c:521): flow->f.state == FLOW_STATE_FREE Backtrace shows that this happens in flow_defer_handler(): #4 0x00005610d6f5b481 flow_alloc (passt + 0xb481) #5 0x00005610d6f74f86 udp_flow_from_sock (passt + 0x24f86) #6 0x00005610d6f737c3 udp_sock_fwd (passt + 0x237c3) #7 0x00005610d6f74c07 udp_flush_flow (passt + 0x24c07) #8 0x00005610d6f752c2 udp_flow_defer (passt + 0x252c2) #9 0x00005610d6f5bce1 flow_defer_handler (passt + 0xbce1) We are trying to allocate a new flow inside the loop freeing them. Inside the loop free_head points to the first free flow entry in the current cluster. But if we allocate a new entry during the loop, free_head is not updated and can point now to the entry we have just allocated. We can fix the problem by spliting the loop in two parts: - first part where we can close some of them and allocate some new flow entries, - second part where we free the entries closed in the previous loop and we aggregate the free entries to merge consecutive the clusters. Link: https://github.com/containers/podman/issues/25959 Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- flow.c | 107 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 57 insertions(+), 50 deletions(-) diff --git a/flow.c b/flow.c index 3c81cb42f921..00c1b2cc316f 100644 --- a/flow.c +++ b/flow.c @@ -788,6 +788,7 @@ void flow_defer_handler(const struct ctx *c, const struct timespec *now) { struct flow_free_cluster *free_head = NULL; unsigned *last_next = &flow_first_free; + bool to_free[FLOW_MAX] = { 0 }; bool timer = false; union flow *flow; @@ -798,9 +799,44 @@ void flow_defer_handler(const struct ctx *c, const struct timespec *now) ASSERT(!flow_new_entry); /* Incomplete flow at end of cycle */ - flow_foreach_slot(flow) { + /* Check which flows we might need to close first, but don't free them + * yet as it's not safe to do that in the middle of flow_foreach(). + */ + flow_foreach(flow) { bool closed = false; + switch (flow->f.type) { + case FLOW_TYPE_NONE: + ASSERT(false); + break; + case FLOW_TCP: + closed = tcp_flow_defer(&flow->tcp); + break; + case FLOW_TCP_SPLICE: + closed = tcp_splice_flow_defer(&flow->tcp_splice); + if (!closed && timer) + tcp_splice_timer(c, &flow->tcp_splice); + break; + case FLOW_PING4: + case FLOW_PING6: + if (timer) + closed = icmp_ping_timer(c, &flow->ping, now); + break; + case FLOW_UDP: + closed = udp_flow_defer(c, &flow->udp, now); + if (!closed && timer) + closed = udp_flow_timer(c, &flow->udp, now); + break; + default: + /* Assume other flow types don't need any handling */ + ; + } + + to_free[FLOW_IDX(flow)] = closed; + } + + /* Second step: actually free the flows */ + flow_foreach_slot(flow) { switch (flow->f.state) { case FLOW_STATE_FREE: { unsigned skip = flow->free.n; @@ -833,60 +869,31 @@ void flow_defer_handler(const struct ctx *c, const struct timespec *now) break; case FLOW_STATE_ACTIVE: - /* Nothing to do */ + if (to_free[FLOW_IDX(flow)]) { + flow_set_state(&flow->f, FLOW_STATE_FREE); + memset(flow, 0, sizeof(*flow)); + + if (free_head) { + /* Add slot to current free cluster */ + ASSERT(FLOW_IDX(flow) == + FLOW_IDX(free_head) + free_head->n); + free_head->n++; + flow->free.n = flow->free.next = 0; + } else { + /* Create new free cluster */ + free_head = &flow->free; + free_head->n = 1; + *last_next = FLOW_IDX(flow); + last_next = &free_head->next; + } + } else { + free_head = NULL; + } break; default: ASSERT(false); } - - switch (flow->f.type) { - case FLOW_TYPE_NONE: - ASSERT(false); - break; - case FLOW_TCP: - closed = tcp_flow_defer(&flow->tcp); - break; - case FLOW_TCP_SPLICE: - closed = tcp_splice_flow_defer(&flow->tcp_splice); - if (!closed && timer) - tcp_splice_timer(c, &flow->tcp_splice); - break; - case FLOW_PING4: - case FLOW_PING6: - if (timer) - closed = icmp_ping_timer(c, &flow->ping, now); - break; - case FLOW_UDP: - closed = udp_flow_defer(c, &flow->udp, now); - if (!closed && timer) - closed = udp_flow_timer(c, &flow->udp, now); - break; - default: - /* Assume other flow types don't need any handling */ - ; - } - - if (closed) { - flow_set_state(&flow->f, FLOW_STATE_FREE); - memset(flow, 0, sizeof(*flow)); - - if (free_head) { - /* Add slot to current free cluster */ - ASSERT(FLOW_IDX(flow) == - FLOW_IDX(free_head) + free_head->n); - free_head->n++; - flow->free.n = flow->free.next = 0; - } else { - /* Create new free cluster */ - free_head = &flow->free; - free_head->n = 1; - *last_next = FLOW_IDX(flow); - last_next = &free_head->next; - } - } else { - free_head = NULL; - } } *last_next = FLOW_MAX; -- 2.49.0

2 1

[PATCH] util: Fix typo, ASSSERTION -> ASSERTION
by Stefano Brivio 30 Apr '25

30 Apr '25

Fixes: 9153aca15bc1 ("util: Add abort_with_msg() and ASSERT_WITH_MSG() helpers") Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util.h b/util.h index cc7d084..5947337 100644 --- a/util.h +++ b/util.h @@ -75,7 +75,7 @@ void abort_with_msg(const char *fmt, ...) #define ASSERT_WITH_MSG(expr, ...) \ ((expr) ? (void)0 : abort_with_msg(__VA_ARGS__)) #define ASSERT(expr) \ - ASSERT_WITH_MSG((expr), "ASSSERTION FAILED in %s (%s:%d): %s", \ + ASSERT_WITH_MSG((expr), "ASSERTION FAILED in %s (%s:%d): %s", \ __func__, __FILE__, __LINE__, STRINGIFY(expr)) #ifdef P_tmpdir -- 2.43.0

2 1

[PATCH] passt-repair: Hide bogus gcc warning from -Og
by Stefano Brivio 30 Apr '25

30 Apr '25

When building with gcc 13 and -Og, we get: passt-repair.c: In function ‘main’: passt-repair.c:161:23: warning: ‘ev’ may be used uninitialized [-Wmaybe-uninitialized] 161 | if (ev->len > NAME_MAX + 1 || ev->name[ev->len - 1] != '\0') { | ~~^~~~~ but that can't actually happen, because we only exit the preceding while loop if 'found' is true, and that only happens, in turn, as we assign 'ev'. Get rid of the warning by (redundantly) initialising ev to NULL. Signed-off-by: Stefano Brivio <sbrivio(a)redhat.com> --- passt-repair.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/passt-repair.c b/passt-repair.c index 256a8c9..ff1c44f 100644 --- a/passt-repair.c +++ b/passt-repair.c @@ -113,7 +113,7 @@ int main(int argc, char **argv) if ((sb.st_mode & S_IFMT) == S_IFDIR) { char buf[sizeof(struct inotify_event) + NAME_MAX + 1] __attribute__ ((aligned(__alignof__(struct inotify_event)))); - const struct inotify_event *ev; + const struct inotify_event *ev = NULL; char path[PATH_MAX + 1]; bool found = false; ssize_t n; -- 2.43.0

1 0

[PATCH] conf: allow --fd 0
by Alyssa Ross 28 Apr '25

28 Apr '25

inetd-style socket passing traditionally starts a service with a connected socket on file descriptors 0 and 1. passt disallowing obtaining its socket from either of these descriptors made it difficult to use with super-servers providing this interface — in my case I wanted to use passt with s6-ipcserver[1]. Since (as far as I can tell) passt does not use standard input for anything else (unlike standard output), it should be safe to relax the restrictions on --fd to allow setting it to 0, enabling this use case. Link: https://skarnet.org/software/s6/s6-ipcserver.html [1] Signed-off-by: Alyssa Ross <hi(a)alyssa.is> --- conf.c | 3 ++- util.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/conf.c b/conf.c index f942851..a6d7e22 100644 --- a/conf.c +++ b/conf.c @@ -1717,7 +1717,8 @@ void conf(struct ctx *c, int argc, char **argv) fd_tap_opt = strtol(optarg, NULL, 0); if (errno || - fd_tap_opt <= STDERR_FILENO || fd_tap_opt > INT_MAX) + (fd_tap_opt != STDIN_FILENO && fd_tap_opt <= STDERR_FILENO) || + fd_tap_opt > INT_MAX) die("Invalid --fd: %s", optarg); c->fd_tap = fd_tap_opt; diff --git a/util.c b/util.c index 62a6003..f5497d4 100644 --- a/util.c +++ b/util.c @@ -875,7 +875,9 @@ void close_open_files(int argc, char **argv) errno = 0; fd = strtol(optarg, NULL, 0); - if (errno || fd <= STDERR_FILENO || fd > INT_MAX) + if (errno || + (fd != STDIN_FILENO && fd <= STDERR_FILENO) || + fd > INT_MAX) die("Invalid --fd: %s", optarg); } } while (name != -1); base-commit: 436afc30447c6f0ce516f2b38c769833114bb5f8 -- 2.47.2

2 2

[PATCH 0/2] udp: copy ttl or hop limit from socket to tap
by Jon Maloy 24 Apr '25

24 Apr '25

The following patches were meant as a preparation for supporting traceroute from remote to internal peers. Unfortunately we ran into two showstoppers during this work. 1: We cannot read IPv4 ttl from the dual-stack socket we are using as listener. setsockopt(IP_RECVTTL) is simply not supported by the kernel for IPv6 socket, even in dual-stack mode. It is not acceptable from a memory consumption viewpoint to create two listener sockets, one for IPv4 and one for IPv6, for each bound port. 2: There is no way we can create and send ICMP response messages to be sent out from the outgoing socket unless we can access it with privileges, which we cannot in PASST. Because of this, these patches are posted mostly as a documentation of the work done, maybe to be applied some time in the future if new conditions permit. Jon Maloy (2): make ttl parametrized udp: copy incoming packet TTL from socket to tap checksum.c | 7 +++-- checksum.h | 2 +- ip.h | 11 ++++--- tap.c | 16 +++++----- tap.h | 6 ++-- tcp.c | 5 ++-- udp.c | 79 +++++++++++++++++++++++++++++++++++++------------- udp_flow.c | 18 ++++++++++++ udp_internal.h | 4 +-- udp_vu.c | 5 ++-- util.c | 5 ++++ 11 files changed, 112 insertions(+), 46 deletions(-) -- 2.48.1

1 2

[0/2] udp: copy ttl or hop limit from socket to tap
by Jon Maloy 24 Apr '25

24 Apr '25

The following patches were meant as a preparation for supporting traceroute from remote to internal peers. Unfortunately we ran into two showstoppers during this work. 1: We cannot read IPv4 ttl from the dual-stack socket we are using as listener. setsockopt(IP_RECVTTL) is simply not supported by the kernel for IPv6 socket, even in dual-stack mode. It is not acceptable from a memory consumption viewpoint to create two listener sockets, one for IPv4 and one for IPv6, for each bound port. 2: There is no way we can create and send ICMP response messages to be sent out from the outgoing socket unless we can access it with privileges, which we cannot in PASST. Because of this, these patches are posted mostly as a documentation of the work done, maybe to be applied some time in the future if new conditions permit. Jon Maloy (2): make ttl parametrized udp: copy incoming packet TTL from socket to tap checksum.c | 7 +++-- checksum.h | 2 +- ip.h | 11 ++++--- tap.c | 16 +++++----- tap.h | 6 ++-- tcp.c | 5 ++-- udp.c | 79 +++++++++++++++++++++++++++++++++++++------------- udp_flow.c | 18 ++++++++++++ udp_internal.h | 4 +-- udp_vu.c | 5 ++-- util.c | 5 ++++ 11 files changed, 112 insertions(+), 46 deletions(-) -- 2.48.1

1 0

[PATCH v2 0/4] Translate source addresses for ICMP errors
by David Gibson 22 Apr '25

22 Apr '25

We now propagate ICMP errors on UDP flows back into ICMP packets on the tap interface. However, we don't always get the source address right for the synthesized message. Because ICMPs can be generated by intermediate routers, that source address might not be one of the endpoints, so the address translation we already have isn't sufficient. Implement properly translating ICMP addresses when we need to. This ended up a bit messier than I hoped, but it seems to work. A simple case to test this is: pasta --config-net --map-host-loopback=172.16.1.1 -- \ sh -c "echo hello | socat STDIO UDP4:172.16.1.1:10001" where 10001 is a port where nothing is listening on the host. Without this series, this will just time out. pasta sends an ICMP Port Unreachable message, but it's sent with source address 127.0.0.1 and so discarded by the guest. With this series, the address is properly translated and we correctly get the error from socat: 2025/04/16 19:02:37 socat[3] E read(5, 0x555c3dbf2000, 8192): Connection refused v2: * Fix a (bogus) coverity warning * Minor cosmetic changes based on Stefano's review David Gibson (4): fwd: Split out helpers for port-independent NAT treewide: Improve robustness against sockaddrs of unexpected family udp: Rework offender address handling in udp_sock_recverr() udp: Translate offender addresses for ICMP messages flow.c | 16 ++++++++-- fwd.c | 87 ++++++++++++++++++++++++++++++++++++++---------------- fwd.h | 3 ++ inany.h | 28 +++++++++++------- tcp.c | 10 +++---- udp.c | 81 ++++++++++++++++++++++++++++++++++++-------------- udp_flow.c | 6 ++-- 7 files changed, 162 insertions(+), 69 deletions(-) -- 2.49.0

2 5

[PATCH 0/4] Translate source addresses for ICMP errors
by David Gibson 17 Apr '25

17 Apr '25

We now propagate ICMP errors on UDP flows back into ICMP packets on the tap interface. However, we don't always get the source address right for the synthesized message. Because ICMPs can be generated by intermediate routers, that source address might not be one of the endpoints, so the address translation we already have isn't sufficient. Implement properly translating ICMP addresses when we need to. This ended up a bit messier than I hoped, but it seems to work. A simple case to test this is: pasta --config-net --map-host-loopback=172.16.1.1 -- \ sh -c "echo hello | socat STDIO UDP4:172.16.1.1:10001" where 10001 is a port where nothing is listening on the host. Without this series, this will just time out. pasta sends an ICMP Port Unreachable message, but it's sent with source address 127.0.0.1 and so discarded by the guest. With this series, the address is properly translated and we correctly get the error from socat: 2025/04/16 19:02:37 socat[3] E read(5, 0x555c3dbf2000, 8192): Connection refused David Gibson (4): fwd: Split out helpers for port-independent NAT treewide: Improve robustness against sockaddrs of unexpected family udp: Rework offender address handling in udp_sock_recverr() udp: Translate offender addresses for ICMP messages flow.c | 16 ++++++++-- fwd.c | 87 ++++++++++++++++++++++++++++++++++++++---------------- fwd.h | 3 ++ inany.h | 22 +++++++++----- tcp.c | 10 +++---- udp.c | 79 +++++++++++++++++++++++++++++++++++-------------- udp_flow.c | 6 ++-- 7 files changed, 157 insertions(+), 66 deletions(-) -- 2.49.0

2 9