From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202510 header.b=e5M6+UL7; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id DE6BD5A0619 for ; Mon, 20 Oct 2025 11:22:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1760952169; bh=XgKnY/kMMFM6iQ33DMhYD38JV2TD5QV8/yohmRBzcpM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=e5M6+UL7a6aYkOQiGe6gBbB0CYAI6PKh36Syi03Z2Th0b0jPPyYYjPZw9BHEMZU4T +cteNgtL87wZHFmezI8HCX6EYsidvarM929Er4ZZDRoy5xRb8EZlWTPcQbAO740lwi Q0dIy5kNjjo4qE830U2Nzi3etSmwLPKKIWYVGvhKbSK9Tkh7mfcnU0h5NXLvR+/aGP +2m6XZjAAmvZlM/gHX2l2ATShAe4gsPfbeamaw98BGvXNhdqMQT5WvMip4mORwKXEL UZlacliLDBhfpSH/VdzyVuwtMuzSgHVFnpDXqr8+VaiiV9S6khFWPyoZ1gJvPaGm78 y+zadBfX1L8Qw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4cqqhn2kXCz4wBB; Mon, 20 Oct 2025 20:22:49 +1100 (AEDT) Date: Mon, 20 Oct 2025 20:17:10 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 4/4] tcp: Update data retransmission timeout Message-ID: References: <20251014073836.18150-1-yuhuang@redhat.com> <20251014073836.18150-5-yuhuang@redhat.com> <20251017202812.173e9352@elisabeth> <20251020071107.42fd40e9@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ibi9ofXesZGnpasP" Content-Disposition: inline In-Reply-To: <20251020071107.42fd40e9@elisabeth> Message-ID-Hash: IBU7SQ7Z2NPL4NAAAE4ZJ2RXOPEE4KIP X-Message-ID-Hash: IBU7SQ7Z2NPL4NAAAE4ZJ2RXOPEE4KIP X-MailFrom: dgibson@gandalf.ozlabs.org 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: Yumei Huang , passt-dev@passt.top 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: --ibi9ofXesZGnpasP Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Oct 20, 2025 at 07:11:07AM +0200, Stefano Brivio wrote: > On Mon, 20 Oct 2025 11:20:19 +1100 > David Gibson wrote: >=20 > > On Fri, Oct 17, 2025 at 08:28:12PM +0200, Stefano Brivio wrote: > > > On Thu, 16 Oct 2025 09:54:25 +1100 > > > David Gibson wrote: > > > =20 > > > > On Wed, Oct 15, 2025 at 02:31:27PM +0800, Yumei Huang wrote: =20 > > > > > On Wed, Oct 15, 2025 at 8:05=E2=80=AFAM David Gibson > > > > > wrote: =20 > > > > > > > > > > > > On Tue, Oct 14, 2025 at 03:38:36PM +0800, Yumei Huang wrote: = =20 > > > > > > > According to RFC 2988 and RFC 6298, we should use an exponent= ial > > > > > > > backoff timeout for data retransmission starting from one sec= ond > > > > > > > (see Appendix A in RFC 6298), and limit it to about 60 seconds > > > > > > > as allowed by the same RFC: > > > > > > > > > > > > > > (2.5) A maximum value MAY be placed on RTO provided it is = at > > > > > > > least 60 seconds. =20 > > > > > > > > > > > > The interpretation of this isn't entirely clear to me. Does it= mean > > > > > > if the total retransmit delay exceeds 60s we give up and RST (w= hat > > > > > > this patch implements)? Or does it mean that if the retransmit= delay > > > > > > reaches 60s we keep retransmitting, but don't increase the dela= y any > > > > > > further? > > > > > > > > > > > > Looking at tcp_bound_rto() and related code in the kernel sugge= sts the > > > > > > second interpretation. > > > > > > =20 > > > > > > > Combine the macros defining the initial timeout for both SYN = and ACK. > > > > > > > And add a macro ACK_RETRIES to limit the total timeout to abo= ut 60s. > > > > > > > > > > > > > > Signed-off-by: Yumei Huang > > > > > > > --- > > > > > > > tcp.c | 32 ++++++++++++++++---------------- > > > > > > > 1 file changed, 16 insertions(+), 16 deletions(-) > > > > > > > > > > > > > > diff --git a/tcp.c b/tcp.c > > > > > > > index 3ce3991..84da069 100644 > > > > > > > --- a/tcp.c > > > > > > > +++ b/tcp.c > > > > > > > @@ -179,16 +179,12 @@ > > > > > > > * > > > > > > > * Timeouts are implemented by means of timerfd timers, set = based on flags: > > > > > > > * > > > > > > > - * - SYN_TIMEOUT_INIT: if no ACK is received from tap/guest = during handshake > > > > > > > - * (flag ACK_FROM_TAP_DUE without ESTABLISHED event) withi= n this time, resend > > > > > > > - * SYN. It's the starting timeout for the first SYN retry.= If this persists > > > > > > > - * for more than TCP_MAX_RETRIES or (tcp_syn_retries + > > > > > > > - * tcp_syn_linear_timeouts) times in a row, reset the conn= ection > > > > > > > - * > > > > > > > - * - ACK_TIMEOUT: if no ACK segment was received from tap/gu= est, after sending > > > > > > > - * data (flag ACK_FROM_TAP_DUE with ESTABLISHED event), re= -send data from the > > > > > > > - * socket and reset sequence to what was acknowledged. If = this persists for > > > > > > > - * more than TCP_MAX_RETRIES times in a row, reset the con= nection > > > > > > > + * - ACK_TIMEOUT_INIT: if no ACK segment was received from t= ap/guest, eiher > > > > > > > + * during handshake(flag ACK_FROM_TAP_DUE without ESTABLIS= HED event) or after > > > > > > > + * sending data (flag ACK_FROM_TAP_DUE with ESTABLISHED ev= ent), re-send data > > > > > > > + * from the socket and reset sequence to what was acknowle= dged. It's the > > > > > > > + * starting timeout for the first retry. If this persists = for more than > > > > > > > + * allowed times in a row, reset the connection > > > > > > > * > > > > > > > * - FIN_TIMEOUT: if a FIN segment was sent to tap/guest (fl= ag ACK_FROM_TAP_DUE > > > > > > > * with TAP_FIN_SENT event), and no ACK is received within= this time, reset > > > > > > > @@ -342,8 +338,7 @@ enum { > > > > > > > #define WINDOW_DEFAULT 14600 = /* RFC 6928 */ > > > > > > > > > > > > > > #define ACK_INTERVAL 10 /* ms */ > > > > > > > -#define SYN_TIMEOUT_INIT 1 /* s */ > > > > > > > -#define ACK_TIMEOUT 2 > > > > > > > +#define ACK_TIMEOUT_INIT 1 /* s, R= FC 6298 */ =20 > > > > > > > > > > > > I'd suggest calling this RTO_INIT to match the terminology used= in the > > > > > > RFCs. =20 > > > > >=20 > > > > > Sure. =20 > > > > > > =20 > > > > > > > #define FIN_TIMEOUT 60 > > > > > > > #define ACT_TIMEOUT 7200 > > > > > > > > > > > > > > @@ -352,6 +347,11 @@ enum { > > > > > > > > > > > > > > #define ACK_IF_NEEDED 0 /* See tcp_send= _flag() */ > > > > > > > > > > > > > > +/* Number of retries calculated from the exponential backoff= formula, limited > > > > > > > + * by a total timeout of about 60 seconds. > > > > > > > + */ > > > > > > > +#define ACK_RETRIES 5 > > > > > > > + =20 > > > > > > > > > > > > As noted above, I think this is based on a misunderstanding of = what > > > > > > the RFC is saying. TCP_MAX_RETRIES should be fine as it is, I = think. > > > > > > We could implement the clamping of the RTO, but it's a "MAY" in= the > > > > > > RFC, so we don't have to, and I don't really see a strong reaso= n to do > > > > > > so. =20 > > > > >=20 > > > > > If we use TCP_MAX_RETRIES and not clamping RTO, the total timeout > > > > > could be 255 seconds. > > > > >=20 > > > > > Stefano mentioned "Retransmitting data after 256 seconds doesn't = make > > > > > a lot of sense to me" in the previous comment. =20 > > > >=20 > > > > That's true, but it's pretty much true for 60s as well. For the lo= cal > > > > link we usually have between passt and guest, even 1s is an eternit= y. =20 > > >=20 > > > Rather than the local link I was thinking of whatever monitor or > > > liveness probe in KubeVirt which might have a 60-second period, or so= me > > > firewall agent, or how long it typically takes for guests to stop and > > > resume again in KubeVirt. =20 > >=20 > > Right, I hadn't considered those. Although.. do those actually re-use > > a single connection? I would have guessed they use a new connection > > each time, making the timeouts here irrelevant. >=20 > It depends on the definition of "each time", because we don't time out > host-side connections immediately. Hm, ok. Is your concern that getting a negative answer from the probe will take too long? > Pretending passt isn't there, the timeout would come from the default > values for TCP connections. It looks like there's no specific > SO_SNDTIMEO value set for those probes, and you can't configure the > timeout, at least according to: >=20 > https://kubernetes.io/docs/tasks/configure-pod-container/configure-live= ness-readiness-startup-probes/#define-a-tcp-liveness-probe My guess would be that the probe would probably time out at the application level long before the TCP layer times out, but I don't know for sure. > and for tcp_syn_retries, tcp(7) says: >=20 > The default value is 6, which corresponds to retrying for up to > approximately 127 seconds. >=20 > In this series, to make things transparent, we read out those values, > so that part is fine. But does the Linux kernel clamp the RTO? >=20 > It turns out that yes, it does, TCP_RTO_MAX_SEC is 120 seconds (before > 1280c26228bd ("tcp: add tcp_rto_max_ms sysctl") that was TCP_RTO_MAX, > same value), and it's used by tcp_retransmit_timer() via tcp_rto_max(). > That change makes it configurable. >=20 > I'm tempted to suggest that we should read out that value as well > (with a 120-second fallback for older kernels) to make our behaviour > as transparent as possible. >=20 > It's slightly more complicated and perhaps not strictly needed, but > we've been bitten a few times by cases where applications and users > expect us to behave like the Linux kernel, and we didn't... so maybe > we could do this as well while at it? Given the rest of this series, > it looks like a relatively small addition to it. I think that's a good idea. It's a bit more work, but it doesn't greatly increase the conceptual complexity and will more closely match the kernel's behaviour. --=20 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 --ibi9ofXesZGnpasP Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmj1/gQACgkQzQJF27ox 2GexkRAAkQESlQoFDNZSIPrmP3Oclm7mKg8Xm776ZxLo7XE5x/RyDeBp7oYKlPZb 5IThTBZ18KBlZG+ONYNB1019RsZknrbQpTaXaJCSKty+U44wfPqJONpBmuMCd0wg 6rs7FOEx35iq1KT0Hkk6fnXITC4RhGYMuDFWFWjSWho+2JgDDEDvapauq5JL+zF/ auASag/xUbzOJOBJrgxZIW4czoT2oIhDOWZjWimg50KyAa+6uDyKrGiTle+yvRWU 6HfHRQBB+duRbRb+2y9u7W3HkeFroONwOLE+zKQWxlICgWK4JtIoKoL8dexhgNdD SjVW+vNbKFtyNWJxEx7nF3Omg1lJw7wY52HDeYxA5E47J8rjH/p5shh3pmBXz1rW mBYSrj8IKN3bY5g39f/4Z/EGUvml2XqIW5qa3XeRgZX0zFR3dg3hsGSS3yB1CJe3 4IXcL1J1v0ThscsCAIH3hHTkk7mUfkgYFn1bJiIYIA/ESR+O07CV6WHbSSpggEO9 saBAVd9QCacDescLQKrfv8bXUGTUw8K5Qu/mwfNpD5Z1aRHmH1Xa06CRZvy7QZwT NXjvB4JhYolK/8XsEqb4LdhjR/uPgk4owgoUCEXma2VkZ5YELAB540BnqwnI6sYe ST5Hmes69k+w9es595hhUv6LvqKR6S2IgpyEYSK1rp2wSWXUeIY= =x75n -----END PGP SIGNATURE----- --ibi9ofXesZGnpasP--