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

[Jon Maloy <jmaloy@redhat.com>]

On 2025-09-09 14:16, Stefano Brivio wrote:
> If the peer shrinks the window to zero, we'll skip storing the new
> window, as a convenient 

Is this really convenient? It looks more like an inconsistency with 
potential for future trouble to me. Wouldn't it be better with just
a SEND_WIN_PROBE flag or similar to be reset as soon as the window goes
non-zero again?

> way to cause window probes (which exceed any
> zero-sized window, strictly speaking) if we don't get window updates
> in a while.
> 
> 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>
> 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 b83510b..9c70a25 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1271,8 +1271,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);
> @@ -1285,13 +1287,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? */

Not so sure. That sender will stop in due time anyway, with no harm 
done. Starting fiddling with SO_RCVBUF sounds like something to avoid.

> +	return true;
>   }
>   
>   /**
> @@ -2101,9 +2104,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;
> @@ -2113,8 +2115,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)