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

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 5 Dec 2025 11:35:22 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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.

Sure, Nagle's algorithm was published almost two years later (RFC 896).

>  * 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.

The practical problem I observed comes from the "clumping" Linux does
while sending (and that's implemented as part of Nagle's algorithm).
But yes I actually ignored section 4 in all this, I'll mention it
explicitly.

> > ---
> >  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

-- 
Stefano