Re: [PATCH v2 6/9] tcp: Don't limit window to less-than-MSS values, use zero instead

[David Gibson <david@gibson.dropbear.id.au>]

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

On Mon, Dec 08, 2025 at 01:22:14AM +0100, Stefano Brivio wrote:
> If the sender uses data clumping (including Nagle's algorithm) for
> Silly Window Syndrome (SWS) avoidance, advertising less than a MSS
> means the sender might stop sending altogether, and window updates
> after a low window condition are just as important as they are in
> a zero-window condition.
> 
> For simplicity, approximate that limit to zero, as we have an
> implementation forcing window updates after zero-sized windows.
> This matches the suggestion from RFC 813, section 4.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Looking at this again, I'm worrying if it might allow a pathalogical
case here: unlikely to hit, but very bad if it did.

Suppose we have:
  1. A receiver that wants to consume its input in fixed largish
     (~64kiB) records
  2. The receiver has locked its SO_RCVBUF to that record length, or
     only slightly more
  3. The receive buf is near full - but not quite a full record's
     worth

The receiver doesn't consume anything, because it doesn't have a full
record.  Its rcvbuf is near full, so its kernel advertises only a
small window.  We approximate that to zero, so the sender can't send
anything.  So, the record never gets completed and we stall
completely.

The solution would be to "time out" this limitation of the advertised
window, not sure how complicated that would be in practice.

> ---
>  tcp.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/tcp.c b/tcp.c
> index 533c8a7..3c046a5 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1155,6 +1155,23 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn,
>  		else
>  			limit = SNDBUF_GET(conn) - (int)sendq;
>  
> +		/* If the sender uses mechanisms to prevent Silly Window
> +		 * Syndrome (SWS, described in RFC 813 Section 3) it's critical
> +		 * that, should the window ever become less than the MSS, we
> +		 * advertise a new value once it increases again to be above it.
> +		 *
> +		 * The mechanism to avoid SWS in the kernel is, implicitly,
> +		 * implemented by Nagle's algorithm (which was proposed after
> +		 * RFC 813).
> +		 *
> +		 * To this end, for simplicity, approximate a window value below
> +		 * the MSS to zero, as we already have mechanisms in place to
> +		 * force updates after the window becomes zero. This matches the
> +		 * suggestion from RFC 813, Section 4.
> +		 */
> +		if (limit < MSS_GET(conn))
> +			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 --]