[PATCH] tcp: Fix rounding issue in check for approximating window to zero

[PATCH] tcp: Fix rounding issue in check for approximating window to zero

[Stefano Brivio]

In general, we approximate the advertised window to zero if we would
otherwise advertise less than a MSS worth, and the reasoning behind
that is explained in cf1925fb7b77 ("tcp: Don't limit window to
less-than-MSS values, use zero instead").

Then, in commit b40f5cd8c8e1 ("tcp: Use less-than-MSS window on no
queued data, or no data sent recently"), I introduced some conditions
under which we won't do that, including a check on whether any data
was sent recently.

As an arbitrary but probably reasonable threshold, we consider data to
have recently been sent if that occurred less than ten times the
round-trip time (RTT) ago.

The time elapsed since the last data transmission is reported by the
kernel in milliseconds, in the tcpi_last_data_sent field of struct
tcp_info, and the RTT is reported in microseconds instead, in
tcpi_rtt.

To avoid the risk of overflow in a simple way, for the purpose of this
comparison, I converted tcpi_rtt to milliseconds first, but this means
that the check will always be false (and we'll never approximate the
window to zero) if the RTT is below one millisecond.

This, in turn, reintroduces nasty delay issues in transfers in
non-local connections which have however almost-local (low) latency.

Given that we want to use ten times the RTT as an arbitrary "long
enough" upper bound, round the RTT up while converting it to
milliseconds.

As an alternative, we could perform the comparison in microseconds,
but we would need a slightly more complicated implementation to
exclude overflows, and it's definitely not worth it given the nature
of this threshold.

Fixes: b40f5cd8c8e1 ("tcp: Use less-than-MSS window on no queued data, or no data sent recently")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 tcp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tcp.c b/tcp.c
index e7fa85f..9b7f505 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1182,6 +1182,7 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn,
 	if ((conn->flags & LOCAL) || tcp_rtt_dst_low(conn)) {
 		new_wnd_to_tap = tinfo->tcpi_snd_wnd;
 	} else {
+		unsigned rtt_ms_ceiling = DIV_ROUND_UP(tinfo->tcpi_rtt, 1000);
 		uint32_t sendq;
 		int limit;
 
@@ -1225,7 +1226,7 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn,
 		 *   with pending data in the outbound queue
 		 */
 		if (limit < MSS_GET(conn) && sendq &&
-		    tinfo->tcpi_last_data_sent < tinfo->tcpi_rtt / 1000 * 10)
+		    tinfo->tcpi_last_data_sent < rtt_ms_ceiling * 10)
 			limit = 0;
 
 		new_wnd_to_tap = MIN((int)tinfo->tcpi_snd_wnd, limit);
-- 
2.43.0

Re: [PATCH] tcp: Fix rounding issue in check for approximating window to zero

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3128 bytes --]

On Fri, Jan 09, 2026 at 03:14:43PM +0100, Stefano Brivio wrote:
> In general, we approximate the advertised window to zero if we would
> otherwise advertise less than a MSS worth, and the reasoning behind
> that is explained in cf1925fb7b77 ("tcp: Don't limit window to
> less-than-MSS values, use zero instead").
> 
> Then, in commit b40f5cd8c8e1 ("tcp: Use less-than-MSS window on no
> queued data, or no data sent recently"), I introduced some conditions
> under which we won't do that, including a check on whether any data
> was sent recently.
> 
> As an arbitrary but probably reasonable threshold, we consider data to
> have recently been sent if that occurred less than ten times the
> round-trip time (RTT) ago.
> 
> The time elapsed since the last data transmission is reported by the
> kernel in milliseconds, in the tcpi_last_data_sent field of struct
> tcp_info, and the RTT is reported in microseconds instead, in
> tcpi_rtt.
> 
> To avoid the risk of overflow in a simple way, for the purpose of this
> comparison, I converted tcpi_rtt to milliseconds first, but this means
> that the check will always be false (and we'll never approximate the
> window to zero) if the RTT is below one millisecond.
> 
> This, in turn, reintroduces nasty delay issues in transfers in
> non-local connections which have however almost-local (low) latency.
> 
> Given that we want to use ten times the RTT as an arbitrary "long
> enough" upper bound, round the RTT up while converting it to
> milliseconds.
> 
> As an alternative, we could perform the comparison in microseconds,
> but we would need a slightly more complicated implementation to
> exclude overflows, and it's definitely not worth it given the nature
> of this threshold.
> 
> Fixes: b40f5cd8c8e1 ("tcp: Use less-than-MSS window on no queued data, or no data sent recently")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tcp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tcp.c b/tcp.c
> index e7fa85f..9b7f505 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1182,6 +1182,7 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn,
>  	if ((conn->flags & LOCAL) || tcp_rtt_dst_low(conn)) {
>  		new_wnd_to_tap = tinfo->tcpi_snd_wnd;
>  	} else {
> +		unsigned rtt_ms_ceiling = DIV_ROUND_UP(tinfo->tcpi_rtt, 1000);
>  		uint32_t sendq;
>  		int limit;
>  
> @@ -1225,7 +1226,7 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn,
>  		 *   with pending data in the outbound queue
>  		 */
>  		if (limit < MSS_GET(conn) && sendq &&
> -		    tinfo->tcpi_last_data_sent < tinfo->tcpi_rtt / 1000 * 10)
> +		    tinfo->tcpi_last_data_sent < rtt_ms_ceiling * 10)
>  			limit = 0;
>  
>  		new_wnd_to_tap = MIN((int)tinfo->tcpi_snd_wnd, limit);
> -- 
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]