From 83745652c427813d7a40b398b32b39d5096e9792 Mon Sep 17 00:00:00 2001 From: GAEHWILER Reto Date: Thu, 22 Oct 2020 12:07:39 +0200 Subject: [PATCH] TCP-stack fix for stalled tcp sockets due to broken keepalive Fixes an issue where tcp sockets with activated keepalives stalled and were not properly closed. Poll would not indicate a POLLHUP and therefore locks down the application. * tcp_conn_s.tcp_conn_s & tcp_conn_s.keepintvl changed to uint32_t According RFC1122 keepidle MUST have a default of 2 hours. --- include/nuttx/clock.h | 6 +++--- net/tcp/tcp.h | 4 ++-- net/tcp/tcp_send.c | 13 +++++++++++++ net/tcp/tcp_timer.c | 35 ++++++++++++++++++++++++++++------- 4 files changed, 46 insertions(+), 12 deletions(-) diff --git a/include/nuttx/clock.h b/include/nuttx/clock.h index 3dffca9a3e..ab6f55b557 100644 --- a/include/nuttx/clock.h +++ b/include/nuttx/clock.h @@ -106,15 +106,15 @@ #define NSEC_PER_MIN (NSEC_PER_SEC * SEC_PER_MIN) #define USEC_PER_MIN (USEC_PER_SEC * SEC_PER_MIN) #define MSEC_PER_MIN (MSEC_PER_SEC * SEC_PER_MIN) -#define DSEC_PER_MIN (HSEC_PER_SEC * SEC_PER_MIN) +#define DSEC_PER_MIN (DSEC_PER_SEC * SEC_PER_MIN) #define HSEC_PER_MIN (HSEC_PER_SEC * SEC_PER_MIN) #define MIN_PER_HOUR 60L #define NSEC_PER_HOUR (NSEC_PER_MIN * MIN_PER_HOUR) #define USEC_PER_HOUR (USEC_PER_MIN * MIN_PER_HOUR) #define MSEC_PER_HOUR (MSEC_PER_MIN * MIN_PER_HOUR) -#define DSEC_PER_HOUR (HSEC_PER_SEC * MIN_PER_HOUR) -#define HSEC_PER_HOUR (DSEC_PER_MIN * MIN_PER_HOUR) +#define DSEC_PER_HOUR (DSEC_PER_MIN * MIN_PER_HOUR) +#define HSEC_PER_HOUR (HSEC_PER_MIN * MIN_PER_HOUR) #define SEC_PER_HOUR (SEC_PER_MIN * MIN_PER_HOUR) #define HOURS_PER_DAY 24L diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h index da7bcb33a1..044869fa54 100644 --- a/net/tcp/tcp.h +++ b/net/tcp/tcp.h @@ -255,8 +255,8 @@ struct tcp_conn_s clock_t keeptime; /* Last time that the TCP socket was known to be * alive (ACK or data received) OR time that the * last probe was sent. */ - uint16_t keepidle; /* Elapsed idle time before first probe sent (dsec) */ - uint16_t keepintvl; /* Interval between probes (dsec) */ + uint32_t keepidle; /* Elapsed idle time before first probe sent (dsec) */ + uint32_t keepintvl; /* Interval between probes (dsec) */ bool keepalive; /* True: KeepAlive enabled; false: disabled */ uint8_t keepcnt; /* Number of retries before the socket is closed */ uint8_t keepretries; /* Number of retries attempted */ diff --git a/net/tcp/tcp_send.c b/net/tcp/tcp_send.c index 882764ce36..8adf61d988 100644 --- a/net/tcp/tcp_send.c +++ b/net/tcp/tcp_send.c @@ -311,6 +311,19 @@ static void tcp_sendcommon(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn, FAR struct tcp_hdr_s *tcp) { + /* If keepalive is outstanding, cancel it */ + +#ifdef CONFIG_NET_TCP_KEEPALIVE + if (conn->keepretries > 0) + { + uint32_t saveseq = tcp_getsequence(conn->sndseq); + saveseq += conn->tx_unacked; + tcp_setsequence(conn->sndseq, saveseq); + conn->tx_unacked--; + conn->keepretries = 0; + } +#endif /* CONFIG_NET_TCP_KEEPALIVE */ + /* Copy the IP address into the IPv6 header */ #ifdef CONFIG_NET_IPv6 diff --git a/net/tcp/tcp_timer.c b/net/tcp/tcp_timer.c index 56e988dbf6..d42914e8ba 100644 --- a/net/tcp/tcp_timer.c +++ b/net/tcp/tcp_timer.c @@ -341,13 +341,36 @@ void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn, case TCP_ESTABLISHED: - /* In the ESTABLISHED state, we call upon the application - * to do the actual retransmit after which we jump into - * the code for sending out the packet. + /* In the ESTABLISHED state, we have to differentiate + * between a keepalive and actual transmitted data. */ - result = tcp_callback(dev, conn, TCP_REXMIT); - tcp_rexmit(dev, conn, result); +#ifdef CONFIG_NET_TCP_KEEPALIVE + if (conn->keepretries > 0) + { + /* In case of a keepalive timeout (based on RTT) the + * state has to be set back into idle so that a new + * keepalive can be fired. + */ + + uint32_t saveseq = tcp_getsequence(conn->sndseq); + saveseq += conn->tx_unacked; + tcp_setsequence(conn->sndseq, saveseq); + conn->tx_unacked--; + } + else +#endif + { + /* If there is a timeout on outstanding data we call + * upon the application to do the actual retransmit + * after which we jump into the code for sending out + * the packet. + */ + + result = tcp_callback(dev, conn, TCP_REXMIT); + tcp_rexmit(dev, conn, result); + } + goto done; case TCP_FIN_WAIT_1: @@ -462,8 +485,6 @@ void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn, tcp_send(dev, conn, TCP_ACK, tcpiplen + 1); - tcp_setsequence(conn->sndseq, saveseq); - /* Increment the number of un-ACKed bytes due to * the dummy byte that we just sent. */