sergiotarxz/nuttx
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

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.

Browse Source
This commit is contained in:
Jakub Łągwa 2016-06-20 06:55:29 -06:00 committed by Gregory Nutt
parent d5b8869b10
commit 338b915008
5 changed files with 43 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
ChangeLog
View File

@ -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).

4
net/tcp/tcp.h
View File

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

10
net/tcp/tcp_conn.c
View File

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

3
net/tcp/tcp_input.c
View File

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

16
net/tcp/tcp_send_buffered.c
View File

@ -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);