- dev - passt

[PATCH v5 02/18] fwd_rule: Move ephemeral port probing to fwd_rule.c
by David Gibson 21 Apr '26

21 Apr '26

We want to move parsing of forward rule options to fwd_rule.c so it can eventually be shared with a configuration client. As a preliminary step, move the ephemeral port probing there, which that will need to use. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> Reviewed-by: Laurent Vivier <lvivier(a)redhat.com> --- fwd.c | 73 -------------------------------------------------- fwd.h | 6 ----- fwd_rule.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ fwd_rule.h | 6 +++++ 4 files changed, 84 insertions(+), 79 deletions(-) diff --git a/fwd.c b/fwd.c index 979c1494..a6d75b74 100644 --- a/fwd.c +++ b/fwd.c @@ -34,12 +34,6 @@ #include "arp.h" #include "ndp.h" -/* Ephemeral port range: values from RFC 6335 */ -static in_port_t fwd_ephemeral_min = (1 << 15) + (1 << 14); -static in_port_t fwd_ephemeral_max = NUM_PORTS - 1; - -#define PORT_RANGE_SYSCTL "/proc/sys/net/ipv4/ip_local_port_range" - #define NEIGH_TABLE_SLOTS 1024 #define NEIGH_TABLE_SIZE (NEIGH_TABLE_SLOTS / 2) static_assert((NEIGH_TABLE_SLOTS & (NEIGH_TABLE_SLOTS - 1)) == 0, @@ -249,73 +243,6 @@ void fwd_neigh_table_init(const struct ctx *c) fwd_neigh_table_update(c, &mga, c->our_tap_mac, true); } -/** fwd_probe_ephemeral() - Determine what ports this host considers ephemeral - * - * Work out what ports the host thinks are emphemeral and record it for later - * use by fwd_port_is_ephemeral(). If we're unable to probe, assume the range - * recommended by RFC 6335. - */ -void fwd_probe_ephemeral(void) -{ - char *line, *tab, *end; - struct lineread lr; - long min, max; - ssize_t len; - int fd; - - fd = open(PORT_RANGE_SYSCTL, O_RDONLY | O_CLOEXEC); - if (fd < 0) { - warn_perror("Unable to open %s", PORT_RANGE_SYSCTL); - return; - } - - lineread_init(&lr, fd); - len = lineread_get(&lr, &line); - close(fd); - - if (len < 0) - goto parse_err; - - tab = strchr(line, '\t'); - if (!tab) - goto parse_err; - *tab = '\0'; - - errno = 0; - min = strtol(line, &end, 10); - if (*end || errno) - goto parse_err; - - errno = 0; - max = strtol(tab + 1, &end, 10); - if (*end || errno) - goto parse_err; - - if (min < 0 || min >= (long)NUM_PORTS || - max < 0 || max >= (long)NUM_PORTS) - goto parse_err; - - fwd_ephemeral_min = min; - fwd_ephemeral_max = max; - - return; - -parse_err: - warn("Unable to parse %s", PORT_RANGE_SYSCTL); -} - -/** - * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap - * @map: Bitmap to update - */ -void fwd_port_map_ephemeral(uint8_t *map) -{ - unsigned port; - - for (port = fwd_ephemeral_min; port <= fwd_ephemeral_max; port++) - bitmap_set(map, port); -} - /* Forwarding table storage, generally accessed via pointers in struct ctx */ static struct fwd_table fwd_in; static struct fwd_table fwd_out; diff --git a/fwd.h b/fwd.h index 3e365d35..e664d1d0 100644 --- a/fwd.h +++ b/fwd.h @@ -20,12 +20,6 @@ struct flowside; -/* Number of ports for both TCP and UDP */ -#define NUM_PORTS (1U << 16) - -void fwd_probe_ephemeral(void); -void fwd_port_map_ephemeral(uint8_t *map); - #define FWD_RULE_BITS 8 #define MAX_FWD_RULES MAX_FROM_BITS(FWD_RULE_BITS) #define FWD_NO_HINT (-1) diff --git a/fwd_rule.c b/fwd_rule.c index 47d8df1c..9d489827 100644 --- a/fwd_rule.c +++ b/fwd_rule.c @@ -15,9 +15,87 @@ * Author: David Gibson <david(a)gibson.dropbear.id.au> */ +#include <errno.h> +#include <fcntl.h> #include <stdio.h> +#include <unistd.h> #include "fwd_rule.h" +#include "lineread.h" +#include "log.h" + +/* Ephemeral port range: values from RFC 6335 */ +static in_port_t fwd_ephemeral_min = (1 << 15) + (1 << 14); +static in_port_t fwd_ephemeral_max = NUM_PORTS - 1; + +#define PORT_RANGE_SYSCTL "/proc/sys/net/ipv4/ip_local_port_range" + +/** fwd_probe_ephemeral() - Determine what ports this host considers ephemeral + * + * Work out what ports the host thinks are emphemeral and record it for later + * use by fwd_port_is_ephemeral(). If we're unable to probe, assume the range + * recommended by RFC 6335. + */ +void fwd_probe_ephemeral(void) +{ + char *line, *tab, *end; + struct lineread lr; + long min, max; + ssize_t len; + int fd; + + fd = open(PORT_RANGE_SYSCTL, O_RDONLY | O_CLOEXEC); + if (fd < 0) { + warn_perror("Unable to open %s", PORT_RANGE_SYSCTL); + return; + } + + lineread_init(&lr, fd); + len = lineread_get(&lr, &line); + close(fd); + + if (len < 0) + goto parse_err; + + tab = strchr(line, '\t'); + if (!tab) + goto parse_err; + *tab = '\0'; + + errno = 0; + min = strtol(line, &end, 10); + if (*end || errno) + goto parse_err; + + errno = 0; + max = strtol(tab + 1, &end, 10); + if (*end || errno) + goto parse_err; + + if (min < 0 || min >= (long)NUM_PORTS || + max < 0 || max >= (long)NUM_PORTS) + goto parse_err; + + fwd_ephemeral_min = min; + fwd_ephemeral_max = max; + + return; + +parse_err: + warn("Unable to parse %s", PORT_RANGE_SYSCTL); +} + +/** + * fwd_port_map_ephemeral() - Mark ephemeral ports in a bitmap + * @map: Bitmap to update + */ +void fwd_port_map_ephemeral(uint8_t *map) +{ + unsigned port; + + for (port = fwd_ephemeral_min; port <= fwd_ephemeral_max; port++) + bitmap_set(map, port); +} /** * fwd_rule_addr() - Return match address for a rule diff --git a/fwd_rule.h b/fwd_rule.h index edba6782..5c7b67aa 100644 --- a/fwd_rule.h +++ b/fwd_rule.h @@ -17,6 +17,9 @@ #include "inany.h" #include "bitmap.h" +/* Number of ports for both TCP and UDP */ +#define NUM_PORTS (1U << 16) + /* Forwarding capability bits */ #define FWD_CAP_IPV4 BIT(0) #define FWD_CAP_IPV6 BIT(1) @@ -51,6 +54,9 @@ struct fwd_rule { uint8_t flags; }; +void fwd_probe_ephemeral(void); +void fwd_port_map_ephemeral(uint8_t *map); + #define FWD_RULE_STRLEN \ (IPPROTO_STRLEN - 1 \ + INANY_ADDRSTRLEN - 1 \ -- 2.53.0

