Now that we have a new clamped_scale() function, which makes it simple to specify a precise usage factor, change the amount of sending buffer we want to use at and above 4 MiB: 75% looks perfectly safe. Signed-off-by: Stefano Brivio --- tcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tcp.c b/tcp.c index 026546a..37aceed 100644 --- a/tcp.c +++ b/tcp.c @@ -773,7 +773,7 @@ static void tcp_rtt_dst_check(const struct tcp_tap_conn *conn, } /** - * tcp_get_sndbuf() - Get, scale SO_SNDBUF between thresholds (1 to 0.5 usage) + * tcp_get_sndbuf() - Get, scale SO_SNDBUF between thresholds (1 to 0.75 usage) * @conn: Connection pointer */ static void tcp_get_sndbuf(struct tcp_tap_conn *conn) @@ -788,7 +788,7 @@ static void tcp_get_sndbuf(struct tcp_tap_conn *conn) return; } - v = clamped_scale(sndbuf, sndbuf, SNDBUF_SMALL, SNDBUF_BIG, 50); + v = clamped_scale(sndbuf, sndbuf, SNDBUF_SMALL, SNDBUF_BIG, 75); SNDBUF_SET(conn, MIN(INT_MAX, v)); } -- 2.43.0

A fixed 10 ms ACK_INTERVAL timer value served us relatively well until the previous change, because we would generally cause retransmissions for non-local outbound transfers with relatively high (> 100 Mbps) bandwidth and non-local but low (< 5 ms) RTT. Now that retransmissions are less frequent, we don't have a proper trigger to check for acknowledged bytes on the socket, and will generally block the sender for a significant amount of time while we could acknowledge more data, instead. Store the RTT reported by the kernel using an approximation (exponent), to keep flow storage size within two (typical) cachelines. Check for socket updates when half of this time elapses: it should be a good indication of the one-way delay we're interested in (peer to us). Representable values are between 100 us and 3.2768 s, and any value outside this range is clamped to these bounds. This choice appears to be a good trade-off between additional overhead and throughput. This mechanism partially overlaps with the "low RTT" destinations, which we use to infer that a socket is connected to an endpoint to the same machine (while possibly in a different namespace) if the RTT is reported as 10 us or less. This change doesn't, however, conflict with it: we are reading TCP_INFO parameters for local connections anyway, so we can always store the RTT approximation opportunistically. Then, if the RTT is "low", we don't really need a timer to acknowledge data as we'll always acknowledge everything to the sender right away. However, we have limited space in the array where we store addresses of local destination, so the low RTT property of a connection might toggle frequently. Because of this, it's actually helpful to always have the RTT approximation stored. This could probably benefit from a future rework, though, introducing a more integrated approach between these two mechanisms. Signed-off-by: Stefano Brivio --- tcp.c | 30 +++++++++++++++++++++++------- tcp_conn.h | 9 +++++++++ util.c | 14 ++++++++++++++ util.h | 1 + 4 files changed, 47 insertions(+), 7 deletions(-) diff --git a/tcp.c b/tcp.c index 28d3304..0167121 100644 --- a/tcp.c +++ b/tcp.c @@ -202,9 +202,13 @@ * - ACT_TIMEOUT, in the presence of any event: if no activity is detected on * either side, the connection is reset * - * - ACK_INTERVAL elapsed after data segment received from tap without having + * - 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 + * ACK_TO_TAP_DUE): forcibly check if an ACK segment can be sent. + * + * RTT, here, is an approximation of the RTT value reported by the kernel via + * TCP_INFO, with a representable range from RTT_STORE_MIN (100 us) to + * RTT_STORE_MAX (3276.8 ms). The timeout value is clamped accordingly. * * * Summary of data flows (with ESTABLISHED event) @@ -341,7 +345,6 @@ enum { #define MSS_DEFAULT 536 #define WINDOW_DEFAULT 14600 /* RFC 6928 */ -#define ACK_INTERVAL 10 /* ms */ #define RTO_INIT 1 /* s, RFC 6298 */ #define RTO_INIT_AFTER_SYN_RETRIES 3 /* s, RFC 6298 */ #define FIN_TIMEOUT 60 @@ -593,7 +596,8 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) } if (conn->flags & ACK_TO_TAP_DUE) { - it.it_value.tv_nsec = (long)ACK_INTERVAL * 1000 * 1000; + it.it_value.tv_sec = RTT_GET(conn) / 2 / (1000 * 1000); + it.it_value.tv_nsec = RTT_GET(conn) / 2 % (1000 * 1000) * 1000; } else if (conn->flags & ACK_FROM_TAP_DUE) { int exp = conn->retries, timeout = RTO_INIT; if (!(conn->events & ESTABLISHED)) @@ -608,9 +612,17 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) it.it_value.tv_sec = ACT_TIMEOUT; } - flow_dbg(conn, "timer expires in %llu.%03llus", - (unsigned long long)it.it_value.tv_sec, - (unsigned long long)it.it_value.tv_nsec / 1000 / 1000); + if (conn->flags & ACK_TO_TAP_DUE) { + flow_trace(conn, "timer expires in %llu.%03llums", + (unsigned long)it.it_value.tv_sec * 1000 + + (unsigned long long)it.it_value.tv_nsec % + (1000 * 1000), + (unsigned long long)it.it_value.tv_nsec / 1000); + } else { + flow_dbg(conn, "timer expires in %llu.%03llus", + (unsigned long long)it.it_value.tv_sec, + (unsigned long long)it.it_value.tv_nsec / 1000 / 1000); + } if (timerfd_settime(conn->timer, 0, &it, NULL)) flow_perror(conn, "failed to set timer"); @@ -1144,6 +1156,10 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, conn_flag(c, conn, ACK_TO_TAP_DUE); out: + /* Opportunistically store RTT approximation on valid TCP_INFO data */ + if (tinfo) + RTT_SET(conn, tinfo->tcpi_rtt); + return new_wnd_to_tap != prev_wnd_to_tap || conn->seq_ack_to_tap != prev_ack_to_tap; } diff --git a/tcp_conn.h b/tcp_conn.h index e36910c..9c6ff9e 100644 --- a/tcp_conn.h +++ b/tcp_conn.h @@ -49,6 +49,15 @@ struct tcp_tap_conn { #define MSS_SET(conn, mss) (conn->tap_mss = (mss >> (16 - TCP_MSS_BITS))) #define MSS_GET(conn) (conn->tap_mss << (16 - TCP_MSS_BITS)) +#define RTT_EXP_BITS 4 + unsigned int rtt_exp :RTT_EXP_BITS; +#define RTT_EXP_MAX MAX_FROM_BITS(RTT_EXP_BITS) +#define RTT_STORE_MIN 100 /* us, minimum representable */ +#define RTT_STORE_MAX ((long)(RTT_STORE_MIN << RTT_EXP_MAX)) +#define RTT_SET(conn, rtt) \ + (conn->rtt_exp = MIN(RTT_EXP_MAX, ilog2(MAX(1, rtt / RTT_STORE_MIN)))) +#define RTT_GET(conn) (RTT_STORE_MIN << conn->rtt_exp) + int sock :FD_REF_BITS; uint8_t events; diff --git a/util.c b/util.c index 2232a24..bfeb619 100644 --- a/util.c +++ b/util.c @@ -614,6 +614,9 @@ int __daemon(int pidfile_fd, int devnull_fd) * fls() - Find last (most significant) bit set in word * @x: Word * + * Note: unlike ffs() and other implementations of fls(), notably the one from + * the Linux kernel, the starting position is 0 and not 1, that is, fls(1) = 0. + * * Return: position of most significant bit set, starting from 0, -1 if none */ int fls(unsigned long x) @@ -629,6 +632,17 @@ int fls(unsigned long x) return y; } +/** + * ilog2() - Integral part (floor) of binary logarithm (logarithm to the base 2) + * @x: Argument + * + * Return: integral part of binary logarithm of @x, -1 if undefined (if @x is 0) + */ +int ilog2(unsigned long x) +{ + return fls(x); +} + /** * write_file() - Replace contents of file with a string * @path: File to write diff --git a/util.h b/util.h index 744880b..f7a941f 100644 --- a/util.h +++ b/util.h @@ -233,6 +233,7 @@ int output_file_open(const char *path, int flags); void pidfile_write(int fd, pid_t pid); int __daemon(int pidfile_fd, int devnull_fd); int fls(unsigned long x); +int ilog2(unsigned long x); int write_file(const char *path, const char *buf); intmax_t read_file_integer(const char *path, intmax_t fallback); int write_all_buf(int fd, const void *buf, size_t len); -- 2.43.0

...instead of checking if the current sending buffer is less than SNDBUF_SMALL, because this isn't simply an optimisation to coalesce ACK segments: we rely on having enough data at once from the sender to make the buffer grow by means of TCP buffer size tuning implemented in the Linux kernel. This is important if we're trying to maximise throughput, but not desirable for interactive traffic, where we want to be transparent as possible and avoid introducing unnecessary latency. Use the tcpi_delivery_rate field reported by the Linux kernel, if available, to calculate the current bandwidth-delay product: if it's significantly smaller than the available sending buffer, conclude that we're not bandwidth-bound and this is likely to be interactive traffic, so acknowledge data only as it's acknowledged by the peer. Conversely, if the bandwidth-delay product is comparable to the size of the sending buffer (more than 5%), we're probably bandwidth-bound or... bound to be: acknowledge everything in that case. Signed-off-by: Stefano Brivio --- tcp.c | 45 +++++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/tcp.c b/tcp.c index b2e4174..923c1f2 100644 --- a/tcp.c +++ b/tcp.c @@ -353,6 +353,9 @@ enum { #define LOW_RTT_TABLE_SIZE 8 #define LOW_RTT_THRESHOLD 10 /* us */ +/* Ratio of buffer to bandwidth * delay product implying interactive traffic */ +#define SNDBUF_TO_BW_DELAY_INTERACTIVE /* > */ 20 /* (i.e. < 5% of buffer) */ + #define ACK_IF_NEEDED 0 /* See tcp_send_flag() */ #define CONN_IS_CLOSING(conn) \ @@ -426,11 +429,13 @@ socklen_t tcp_info_size; sizeof(((struct tcp_info_linux *)NULL)->tcpi_##f_)) <= tcp_info_size) /* Kernel reports sending window in TCP_INFO (kernel commit 8f7baad7f035) */ -#define snd_wnd_cap tcp_info_cap(snd_wnd) +#define snd_wnd_cap tcp_info_cap(snd_wnd) /* Kernel reports bytes acked in TCP_INFO (kernel commit 0df48c26d84) */ -#define bytes_acked_cap tcp_info_cap(bytes_acked) +#define bytes_acked_cap tcp_info_cap(bytes_acked) /* Kernel reports minimum RTT in TCP_INFO (kernel commit cd9b266095f4) */ -#define min_rtt_cap tcp_info_cap(min_rtt) +#define min_rtt_cap tcp_info_cap(min_rtt) +/* Kernel reports delivery rate in TCP_INFO (kernel commit eb8329e0a04d) */ +#define delivery_rate_cap tcp_info_cap(delivery_rate) /* sendmsg() to socket */ static struct iovec tcp_iov [UIO_MAXIOV]; @@ -1050,6 +1055,7 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, socklen_t sl = sizeof(*tinfo); struct tcp_info_linux tinfo_new; uint32_t new_wnd_to_tap = prev_wnd_to_tap; + bool ack_everything = true; int s = conn->sock; /* At this point we could ack all the data we've accepted for forwarding @@ -1059,7 +1065,8 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, * control behaviour. * * For it to be possible and worth it we need: - * - The TCP_INFO Linux extension which gives us the peer acked bytes + * - The TCP_INFO Linux extensions which give us the peer acked bytes + * and the delivery rate (outbound bandwidth at receiver) * - Not to be told not to (force_seq) * - Not half-closed in the peer->guest direction * With no data coming from the peer, we might not get events which @@ -1069,19 +1076,36 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, * Data goes from socket to socket, with nothing meaningfully "in * flight". * - Not a pseudo-local connection (e.g. to a VM on the same host) - * - Large enough send buffer - * In these cases, there's not enough in flight to bother. + * If it is, there's not enough in flight to bother. + * - Sending buffer significantly larger than bandwidth * delay product + * Meaning we're not bandwidth-bound and this is likely to be + * interactive traffic where we want to preserve transparent + * connection behaviour and latency. + * + * Otherwise, we probably want to maximise throughput, which needs + * sending buffer auto-tuning, triggered in turn by filling up the + * outbound socket queue. */ - if (bytes_acked_cap && !force_seq && + if (bytes_acked_cap && delivery_rate_cap && !force_seq && !CONN_IS_CLOSING(conn) && - !(conn->flags & LOCAL) && !tcp_rtt_dst_low(conn) && - (unsigned)SNDBUF_GET(conn) >= SNDBUF_SMALL) { + !(conn->flags & LOCAL) && !tcp_rtt_dst_low(conn)) { if (!tinfo) { tinfo = &tinfo_new; if (getsockopt(s, SOL_TCP, TCP_INFO, tinfo, &sl)) return 0; } + if ((unsigned)SNDBUF_GET(conn) > (long long)tinfo->tcpi_rtt * + tinfo->tcpi_delivery_rate / + 1000 / 1000 * + SNDBUF_TO_BW_DELAY_INTERACTIVE) + ack_everything = false; + } + + if (ack_everything) { + /* Fall back to acknowledging everything we got */ + conn->seq_ack_to_tap = conn->seq_from_tap; + } else { /* This trips a cppcheck bug in some versions, including * cppcheck 2.18.3. * https://sourceforge.net/p/cppcheck/discussion/general/thread/fecde59085/ @@ -1089,9 +1113,6 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, /* cppcheck-suppress [uninitvar,unmatchedSuppression] */ conn->seq_ack_to_tap = tinfo->tcpi_bytes_acked + conn->seq_init_from_tap; - } else { - /* Fall back to acknowledging everything we got */ - conn->seq_ack_to_tap = conn->seq_from_tap; } /* It's occasionally possible for us to go from using the fallback above -- 2.43.0

If the remote peer is advertising a bigger value than our current sending buffer, it means that a bigger sending buffer is likely to benefit throughput. We can get a bigger sending buffer by means of the buffer size auto-tuning performed by the Linux kernel, which is triggered by aggressively filling the sending buffer. Use an adaptive boost factor, up to 150%, depending on: - how much data we sent so far: we don't want to risk retransmissions for short-lived connections, as the latency cost would be unacceptable, and - the current RTT value, as we need a bigger buffer for higher transmission delays The factor we use is not quite a bandwidth-delay product, as we're missing the time component of the bandwidth, which is not interesting here: we are trying to make the buffer grow at the beginning of a connection, progressively, as more data is sent. The tuning of the amount of boost factor we want to apply was done somewhat empirically but it appears to yield the available throughput in rather different scenarios (from ~ 10 Gbps bandwidth with 500ns to ~ 1 Gbps with 300 ms RTT) and it allows getting there rather quickly, within a few seconds for the 300 ms case. Note that we want to apply this factor only if the window advertised by the peer is bigger than the current sending buffer, as we only need this for auto-tuning, and we absolutely don't want to incur unnecessary retransmissions otherwise. The related condition in tcp_update_seqack_wnd() is not redundant as there's a subtractive factor, sendq, in the calculation of the window limit. If the sending buffer is smaller than the peer's advertised window, the additional limit we might apply might be lower than we would do otherwise. Assuming that the sending buffer is reported as 100k, sendq is 20k, we could have these example cases: 1. tinfo->tcpi_snd_wnd is 120k, which is bigger than the sending buffer, so we boost its size to 150k, and we limit the window to 120k 2. tinfo->tcpi_snd_wnd is 90k, which is smaller than the sending buffer, so we aren't trying to trigger buffer auto-tuning and we'll stick to the existing, more conservative calculation, by limiting the window to 100 - 20 = 80k If we omitted the new condition, we would always use the boosted value, that is, 120k, even if potentially causing unnecessary retransmissions. Signed-off-by: Stefano Brivio --- tcp.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tcp.c b/tcp.c index 6218f7c..07ef5f0 100644 --- a/tcp.c +++ b/tcp.c @@ -353,6 +353,13 @@ 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) */ @@ -1035,6 +1042,35 @@ void tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn, tap_hdr_update(taph, 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(struct tcp_tap_conn *conn, + 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 scale_x_to_y_slope(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 @@ -1154,6 +1190,8 @@ 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; -- 2.43.0

...we'll send a duplicate ACK right away in this case, and this redundant, earlier check is not just useless, but it might actually be harmful as we'll now send a triple ACK which might cause two retransmissions. Signed-off-by: Stefano Brivio Reviewed-by: David Gibson --- tcp.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tcp.c b/tcp.c index 9c0fe42..0c1e974 100644 --- a/tcp.c +++ b/tcp.c @@ -2026,13 +2026,10 @@ eintr: 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; - conn->seq_from_tap += n; - tcp_send_flag(c, conn, ACK_IF_NEEDED); - } else { - conn->seq_from_tap += n; - } + + conn->seq_from_tap += n; out: if (keep != -1 || partial_send) { -- 2.43.0

Hi Stefano, On Mon, 2025-12-08 at 08:20 +0100, Stefano Brivio wrote:

@@ -1035,6 +1042,35 @@ void tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn, tap_hdr_update(taph, MAX(l3len + sizeof(struct ethhdr), ETH_ZLEN)); }

This chunk fails to apply because master currently has the following line instead: tap_hdr_update(taph, l3len + sizeof(struct ethhdr)); The patch applied cleanly after I fixed the context though. Thanks, -- Max

On Mon, Dec 08, 2025 at 08:20:15AM +0100, Stefano Brivio wrote:

Now that we have a new clamped_scale() function, which makes it simple to specify a precise usage factor, change the amount of sending buffer we want to use at and above 4 MiB: 75% looks perfectly safe.

Signed-off-by: Stefano Brivio

Reviewed-by: David Gibson

--- tcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tcp.c b/tcp.c index 026546a..37aceed 100644 --- a/tcp.c +++ b/tcp.c @@ -773,7 +773,7 @@ static void tcp_rtt_dst_check(const struct tcp_tap_conn *conn, }

/** - * tcp_get_sndbuf() - Get, scale SO_SNDBUF between thresholds (1 to 0.5 usage) + * tcp_get_sndbuf() - Get, scale SO_SNDBUF between thresholds (1 to 0.75 usage) * @conn: Connection pointer */ static void tcp_get_sndbuf(struct tcp_tap_conn *conn) @@ -788,7 +788,7 @@ static void tcp_get_sndbuf(struct tcp_tap_conn *conn) return; }

