From f211cd6345555086307da8c4998783ebe0ec0027 Mon Sep 17 00:00:00 2001 From: Robert Lubos Date: Tue, 26 Mar 2024 11:34:29 +0100 Subject: [PATCH] net: tcp: Deprecate CONFIG_NET_TCP_ACK_TIMEOUT Deprecate CONFIG_NET_TCP_ACK_TIMEOUT as it is redundant with the combination of CONFIG_NET_TCP_INIT_RETRANSMISSION_TIMEOUT and CONFIG_NET_TCP_RETRY_COUNT. The total retransmission timeout (i. e. waiting for ACK) should depend on the individual retransmission timeout and retry count, having separate config is simply ambiguous and confusing for users. Moreover, the config was currently only used during TCP handshake, and for that purpose we could use the very same timeout that is used for the FIN timeout. Therefore, repurpose the fin_timeout_ms to be a generic, maximum timeout at the TCP stack. Signed-off-by: Robert Lubos --- doc/releases/migration-guide-3.7.rst | 7 +++++++ subsys/net/ip/Kconfig.tcp | 8 ++++---- subsys/net/ip/tcp.c | 22 +++++++++++----------- tests/net/all/prj.conf | 1 - 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/doc/releases/migration-guide-3.7.rst b/doc/releases/migration-guide-3.7.rst index be9a9ef489f..6c6a600eff8 100644 --- a/doc/releases/migration-guide-3.7.rst +++ b/doc/releases/migration-guide-3.7.rst @@ -220,6 +220,13 @@ Networking ``wifi -h`` will give more information about the usage of connect command. (:github:`70024`) +* The Kconfig ``CONFIG_NET_TCP_ACK_TIMEOUT`` has been deprecated. Its usage was + limited to TCP handshake only, and in such case the total timeout should depend + on the total retransmission timeout (as in other cases) making the config + redundant and confusing. Use ``CONFIG_NET_TCP_INIT_RETRANSMISSION_TIMEOUT`` and + ``CONFIG_NET_TCP_RETRY_COUNT`` instead to control the total timeout at the + TCP level. (:github:`70731`) + Other Subsystems **************** diff --git a/subsys/net/ip/Kconfig.tcp b/subsys/net/ip/Kconfig.tcp index 8342acfb59a..e7d45f46c7a 100644 --- a/subsys/net/ip/Kconfig.tcp +++ b/subsys/net/ip/Kconfig.tcp @@ -66,14 +66,14 @@ config NET_TCP_TIME_WAIT_DELAY Value of 0 disables TIME_WAIT state completely. config NET_TCP_ACK_TIMEOUT - int "How long to wait for ACK (in milliseconds)" + int "[DEPRECATED] How long to wait for ACK (in milliseconds)" depends on NET_TCP default 1000 range 1 2147483647 help - This value affects the timeout when waiting ACK to arrive in - various TCP states. The value is in milliseconds. Note that - having a very low value here could prevent connectivity. + Deprecated. Use CONFIG_NET_TCP_INIT_RETRANSMISSION_TIMEOUT and + CONFIG_NET_TCP_RETRY_COUNT to control the total timeout at the TCP + level. config NET_TCP_INIT_RETRANSMISSION_TIMEOUT int "Initial value of Retransmission Timeout (RTO) (in milliseconds)" diff --git a/subsys/net/ip/tcp.c b/subsys/net/ip/tcp.c index 49ad112b527..3aee7e047ef 100644 --- a/subsys/net/ip/tcp.c +++ b/subsys/net/ip/tcp.c @@ -26,18 +26,18 @@ LOG_MODULE_REGISTER(net_tcp, CONFIG_NET_TCP_LOG_LEVEL); #include "net_private.h" #include "tcp_internal.h" -#define ACK_TIMEOUT_MS CONFIG_NET_TCP_ACK_TIMEOUT +#define ACK_TIMEOUT_MS tcp_max_timeout_ms #define ACK_TIMEOUT K_MSEC(ACK_TIMEOUT_MS) -#define LAST_ACK_TIMEOUT_MS tcp_fin_timeout_ms +#define LAST_ACK_TIMEOUT_MS tcp_max_timeout_ms #define LAST_ACK_TIMEOUT K_MSEC(LAST_ACK_TIMEOUT_MS) -#define FIN_TIMEOUT K_MSEC(tcp_fin_timeout_ms) +#define FIN_TIMEOUT K_MSEC(tcp_max_timeout_ms) #define ACK_DELAY K_MSEC(100) #define ZWP_MAX_DELAY_MS 120000 #define DUPLICATE_ACK_RETRANSMIT_TRHESHOLD 3 static int tcp_rto = CONFIG_NET_TCP_INIT_RETRANSMISSION_TIMEOUT; static int tcp_retries = CONFIG_NET_TCP_RETRY_COUNT; -static int tcp_fin_timeout_ms; +static int tcp_max_timeout_ms; static int tcp_rx_window = #if (CONFIG_NET_TCP_MAX_RECV_WINDOW_SIZE != 0) CONFIG_NET_TCP_MAX_RECV_WINDOW_SIZE; @@ -1821,7 +1821,7 @@ static void tcp_resend_data(struct k_work *work) if (conn->in_close && conn->send_data_total == 0) { NET_DBG("TCP connection in %s close, " "not disposing yet (waiting %dms)", - "active", tcp_fin_timeout_ms); + "active", tcp_max_timeout_ms); k_work_reschedule_for_queue(&tcp_work_q, &conn->fin_timer, FIN_TIMEOUT); @@ -1896,7 +1896,7 @@ static void tcp_fin_timeout(struct k_work *work) return; } - NET_DBG("Did not receive %s in %dms", "FIN", tcp_fin_timeout_ms); + NET_DBG("Did not receive %s in %dms", "FIN", tcp_max_timeout_ms); NET_DBG("conn: %p %s", conn, tcp_conn_state(conn, NULL)); (void)tcp_conn_close(conn, -ETIMEDOUT); @@ -3564,7 +3564,7 @@ int net_tcp_put(struct net_context *context) NET_DBG("TCP connection in %s close, " "not disposing yet (waiting %dms)", - "active", tcp_fin_timeout_ms); + "active", tcp_max_timeout_ms); k_work_reschedule_for_queue(&tcp_work_q, &conn->fin_timer, FIN_TIMEOUT); @@ -4445,18 +4445,18 @@ void net_tcp_init(void) NULL); /* Compute the largest possible retransmission timeout */ - tcp_fin_timeout_ms = 0; + tcp_max_timeout_ms = 0; rto = tcp_rto; for (i = 0; i < tcp_retries; i++) { - tcp_fin_timeout_ms += rto; + tcp_max_timeout_ms += rto; rto += rto >> 1; } /* At the last timeout cicle */ - tcp_fin_timeout_ms += tcp_rto; + tcp_max_timeout_ms += tcp_rto; /* When CONFIG_NET_TCP_RANDOMIZED_RTO is active in can be worse case 1.5 times larger */ if (IS_ENABLED(CONFIG_NET_TCP_RANDOMIZED_RTO)) { - tcp_fin_timeout_ms += tcp_fin_timeout_ms >> 1; + tcp_max_timeout_ms += tcp_max_timeout_ms >> 1; } k_thread_name_set(&tcp_work_q.thread, "tcp_work"); diff --git a/tests/net/all/prj.conf b/tests/net/all/prj.conf index dfdb46eeaf6..90f0d6cfcd5 100644 --- a/tests/net/all/prj.conf +++ b/tests/net/all/prj.conf @@ -126,7 +126,6 @@ CONFIG_NET_MAX_ROUTERS=3 CONFIG_NET_TCP=y CONFIG_NET_TCP_LOG_LEVEL_DBG=y CONFIG_NET_TCP_TIME_WAIT_DELAY=20000 -CONFIG_NET_TCP_ACK_TIMEOUT=30 CONFIG_NET_TCP_CHECKSUM=y CONFIG_NET_TCP_INIT_RETRANSMISSION_TIMEOUT=400 CONFIG_NET_TCP_RETRY_COUNT=10