1 0

[PATCH v4 01/17] conf, fwd: Stricter rule checking in fwd_rule_add()
by David Gibson 21 Apr '26

21 Apr '26

Although fwd_rule_add() performs some sanity checks on the rule it is given, there are invalid rules we don't check for, assuming that its callers will do that. That won't be enough when we can get rules inserted by a dynamic update client without going through the existing parsing code. So, add stricter checks to fwd_rule_add(), which is now possible thanks to the capabilities bits in the struct fwd_table. Where those duplicate existing checks in the callers, remove the old copies. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- conf.c | 21 --------------------- fwd.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 26 deletions(-) diff --git a/conf.c b/conf.c index 6e884e54..b470b0d8 100644 --- a/conf.c +++ b/conf.c @@ -176,8 +176,6 @@ static void conf_ports_range_except(struct fwd_table *fwd, uint8_t proto, die("Invalid interface name: %s", ifname); } - assert(first != 0); - for (base = first; base <= last; base++) { if (exclude && bitmap_isset(exclude, base)) continue; @@ -310,10 +308,6 @@ static void conf_ports_spec(struct fwd_table *fwd, uint8_t proto, if (p != ep) /* Garbage after the ranges */ goto bad; - if (orig_range.first == 0) { - die("Can't forward port 0 included in '%s'", spec); - } - conf_ports_range_except(fwd, proto, addr, ifname, orig_range.first, orig_range.last, exclude, @@ -356,11 +350,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) return; } - if (proto == IPPROTO_TCP && !(fwd->caps & FWD_CAP_TCP)) - die("TCP port forwarding requested but TCP is disabled"); - if (proto == IPPROTO_UDP && !(fwd->caps & FWD_CAP_UDP)) - die("UDP port forwarding requested but UDP is disabled"); - strncpy(buf, optarg, sizeof(buf) - 1); if ((spec = strchr(buf, '/'))) { @@ -405,16 +394,6 @@ static void conf_ports(char optname, const char *optarg, struct fwd_table *fwd) addr = NULL; } - if (addr) { - if (!(fwd->caps & FWD_CAP_IPV4) && inany_v4(addr)) { - die("IPv4 is disabled, can't use -%c %s", - optname, optarg); - } else if (!(fwd->caps & FWD_CAP_IPV6) && !inany_v4(addr)) { - die("IPv6 is disabled, can't use -%c %s", - optname, optarg); - } - } - if (optname == 'T' || optname == 'U') { assert(!addr && !ifname); diff --git a/fwd.c b/fwd.c index c7fd1a9d..979c1494 100644 --- a/fwd.c +++ b/fwd.c @@ -367,17 +367,59 @@ int fwd_rule_add(struct fwd_table *fwd, const struct fwd_rule *new) new->first, new->last); return -EINVAL; } + if (!new->first) { + warn("Forwarding rule attempts to map from port 0"); + return -EINVAL; + } + if (!new->to || + (in_port_t)(new->to + new->last - new->first) < new->to) { + warn("Forwarding rule attempts to map to port 0"); + return -EINVAL; + } if (new->flags & ~allowed_flags) { warn("Rule has invalid flags 0x%hhx", new->flags & ~allowed_flags); return -EINVAL; } - if (new->flags & FWD_DUAL_STACK_ANY && - !inany_equals(&new->addr, &inany_any6)) { - char astr[INANY_ADDRSTRLEN]; + if (new->flags & FWD_DUAL_STACK_ANY) { + if (!inany_equals(&new->addr, &inany_any6)) { + char astr[INANY_ADDRSTRLEN]; - warn("Dual stack rule has non-wildcard address %s", - inany_ntop(&new->addr, astr, sizeof(astr))); + warn("Dual stack rule has non-wildcard address %s", + inany_ntop(&new->addr, astr, sizeof(astr))); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV4)) { + warn("Dual stack forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!(fwd->caps & FWD_CAP_IPV6)) { + warn("Dual stack forward, but IPv6 not enabled"); + return -EINVAL; + } + } else { + if (inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV4)) { + warn("IPv4 forward, but IPv4 not enabled"); + return -EINVAL; + } + if (!inany_v4(&new->addr) && !(fwd->caps & FWD_CAP_IPV6)) { + warn("IPv6 forward, but IPv6 not enabled"); + return -EINVAL; + } + } + if (new->proto == IPPROTO_TCP) { + if (!(fwd->caps & FWD_CAP_TCP)) { + warn("Can't add TCP forwarding rule, TCP not enabled"); + return -EINVAL; + } + } else if (new->proto == IPPROTO_UDP) { + if (!(fwd->caps & FWD_CAP_UDP)) { + warn("Can't add UDP forwarding rule, UDP not enabled"); + return -EINVAL; + } + } else { + warn("Unsupported protocol 0x%hhx (%s) for forwarding rule", + new->proto, ipproto_name(new->proto)); return -EINVAL; } -- 2.53.0

1 0

[PATCH v1] tcp: Handle errors from tcp_send_flag()
by Anshu Kumari 21 Apr '26

21 Apr '26

tcp_send_flag() can return error codes from tcp_prepare_flags() failing TCP_INFO, or from failure to collect buffers on the vhost-user path. These errors indicate the connection requires resetting. Most callers of tcp_send_flag() were ignoring the error code and carrying on as if nothing was wrong. Check the return value at each call site and handle the error appropriately: - in tcp_data_from_tap(), return -1 so the caller resets - in tcp_tap_handler(), goto reset - in tcp_timer_handler()/tcp_sock_handler()/tcp_conn_from_sock_finish(), call tcp_rst() and return - in tcp_tap_conn_from_sock(), set CLOSING flag (flow not yet active) - in tcp_keepalive(), call tcp_rst() and continue the loop - in tcp_flow_migrate_target_ext(), goto fail The call in tcp_rst_do() is left unchecked: we are already resetting, and tcp_sock_rst() still needs to run regardless. Bug: https://bugs.passt.top/show_bug.cgi?id=194 Signed-off-by: Anshu Kumari <anskuma(a)redhat.com> --- tcp.c | 59 ++++++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 15 deletions(-) diff --git a/tcp.c b/tcp.c index 8ea9be8..9ce671a 100644 --- a/tcp.c +++ b/tcp.c @@ -1917,7 +1917,9 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn, "keep-alive sequence: %u, previous: %u", seq, conn->seq_from_tap); - tcp_send_flag(c, conn, ACK); + if (tcp_send_flag(c, conn, ACK)) + return -1; + tcp_timer_ctl(c, conn); if (setsockopt(conn->sock, SOL_SOCKET, SO_KEEPALIVE, @@ -2043,14 +2045,16 @@ eintr: * Then swiftly looked away and left. */ conn->seq_from_tap = seq_from_tap; - tcp_send_flag(c, conn, ACK); + if (tcp_send_flag(c, conn, ACK)) + return -1; } if (errno == EINTR) goto eintr; if (errno == EAGAIN || errno == EWOULDBLOCK) { - tcp_send_flag(c, conn, ACK | DUP_ACK); + if (tcp_send_flag(c, conn, ACK | DUP_ACK)) + return -1; return p->count - idx; } @@ -2070,7 +2074,8 @@ out: */ if (conn->seq_dup_ack_approx != (conn->seq_from_tap & 0xff)) { conn->seq_dup_ack_approx = conn->seq_from_tap & 0xff; - tcp_send_flag(c, conn, ACK | DUP_ACK); + if (tcp_send_flag(c, conn, ACK | DUP_ACK)) + return -1; } return p->count - idx; } @@ -2084,7 +2089,8 @@ out: conn_event(c, conn, TAP_FIN_RCVD); } else { - tcp_send_flag(c, conn, ACK_IF_NEEDED); + if (tcp_send_flag(c, conn, ACK_IF_NEEDED)) + return -1; } return p->count - idx; @@ -2122,7 +2128,10 @@ static void tcp_conn_from_sock_finish(const struct ctx *c, return; } - tcp_send_flag(c, conn, ACK); + if (tcp_send_flag(c, conn, ACK)) { + tcp_rst(c, conn); + return; + } /* The client might have sent data already, which we didn't * dequeue waiting for SYN,ACK from tap -- check now. @@ -2308,7 +2317,9 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, goto reset; } - tcp_send_flag(c, conn, ACK); + if (tcp_send_flag(c, conn, ACK)) + goto reset; + conn_event(c, conn, SOCK_FIN_SENT); return 1; @@ -2388,7 +2399,9 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, } conn_event(c, conn, SOCK_FIN_SENT); - tcp_send_flag(c, conn, ACK); + if (tcp_send_flag(c, conn, ACK)) + goto reset; + ack_due = 0; /* If we received a FIN, but the socket is in TCP_ESTABLISHED @@ -2478,7 +2491,11 @@ static void tcp_tap_conn_from_sock(const struct ctx *c, union flow *flow, conn->wnd_from_tap = WINDOW_DEFAULT; - tcp_send_flag(c, conn, SYN); + if (tcp_send_flag(c, conn, SYN)) { + conn_flag(c, conn, CLOSING); + return; + } + conn_flag(c, conn, ACK_FROM_TAP_DUE); tcp_get_sndbuf(conn); @@ -2585,7 +2602,10 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) return; if (conn->flags & ACK_TO_TAP_DUE) { - tcp_send_flag(c, conn, ACK_IF_NEEDED); + if (tcp_send_flag(c, conn, ACK_IF_NEEDED)) { + tcp_rst(c, conn); + return; + } tcp_timer_ctl(c, conn); } else if (conn->flags & ACK_FROM_TAP_DUE) { if (!(conn->events & ESTABLISHED)) { @@ -2598,7 +2618,10 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) tcp_rst(c, conn); } else { flow_trace(conn, "SYN timeout, retry"); - tcp_send_flag(c, conn, SYN); + if (tcp_send_flag(c, conn, SYN)) { + tcp_rst(c, conn); + return; + } conn->retries++; conn_flag(c, conn, SYN_RETRIED); tcp_timer_ctl(c, conn); @@ -2662,8 +2685,11 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref, tcp_data_from_sock(c, conn); if (events & EPOLLOUT) { - if (tcp_update_seqack_wnd(c, conn, false, NULL)) - tcp_send_flag(c, conn, ACK); + if (tcp_update_seqack_wnd(c, conn, false, NULL) && + tcp_send_flag(c, conn, ACK)) { + tcp_rst(c, conn); + return; + } } return; @@ -2903,7 +2929,8 @@ static void tcp_keepalive(struct ctx *c, const struct timespec *now) if (conn->tap_inactive) { flow_dbg(conn, "No tap activity for least %us, send keepalive", KEEPALIVE_INTERVAL); - tcp_send_flag(c, conn, KEEPALIVE); + if (tcp_send_flag(c, conn, KEEPALIVE)) + tcp_rst(c, conn); } /* Ready to check fot next interval */ @@ -3926,7 +3953,9 @@ int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd if (tcp_set_peek_offset(conn, peek_offset)) goto fail; - tcp_send_flag(c, conn, ACK); + if (tcp_send_flag(c, conn, ACK)) + goto fail; + tcp_data_from_sock(c, conn); if ((rc = tcp_epoll_ctl(conn))) { -- 2.53.0

3 5

[PATCH 00/13] Improvements to static checker invocation
by David Gibson 21 Apr '26

21 Apr '26

While working on pesto, I ran into a number of awkward errors with the static checkers. This series reworks the invocation of the checkers in a way that will let us deal with that. As a bonus, it also gives us static checking for passt-repair and qrap. It also makes a few other cleanups to the Makefile that seemed natural along the way. David Gibson (13): Makefile: Use make variables for static checker configuration cppcheck: Split out essential defines into a BASE_CPPFLAGS variable Makefile: Remove preprocessor flags from $(FLAGS) Makefile: Remove non-standard $(FLAGS) variable Makefile: Make conditional definition of $(BIN) clearer Makefile: Use common binary compilation rule Makefile: Remove unhelpful $(HEADERS) variable Makefile: Add header dependencies for secondary binaries Makefile: Split static checker targets passt-repair: Split out inotify handling to its own function passt-repair: Simplify construction of Unix path from inotify passt-repair: Run static checkers qrap: Run static checkers Makefile | 106 ++++++++++++++++++------------ linux_dep.h | 2 +- passt-repair.c | 171 +++++++++++++++++++++++++++---------------------- qrap.c | 42 +++++++----- 4 files changed, 188 insertions(+), 133 deletions(-) -- 2.53.0

1 13

[PATCH 02/13] cppcheck: Split out essential defines into a BASE_CPPFLAGS variable
by David Gibson 21 Apr '26

21 Apr '26

Our cppcheck target need certain flags from the compiler so that they it can analyse the code correctly. Currently we extract these rather awkwardly from FLAGS / CFLAGS / CPPFLAGS. But this means we inhibit one of cppcheck's features: by default it will attempt to analyse paths for all combinations of compile time options, not just a single one. Analysing *all* paths doesn't work for us because many of the -D options we use are essential to compile at all, so unless we supply those to cppcheck, overriding the default behaviour we get many spurious errors. At the moment, however, we give cppcheck *all* our -D options, including conditional / configurable ones, not just the essential ones. All cppcheck really needs here is those essential -D options. Split those into a separate variable, and use that directly rather than the clunky $(filter) expression. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- Makefile | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 17e70d22..0de98375 100644 --- a/Makefile +++ b/Makefile @@ -30,11 +30,15 @@ ifeq ($(shell $(CC) -O2 -dM -E - < /dev/null 2>&1 | grep ' _FORTIFY_SOURCE ' > / FORTIFY_FLAG := -D_FORTIFY_SOURCE=2 endif +# Require preprocessor flags we can't build without +BASE_CPPFLAGS := -D_XOPEN_SOURCE=700 -D_GNU_SOURCE \ + -DPAGE_SIZE=$(shell getconf PAGE_SIZE) \ + -DVERSION=\"$(VERSION)\" + FLAGS := -Wall -Wextra -Wno-format-zero-length -Wformat-security -FLAGS += -pedantic -std=c11 -D_XOPEN_SOURCE=700 -D_GNU_SOURCE +FLAGS += -pedantic -std=c11 FLAGS += $(FORTIFY_FLAG) -O2 -pie -fPIE -FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE) -FLAGS += -DVERSION=\"$(VERSION)\" +FLAGS += $(BASE_CPPFLAGS) FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ @@ -195,6 +199,4 @@ CPPCHECK_FLAGS = --std=c11 --error-exitcode=1 --enable=all --force \ -D CPPCHECK_6936 cppcheck: $(PASST_SRCS) $(HEADERS) - $(CPPCHECK) $(CPPCHECK_FLAGS) \ - $(filter -D%,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) $^ \ - $^ + $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^ -- 2.53.0

1 0

[PATCH v3 00/11] Rework forwarding option parsing
by David Gibson 21 Apr '26

21 Apr '26

This series makes a number of significant reworks to how we process forwarding options (-t, -u, -T and -U) in passt & pasta. This is largely motivated by moving towards being able to share this code with a configuration update tool. However, along the way it also enables some forwarding configurations that were technically possible with the forwarding table but couldn't be specified on the command line, in particular bug 180. There is still a bunch of work needed to make the parsing code truly shareable with pesto, but this is a solid start. v3: * Removed already merged patches * Further revisions based on Stefano's review, including * Improved example text in manpage and usage * More flexible rule dumping * Re-integrated conflict checking in fwd_rule_add() v2: * Assorted minor changes based on Stefano's review, including * Explicitly state "guest or namespace" in the manpage * Clearer description of @rulesocks field * Worked around a gcc < 15 bug causing a false positive warning * Update man page and usage() for new capabilities * Additional patches moving rule parsing out of conf.c David Gibson (11): doc: Rework man page description of port specifiers conf: Move "all" handling to port specifier conf: Allow user-specified auto-scanned port forwarding ranges conf: Move SO_BINDTODEVICE workaround to conf_ports() conf: Don't pass raw commandline argument to conf_ports_spec() fwd, conf: Add capabilities bits to each forwarding table conf, fwd: Stricter rule checking in fwd_rule_add() fwd_rule: Move ephemeral port probing to fwd_rule.c fwd, conf: Move rule parsing code to fwd_rule.[ch] fwd_rule: Move conflict checking back within fwd_rule_add() fwd: Generalise fwd_rules_info() conf.c | 434 +++-------------------------------------- fwd.c | 142 ++------------ fwd.h | 37 ---- fwd_rule.c | 563 ++++++++++++++++++++++++++++++++++++++++++++++++++--- fwd_rule.h | 68 ++++++- passt.1 | 72 +++++-- 6 files changed, 704 insertions(+), 612 deletions(-) -- 2.53.0

3 26

[RFC PATCH] tcp: Replace send buffer boost with EPOLLOUT monitoring
by Yumei Huang 21 Apr '26

21 Apr '26

Currently we use the SNDBUF boost mechanism to force TCP auto-tuning. However, it doesn't always work, and sometimes causes a lot of retransmissions. As a result, the throughput suffers. This patch replaces it with monitoring EPOLLOUT when sendmsg() failure (with EAGAIN and EWOULDBLOCK) and partial sends occur. Tested with iperf3 inside pasta: throughput is now comparable to running iperf3 directly on the host without pasta. However, retransmissions can still be elevated when RTT >= 50ms. For example, when RTT is between 200ms and 500ms, retransmission count varies from 30 to 120 in roughly 80% of test runs. Link: https://bugs.passt.top/show_bug.cgi?id=138 Suggested-by: Stefano Brivio <sbrivio(a)redhat.com> Signed-off-by: Yumei Huang <yuhuang(a)redhat.com> --- tcp.c | 57 +++++++++++++++++---------------------------------------- 1 file changed, 17 insertions(+), 40 deletions(-) diff --git a/tcp.c b/tcp.c index 9d91c3c..f7e3932 100644 --- a/tcp.c +++ b/tcp.c @@ -353,13 +353,6 @@ enum { #define LOW_RTT_TABLE_SIZE 8 #define LOW_RTT_THRESHOLD 10 /* us */ -/* Parameters to temporarily exceed sending buffer to force TCP auto-tuning */ -#define SNDBUF_BOOST_BYTES_RTT_LO 2500 /* B * s: no boost until here */ -/* ...examples: 5 MB sent * 500 ns RTT, 250 kB * 10 ms, 8 kB * 300 ms */ -#define SNDBUF_BOOST_FACTOR 150 /* % */ -#define SNDBUF_BOOST_BYTES_RTT_HI 6000 /* apply full boost factor */ -/* 12 MB sent * 500 ns RTT, 600 kB * 10 ms, 20 kB * 300 ms */ - /* Ratio of buffer to bandwidth * delay product implying interactive traffic */ #define SNDBUF_TO_BW_DELAY_INTERACTIVE /* > */ 20 /* (i.e. < 5% of buffer) */ @@ -1023,35 +1016,6 @@ size_t tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn, return MAX(l3len + sizeof(struct ethhdr), ETH_ZLEN); } -/** - * tcp_sndbuf_boost() - Calculate limit of sending buffer to force auto-tuning - * @conn: Connection pointer - * @tinfo: tcp_info from kernel, must be pre-fetched - * - * Return: increased sending buffer to use as a limit for advertised window - */ -static unsigned long tcp_sndbuf_boost(const struct tcp_tap_conn *conn, - const struct tcp_info_linux *tinfo) -{ - unsigned long bytes_rtt_product; - - if (!bytes_acked_cap) - return SNDBUF_GET(conn); - - /* This is *not* a bandwidth-delay product, but it's somewhat related: - * as we send more data (usually at the beginning of a connection), we - * try to make the sending buffer progressively grow, with the RTT as a - * factor (longer delay, bigger buffer needed). - */ - bytes_rtt_product = (long long)tinfo->tcpi_bytes_acked * - tinfo->tcpi_rtt / 1000 / 1000; - - return clamped_scale(SNDBUF_GET(conn), bytes_rtt_product, - SNDBUF_BOOST_BYTES_RTT_LO, - SNDBUF_BOOST_BYTES_RTT_HI, - SNDBUF_BOOST_FACTOR); -} - /** * tcp_update_seqack_wnd() - Update ACK sequence and window to guest/tap * @c: Execution context @@ -1174,8 +1138,6 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, if ((int)sendq > SNDBUF_GET(conn)) /* Due to memory pressure? */ limit = 0; - else if ((int)tinfo->tcpi_snd_wnd > SNDBUF_GET(conn)) - limit = tcp_sndbuf_boost(conn, tinfo) - (int)sendq; else limit = SNDBUF_GET(conn) - (int)sendq; @@ -2050,14 +2012,28 @@ eintr: if (errno == EAGAIN || errno == EWOULDBLOCK) { tcp_send_flag(c, conn, ACK | DUP_ACK); + uint32_t events = tcp_conn_epoll_events(conn->events, + conn->flags); + events |= EPOLLOUT; + if (flow_epoll_set(&conn->f, EPOLL_CTL_MOD, events, + conn->sock, !TAPSIDE(conn)) < 0) + debug("Failed to add EPOLLOUT"); return p->count - idx; - } return -1; } - if (n < (int)(seq_from_tap - conn->seq_from_tap)) + if (n < (int)(seq_from_tap - conn->seq_from_tap)) { partial_send = 1; + uint32_t events = tcp_conn_epoll_events(conn->events, + conn->flags); + events |= EPOLLOUT; + if (flow_epoll_set(&conn->f, EPOLL_CTL_MOD, events, conn->sock, + !TAPSIDE(conn)) < 0) + debug("Failed to add EPOLLOUT"); + } else { + tcp_epoll_ctl(conn); + } conn->seq_from_tap += n; @@ -2661,6 +2637,7 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref, tcp_data_from_sock(c, conn); if (events & EPOLLOUT) { + tcp_epoll_ctl(conn); if (tcp_update_seqack_wnd(c, conn, false, NULL)) tcp_send_flag(c, conn, ACK); } -- 2.53.0

2 1

[PATCH v2 00/23] Rework forwarding option parsing
by David Gibson 16 Apr '26

16 Apr '26

This series makes a number of significant reworks to how we process forwarding options (-t, -u, -T and -U) in passt & pasta. This is largely motivated by moving towards being able to share this code with a configuration update tool. However, along the way it also enables some forwarding configurations that were technically possible with the forwarding table but couldn't be specified on the command line, in particular bug 180. There is still a bunch of work needed to make the parsing code truly shareable with pesto, but this is a solid start. v2: * Assorted minor changes based on Stefano's review, including * Explicitly state "guest or namespace" in the manpage * Clearer description of @rulesocks field * Worked around a gcc < 15 bug causing a false positive warning * Update man page and usage() for new capabilities * Additional patches moving rule parsing out of conf.c David Gibson (23): conf: Split parsing of port specifiers from the rest of -[tuTU] parsing conf: Simplify handling of default forwarding mode conf: Move first pass handling of -[TU] next to handling of -[tu] doc: Consolidate -[tu] option descriptions for passt and pasta conf: Permit -[tTuU] all in pasta mode fwd: Better split forwarding rule specification from associated sockets fwd_rule: Move forwarding rule formatting conf: Pass protocol explicitly to conf_ports_range_except() fwd: Split rule building from rule adding fwd_rule: Move rule conflict checking from fwd_rule_add() to caller fwd: Improve error handling in fwd_rule_add() conf: Don't be strict about exclusivity of forwarding mode conf: Rework stepping through chunks of port specifiers conf: Rework checking for garbage after a range doc: Rework man page description of port specifiers conf: Move "all" handling to port specifier conf: Allow user-specified auto-scanned port forwarding ranges conf: Move SO_BINDTODEVICE workaround to conf_ports() conf: Don't pass raw commandline argument to conf_ports_spec() fwd, conf: Add capabilities bits to each forwarding table conf, fwd: Stricter rule checking in fwd_rule_add() fwd_rule: Move ephemeral port probing to fwd_rule.c fwd, conf: Move rule parsing code to fwd_rule.[ch] Makefile | 10 +- conf.c | 524 ++++++----------------------------------- fwd.c | 261 +++------------------ fwd.h | 46 ---- fwd_rule.c | 676 +++++++++++++++++++++++++++++++++++++++++++++++++++++ fwd_rule.h | 59 +++++ passt.1 | 288 ++++++++++------------- 7 files changed, 973 insertions(+), 891 deletions(-) create mode 100644 fwd_rule.c -- 2.53.0

2 36

[PATCH v3] tap, tcp, udp: Use rate-limited logging
by Anshu Kumari 16 Apr '26

16 Apr '26

Now that rate-limited logging macros are available, promote several debug messages to higher severity levels. These messages were previously kept at debug to prevent guests from flooding host logs, but with rate limiting they can safely be made visible in normal operation. In tap.c, refactor tap4_is_fragment() to use warn_ratelimit() instead of its ad-hoc rate limiting, and promote the guest MAC address change message to info level. In tcp.c, promote the invalid TCP SYN endpoint message to warn level. In udp.c, promote dropped datagram messages to warn level, and rate-limit the unrecoverable socket error message. In udp_flow.c, promote flow allocation failures to err_ratelimit. Link: https://bugs.passt.top/show_bug.cgi?id=134 Signed-off-by: Anshu Kumari <anskuma(a)redhat.com> --- v3: - Promote flow allocation failures to err_ratelimit from warn_ratelimit. - Fix remaining alignment issues. - Cc Volker --- tap.c | 16 +++++----------- tcp.c | 6 +++--- udp.c | 12 ++++++------ udp_flow.c | 13 +++++++------ 4 files changed, 21 insertions(+), 26 deletions(-) diff --git a/tap.c b/tap.c index 1049e02..7d06189 100644 --- a/tap.c +++ b/tap.c @@ -99,7 +99,6 @@ static PACKET_POOL_NOINIT(pool_tap4, TAP_MSGS_IP4); static PACKET_POOL_NOINIT(pool_tap6, TAP_MSGS_IP6); #define TAP_SEQS 128 /* Different L4 tuples in one batch */ -#define FRAGMENT_MSG_RATE 10 /* # seconds between fragment warnings */ /** * tap_l2_max_len() - Maximum frame size (including L2 header) for current mode @@ -686,17 +685,11 @@ static bool tap4_is_fragment(const struct iphdr *iph, const struct timespec *now) { if (ntohs(iph->frag_off) & ~IP_DF) { - /* Ratelimit messages */ - static time_t last_message; static unsigned num_dropped; num_dropped++; - if (now->tv_sec - last_message > FRAGMENT_MSG_RATE) { - warn("Can't process IPv4 fragments (%u dropped)", - num_dropped); - last_message = now->tv_sec; - num_dropped = 0; - } + warn_ratelimit(now, "Can't process IPv4 fragments (%u dropped)", + num_dropped); return true; } return false; @@ -1115,8 +1108,9 @@ void tap_add_packet(struct ctx *c, struct iov_tail *data, char bufmac[ETH_ADDRSTRLEN]; memcpy(c->guest_mac, eh->h_source, ETH_ALEN); - debug("New guest MAC address observed: %s", - eth_ntop(c->guest_mac, bufmac, sizeof(bufmac))); + info_ratelimit(now, "New guest MAC address observed: %s", + eth_ntop(c->guest_mac, bufmac, + sizeof(bufmac))); proto_update_l2_buf(c->guest_mac); } diff --git a/tcp.c b/tcp.c index 8ea9be8..071bcae 100644 --- a/tcp.c +++ b/tcp.c @@ -1688,9 +1688,9 @@ static void tcp_conn_from_tap(const struct ctx *c, sa_family_t af, !inany_is_unicast(&ini->oaddr) || ini->oport == 0) { char sstr[INANY_ADDRSTRLEN], dstr[INANY_ADDRSTRLEN]; - debug("Invalid endpoint in TCP SYN: %s:%hu -> %s:%hu", - inany_ntop(&ini->eaddr, sstr, sizeof(sstr)), ini->eport, - inany_ntop(&ini->oaddr, dstr, sizeof(dstr)), ini->oport); + warn_ratelimit(now, "Invalid endpoint in TCP SYN: %s:%hu -> %s:%hu", + inany_ntop(&ini->eaddr, sstr, sizeof(sstr)), ini->eport, + inany_ntop(&ini->oaddr, dstr, sizeof(dstr)), ini->oport); goto cancel; } diff --git a/udp.c b/udp.c index 1fc5a42..52260b9 100644 --- a/udp.c +++ b/udp.c @@ -871,7 +871,7 @@ void udp_sock_fwd(const struct ctx *c, int s, int rule_hint, /* Clear errors & carry on */ if (udp_sock_errs(c, s, FLOW_SIDX_NONE, frompif, port) < 0) { - err( + err_ratelimit(now, "UDP: Unrecoverable error on listening socket: (%s port %hu)", pif_name(frompif), port); /* FIXME: what now? close/re-open socket? */ @@ -898,7 +898,7 @@ void udp_sock_fwd(const struct ctx *c, int s, int rule_hint, pif_name(frompif), pif_name(topif)); discard = true; } else { - debug("Discarding datagram without flow"); + warn_ratelimit(now, "Discarding datagram without flow"); discard = true; } @@ -1042,10 +1042,10 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, if (!(uflow = udp_at_sidx(tosidx))) { char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN]; - debug("Dropping datagram with no flow %s %s:%hu -> %s:%hu", - pif_name(pif), - inet_ntop(af, saddr, sstr, sizeof(sstr)), src, - inet_ntop(af, daddr, dstr, sizeof(dstr)), dst); + warn_ratelimit(now, "Dropping datagram with no flow %s %s:%hu -> %s:%hu", + pif_name(pif), + inet_ntop(af, saddr, sstr, sizeof(sstr)), src, + inet_ntop(af, daddr, dstr, sizeof(dstr)), dst); return 1; } diff --git a/udp_flow.c b/udp_flow.c index 7e2453e..35417bc 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -235,8 +235,9 @@ flow_sidx_t udp_flow_from_sock(const struct ctx *c, uint8_t pif, if (!(flow = flow_alloc())) { char sastr[SOCKADDR_STRLEN]; - debug("Couldn't allocate flow for UDP datagram from %s %s", - pif_name(pif), sockaddr_ntop(s_in, sastr, sizeof(sastr))); + err_ratelimit(now, "Couldn't allocate flow for UDP datagram from %s %s", + pif_name(pif), + sockaddr_ntop(s_in, sastr, sizeof(sastr))); return FLOW_SIDX_NONE; } @@ -292,10 +293,10 @@ flow_sidx_t udp_flow_from_tap(const struct ctx *c, if (!(flow = flow_alloc())) { char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN]; - debug("Couldn't allocate flow for UDP datagram from %s %s:%hu -> %s:%hu", - pif_name(pif), - inet_ntop(af, saddr, sstr, sizeof(sstr)), srcport, - inet_ntop(af, daddr, dstr, sizeof(dstr)), dstport); + err_ratelimit(now, "Couldn't allocate flow for UDP datagram from %s %s:%hu -> %s:%hu", + pif_name(pif), + inet_ntop(af, saddr, sstr, sizeof(sstr)), srcport, + inet_ntop(af, daddr, dstr, sizeof(dstr)), dstport); return FLOW_SIDX_NONE; } -- 2.53.0

3 4

[PATCH v2] conf: use a single buffer for print formatting in conf_print()
by Jon Maloy 15 Apr '26

15 Apr '26

The function conf_print() uses three different buffers as target for address print formatting. This is unnecessary, as a single buffer of length INANY_ADDRSTRLEN has sufficient space for all address types, IPv4, IPv6 and MAC. There is no risk for conflicts, since all formatting is followed by an immediate info() printout. To make our life easier in the following commits, we do this simplification here. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v2: - Changed buffer length to INANY_ADDSTRLEN - Eliminated use of same buffer in same info() printout. --- conf.c | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/conf.c b/conf.c index dafac46..f13fef6 100644 --- a/conf.c +++ b/conf.c @@ -1136,11 +1136,12 @@ enum passt_modes conf_mode(int argc, char *argv[]) */ static void conf_print(const struct ctx *c) { - char buf4[INET_ADDRSTRLEN], buf6[INET6_ADDRSTRLEN]; - char bufmac[ETH_ADDRSTRLEN], ifn[IFNAMSIZ]; + char buf[INANY_ADDRSTRLEN]; int i; if (c->ifi4 > 0 || c->ifi6 > 0) { + char ifn[IFNAMSIZ]; + info("Template interface: %s%s%s%s%s", c->ifi4 > 0 ? if_indextoname(c->ifi4, ifn) : "", c->ifi4 > 0 ? " (IPv4)" : "", @@ -1158,28 +1159,27 @@ static void conf_print(const struct ctx *c) *c->ip6.ifname_out ? " (IPv6)" : ""); } - if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_out) || - !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out)) { - info("Outbound address: %s%s%s", - IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_out) ? "" : - inet_ntop(AF_INET, &c->ip4.addr_out, buf4, sizeof(buf4)), - (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_out) && - !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out)) ? ", " : "", - IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out) ? "" : - inet_ntop(AF_INET6, &c->ip6.addr_out, buf6, sizeof(buf6))); + if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr_out)) { + inet_ntop(AF_INET, &c->ip4.addr_out, buf, sizeof(buf)); + info("Outbound IPv4 address: %s", buf); + } + + if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr_out)) { + inet_ntop(AF_INET6, &c->ip6.addr_out, buf, sizeof(buf)); + info("Outbound IPv6 address: %s", buf); } if (c->mode == MODE_PASTA && !c->splice_only) info("Namespace interface: %s", c->pasta_ifn); info("MAC:"); - info(" host: %s", eth_ntop(c->our_tap_mac, bufmac, sizeof(bufmac))); + info(" host: %s", eth_ntop(c->our_tap_mac, buf, sizeof(buf))); if (c->ifi4) { if (!IN4_IS_ADDR_UNSPECIFIED(&c->ip4.map_host_loopback)) info(" NAT to host 127.0.0.1: %s", inet_ntop(AF_INET, &c->ip4.map_host_loopback, - buf4, sizeof(buf4))); + buf, sizeof(buf))); if (!c->no_dhcp) { uint32_t mask; @@ -1188,12 +1188,12 @@ static void conf_print(const struct ctx *c) info("DHCP:"); info(" assign: %s", - inet_ntop(AF_INET, &c->ip4.addr, buf4, sizeof(buf4))); + inet_ntop(AF_INET, &c->ip4.addr, buf, sizeof(buf))); info(" mask: %s", - inet_ntop(AF_INET, &mask, buf4, sizeof(buf4))); + inet_ntop(AF_INET, &mask, buf, sizeof(buf))); info(" router: %s", inet_ntop(AF_INET, &c->ip4.guest_gw, - buf4, sizeof(buf4))); + buf, sizeof(buf))); } for (i = 0; i < ARRAY_SIZE(c->ip4.dns); i++) { @@ -1201,8 +1201,8 @@ static void conf_print(const struct ctx *c) break; if (!i) info("DNS:"); - inet_ntop(AF_INET, &c->ip4.dns[i], buf4, sizeof(buf4)); - info(" %s", buf4); + inet_ntop(AF_INET, &c->ip4.dns[i], buf, sizeof(buf)); + info(" %s", buf); } for (i = 0; *c->dns_search[i].n; i++) { @@ -1216,7 +1216,7 @@ static void conf_print(const struct ctx *c) if (!IN6_IS_ADDR_UNSPECIFIED(&c->ip6.map_host_loopback)) info(" NAT to host ::1: %s", inet_ntop(AF_INET6, &c->ip6.map_host_loopback, - buf6, sizeof(buf6))); + buf, sizeof(buf))); if (!c->no_ndp && !c->no_dhcpv6) info("NDP/DHCPv6:"); @@ -1228,12 +1228,12 @@ static void conf_print(const struct ctx *c) goto dns6; info(" assign: %s", - inet_ntop(AF_INET6, &c->ip6.addr, buf6, sizeof(buf6))); + inet_ntop(AF_INET6, &c->ip6.addr, buf, sizeof(buf))); info(" router: %s", - inet_ntop(AF_INET6, &c->ip6.guest_gw, buf6, sizeof(buf6))); + inet_ntop(AF_INET6, &c->ip6.guest_gw, buf, sizeof(buf))); info(" our link-local: %s", inet_ntop(AF_INET6, &c->ip6.our_tap_ll, - buf6, sizeof(buf6))); + buf, sizeof(buf))); dns6: for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) { @@ -1241,8 +1241,8 @@ dns6: break; if (!i) info("DNS:"); - inet_ntop(AF_INET6, &c->ip6.dns[i], buf6, sizeof(buf6)); - info(" %s", buf6); + inet_ntop(AF_INET6, &c->ip6.dns[i], buf, sizeof(buf)); + info(" %s", buf); } for (i = 0; *c->dns_search[i].n; i++) { -- 2.52.0

2 1