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=uibYezxZ; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id C04C35A061A for ; Wed, 15 Oct 2025 02:05:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202510; t=1760486709; bh=YTi5zEJEB1IOUMBGBo/ftBiJXHb9w6mBh+VJka23w+Q=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=uibYezxZ55zHZktUQCjLUjY9zSy2Uz4Cmb2swlRtEVPFpgvQ2pQwz7wI2uoCpbk17 rRVPwr1MlP4ws+BEllIgMvIW2XEviB6XOvNRlnc+F4HDYe2p5Ai4hqqXu0s5qsYdw9 7JorxAmYLSR1umR54uFQNe/YWg0Pp9lrIc6d5IvN08Rc0i9QqTBWTDIGX99ns9/TxZ 38Ca1wT/qv5+Pk8PLEhxhbnQHj9+n+9dyHH67X1nTGNOdCqFyW/QIb4FXsVmHwbLNA AH+5AL3T36WaADQEPb87FR4VX9tDBHrhOIUjPcMEccS3XqAKwSy5cm+OGp8YVxpeHg dVWJgaicHCezg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4cmWYd3MpYz4w9w; Wed, 15 Oct 2025 11:05:09 +1100 (AEDT) Date: Wed, 15 Oct 2025 11:05:05 +1100 From: David Gibson To: Yumei Huang 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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="IyG5BJXqphxD4V+o" Content-Disposition: inline In-Reply-To: <20251014073836.18150-5-yuhuang@redhat.com> Message-ID-Hash: NQGJ2FQFP5HS4HLMDNUMQVRARLBST6HU X-Message-ID-Hash: NQGJ2FQFP5HS4HLMDNUMQVRARLBST6HU 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: passt-dev@passt.top, sbrivio@redhat.com 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: --IyG5BJXqphxD4V+o Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 14, 2025 at 03:38:36PM +0800, Yumei Huang wrote: > According to RFC 2988 and RFC 6298, we should use an exponential > backoff timeout for data retransmission starting from one second > (see Appendix A in RFC 6298), and limit it to about 60 seconds > as allowed by the same RFC: >=20 > (2.5) A maximum value MAY be placed on RTO provided it is at > least 60 seconds. 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 (what this patch implements)? Or does it mean that if the retransmit delay reaches 60s we keep retransmitting, but don't increase the delay any further? Looking at tcp_bound_rto() and related code in the kernel suggests the second interpretation. > Combine the macros defining the initial timeout for both SYN and ACK. > And add a macro ACK_RETRIES to limit the total timeout to about 60s. >=20 > Signed-off-by: Yumei Huang > --- > tcp.c | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) >=20 > 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 fla= gs: > * > - * - SYN_TIMEOUT_INIT: if no ACK is received from tap/guest during hands= hake > - * (flag ACK_FROM_TAP_DUE without ESTABLISHED event) within this time,= resend > - * SYN. It's the starting timeout for the first SYN retry. If this per= sists > - * for more than TCP_MAX_RETRIES or (tcp_syn_retries + > - * tcp_syn_linear_timeouts) times in a row, reset the connection > - * > - * - ACK_TIMEOUT: if no ACK segment was received from tap/guest, after s= ending > - * data (flag ACK_FROM_TAP_DUE with ESTABLISHED event), re-send data f= rom the > - * socket and reset sequence to what was acknowledged. If this persist= s for > - * more than TCP_MAX_RETRIES times in a row, reset the connection > + * - ACK_TIMEOUT_INIT: if no ACK segment was received from tap/guest, ei= her > + * during handshake(flag ACK_FROM_TAP_DUE without ESTABLISHED event) o= r after > + * sending data (flag ACK_FROM_TAP_DUE with ESTABLISHED event), re-sen= d data > + * from the socket and reset sequence to what was acknowledged. It's t= he > + * starting timeout for the first retry. If this persists for more tha= n=20 > + * allowed times in a row, reset the connection > * > * - FIN_TIMEOUT: if a FIN segment was sent to tap/guest (flag 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 */ > =20 > #define ACK_INTERVAL 10 /* ms */ > -#define SYN_TIMEOUT_INIT 1 /* s */ > -#define ACK_TIMEOUT 2 > +#define ACK_TIMEOUT_INIT 1 /* s, RFC 6298 */ I'd suggest calling this RTO_INIT to match the terminology used in the RFCs. > #define FIN_TIMEOUT 60 > #define ACT_TIMEOUT 7200 > =20 > @@ -352,6 +347,11 @@ enum { > =20 > #define ACK_IF_NEEDED 0 /* See tcp_send_flag() */ > =20 > +/* Number of retries calculated from the exponential backoff formula, li= mited > + * by a total timeout of about 60 seconds. > + */ > +#define ACK_RETRIES 5 > + 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 reason to do so. > #define CONN_IS_CLOSING(conn) \ > (((conn)->events & ESTABLISHED) && \ > ((conn)->events & (SOCK_FIN_RCVD | TAP_FIN_RCVD))) > @@ -589,13 +589,13 @@ static void tcp_timer_ctl(const struct ctx *c, stru= ct tcp_tap_conn *conn) > } else if (conn->flags & ACK_FROM_TAP_DUE) { > if (!(conn->events & ESTABLISHED)) { > if (conn->retries < c->tcp.syn_linear_timeouts) > - it.it_value.tv_sec =3D SYN_TIMEOUT_INIT; > + it.it_value.tv_sec =3D ACK_TIMEOUT_INIT; > else > - it.it_value.tv_sec =3D SYN_TIMEOUT_INIT << > + it.it_value.tv_sec =3D ACK_TIMEOUT_INIT << > (conn->retries - c->tcp.syn_linear_timeouts); > } > else > - it.it_value.tv_sec =3D ACK_TIMEOUT; > + it.it_value.tv_sec =3D ACK_TIMEOUT_INIT << conn->retries; > } else if (CONN_HAS(conn, SOCK_FIN_SENT | TAP_FIN_ACKED)) { > it.it_value.tv_sec =3D FIN_TIMEOUT; > } else { > @@ -2433,7 +2433,7 @@ void tcp_timer_handler(const struct ctx *c, union e= poll_ref ref) > } else if (CONN_HAS(conn, SOCK_FIN_SENT | TAP_FIN_ACKED)) { > flow_dbg(conn, "FIN timeout"); > tcp_rst(c, conn); > - } else if (conn->retries =3D=3D TCP_MAX_RETRIES) { > + } else if (conn->retries >=3D ACK_RETRIES) { > flow_dbg(conn, "retransmissions count exceeded"); > tcp_rst(c, conn); > } else { > --=20 > 2.47.0 >=20 --=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 --IyG5BJXqphxD4V+o Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmju5TAACgkQzQJF27ox 2GcDLA/+O2nMwHwkma+byoAGu+g0GiqGTSdqL//NyuKCkQZZ/vYZwMx1xVNRkzLN Z/UBnOXSb/raMr5F9ZCOoKp8E4HuuLOKgKlFh7++e/NavDp81olN5EzNXq/8bjaX rJdFL+/LQeouXnbUYa1Ab4tJrOTjhLjIxtEDlu3Ic7TT3f1TZaWsO+/OWXe3cfwz GhWjoDdGWXZiB86hQcBm40i1QSe4uWfQPUIqw1KYg9Azvhtlu+BH94jK1x2caOSN I4oA+4FVgUxVgvKC+GXSZWXrA7LfRldGUF2MrYzdf57eXIqiq/YYPjBSsxlY0Y8C sbb7Rq8PPYAFxLN3RlTV2tIseiT0uOccEnKlrIZU3NbmBsvkHkblnoy0Jtx9nmQD TasPSKxXYjZeApMTBKrLvzDg+lybtEzM01eTHdrUEOeIpEimSgNO/LSXlnLVM5TI gwnippQFELD3LgjIngwXHVEf9Yxw81NW92J9aEhZjgRP1U4nWKmZWBDuWbvbRNTS 2eIxQKTmbuk5ZCTX8nrFJq2TGevc4LwAztcKp5wo2f5/I/BcVYRifIaGDi4vrl6X kIYw3+DWVKqpjWozfnl/CvrDn2NhCh9Y5bJO3IUTeFJv/H52rBONMQv9RZJobULY 7FUmQEc4kgXCI9jPdoO3p8P7Lpi7sa2BDUiMEuWde1Ux/snhzSI= =EcTY -----END PGP SIGNATURE----- --IyG5BJXqphxD4V+o--