You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by xi...@apache.org on 2020/10/27 18:22:08 UTC
[incubator-nuttx] branch master updated: TCP-stack fix for stalled
tcp sockets due to broken keepalive
This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-nuttx.git
The following commit(s) were added to refs/heads/master by this push:
new 8374565 TCP-stack fix for stalled tcp sockets due to broken keepalive
8374565 is described below
commit 83745652c427813d7a40b398b32b39d5096e9792
Author: GAEHWILER Reto <re...@hexagon.com>
AuthorDate: Thu Oct 22 12:07:39 2020 +0200
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 3dffca9..ab6f55b 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 da7bcb3..044869f 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 882764c..8adf61d 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 56e988d..d42914e 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.
*/