- dev - passt

[PATCH v2 3/4] tcp: Extend tcp_send_flag() to send TCP keepalive segments
by David Gibson 06 Feb '26

06 Feb '26

TCP keepalives aren't technically a flag, but they are a zero-data segment so they can be generated with only a small modification to tcp_{buf,vu}_send_flag(). Implement this, using a new "pseudo-flag" value (similar to DUP_ACK), KEEPALIVE. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp_buf.c | 4 ++++ tcp_internal.h | 2 ++ tcp_vu.c | 3 +++ 3 files changed, 9 insertions(+) diff --git a/tcp_buf.c b/tcp_buf.c index d2925410..18bdce81 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -227,6 +227,10 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) tcp_frame_conns[tcp_payload_used++] = conn; l4len = optlen + sizeof(struct tcphdr); iov[TCP_IOV_PAYLOAD].iov_len = l4len; + + if (flags & KEEPALIVE) + seq--; + tcp_l2_buf_fill_headers(c, conn, iov, NULL, seq, false); tcp_l2_buf_pad(iov); diff --git a/tcp_internal.h b/tcp_internal.h index 5f8fb358..36f443b4 100644 --- a/tcp_internal.h +++ b/tcp_internal.h @@ -38,6 +38,8 @@ /* Flags for internal usage */ #define DUP_ACK (1 << 5) +#define KEEPALIVE (1 << 6) + #define OPT_EOL 0 #define OPT_NOP 1 #define OPT_MSS 2 diff --git a/tcp_vu.c b/tcp_vu.c index b9e9b55e..f2d38aa5 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -135,6 +135,9 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) flags_elem[0].in_sg[0].iov_len = hdrlen + optlen; payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen); + if (flags & KEEPALIVE) + seq--; + tcp_fill_headers(c, conn, NULL, eh, ip4h, ip6h, th, &payload, NULL, seq, !*c->pcap); -- 2.53.0

1 0

[PATCH v2 1/4] tcp: Remove non-working activity timeout mechanism
by David Gibson 06 Feb '26

06 Feb '26

This mechanism was intended to remove connections which have had no activity for two hours, even if they haven't closed or been reset internally. It operated by setting the two hour timeout if there are no sooner TCP timeouts to schedule. However, when the timer fires, the way we detect the case of the activity timeout doesn't work: it resets the timer for another two hours, then checks if the old timeout was two hours. But the old timeout returned by timerfd_settime() is not the original value of the timer, but the remaining time. Since the timer has just fired it will essentially always be 0. For now, just remove the mechanism, disarming the timer entirely if there isn't another upcoming event. We'll re-introduce some sort of activity timeout by a different means later. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/tcp.c b/tcp.c index 0a64892a..f8663369 100644 --- a/tcp.c +++ b/tcp.c @@ -190,9 +190,6 @@ * - RTO_INIT_AFTER_SYN_RETRIES: if SYN retries happened during handshake and * RTO is less than this, re-initialise RTO to this for data retransmissions * - * - ACT_TIMEOUT, in the presence of any event: if no activity is detected on - * either side, the connection is reset - * * - RTT / 2 elapsed after data segment received from tap without having * sent an ACK segment, or zero-sized window advertised to tap/guest (flag * ACK_TO_TAP_DUE): forcibly check if an ACK segment can be sent. @@ -589,7 +586,9 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) timeout <<= MAX(exp, 0); it.it_value.tv_sec = MIN(timeout, c->tcp.rto_max); } else { - it.it_value.tv_sec = ACT_TIMEOUT; + /* Disarm */ + it.it_value.tv_sec = 0; + it.it_value.tv_nsec = 0; } if (conn->flags & ACK_TO_TAP_DUE) { @@ -2598,23 +2597,6 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) tcp_data_from_sock(c, conn); tcp_timer_ctl(c, conn); } - } else { - struct itimerspec new = { { 0 }, { ACT_TIMEOUT, 0 } }; - struct itimerspec old = { { 0 }, { 0 } }; - - /* Activity timeout: if it was already set, reset the - * connection, otherwise, it was a left-over from ACK_TO_TAP_DUE - * or ACK_FROM_TAP_DUE, so just set the long timeout in that - * case. This avoids having to preemptively reset the timer on - * ~ACK_TO_TAP_DUE or ~ACK_FROM_TAP_DUE. - */ - if (timerfd_settime(conn->timer, 0, &new, &old)) - flow_perror(conn, "failed to set timer"); - - if (old.it_value.tv_sec == ACT_TIMEOUT) { - flow_dbg(conn, "activity timeout"); - tcp_rst(c, conn); - } } } -- 2.53.0

