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

[Stefano Brivio <sbrivio@redhat.com>]

On Sun, 28 Sep 2025 15:29:46 +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 SYN_TIMEOUT.
> 
> Signed-off-by: Yumei Huang <yuhuang@redhat.com>
> ---
>  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){

Nit: missing whitespace between ) and {.

Now that you use conn->retries for this purpose as well, I guess you
should either rename TCP_MAX_RETRANS to TCP_MAX_RETRIES, or use another
constant.

This also depends on the timing behaviour we want to have here, see the
other part of the thread. In any case, you should update the "Theory of
Operation" documentation at the top of tcp.c, see the "Aging and timeout"
section.

> +				flow_dbg(conn, "handshake timeout");
> +				tcp_rst(c, conn);
> +			} else {
> +				flow_dbg(conn, "SYN timeout, retry");
> +				tcp_send_flag(c, conn, SYN);
> +				conn->retries++;

You should reset this to zero once the connection is established,
otherwise we might start the connection with a value of conn->retries
that indicates that we already retransmitted data a number of times,
even if we didn't.

> +				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);

-- 
Stefano