- v = clamped_scale(sndbuf, sndbuf, SNDBUF_SMALL, SNDBUF_BIG, 50); + v = clamped_scale(sndbuf, sndbuf, SNDBUF_SMALL, SNDBUF_BIG, 75);

SNDBUF_SET(conn, MIN(INT_MAX, v)); } -- 2.43.0

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson

On Mon, Dec 08, 2025 at 08:20:19AM +0100, Stefano Brivio wrote:

...instead of checking if the current sending buffer is less than SNDBUF_SMALL, because this isn't simply an optimisation to coalesce ACK segments: we rely on having enough data at once from the sender to make the buffer grow by means of TCP buffer size tuning implemented in the Linux kernel.

This is important if we're trying to maximise throughput, but not desirable for interactive traffic, where we want to be transparent as possible and avoid introducing unnecessary latency.

Use the tcpi_delivery_rate field reported by the Linux kernel, if available, to calculate the current bandwidth-delay product: if it's significantly smaller than the available sending buffer, conclude that we're not bandwidth-bound and this is likely to be interactive traffic, so acknowledge data only as it's acknowledged by the peer.

Conversely, if the bandwidth-delay product is comparable to the size of the sending buffer (more than 5%), we're probably bandwidth-bound or... bound to be: acknowledge everything in that case.

