Re: [PATCH 2/2] tcp: Resend SYN for inbound connections

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

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

On Sun, Sep 28, 2025 at 03:29:46PM +0800, Yumei Huang wrote:
> If a client connects while guest is not connected or ready yet,
> resend SYN instead of just resetting connection after SYN_TIMEOUT.
> 
> Signed-off-by: Yumei Huang <yuhuang@redhat.com>

Simpler than I thought.  Nice.

However, I think now that we're retrying we probably want to adjust
SYN_TIMEOUT.  I suspect the 10s was a generous amount to mitigate the
fact we didn't retry.  However, AFAICT most OSes resend SYNs faster
than that (after 1-3s initially).  They also typically slow down the
resents on subsequent retries.  I'm not sure if that last is important
in our case - since we're talking directly to a guest, we're unlikely
to flood the link this way.

In fact, I haven't read closely enough to be sure, but there was some
language in RFC 6298 and RFC 1122 that suggested to me maybe we should
be using the same backoff calculation for SYN retries as for regular
retransmits.  Which as a bonus might simplify our logic a little bit.

Documentation/networking/ip-sysctl.rst has some information on how
Linux handles this (tcp_syn_retries and tcp_syn_linear_timeouts in
particular).  I guess we could configure ourselves to match the host's
settings - we do something similar to determine what we consider
ephemeral ports.

Stefano, thoughts?

> ---
>  tcp.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 21b75a5..6fe8678 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2378,8 +2378,15 @@ 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 == TCP_MAX_RETRANS){
> +				flow_dbg(conn, "handshake timeout");
> +				tcp_rst(c, conn);
> +			} else {
> +				flow_dbg(conn, "SYN timeout, retry");
> +				tcp_send_flag(c, conn, SYN);
> +				conn->retries++;
> +				tcp_timer_ctl(c, conn);
> +			}
>  		} else if (CONN_HAS(conn, SOCK_FIN_SENT | TAP_FIN_ACKED)) {
>  			flow_dbg(conn, "FIN timeout");
>  			tcp_rst(c, conn);
> -- 
> 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 way
				| around.
http://www.ozlabs.org/~dgibson

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