From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=GdalrAQd; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id 70C1B5A0279 for ; Thu, 11 Sep 2025 10:59:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1757581159; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=N5krBS4z3iZnAO69pQcH5aJ8eorL2N06+GU92pQGjuk=; b=GdalrAQddRUJUmWuZY0MgiwwArLrCtf7qsJGs66C0yEnP9pNdvPXx69gZSOE70MOZeFwXR 1yFXZAsnIRR2EUO8bLYssQtjH3KAFCpCPdjjvs10FcAzheL+vAWFMra1M9ueQouI8nGQwt pEUrMZUbRPmPMpDaxg/zWwkFozILzWY= Received: from mail-wm1-f72.google.com (mail-wm1-f72.google.com [209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-582-DC33L8-MNTCkQWeTzdD9pQ-1; Thu, 11 Sep 2025 04:59:17 -0400 X-MC-Unique: DC33L8-MNTCkQWeTzdD9pQ-1 X-Mimecast-MFC-AGG-ID: DC33L8-MNTCkQWeTzdD9pQ_1757581157 Received: by mail-wm1-f72.google.com with SMTP id 5b1f17b1804b1-45df9e11fc6so2707485e9.3 for ; Thu, 11 Sep 2025 01:59:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1757581157; x=1758185957; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=N5krBS4z3iZnAO69pQcH5aJ8eorL2N06+GU92pQGjuk=; b=Mon8CwRW2UNAh7H1elYTcz8z9jNAfVrXki5p6iGwedAYoadVIgHTqQcVtRNf3g23Q1 ph2wIG8xGQ6tW+p57aUTOLSRAzhhtS79+mcChkR5K2DY1Q9FNk5+BA8HHqR5b10cnvsr gIA7cfFdRZdK/YrB1hpVN7cgLwLQ7tX1UF5d2TSo9A1HO2YKaBN9QKxbsCLrc4/xr38X bfx9U+FSCIzDYU4DwnZp5355FgJ9FmFaGBu4wSKBfboxmlOEQRaB8/+8TZpKV/+6/E65 kXVW9KgAdpZ7qwqIXkD2tuCxj9GNkQrEp3Q7jWEYdZfQt2naKY9VTxPaJBu7SOCSjpxp w4kA== X-Gm-Message-State: AOJu0Yy0js91TaUGVSr6lAUAotsbrVvz5rf5nMzp0NaYI+DV289LlBii 31nkcMu00y+Z/QhF80/WzdjVwoSGVgn0DAP2ZP8XtGYeKV4z5BLzdNMaGdB6q6sfSnZLt9gj6Jq 7v9ey3xcrG13nzG6W5SZVLanjOihOqGK1NFyY15Pw3DWs6EO8ZUOuTQ== X-Gm-Gg: ASbGnctgvQ9A2Q7QcMtHuveEyztJOL0VKK5/HF0HFxurtBXY0BHtDWybLhtNbuCTfaV EqonrzZp5ffOQHDWtNSubp8lzFUx/w60MdlVUHptv++pPA4LoihuKJ0wCXpHW4Vu38Pf/KiXSJh dfm6L6r7oxT81rf3pAReG4AlfRiujp2RNt/3/nrfyPeDSC0l6iPT3ltPZkAvpqlYUUyqSWiTCxE hpwnE42J/YryN21J73CItJ3yaNUStx+j5Vg8mz0SEs0Nhqias0VrrZ8GkPDvfZI4Tpr0RoRfHH1 3HSiMdgd5CvS6jJfS1+ugxH3vyCQwuHP+QGF6X+qlrGjels/7F80r6BGp6+LZR7IhH/u X-Received: by 2002:a05:600c:4ed1:b0:45b:8b51:b1dd with SMTP id 5b1f17b1804b1-45ddded6f06mr169221585e9.32.1757581156491; Thu, 11 Sep 2025 01:59:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFVVSNYmj+okeVAjiliNSYL53wemceNrMY1w4HBhLH80CKRn0pFNfTDOBApAbdVDKQfMlAk4w== X-Received: by 2002:a05:600c:4ed1:b0:45b:8b51:b1dd with SMTP id 5b1f17b1804b1-45ddded6f06mr169220715e9.32.1757581155951; Thu, 11 Sep 2025 01:59:15 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-3e7607e13f4sm1595437f8f.57.2025.09.11.01.59.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 11 Sep 2025 01:59:15 -0700 (PDT) Date: Thu, 11 Sep 2025 10:59:13 +0200 From: Stefano Brivio To: Jon Maloy Subject: Re: [PATCH v4 5/8] tcp: Don't try to transmit right after the peer shrank the window to zero Message-ID: <20250911105913.031ff3bf@elisabeth> In-Reply-To: <2aa49410-9bf9-40d0-bfcf-c88c80c0430a@redhat.com> References: <20250909181655.2990223-1-sbrivio@redhat.com> <20250909181655.2990223-6-sbrivio@redhat.com> <2aa49410-9bf9-40d0-bfcf-c88c80c0430a@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: 6ZE-OHkd_w8SLfQZAtKlmEVFBZp0xAjmnWYYwDnzFhs_1757581157 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: 3OY3WO3L4H2RFDMUBSHQTRHRZUY6JWW3 X-Message-ID-Hash: 3OY3WO3L4H2RFDMUBSHQTRHRZUY6JWW3 X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top, Paul Holzinger , David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, 10 Sep 2025 20:12:13 -0400 Jon Maloy wrote: > 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. See your own reasoning in commit a740e16fd1b9 ("tcp: handle shrunk window advertisements from guest"). You described it as "extremely simple" back then, and now this is becoming marginally more complicated, but not because of this patch. > 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? We could also store the actual window value, instead of an explicit flag, but yes, it's becoming clear for other reasons that we need to introduce a couple of new flags to simplify all this, see: https://archives.passt.top/passt-dev/20250910083754.323c0b1e@elisabeth/ > > 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 > > Reviewed-by: David Gibson > > --- > > 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. This is all not related to this patch, but see commit cf3eeba6c0d7 ("tcp: Don't use TCP_WINDOW_CLAMP") for the reasoning. It's not about harm, it's rather about making our behaviour as transparent as possible. > > + 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) -- Stefano