Re: [PATCH v2 3/3] tcp: Update data retransmission timeout

On Fri, Oct 10, 2025 at 03:47:00PM +0800, Yumei Huang wrote:

Use an exponential backoff timeout with the initial timeout 1s and total timeout 60s.

The commit message needs more information on why making this change to behaviour is desirable (e.g. referencing RFCs).

Signed-off-by: Yumei Huang --- tcp.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/tcp.c b/tcp.c index 85bbdac..0db7f30 100644 --- a/tcp.c +++ b/tcp.c @@ -185,10 +185,15 @@ * 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 persists 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, after + * sending data (flag ACK_FROM_TAP_DUE with ESTABLISHED event), re-send data + * from the socket and reset sequence to what was acknowledged. It's the + * starting timeout for the first retransmission. If this persists for more + * than TCP_MAX_RETRIES times in a row, reset the connection + * + * - ACK_TIMEOUT_TOTAL: if no ACK segment was received from tap/guest after + * retransmitting data repeatedly from the socket within this time, 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 @@ -344,7 +349,8 @@ enum {

#define ACK_INTERVAL 10 /* ms */ #define SYN_TIMEOUT_INIT 1 /* s */ -#define ACK_TIMEOUT 2 +#define ACK_TIMEOUT_INIT 1 +#define ACK_TIMEOUT_TOTAL 60 #define FIN_TIMEOUT 60 #define ACT_TIMEOUT 7200

@@ -358,6 +364,9 @@ enum { ((conn)->events & (SOCK_FIN_RCVD | TAP_FIN_RCVD))) #define CONN_HAS(conn, set) (((conn)->events & (set)) == (set))

+#define RETRY_ELAPSED(timeout_init, retries) \ + ((timeout_init) * ((1 << ((retries) + 1)) - 2)) + /* Buffers to migrate pending data from send and receive queues. No, they don't * use memory if we don't use them. And we're going away after this, so splurge. */ @@ -596,7 +605,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) (conn->retries - c->tcp.syn_linear_timeouts); } else - it.it_value.tv_sec = ACK_TIMEOUT; + it.it_value.tv_sec = ACK_TIMEOUT_INIT << conn->retries; } else if (CONN_HAS(conn, SOCK_FIN_SENT | TAP_FIN_ACKED)) { it.it_value.tv_sec = FIN_TIMEOUT; } else { @@ -2438,6 +2447,10 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) } else if (conn->retries == TCP_MAX_RETRIES) { flow_dbg(conn, "retransmissions count exceeded"); tcp_rst(c, conn); + } else if(RETRY_ELAPSED(ACK_TIMEOUT_INIT, conn->retries) >= + ACK_TIMEOUT_TOTAL) { + flow_dbg(conn, "retransmissions timeout exceeded"); + tcp_rst(c, conn);

Having a test both for number of retries and time elapsed seems redundant. RETRY_ELAPSED is pure function of the number of retries, so it should be possible to just have a threshold on the number of retries, which can be calculated from the target total timeout.

} else { flow_dbg(conn, "ACK timeout, retry");

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