1 0

[PATCH v6] conf: Support CIDR notation for -a/--address option
by Jon Maloy 06 Feb '26

06 Feb '26

Extend the -a/--address option to accept addresses in CIDR notation (e.g., 192.168.1.1/24 or 2001:db8::1/64) as an alternative to using separate -a and -n options. We add a new inany_prefix_pton() helper function that: - Parses address strings with a compulsory /prefix_len suffix - Validates prefix length based on address family (0-32 for IPv4, 0-128 for IPv6), including handling of IPv4-to-IPv6 mapping case. For IPv4, the prefix length is stored in ip4.prefix_len when provided. Mixing -n and CIDR notation results in an error to catch likely user mistakes. Also fix a bug in conf_ip4_prefix() that was incorrectly using the global 'optarg' instead of its 'arg' parameter. Signed-off-by: Jon Maloy <jmaloy(a)redhat.com> --- v3: Fixes after feedback from Laurent, David and Stefano Notably, updated man page for the -a option v4: Fixes based on feedback from David G: - Handling prefix length adjustment when IPv4-to-IPv6 mapping - Removed redundant !IN6_IS_ADDR_V4MAPPED(&addr.a6) test - Simplified tests of acceptable address types - Merged documentation and code commits - Some documentation text clarifications v5: - Moved address/prefix parsing into a refactored inany_prefix_pton() function. - inany_prefix_pton() now only caluclates IPv6 style prefix lengths - Stricter distinction between error causes. - Some refactoring of the 'case a:' branch in conf() - Some small fixes in passt.1 v6: - Refactored inany_prefix_pton() and conf()::'case -a' code after input from David Gibson. --- conf.c | 56 +++++++++++++++++++++++++++++++++----------------------- inany.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ inany.h | 14 ++++++++++++++ ip.c | 21 +++++++++++++++++++++ ip.h | 2 ++ passt.1 | 17 ++++++++++++----- 6 files changed, 134 insertions(+), 28 deletions(-) diff --git a/conf.c b/conf.c index 2942c8c..3cf825e 100644 --- a/conf.c +++ b/conf.c @@ -682,7 +682,7 @@ static int conf_ip4_prefix(const char *arg) return -1; } else { errno = 0; - len = strtoul(optarg, NULL, 0); + len = strtoul(arg, NULL, 0); if (len > 32 || errno) return -1; } @@ -896,7 +896,7 @@ static void usage(const char *name, FILE *f, int status) " a zero value disables assignment\n" " default: 65520: maximum 802.3 MTU minus 802.3 header\n" " length, rounded to 32 bits (IPv4 words)\n" - " -a, --address ADDR Assign IPv4 or IPv6 address ADDR\n" + " -a, --address ADDR Assign IPv4 or IPv6 address ADDR[/PREFIXLEN]\n" " can be specified zero to two times (for IPv4 and IPv6)\n" " default: use addresses from interface with default route\n" " -n, --netmask MASK Assign IPv4 MASK, dot-decimal or bits\n" @@ -1498,6 +1498,7 @@ void conf(struct ctx *c, int argc, char **argv) const char *optstring = "+dqfel:hs:F:I:p:P:m:a:n:M:g:i:o:D:S:H:461t:u:T:U:"; const char *logname = (c->mode == MODE_PASTA) ? "pasta" : "passt"; char userns[PATH_MAX] = { 0 }, netns[PATH_MAX] = { 0 }; + bool prefix_from_cidr = false, prefix_from_opt = false; bool copy_addrs_opt = false, copy_routes_opt = false; enum fwd_ports_mode fwd_default = FWD_NONE; bool v4_only = false, v6_only = false; @@ -1808,35 +1809,44 @@ void conf(struct ctx *c, int argc, char **argv) c->mtu = mtu; break; } - case 'a': - if (inet_pton(AF_INET6, optarg, &c->ip6.addr) && - !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.addr) && - !IN6_IS_ADDR_LOOPBACK(&c->ip6.addr) && - !IN6_IS_ADDR_V4MAPPED(&c->ip6.addr) && - !IN6_IS_ADDR_V4COMPAT(&c->ip6.addr) && - !IN6_IS_ADDR_MULTICAST(&c->ip6.addr)) { - if (c->mode == MODE_PASTA) - c->ip6.no_copy_addrs = true; - break; - } - - if (inet_pton(AF_INET, optarg, &c->ip4.addr) && - !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.addr) && - !IN4_IS_ADDR_BROADCAST(&c->ip4.addr) && - !IN4_IS_ADDR_LOOPBACK(&c->ip4.addr) && - !IN4_IS_ADDR_MULTICAST(&c->ip4.addr)) { + case 'a': { + union inany_addr addr; + int prefix_len; + + if (inany_pton(optarg, &addr)) + prefix_len = inany_default_prefix_len(&addr); + else if (!inany_prefix_pton(optarg, &addr, &prefix_len)) + die("Invalid address/prefix_len: %s", optarg); + else if (prefix_from_opt) + die("Can't mix CIDR with -n"); + else + prefix_from_cidr = true; + + if (inany_is_unspecified(&addr) || + inany_is_multicast(&addr) || + inany_is_loopback(&addr) || + IN6_IS_ADDR_V4COMPAT(&addr.a6)) + die("Invalid address: %s", optarg); + + if (inany_v4(&addr)) { + c->ip4.addr = *inany_v4(&addr); + c->ip4.prefix_len = prefix_len - 96; if (c->mode == MODE_PASTA) c->ip4.no_copy_addrs = true; - break; + } else { + c->ip6.addr = addr.a6; + if (c->mode == MODE_PASTA) + c->ip6.no_copy_addrs = true; } - - die("Invalid address: %s", optarg); break; + } case 'n': + if (prefix_from_cidr) + die("Can't use both -n and CIDR prefix length"); c->ip4.prefix_len = conf_ip4_prefix(optarg); if (c->ip4.prefix_len < 0) die("Invalid netmask: %s", optarg); - + prefix_from_opt = true; break; case 'M': parse_mac(c->our_tap_mac, optarg); diff --git a/inany.c b/inany.c index 7680439..d9a592a 100644 --- a/inany.c +++ b/inany.c @@ -11,6 +11,7 @@ #include <assert.h> #include <netinet/in.h> #include <arpa/inet.h> +#include <errno.h> #include "util.h" #include "ip.h" @@ -57,3 +58,54 @@ int inany_pton(const char *src, union inany_addr *dst) return 0; } + +/** inany_prefix_pton - Parse an IPv[46] address with prefix length + * @src: IPv[46] address and prefix length string in CIDR format + * @dst: output buffer, filled with parsed address + * @prefix_len: pointer to prefix length + * + * Return: on success, 1, if no parseable address or prefix is found, 0 + */ +int inany_prefix_pton(char *src, union inany_addr *dst, int *prefix_len) +{ + char astr[INANY_ADDRSTRLEN] = {0,}; + size_t alen = strcspn(src, "/"); + char pstr[4] = {0,}; + char *end; + + if (alen >= INANY_ADDRSTRLEN) + return 0; + + if (src[alen] != '/') + return 0; + + strncpy(astr, src, alen); + strncpy(pstr, &src[alen + 1], sizeof(pstr) - 1); + + info("addr %s, prefix %s", astr, pstr); + + /* Read prefix length */ + errno = 0; + *prefix_len = strtoul(pstr, &end, 10); + if (errno || *end) + return 0; + + if (*prefix_len > 128) + return 0; + + /* Read address */ + if (inet_pton(AF_INET6, astr, dst)) { + if (!inany_v4(dst)) + return 1; + if (*prefix_len < 96) + return 0; + return 1; + } else if (inany_pton(astr, dst)) { + if (*prefix_len > 32) + return 0; + *prefix_len += 96; + return 1; + } + + return 0; +} diff --git a/inany.h b/inany.h index 61b36fb..675e7e7 100644 --- a/inany.h +++ b/inany.h @@ -96,6 +96,19 @@ static inline struct in_addr *inany_v4(const union inany_addr *addr) return (struct in_addr *)&addr->v4mapped.a4; } +/** inany_default_prefix_len() - Get default prefix length for address + * @addr: IPv4 or iPv6 address + * + * Return: Class-based prefix length for IPv4 (in IPv6 format: 104-128), + * or 64 for IPv6 + */ +static inline int inany_default_prefix_len(const union inany_addr *addr) +{ + const struct in_addr *v4 = inany_v4(addr); + + return v4 ? ip4_class_prefix_len(v4) : 64; +} + /** inany_equals - Compare two IPv[46] addresses * @a, @b: IPv[46] addresses * @@ -295,5 +308,6 @@ static inline void inany_siphash_feed(struct siphash_state *state, const char *inany_ntop(const union inany_addr *src, char *dst, socklen_t size); int inany_pton(const char *src, union inany_addr *dst); +int inany_prefix_pton(char *src, union inany_addr *dst, int *prefix_len); #endif /* INANY_H */ diff --git a/ip.c b/ip.c index 9a7f4c5..40dc24e 100644 --- a/ip.c +++ b/ip.c @@ -13,6 +13,8 @@ */ #include <stddef.h> +#include <netinet/in.h> + #include "util.h" #include "ip.h" @@ -67,3 +69,22 @@ found: *proto = nh; return true; } + +/** + * ip4_class_prefix_len() - Get class based prefix length for IPv4 address + * @addr: IPv4 address + * + * Return: prefix length based on address class, or 32 for other + */ +int ip4_class_prefix_len(const struct in_addr *addr) +{ + in_addr_t a = ntohl(addr->s_addr); + + if (IN_CLASSA(a)) + return 32 - IN_CLASSA_NSHIFT; + if (IN_CLASSB(a)) + return 32 - IN_CLASSB_NSHIFT; + if (IN_CLASSC(a)) + return 32 - IN_CLASSC_NSHIFT; + return 32; +} diff --git a/ip.h b/ip.h index 5830b92..bd28640 100644 --- a/ip.h +++ b/ip.h @@ -135,4 +135,6 @@ static const struct in_addr in4addr_broadcast = { 0xffffffff }; #define IPV6_MIN_MTU 1280 #endif +int ip4_class_prefix_len(const struct in_addr *addr); + #endif /* IP_H */ diff --git a/passt.1 b/passt.1 index db0d662..2c3eae9 100644 --- a/passt.1 +++ b/passt.1 @@ -156,10 +156,14 @@ By default, the advertised MTU is 65520 bytes, that is, the maximum 802.3 MTU minus the length of a 802.3 header, rounded to 32 bits (IPv4 words). .TP -.BR \-a ", " \-\-address " " \fIaddr +.BR \-a ", " \-\-address " " \fIaddr\fR[/\fIprefix_len\fR] Assign IPv4 \fIaddr\fR via DHCP (\fByiaddr\fR), or \fIaddr\fR via DHCPv6 (option 5) and an \fIaddr\fR-based prefix via NDP Router Advertisement (option type 3) for an IPv6 \fIaddr\fR. +An optional /\fIprefix_len\fR (0-32 for IPv4, 0-128 for IPv6) can be +appended in CIDR notation (e.g. 192.0.2.1/24). This is an alternative to +using the \fB-n\fR, \fB--netmask\fR option. Mixing CIDR notation with +\fB-n\fR results in an error. This option can be specified zero (for defaults) to two times (once for IPv4, once for IPv6). By default, assigned IPv4 and IPv6 addresses are taken from the host interfaces @@ -172,10 +176,13 @@ is assigned for IPv4, and no additional address will be assigned for IPv6. .TP .BR \-n ", " \-\-netmask " " \fImask Assign IPv4 netmask \fImask\fR, expressed as dot-decimal or number of bits, via -DHCP (option 1). -By default, the netmask associated to the host address matching the assigned one -is used. If there's no matching address on the host, the netmask is determined -according to the CIDR block of the assigned address (RFC 4632). +DHCP (option 1). Alternatively, the prefix length can be specified using CIDR +notation with the \fB-a\fR, \fB--address\fR option (e.g. \fB-a\fR 192.0.2.1/24). +Mixing \fB-n\fR with CIDR notation results in an error. +If no address is indicated, the netmask associated with the adopted host address, +if any, is used. If an address is indicated, but without a prefix length, the +netmask is determined based on the corresponding network class. In all other +cases, the netmask is determined by using the indicated prefix length. .TP .BR \-M ", " \-\-mac-addr " " \fIaddr -- 2.52.0

2 1

[PATCH 4/4] tcp: Send TCP keepalive segments after a period of tap-side inactivity
by David Gibson 06 Feb '26

06 Feb '26

There are several circumstances in which a live, but idle TCP connection can be forgotten by a guest, with no "on the wire" indication that this has happened. The most obvious is if the guest abruptly reboots. A more subtle case can happen with a half-closed connection, specifically one in FIN_WAIT_2 state on the guest. A connection can, legitimately, remain in this state indefinitely. If however, a socket in this state is closed by userspace, Linux at least will remove the kernel socket after 60s (or as configured in the net.ipv4.tcp_fin_timeout sysctl). Because there's no on the wire indication in these cases, passt will pointlessly retain the connection in its flow table, at least until it is removed by the inactivity timeout after several hours. To avoid keeping connections around for so long in this state, add functionality to periodically send TCP keepalive segments to the guest if we've seen no activity on the tap interface. If the guest is no longer aware of the connection, it should respond with an RST which will let passt remove the stale entry. To do this we use a method similar to the inactivity timeout - a 1-bit page replacement / clock algorithm, but with a shorter interval, and only checking for tap side activity. Currently we use a 300s interval, meaning we'll send a keepalive after 5-10 minutes of (tap side) inactivity. Link: https://bugs.passt.top/show_bug.cgi?id=179 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp.c | 39 +++++++++++++++++++++++++++++++++++++++ tcp.h | 2 ++ tcp_conn.h | 2 ++ 3 files changed, 43 insertions(+) diff --git a/tcp.c b/tcp.c index acdac7df..bf57be23 100644 --- a/tcp.c +++ b/tcp.c @@ -206,6 +206,12 @@ * keepalives) will be removed between INACTIVITY_INTERVAL s and * 2*INACTIVITY_INTERVAL s after the last activity. * + * - KEEPALIVE_INTERVAL: if a connection has had no tap-side activity for an + * entire interval, send a tap-side keepalive. If the endpoint is no longer + * aware of the connection (due to a reboot, or a kernel timeout in FIN_WAIT_2 + * state) that should trigger an RST, so we won't keep track of connections + * that the guest endpoint no longer cares about. + * * Summary of data flows (with ESTABLISHED event) * ---------------------------------------------- * @@ -342,6 +348,7 @@ enum { #define RTO_INIT_AFTER_SYN_RETRIES 3 /* s, RFC 6298 */ #define INACTIVITY_INTERVAL 7200 /* s */ +#define KEEPALIVE_INTERVAL 30 /* s */ #define LOW_RTT_TABLE_SIZE 8 #define LOW_RTT_THRESHOLD 10 /* us */ @@ -2263,6 +2270,7 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, } conn->inactive = false; + conn->tap_inactive = false; if (th->ack && !(conn->events & ESTABLISHED)) tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq)); @@ -2884,6 +2892,36 @@ int tcp_init(struct ctx *c) return 0; } +/** + * tcp_keepalive() - Send keepalives for connections which need it + * @: Execution context + */ +static void tcp_keepalive(struct ctx *c, const struct timespec *now) +{ + union flow *flow; + + if (now->tv_sec - c->tcp.keepalive_run < KEEPALIVE_INTERVAL) + return; + + c->tcp.keepalive_run = now->tv_sec; + + flow_foreach(flow) { + struct tcp_tap_conn *conn = &flow->tcp; + + if (flow->f.type != FLOW_TCP) + continue; + + if (conn->tap_inactive) { + flow_dbg(conn, "No tap activity for least %us, send keepalive", + KEEPALIVE_INTERVAL); + tcp_send_flag(c, conn, KEEPALIVE); + } + + /* Ready to check fot next interval */ + conn->tap_inactive = true; + } +} + /** * tcp_inactivity() - Scan for and close long-inactive connections * @: Execution context @@ -2927,6 +2965,7 @@ void tcp_timer(struct ctx *c, const struct timespec *now) if (c->mode == MODE_PASTA) tcp_splice_refill(c); + tcp_keepalive(c, now); tcp_inactivity(c, now); } diff --git a/tcp.h b/tcp.h index e104d453..2739f309 100644 --- a/tcp.h +++ b/tcp.h @@ -38,6 +38,7 @@ extern bool peek_offset_cap; * @rto_max: Maximum retry timeout (in s) * @syn_retries: SYN retries using exponential backoff timeout * @syn_linear_timeouts: SYN retries before using exponential backoff timeout + * @keepalive_run: Time we last issued tap-side keepalives * @inactivity_run: Time we last scanned for inactive connections */ struct tcp_ctx { @@ -48,6 +49,7 @@ struct tcp_ctx { int rto_max; uint8_t syn_retries; uint8_t syn_linear_timeouts; + time_t keepalive_run; time_t inactivity_run; }; diff --git a/tcp_conn.h b/tcp_conn.h index 7197ff63..c82e1441 100644 --- a/tcp_conn.h +++ b/tcp_conn.h @@ -16,6 +16,7 @@ * @ws_from_tap: Window scaling factor advertised from tap/guest * @ws_to_tap: Window scaling factor advertised to tap/guest * @tap_mss: MSS advertised by tap/guest, rounded to 2 ^ TCP_MSS_BITS + * @tapinactive: No tao activity within the current KEEPALIVE_INTERVAL * @inactive: No activity within the current INACTIVITY_INTERVAL * @sock: Socket descriptor number * @events: Connection events, implying connection states @@ -58,6 +59,7 @@ struct tcp_tap_conn { (conn->rtt_exp = MIN(RTT_EXP_MAX, ilog2(MAX(1, rtt / RTT_STORE_MIN)))) #define RTT_GET(conn) (RTT_STORE_MIN << conn->rtt_exp) + bool tap_inactive :1; bool inactive :1; int sock :FD_REF_BITS; -- 2.52.0

2 2

[PATCH 0/4] RFC: Reworks and improvements to TCP activity timers
by David Gibson 06 Feb '26

06 Feb '26

Here's a bunch of patches aimed at fixing bug 179, and reworking the currently broken inactivity timer along the way. I believe patches 1..2/4 are ready to go - I've tested them, and I'm happy with how they're behaving. Patches 3..4/4 I think are correct, but I've been getting bogged down in details trying to test them in the specific FIN_WAIT_2 situation that occurs in bug 179. I'm sending this out for comment, while I look at some other things to clear my head. Caveats: * Currently the inactivity timer uses an interval of 2h to match the intended behaviour of the existig non-working activity timeout. Arguably it should be much longer (several days), like the kernel NAT timeout for idle connection tracking. * The keepalive interval is currently 30s. That's too short - I have it there just for testing. I was intending to change it to 300s (5m) for "real", but that's certainly open to discussion. * This introduces two new fields in the connection structure, as the clock values for the two new timers. These are new 1-bit bool fields slotted into a 3-bit gap. Arguably these would be cleaner as new bits in the 'flags' field. However, since we only have one spare bit there at the moment, that would require some more complex reorganization which I didn't want to tackle right now. David Gibson (4): tcp: Remove non-working activity timeout mechanism tcp: Re-introduce inactivity timeouts based on a clock algorithm tcp: Extend tcp_send_flag() to send TCP keepalive segments tcp: Send TCP keepalive segments after a period of tap-side inactivity tcp.c | 115 ++++++++++++++++++++++++++++++++++++++----------- tcp.h | 6 ++- tcp_buf.c | 4 ++ tcp_conn.h | 5 +++ tcp_internal.h | 2 + tcp_vu.c | 3 ++ 6 files changed, 109 insertions(+), 26 deletions(-) -- 2.52.0

2 4

[PATCH] virtio: Remove duplicate VIRTQUEUE_MAX_SIZE definition
by Laurent Vivier 04 Feb '26

04 Feb '26

VIRTQUEUE_MAX_SIZE is already defined in virtio.h, which virtio.c includes. Remove the redundant local definition. Signed-off-by: Laurent Vivier <lvivier(a)redhat.com> --- virtio.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/virtio.c b/virtio.c index 447137ee83dd..7e31e799f881 100644 --- a/virtio.c +++ b/virtio.c @@ -83,8 +83,6 @@ #include "virtio.h" #include "vhost_user.h" -#define VIRTQUEUE_MAX_SIZE 1024 - /** * vu_gpa_to_va() - Translate guest physical address to our virtual address. * @dev: Vhost-user device -- 2.52.0

2 1

[PATCH 3/4] tcp: Extend tcp_send_flag() to send TCP keepalive segments
by David Gibson 04 Feb '26

04 Feb '26

TCP keepalives aren't technically a flag, but they are a zero-data segment so they can be generated with only a small modification to tcp_{buf,vu}_send_flag(). Implement this, using a new "pseudo-flag" value (similar to DUP_ACK), KEEPALIVE. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp_buf.c | 4 ++++ tcp_internal.h | 2 ++ tcp_vu.c | 3 +++ 3 files changed, 9 insertions(+) diff --git a/tcp_buf.c b/tcp_buf.c index d2925410..18bdce81 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -227,6 +227,10 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) tcp_frame_conns[tcp_payload_used++] = conn; l4len = optlen + sizeof(struct tcphdr); iov[TCP_IOV_PAYLOAD].iov_len = l4len; + + if (flags & KEEPALIVE) + seq--; + tcp_l2_buf_fill_headers(c, conn, iov, NULL, seq, false); tcp_l2_buf_pad(iov); diff --git a/tcp_internal.h b/tcp_internal.h index 5f8fb358..36f443b4 100644 --- a/tcp_internal.h +++ b/tcp_internal.h @@ -38,6 +38,8 @@ /* Flags for internal usage */ #define DUP_ACK (1 << 5) +#define KEEPALIVE (1 << 6) + #define OPT_EOL 0 #define OPT_NOP 1 #define OPT_MSS 2 diff --git a/tcp_vu.c b/tcp_vu.c index b9e9b55e..f2d38aa5 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -135,6 +135,9 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags) flags_elem[0].in_sg[0].iov_len = hdrlen + optlen; payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen); + if (flags & KEEPALIVE) + seq--; + tcp_fill_headers(c, conn, NULL, eh, ip4h, ip6h, th, &payload, NULL, seq, !*c->pcap); -- 2.52.0

1 0

[PATCH v2 0/3] Fix errors in FIN timeout logic
by David Gibson 31 Jan '26

31 Jan '26

While investigating bug 179, I found a number of things that confused me about the TCP timer handling. One of them, I think I figured out what's going on and what should be done about it. So, here are the changes. This is mostly about FIN handling and only tangentially about the timer, but it does at least slightly simplify the timer handling while I figure out the rest of it. Changes in v2: * Stefano pointed out some errors in my guesses at the history of things, evised commit message of 2/3 accordingly * Added 3/3 checking for shutdown(2) failures David Gibson (3): tcp: Retransmit FINs like data segments tcp: Eliminate FIN_TIMEOUT tcp, tcp_splice: Check for failures of shutdown(2) tcp.c | 49 ++++++++++++++++++++++++------------------------- tcp_buf.c | 1 + tcp_splice.c | 3 ++- tcp_vu.c | 1 + 4 files changed, 28 insertions(+), 26 deletions(-) -- 2.52.0

2 2

[PATCH v2 3/3] tcp, tcp_splice: Check for failures of shutdown(2)
by David Gibson 30 Jan '26

30 Jan '26

shutdown(2) should never fail, unless we give it bad parameters (e.g. passing it an fd which isn't a connected socket). However, if it ever did, we'd currently ignore the error and carry on which could lead to very confusing behaviour. In the interests of debugability, check for failure of shutdown(2), log an error and: - during runtime, reset the affected connection - during migration, fail the migration Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp.c | 31 ++++++++++++++++++++++++------- tcp_splice.c | 3 ++- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/tcp.c b/tcp.c index 0be871a4..9dd02cd8 100644 --- a/tcp.c +++ b/tcp.c @@ -2284,7 +2284,11 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, if (th->fin) { conn->seq_from_tap++; - shutdown(conn->sock, SHUT_WR); + if (shutdown(conn->sock, SHUT_WR) < 0) { + flow_dbg_perror(conn, "shutdown() failed"); + goto reset; + } + tcp_send_flag(c, conn, ACK); conn_event(c, conn, SOCK_FIN_SENT); @@ -2359,7 +2363,11 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, socklen_t sl; struct tcp_info tinfo; - shutdown(conn->sock, SHUT_WR); + if (shutdown(conn->sock, SHUT_WR) < 0) { + flow_dbg_perror(conn, "shutdown() failed"); + goto reset; + } + conn_event(c, conn, SOCK_FIN_SENT); tcp_send_flag(c, conn, ACK); ack_due = 0; @@ -3831,10 +3839,15 @@ int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd int v; v = TCP_SEND_QUEUE; - if (setsockopt(s, SOL_TCP, TCP_REPAIR_QUEUE, &v, sizeof(v))) + if (setsockopt(s, SOL_TCP, TCP_REPAIR_QUEUE, &v, sizeof(v))) { flow_perror(conn, "Selecting repair queue"); - else - shutdown(s, SHUT_WR); + } else { + if (shutdown(s, SHUT_WR) < 0) { + flow_perror(conn, + "Repair mode shutdown() failed"); + goto fail; + } + } } if (tcp_flow_repair_wnd(conn, &t)) @@ -3861,8 +3874,12 @@ int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd * Call shutdown(x, SHUT_WR) *not* in repair mode, which moves us to * TCP_FIN_WAIT1. */ - if (t.tcpi_state == TCP_FIN_WAIT1) - shutdown(s, SHUT_WR); + if (t.tcpi_state == TCP_FIN_WAIT1) { + if (shutdown(s, SHUT_WR) < 0) { + flow_perror(conn, "Post-repair shutdown() failed"); + goto fail; + } + } if (tcp_set_peek_offset(conn, peek_offset)) goto fail; diff --git a/tcp_splice.c b/tcp_splice.c index 8806523a..d60981ca 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -627,7 +627,8 @@ retry: flow_foreach_sidei(sidei) { if ((conn->events & FIN_RCVD(sidei)) && !(conn->events & FIN_SENT(!sidei))) { - shutdown(conn->s[!sidei], SHUT_WR); + if (shutdown(conn->s[!sidei], SHUT_WR) < 0) + goto reset; conn_event(conn, FIN_SENT(!sidei)); } } -- 2.52.0

1 0

[PATCH v2 1/3] tcp: Retransmit FINs like data segments
by David Gibson 30 Jan '26

30 Jan '26

RFC 9293 doesn't distinguish between regular data segments and FIN segments for the purposes of retransmissions. Our existing retransmission logic will also work for FIN segments, except for one detail: we don't currently set the ACK_FROM_TAP_DUE flag when we send a FIN. Add the flag, so that we'll properly retransmit FIN segments like data segments. Remove the section from the theory of operation comment that describes a different way of handling FIN timeouts which (a) isn't correct behaviour and (b) doesn't appear to be implemented. I've tested this by adding logic to suppress sending the FIN if retries < some non-zero value. We correctly resend the FIN and close normally after the expected timeouts. Link: https://bugs.passt.top/show_bug.cgi?id=195 Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- tcp.c | 4 ---- tcp_buf.c | 1 + tcp_vu.c | 1 + 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tcp.c b/tcp.c index 17e5b006..dbfde2e0 100644 --- a/tcp.c +++ b/tcp.c @@ -190,10 +190,6 @@ * - RTO_INIT_AFTER_SYN_RETRIES: if SYN retries happened during handshake and * RTO is less than this, re-initialise RTO to this for data retransmissions * - * - FIN_TIMEOUT: if a FIN segment was sent to tap/guest (flag ACK_FROM_TAP_DUE - * with TAP_FIN_SENT event), and no ACK is received within this time, reset - * the connection - * * - FIN_TIMEOUT: if a FIN segment was acknowledged by tap/guest and a FIN * segment (write shutdown) was sent via socket (events SOCK_FIN_SENT and * TAP_FIN_ACKED), but no socket activity is detected from the socket within diff --git a/tcp_buf.c b/tcp_buf.c index 5d419d36..d2925410 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -407,6 +407,7 @@ int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) } conn_event(c, conn, TAP_FIN_SENT); + conn_flag(c, conn, ACK_FROM_TAP_DUE); } return 0; diff --git a/tcp_vu.c b/tcp_vu.c index db9db78a..b9e9b55e 100644 --- a/tcp_vu.c +++ b/tcp_vu.c @@ -425,6 +425,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn) } conn_event(c, conn, TAP_FIN_SENT); + conn_flag(c, conn, ACK_FROM_TAP_DUE); } return 0; -- 2.52.0

1 0