Signed-off-by: Stefano Brivio

Reviewed-by: David Gibson

--- tcp.c | 45 +++++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/tcp.c b/tcp.c index b2e4174..923c1f2 100644 --- a/tcp.c +++ b/tcp.c @@ -353,6 +353,9 @@ enum { #define LOW_RTT_TABLE_SIZE 8 #define LOW_RTT_THRESHOLD 10 /* us */

+/* Ratio of buffer to bandwidth * delay product implying interactive traffic */ +#define SNDBUF_TO_BW_DELAY_INTERACTIVE /* > */ 20 /* (i.e. < 5% of buffer) */ + #define ACK_IF_NEEDED 0 /* See tcp_send_flag() */

#define CONN_IS_CLOSING(conn) \ @@ -426,11 +429,13 @@ socklen_t tcp_info_size; sizeof(((struct tcp_info_linux *)NULL)->tcpi_##f_)) <= tcp_info_size)

/* Kernel reports sending window in TCP_INFO (kernel commit 8f7baad7f035) */ -#define snd_wnd_cap tcp_info_cap(snd_wnd) +#define snd_wnd_cap tcp_info_cap(snd_wnd) /* Kernel reports bytes acked in TCP_INFO (kernel commit 0df48c26d84) */ -#define bytes_acked_cap tcp_info_cap(bytes_acked) +#define bytes_acked_cap tcp_info_cap(bytes_acked) /* Kernel reports minimum RTT in TCP_INFO (kernel commit cd9b266095f4) */ -#define min_rtt_cap tcp_info_cap(min_rtt) +#define min_rtt_cap tcp_info_cap(min_rtt) +/* Kernel reports delivery rate in TCP_INFO (kernel commit eb8329e0a04d) */ +#define delivery_rate_cap tcp_info_cap(delivery_rate)

/* sendmsg() to socket */ static struct iovec tcp_iov [UIO_MAXIOV]; @@ -1050,6 +1055,7 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, socklen_t sl = sizeof(*tinfo); struct tcp_info_linux tinfo_new; uint32_t new_wnd_to_tap = prev_wnd_to_tap; + bool ack_everything = true; int s = conn->sock;

/* At this point we could ack all the data we've accepted for forwarding @@ -1059,7 +1065,8 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, * control behaviour. * * For it to be possible and worth it we need: - * - The TCP_INFO Linux extension which gives us the peer acked bytes + * - The TCP_INFO Linux extensions which give us the peer acked bytes + * and the delivery rate (outbound bandwidth at receiver) * - Not to be told not to (force_seq) * - Not half-closed in the peer->guest direction * With no data coming from the peer, we might not get events which @@ -1069,19 +1076,36 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, * Data goes from socket to socket, with nothing meaningfully "in * flight". * - Not a pseudo-local connection (e.g. to a VM on the same host) - * - Large enough send buffer - * In these cases, there's not enough in flight to bother. + * If it is, there's not enough in flight to bother. + * - Sending buffer significantly larger than bandwidth * delay product + * Meaning we're not bandwidth-bound and this is likely to be + * interactive traffic where we want to preserve transparent + * connection behaviour and latency. + * + * Otherwise, we probably want to maximise throughput, which needs + * sending buffer auto-tuning, triggered in turn by filling up the + * outbound socket queue. */ - if (bytes_acked_cap && !force_seq && + if (bytes_acked_cap && delivery_rate_cap && !force_seq && !CONN_IS_CLOSING(conn) && - !(conn->flags & LOCAL) && !tcp_rtt_dst_low(conn) && - (unsigned)SNDBUF_GET(conn) >= SNDBUF_SMALL) { + !(conn->flags & LOCAL) && !tcp_rtt_dst_low(conn)) { if (!tinfo) { tinfo = &tinfo_new; if (getsockopt(s, SOL_TCP, TCP_INFO, tinfo, &sl)) return 0; }

+ if ((unsigned)SNDBUF_GET(conn) > (long long)tinfo->tcpi_rtt * + tinfo->tcpi_delivery_rate / + 1000 / 1000 * + SNDBUF_TO_BW_DELAY_INTERACTIVE) + ack_everything = false; + } + + if (ack_everything) { + /* Fall back to acknowledging everything we got */ + conn->seq_ack_to_tap = conn->seq_from_tap; + } else { /* This trips a cppcheck bug in some versions, including * cppcheck 2.18.3. * https://sourceforge.net/p/cppcheck/discussion/general/thread/fecde59085/ @@ -1089,9 +1113,6 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, /* cppcheck-suppress [uninitvar,unmatchedSuppression] */ conn->seq_ack_to_tap = tinfo->tcpi_bytes_acked + conn->seq_init_from_tap; - } else { - /* Fall back to acknowledging everything we got */ - conn->seq_ack_to_tap = conn->seq_from_tap; }

/* It's occasionally possible for us to go from using the fallback above -- 2.43.0

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson

On Tue, 9 Dec 2025 16:10:36 +1100 David Gibson wrote:

On Mon, Dec 08, 2025 at 08:20:17AM +0100, Stefano Brivio wrote:

...

A fixed 10 ms ACK_INTERVAL timer value served us relatively well until the previous change, because we would generally cause retransmissions for non-local outbound transfers with relatively high (> 100 Mbps) bandwidth and non-local but low (< 5 ms) RTT.

Now that retransmissions are less frequent, we don't have a proper trigger to check for acknowledged bytes on the socket, and will generally block the sender for a significant amount of time while we could acknowledge more data, instead.

Store the RTT reported by the kernel using an approximation (exponent), to keep flow storage size within two (typical) cachelines. Check for socket updates when half of this time elapses: it should be a good indication of the one-way delay we're interested in (peer to us).

Representable values are between 100 us and 3.2768 s, and any value outside this range is clamped to these bounds. This choice appears to be a good trade-off between additional overhead and throughput.

This mechanism partially overlaps with the "low RTT" destinations, which we use to infer that a socket is connected to an endpoint to the same machine (while possibly in a different namespace) if the RTT is reported as 10 us or less.

This change doesn't, however, conflict with it: we are reading TCP_INFO parameters for local connections anyway, so we can always store the RTT approximation opportunistically.

Then, if the RTT is "low", we don't really need a timer to acknowledge data as we'll always acknowledge everything to the sender right away. However, we have limited space in the array where we store addresses of local destination, so the low RTT property of a connection might toggle frequently. Because of this, it's actually helpful to always have the RTT approximation stored.

This could probably benefit from a future rework, though, introducing a more integrated approach between these two mechanisms.

Signed-off-by: Stefano Brivio --- tcp.c | 30 +++++++++++++++++++++++------- tcp_conn.h | 9 +++++++++ util.c | 14 ++++++++++++++ util.h | 1 + 4 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/tcp.c b/tcp.c index 28d3304..0167121 100644 --- a/tcp.c +++ b/tcp.c @@ -202,9 +202,13 @@ * - ACT_TIMEOUT, in the presence of any event: if no activity is detected on * either side, the connection is reset * - * - ACK_INTERVAL elapsed after data segment received from tap without having + * - 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 + * ACK_TO_TAP_DUE): forcibly check if an ACK segment can be sent. + * + * RTT, here, is an approximation of the RTT value reported by the kernel via + * TCP_INFO, with a representable range from RTT_STORE_MIN (100 us) to + * RTT_STORE_MAX (3276.8 ms). The timeout value is clamped accordingly. * * * Summary of data flows (with ESTABLISHED event) @@ -341,7 +345,6 @@ enum { #define MSS_DEFAULT 536 #define WINDOW_DEFAULT 14600 /* RFC 6928 */

-#define ACK_INTERVAL 10 /* ms */ #define RTO_INIT 1 /* s, RFC 6298 */ #define RTO_INIT_AFTER_SYN_RETRIES 3 /* s, RFC 6298 */ #define FIN_TIMEOUT 60 @@ -593,7 +596,8 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) }

if (conn->flags & ACK_TO_TAP_DUE) { - it.it_value.tv_nsec = (long)ACK_INTERVAL * 1000 * 1000; + it.it_value.tv_sec = RTT_GET(conn) / 2 / (1000 * 1000); + it.it_value.tv_nsec = RTT_GET(conn) / 2 % (1000 * 1000) * 1000; } else if (conn->flags & ACK_FROM_TAP_DUE) { int exp = conn->retries, timeout = RTO_INIT; if (!(conn->events & ESTABLISHED)) @@ -608,9 +612,17 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) it.it_value.tv_sec = ACT_TIMEOUT; }

- flow_dbg(conn, "timer expires in %llu.%03llus", - (unsigned long long)it.it_value.tv_sec, - (unsigned long long)it.it_value.tv_nsec / 1000 / 1000); + if (conn->flags & ACK_TO_TAP_DUE) { + flow_trace(conn, "timer expires in %llu.%03llums", + (unsigned long)it.it_value.tv_sec * 1000 + + (unsigned long long)it.it_value.tv_nsec % + (1000 * 1000), + (unsigned long long)it.it_value.tv_nsec / 1000);

This is the wrong way around. The ms part needs to be:

Ouch, I... half swapped them. Almost. This risks haunting us for a bit. Luckily we have discrete values there so I could make this small table for --trace logs from the broken implementation: #include int main() { unsigned long rtt, n, s; printf("before fix\t\tafter fix\tRTT\n"); for (rtt = 100; rtt < 100 << 16; rtt <<= 1) { s = rtt / 2 / (1000 * 1000); n = rtt / 2 % (1000 * 1000) * 1000; printf("%llu.%03llu\t\t%lu.%02lu\t\t%lu\n", s * 1000 + n % (1000 * 1000), n / 1000, s * 1000 + n / (1000 * 1000), (n / 10000) % 100, rtt); } } before fix after fix RTT 50000.050 0.05 100 100000.100 0.10 200 200000.200 0.20 400 400000.400 0.40 800 800000.800 0.80 1600 600000.1600 1.60 3200 200000.3200 3.20 6400 400000.6400 6.40 12800 800000.12800 12.80 25600 600000.25600 25.60 51200 200000.51200 51.20 102400 400000.102400 102.40 204800 800000.204800 204.80 409600 600000.409600 409.60 819200 200000.819200 819.20 1638400 401000.638400 1638.40 3276800

tv_sec * 1000 + tv_nsec / 1000000 and the fractional (us) part: (tv_nsec / 1000) % 1000

(or if you did just want a single digit after the ., then: tv_nsec / 100000 % 10 )

I guess I'll display two digits instead, I never had a ~100 ns RTT in my tests so I didn't notice until now that the resulting timeout would be displayed as 0.0ms with one fractional digit. Thanks for the snippet, I'll post a patch soon (of course, feel free to do so meanwhile if you already have a local change...). -- Stefano