Re: [PATCH v4 3/4] tcp: Resend SYN for inbound connections

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4372 bytes --]

On Fri, Oct 17, 2025 at 12:22:46AM +0200, Stefano Brivio wrote:
> On Thu, 16 Oct 2025 10:34:22 +0800
> Yumei Huang <yuhuang@redhat.com> wrote:
> 
> > If a client connects while guest is not connected or ready yet,
> > resend SYN instead of just resetting connection after 10 seconds.
> > 
> > Use the same backoff calculation for the timeout as linux kernel.
> > 
> > Signed-off-by: Yumei Huang <yuhuang@redhat.com>
> > ---
> >  tcp.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++--------
> >  tcp.h |  2 ++
> >  2 files changed, 49 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tcp.c b/tcp.c
> > index 2ec4b0c..3003333 100644
> > --- a/tcp.c
> > +++ b/tcp.c
> > @@ -179,9 +179,11 @@
> >   *
> >   * Timeouts are implemented by means of timerfd timers, set based on flags:
> >   *
> > - * - SYN_TIMEOUT: if no ACK is received from tap/guest during handshake (flag
> > - *   ACK_FROM_TAP_DUE without ESTABLISHED event) within this time, reset the
> > - *   connection
> > + * - SYN_TIMEOUT_INIT: if no ACK is received from tap/guest during handshake
> > + *   (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 persists
> > + *   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
> > @@ -340,7 +342,7 @@ enum {
> >  #define WINDOW_DEFAULT			14600		/* RFC 6928 */
> >  
> >  #define ACK_INTERVAL			10		/* ms */
> > -#define SYN_TIMEOUT			10		/* s */
> > +#define SYN_TIMEOUT_INIT		1		/* s */
> >  #define ACK_TIMEOUT			2
> >  #define FIN_TIMEOUT			60
> >  #define ACT_TIMEOUT			7200
> > @@ -365,6 +367,10 @@ uint8_t tcp_migrate_rcv_queue		[TCP_MIGRATE_RCV_QUEUE_MAX];
> >  
> >  #define TCP_MIGRATE_RESTORE_CHUNK_MIN	1024 /* Try smaller when above this */
> >  
> > +#define TCP_SYN_RETRIES_SYSCTL		"/proc/sys/net/ipv4/tcp_syn_retries"
> > +#define TCP_SYN_LINEAR_TIMEOUTS_SYSCTL						\
> > +	"/proc/sys/net/ipv4/tcp_syn_linear_timeouts"
> 
> It's quite obvious those are names of sysctl entries, I think we can
> drop the _SYSCTL suffix and keep this shorter without losing any
> information/indication.
> 
> > +
> >  /* "Extended" data (not stored in the flow table) for TCP flow migration */
> >  static struct tcp_tap_transfer_ext migrate_ext[FLOW_MAX];
> >  
> > @@ -581,8 +587,13 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn)
> >  	if (conn->flags & ACK_TO_TAP_DUE) {
> >  		it.it_value.tv_nsec = (long)ACK_INTERVAL * 1000 * 1000;
> >  	} else if (conn->flags & ACK_FROM_TAP_DUE) {
> > -		if (!(conn->events & ESTABLISHED))
> > -			it.it_value.tv_sec = SYN_TIMEOUT;
> > +		if (!(conn->events & ESTABLISHED)) {
> > +			if (conn->retries < c->tcp.syn_linear_timeouts)
> > +				it.it_value.tv_sec = SYN_TIMEOUT_INIT;
> > +			else
> > +				it.it_value.tv_sec = SYN_TIMEOUT_INIT <<
> > +					(conn->retries - c->tcp.syn_linear_timeouts);
> > +		}
> >  		else
> >  			it.it_value.tv_sec = ACK_TIMEOUT;
> >  	} else if (CONN_HAS(conn, SOCK_FIN_SENT | TAP_FIN_ACKED)) {
> > @@ -2409,8 +2420,16 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)
> >  		tcp_timer_ctl(c, conn);
> >  	} else if (conn->flags & ACK_FROM_TAP_DUE) {
> >  		if (!(conn->events & ESTABLISHED)) {
> > -			flow_dbg(conn, "handshake timeout");
> > -			tcp_rst(c, conn);
> > +			if (conn->retries >= MIN(TCP_MAX_RETRIES,
> > +				(c->tcp.tcp_syn_retries + c->tcp.syn_linear_timeouts))) {
> 
> That doesn't seem to match the sysctl documentation for
> tcp_syn_retries, which should be the *total* number of retries, not
> excluding the ones with "linear timeouts".

No, but it does match the actual kernel behaviour (Yumei already
experimented and discovered this).  Confirmed it in the kernel code,
here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ipv4/tcp_timer.c#n258

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]