Re: [PATCH 5/8] 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: 2433 bytes --]

On Thu, Dec 04, 2025 at 08:45:38AM +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.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

The logic change looks good to me, so,

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

However, a couple of points about the description (both commit message
and comment).

 * Nagle's algorithm is certainly related, but it's not clear to me
   it's quite the same thing as the sender-side SWS avoidance
   algorithm - Nagle's exists for a different purpose, certainly.
   RFC 813 doesn't name Nagle's algorithm anywhere,    although that
   could because the name wasn't as established at the time.
 * Since you're referencing RFC 813 anyway, it seems relevant that
   what you're doing here is pretty similar to the receiver-side SWS
   avoidance algorithm described in section 4.

> ---
>  tcp.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/tcp.c b/tcp.c
> index fbf97a0..2220059 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1140,6 +1140,18 @@ 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 Nagle's algorithm to prevent Silly Window
> +		 * Syndrome (SWS, 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.
> +		 *
> +		 * 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.
> +		 */
> +		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 --]