diff --git a/ChangeLog b/ChangeLog index 04e17597a0..613c081ed5 100755 --- a/ChangeLog +++ b/ChangeLog @@ -12084,3 +12084,21 @@ option. Without this configuration setting, interrupt level output will be asynchronous. And (3) vsyslog is now a system call and is usable with other-than-FLAT builds (2016-06-19). + * TCP Networking: While working with version 7.10 I discovered a + problem in TCP stack that could be observed on high network load. + Generally speaking, the problem is that RST flag is set in + unnecessary case, in which between loss of some TCP packet and its + proper retransmission, another packets had been successfully sent. + The scenario is as follows: NuttX did not receive ACK for some + sent packet, so it has been probably lost somewhere. But before + its retransmission starts, NuttX is correctly issuing next TCP + packets, with sequence numbers increasing properly. When the + retransmission of previously lost packet finally succeeds, tcp_input + receives the accumulated ACK value, which acknowledges also the + packets sent in the meantime (i.e. between unsuccessful sending of + lost packet and its proper retransmission). However, variable unackseq + is still set to conn->isn + conn->sent, which is truth only if no + further packets transmission occurred in the meantime. Because of + incorrect (in such specific case) unackseq value, few lines further + condition if (ackseq <= unackseq)is not met, and, as a result, we + are going to reset label. From Jakub Łągwa (2016-06-20). diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h index e95d50cdf4..c53475d75a 100644 --- a/net/tcp/tcp.h +++ b/net/tcp/tcp.h @@ -206,6 +206,8 @@ struct tcp_conn_s * it can only be updated at TCP_ESTABLISHED state */ uint32_t sent; /* The number of bytes sent (ACKed and un-ACKed) */ uint32_t isn; /* Initial sequence number */ + uint32_t sndseq_max; /* The sequence number of next not-retransmitted + * segment (next greater sndseq) */ #endif #ifdef CONFIG_NET_TCPBACKLOG @@ -665,7 +667,7 @@ void tcp_poll(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn); * Parameters: * dev - The device driver structure to use in the send operation * conn - The TCP "connection" to poll for TX data - * hsed - The polling interval in halves of a second + * hsec - The polling interval in halves of a second * * Return: * None diff --git a/net/tcp/tcp_conn.c b/net/tcp/tcp_conn.c index fa11c42c4a..83ca79f8e4 100644 --- a/net/tcp/tcp_conn.c +++ b/net/tcp/tcp_conn.c @@ -1083,11 +1083,12 @@ FAR struct tcp_conn_s *tcp_alloc_accept(FAR struct net_driver_s *dev, conn->tcpstateflags = TCP_SYN_RCVD; tcp_initsequence(conn->sndseq); - conn->unacked = 1; + conn->unacked = 1; #ifdef CONFIG_NET_TCP_WRITE_BUFFERS - conn->expired = 0; - conn->isn = 0; - conn->sent = 0; + conn->expired = 0; + conn->isn = 0; + conn->sent = 0; + conn->sndseq_max = 0; #endif /* rcvseq should be the seqno from the incoming packet + 1. */ @@ -1344,6 +1345,7 @@ int tcp_connect(FAR struct tcp_conn_s *conn, FAR const struct sockaddr *addr) conn->expired = 0; conn->isn = 0; conn->sent = 0; + conn->sndseq_max = 0; #endif #ifdef CONFIG_NET_TCP_READAHEAD diff --git a/net/tcp/tcp_input.c b/net/tcp/tcp_input.c index 277db67412..3834a5cf47 100644 --- a/net/tcp/tcp_input.c +++ b/net/tcp/tcp_input.c @@ -364,7 +364,7 @@ found: */ #ifdef CONFIG_NET_TCP_WRITE_BUFFERS - unackseq = conn->isn + conn->sent; + unackseq = conn->sndseq_max; #else unackseq = tcp_addsequence(conn->sndseq, conn->unacked); #endif @@ -468,6 +468,7 @@ found: conn->isn = tcp_getsequence(tcp->ackno); tcp_setsequence(conn->sndseq, conn->isn); conn->sent = 0; + conn->sndseq_max = 0; #endif conn->unacked = 0; flags = TCP_CONNECTED; diff --git a/net/tcp/tcp_send_buffered.c b/net/tcp/tcp_send_buffered.c index a343d0d046..9d4068af24 100644 --- a/net/tcp/tcp_send_buffered.c +++ b/net/tcp/tcp_send_buffered.c @@ -201,7 +201,8 @@ static inline void psock_lost_connection(FAR struct socket *psock, sq_init(&conn->unacked_q); sq_init(&conn->write_q); - conn->sent = 0; + conn->sent = 0; + conn->sndseq_max = 0; } /**************************************************************************** @@ -654,7 +655,7 @@ static uint16_t psock_send_interrupt(FAR struct net_driver_s *dev, { /* Insert the write buffer into the write_q (in sequence * number order). The retransmission will occur below - * when the write buffer with the lowest sequenc number + * when the write buffer with the lowest sequence number * is pulled from the write_q again. */ @@ -696,6 +697,7 @@ static uint16_t psock_send_interrupt(FAR struct net_driver_s *dev, if (psock_send_addrchck(conn)) { FAR struct tcp_wrbuffer_s *wrb; + uint32_t predicted_seqno; size_t sndlen; /* Peek at the head of the write queue (but don't remove anything @@ -770,6 +772,16 @@ static uint16_t psock_send_interrupt(FAR struct net_driver_s *dev, conn->unacked += sndlen; conn->sent += sndlen; + /* Below prediction will become true, unless retransmission occurrence */ + + predicted_seqno = tcp_getsequence(conn->sndseq) + sndlen; + + if ((predicted_seqno > conn->sndseq_max) || + (tcp_getsequence(conn->sndseq) > predicted_seqno)) /* overflow */ + { + conn->sndseq_max = predicted_seqno; + } + nllinfo("SEND: wrb=%p nrtx=%u unacked=%u sent=%u\n", wrb, WRB_NRTX(wrb), conn->unacked, conn->sent);