Re: [PATCH 5/6] tcp: Don't try to transmit right after the peer shrank the window to zero

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

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

On Fri, Aug 15, 2025 at 06:10:41PM +0200, Stefano Brivio wrote:
> If the peer shrinks the window to zero, we'll skip storing the new
> window, as a convenient way to cause window probes (which exceed any
> zero-sized window, strictly speaking) if we don't get window updates
> in a while.

Strictly speaking, not storing the new zero window feels slightly
wrong to me - I wonder if it would be more correct to store the zero
window, but still send window probes as a special case.

> As we do so, though, we need to ensure we don't try to queue more data
> from the socket right after we process this window update, as the
> entire point of a zero-window advertisement is to keep us from sending
> more data.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

For the meantime, though, I'm reasonably confident that this is still
an improvement, so,

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


> ---
>  tcp.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index aed25a9..624e7f4 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1260,8 +1260,10 @@ static void tcp_get_tap_ws(struct tcp_tap_conn *conn,
>   * @c:		Execution context
>   * @conn:	Connection pointer
>   * @wnd:	Window value, host order, unscaled
> + *
> + * Return: false on zero window (not stored to wnd_from_tap), true otherwise
>   */
> -static void tcp_tap_window_update(const struct ctx *c,
> +static bool tcp_tap_window_update(const struct ctx *c,
>  				  struct tcp_tap_conn *conn, unsigned wnd)
>  {
>  	wnd = MIN(MAX_WINDOW, wnd << conn->ws_from_tap);
> @@ -1274,13 +1276,14 @@ static void tcp_tap_window_update(const struct ctx *c,
>  	 */
>  	if (!wnd && SEQ_LT(conn->seq_ack_from_tap, conn->seq_to_tap)) {
>  		tcp_rewind_seq(c, conn);
> -		return;
> +		return false;
>  	}
>  
>  	conn->wnd_from_tap = MIN(wnd >> conn->ws_from_tap, USHRT_MAX);
>  
>  	/* FIXME: reflect the tap-side receiver's window back to the sock-side
>  	 * sender by adjusting SO_RCVBUF? */
> +	return true;
>  }
>  
>  /**
> @@ -2066,9 +2069,8 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
>  		if (!th->ack)
>  			goto reset;
>  
> -		tcp_tap_window_update(c, conn, ntohs(th->window));
> -
> -		tcp_data_from_sock(c, conn);
> +		if (tcp_tap_window_update(c, conn, ntohs(th->window)))
> +			tcp_data_from_sock(c, conn);
>  
>  		if (p->count - idx == 1)
>  			return 1;
> @@ -2078,8 +2080,8 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
>  	if (conn->events & TAP_FIN_RCVD) {
>  		tcp_sock_consume(conn, ntohl(th->ack_seq));
>  		tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq));
> -		tcp_tap_window_update(c, conn, ntohs(th->window));
> -		tcp_data_from_sock(c, conn);
> +		if (tcp_tap_window_update(c, conn, ntohs(th->window)))
> +			tcp_data_from_sock(c, conn);
>  
>  		if (conn->seq_ack_from_tap == conn->seq_to_tap) {
>  			if (th->ack && conn->events & TAP_FIN_SENT)

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