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: 3302 bytes --]

On Fri, Dec 05, 2025 at 02:20:16AM +0100, Stefano Brivio wrote:
> 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).

Ok.  I guess you could look at Nagle's algorithm as (amongst other
things) a refinement of the sender-side anti-SWS angorithm in RFC 813.

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

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