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=RwIi0PT4; 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 C844C5A026F for ; Wed, 15 Oct 2025 08:31:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1760509903; 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=JMxJrxvlDo1vSwoeqf8nLlXUI74+kKl7W50Ucjoeyd0=; b=RwIi0PT4dM80XSXXiXQ+pKXC+FPqIoUh/f8bW4s3fe04/X9oTejFIprGtkQ24T+qZK85Hl W5pcR8kj5BeI2/zCunq+RyXZAwef8hC8apxxJFeHTOG42IEdunYX4NsYGPm8mBZnO0q+Sa LQKM8F3V+iC+lmTL0/iIWQnV+86kujg= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-470-WCOtk97vO_CU-4L_NeU4mA-1; Wed, 15 Oct 2025 02:31:42 -0400 X-MC-Unique: WCOtk97vO_CU-4L_NeU4mA-1 X-Mimecast-MFC-AGG-ID: WCOtk97vO_CU-4L_NeU4mA_1760509901 Received: by mail-ed1-f69.google.com with SMTP id 4fb4d7f45d1cf-6394a37e473so11806935a12.1 for ; Tue, 14 Oct 2025 23:31:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1760509901; x=1761114701; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JMxJrxvlDo1vSwoeqf8nLlXUI74+kKl7W50Ucjoeyd0=; b=C6kA7MVWdmxGeW5MgScZ2U1i9US9HUejs8pyQXQwctSmsF2CrKz4cZTtv2XhjaAMEL Gb3R/iqjmG/P80LKZweGA/UeZ6sp/uQ8NJHDiYJIEvRIDYVJsGizngcl5ov4KdIsAset rMciPdOJ6PNnkWI96CAInGA5GdPdwM8IJKQPEFPqR6hRNxyHNCF/SSJ5vafhH1N/EvKM 5oBk84dcIcq0zXNTAASxdSa8sU92si609nMod2Isy3sR3R+wNrrIE6lu7XzYw+wBk2t1 0dwo0fsZawxDvG9it5q0A5b7MC2Sl/O/J28EVnDQRSbdLfmKOr/Jxn1Auvm0uEh3Dohb 1Png== X-Gm-Message-State: AOJu0YwtVuanHd5ivxuXJbZJinnqn5FCkxhcMAZVnAokrOdGnX4xozhp SNR4cvG19dVc3BZxtz+/l8YCoQ3L0eS1ON6Bj0YYcmW+Q4Rva4iXLTWNEw8MzWg94hDOpGZoEsE 9+Dtp/R15bZOAi0DzLsDkxPc3+0Iby7y6baGKiNoCiLkyoQrjBffd4hEtxrToLj9F2QjlWUPjrT mqTHkyG+pIhgxbxUblJr/upVpwFQ71Y8T8g3x+1J2UhA== X-Gm-Gg: ASbGncun0YE35YMRKXN//cTENaxDXQmvTyj/7Ilt1qAPwEF6TNfqCIVYoYbP/Qm4KE3 0xBUR5bXIMXgfrK7mUJlWwUnrvWIsE+QA1SGfvKyr85L6u7bafXpkXoWe98axotetyegZrLbS8r RttPxcplJzJOkvqA9Wyk+CmQ== X-Received: by 2002:a05:6402:40d3:b0:636:23c2:e62a with SMTP id 4fb4d7f45d1cf-639d5b5de0bmr25921603a12.8.1760509900790; Tue, 14 Oct 2025 23:31:40 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHKUATyuGjKjG8trMWHfZB0GVeilVbdCrw1PlSx/bcWMCq22w70vTJPBBeFYez2Z2JULaW4cIkwGWmWZMzvh7I= X-Received: by 2002:a05:6402:40d3:b0:636:23c2:e62a with SMTP id 4fb4d7f45d1cf-639d5b5de0bmr25921586a12.8.1760509900354; Tue, 14 Oct 2025 23:31:40 -0700 (PDT) MIME-Version: 1.0 References: <20251014073836.18150-1-yuhuang@redhat.com> <20251014073836.18150-5-yuhuang@redhat.com> In-Reply-To: From: Yumei Huang Date: Wed, 15 Oct 2025 14:31:27 +0800 X-Gm-Features: AS18NWDJOrrVtuOYimYFDWPDGMYSU9sy-1395CFcHF4ABCdTuHUhsucwFmDgZsY Message-ID: Subject: Re: [PATCH v3 4/4] tcp: Update data retransmission timeout To: David Gibson X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: h1698KROCIHpV63EQBEFChdEk1fWEoOvmqXeXbtBDyY_1760509901 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Message-ID-Hash: 7TCFKU4DH74TNFAMF4R3EBAKS3HW6WPQ X-Message-ID-Hash: 7TCFKU4DH74TNFAMF4R3EBAKS3HW6WPQ X-MailFrom: yuhuang@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, 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: On Wed, Oct 15, 2025 at 8:05=E2=80=AFAM David Gibson wrote: > > 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: > > > > (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. > > > > 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 f= lags: > > * > > - * - SYN_TIMEOUT_INIT: if no ACK is received from tap/guest during han= dshake > > - * (flag ACK_FROM_TAP_DUE without ESTABLISHED event) within this tim= e, resend > > - * SYN. It's the starting timeout for the first SYN retry. If this p= ersists > > - * 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= 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 persi= sts 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, = eiher > > + * during handshake(flag ACK_FROM_TAP_DUE without ESTABLISHED event)= or after > > + * sending data (flag ACK_FROM_TAP_DUE with ESTABLISHED event), re-s= end data > > + * from the socket and reset sequence to what was acknowledged. It's= the > > + * starting timeout for the first retry. If this persists for more t= han > > + * allowed times in a row, reset the connection > > * > > * - FIN_TIMEOUT: if a FIN segment was sent to tap/guest (flag ACK_FRO= M_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 69= 28 */ > > > > #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. Sure. > > > #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 > > + > > 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. If we use TCP_MAX_RETRIES and not clamping RTO, the total timeout could be 255 seconds. Stefano mentioned "Retransmitting data after 256 seconds doesn't make a lot of sense to me" in the previous comment. Not sure what the reasonable timeout should be. BTW, clamping the RTO to limit the delay to 60s should be easy to implement, and it leads to 183s for the total timeout. I'm okay with either approach. Please let me know your thoughts. Thanks. > > > #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, st= ruct 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_linea= r_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= epoll_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 { > > -- > > 2.47.0 > > > > -- > 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 wa= y > | around. > http://www.ozlabs.org/~dgibson --=20 Thanks, Yumei Huang