Fix an error in the handling of TCP/IP sequence numbers

git-svn-id: svn://svn.code.sf.net/p/nuttx/code/trunk@2392 42af7a65-404d-4744-a932-0658087f49c3
This commit is contained in:
patacongo 2009-12-19 22:05:12 +00:00
parent 29d789372a
commit 7d63ffbf72
11 changed files with 227 additions and 116 deletions

View file

@ -1016,3 +1016,10 @@
account for the 2-byte packet length and the 4-byte packet FCS in the
FIFO).
* net/accept.c - Fixed a bad assertion (only happens when debug is enabled).
* net/send.c net/uip/uip_tcpseqno.c - Fixed a critical error in the TCP/IP
logic. The NuttX port of uIP imcludes logic to send data ahead without
waiting for ACKs from the recipient; this greatly improves throughput.
However, the packet sequence number was not being updated correctly and,
as a result, packets were not be ACKed by the recipient and transfers
would sometimes stall. This is a very important bug fix (in fact, I
don't understand how TCP/IP worked at all without this fix???)

View file

@ -8,7 +8,7 @@
<tr align="center" bgcolor="#e4e4e4">
<td>
<h1><big><font color="#3c34ec"><i>NuttX RTOS</i></font></big></h1>
<p>Last Updated: December 18, 2009</p>
<p>Last Updated: December 19, 2009</p>
</td>
</tr>
</table>
@ -1642,6 +1642,13 @@ nuttx-5.0 2009-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;
account for the 2-byte packet length and the 4-byte packet FCS in the
FIFO).
* net/accept.c - Fixed a bad assertion (only happens when debug is enabled).
* net/send.c net/uip/uip_tcpseqno.c - Fixed a critical error in the TCP/IP
logic. The NuttX port of uIP imcludes logic to send data ahead without
waiting for ACKs from the recipient; this greatly improves throughput.
However, the packet sequence number was not being updated correctly and,
as a result, packets were not be ACKed by the recipient and transfers
would sometimes stall. This is a very important bug fix (in fact, I
don't understand how TCP/IP worked at all without this fix???)
pascal-2.0 2010-xx-xx Gregory Nutt &lt;spudmonkey@racsa.co.cr&gt;

View file

@ -76,7 +76,7 @@
#endif
#define NTOHS(hs) HTONS(hs)
#define NTOHL(hl) NTOHL(hl)
#define NTOHL(hl) HTONL(hl)
/****************************************************************************
* Public Function Prototypes

View file

@ -295,28 +295,19 @@ typedef int (*uip_poll_callback_t)(struct uip_driver_s *dev);
extern int uip_poll(struct uip_driver_s *dev, uip_poll_callback_t callback);
extern int uip_timer(struct uip_driver_s *dev, uip_poll_callback_t callback, int hsec);
/* By defining UIP_ARCH_CHKSUM, the architecture can replace the following
* functions with hardware assisted solutions.
/* By defining UIP_ARCH_CHKSUM, the architecture can replace up_incr32
* with hardware assisted solutions.
*/
/* Carry out a 32-bit addition.
*
* op32 - A pointer to a 4-byte array representing a 32-bit
* integer in network byte order (big endian). This value may not
* be word aligned.
* be word aligned. The value pointed to by op32 is modified in place
*
* For uip_incr32, the value pointed to by op32 is modified in place
* For uip_add32, the value pointed to by op32 is unmodified
*
* op16 - A 16-bit integer in host byte order.
*
* sum - The location to return the result (32-bit, network byte order,
* possibly unaligned).
*
* uip_add32 only.
*/
extern void uip_add32(const uint8_t *op32, uint16_t op16, uint8_t *sum);
extern void uip_incr32(uint8_t *op32, uint16_t op16);
/* Calculate the Internet checksum over a buffer.

View file

@ -87,54 +87,6 @@ struct send_s
* Private Functions
****************************************************************************/
/****************************************************************************
* Function: send_getisn
*
* Description:
* Get the next initial sequence number from the connection stucture
*
* Parameters:
* conn The connection structure associated with the socket
*
* Returned Value:
* None
*
* Assumptions:
* Running at the interrupt level
*
****************************************************************************/
static uint32_t send_getisn(struct uip_conn *conn)
{
uint32_t tmp;
memcpy(&tmp, conn->snd_nxt, 4);
return ntohl(tmp);
}
/****************************************************************************
* Function: send_getackno
*
* Description:
* Extract the current acknowledgement sequence number from the incoming packet
*
* Parameters:
* dev - The sructure of the network driver that caused the interrupt
*
* Returned Value:
* None
*
* Assumptions:
* Running at the interrupt level
*
****************************************************************************/
static uint32_t send_getackno(struct uip_driver_s *dev)
{
uint32_t tmp;
memcpy(&tmp, TCPBUF->ackno, 4);
return ntohl(tmp);
}
/****************************************************************************
* Function: send_timeout
*
@ -216,7 +168,7 @@ static uint16_t send_interrupt(struct uip_driver_s *dev, void *pvconn,
* is the number of bytes to be acknowledged.
*/
pstate->snd_acked = send_getackno(dev) - pstate->snd_isn;
pstate->snd_acked = uip_tcpgetsequence(TCPBUF->ackno) - pstate->snd_isn;
nllvdbg("ACK: acked=%d sent=%d buflen=%d\n",
pstate->snd_acked, pstate->snd_sent, pstate->snd_buflen);
@ -259,7 +211,7 @@ static uint16_t send_interrupt(struct uip_driver_s *dev, void *pvconn,
}
/* Check if the outgoing packet is available (it may have been claimed
* by a sendto interrupt serving a different thread.
* by a sendto interrupt serving a different thread).
*/
#if 0 /* We can't really support multiple senders on the same TCP socket */
@ -289,6 +241,10 @@ static uint16_t send_interrupt(struct uip_driver_s *dev, void *pvconn,
sndlen = uip_mss(conn);
}
/* Set the sequence number for this packet */
uip_tcpsetsequence(conn->snd_nxt, pstate->snd_sent + pstate->snd_isn);
/* Then send that amount of data */
uip_send(dev, &pstate->snd_buffer[pstate->snd_sent], sndlen);
@ -459,7 +415,7 @@ ssize_t send(int sockfd, const void *buf, size_t len, int flags)
{
/* Get the initial sequence number that will be used */
state.snd_isn = send_getisn(conn); /* Initial sequence number */
state.snd_isn = uip_tcpgetsequence(conn->snd_nxt);
/* Update the initial time for calculating timeouts */

View file

@ -51,7 +51,7 @@ endif
ifeq ($(CONFIG_NET_TCP),y)
UIP_CSRCS += uip_tcpconn.c uip_tcppoll.c uip_tcptimer.c uip_tcpsend.c \
UIP_CSRCS += uip_tcpconn.c uip_tcpseqno.c uip_tcppoll.c uip_tcptimer.c uip_tcpsend.c \
uip_tcpinput.c uip_tcpappsend.c uip_listen.c uip_tcpcallback.c \
uip_tcpreadahead.c uip_tcpbacklog.c

View file

@ -148,7 +148,7 @@ static uint16_t uip_icmp6chksum(struct uip_driver_s *dev)
/* Calculate the Internet checksum over a buffer. */
#if !UIP_ARCH_ADD32
static void uip_carry32(uint8_t *sum, uint16_t op16)
static inline void uip_carry32(uint8_t *sum, uint16_t op16)
{
if (sum[2] < (op16 >> 8))
{
@ -173,17 +173,6 @@ static void uip_carry32(uint8_t *sum, uint16_t op16)
}
}
void uip_add32(const uint8_t *op32, uint16_t op16, uint8_t *sum)
{
/* op32 and the sum are in network order (big-endian); op16 is host order. */
sum[3] = op32[3] + (op16 & 0xff);
sum[2] = op32[2] + (op16 >> 8);
sum[1] = op32[1];
sum[0] = op32[0];
uip_carry32(sum, op16);
}
void uip_incr32(uint8_t *op32, uint16_t op16)
{
op32[3] += (op16 & 0xff);

View file

@ -110,6 +110,13 @@ EXTERN struct uip_conn *uip_tcpactive(struct uip_tcpip_hdr *buf);
EXTERN struct uip_conn *uip_nexttcpconn(struct uip_conn *conn);
EXTERN struct uip_conn *uip_tcplistener(uint16_t portno);
EXTERN struct uip_conn *uip_tcpaccept(struct uip_tcpip_hdr *buf);
/* Defined in uip_tcpseqno.c ************************************************/
EXTERN void uip_tcpsetsequence(FAR uint8_t *seqno, uint32_t value);
EXTERN uint32_t uip_tcpgetsequence(FAR uint8_t *seqno);
EXTERN uint32_t uip_tcpaddsequence(FAR uint8_t *seqno, uint16_t len);
EXTERN void uip_tcpinitsequence(FAR uint8_t *seqno);
EXTERN void uip_tcpnextsequence(void);
/* Defined in uip_tcppoll.c *************************************************/

View file

@ -85,10 +85,6 @@ static dq_queue_t g_active_tcp_connections;
static uint16_t g_last_tcp_port;
/* g_tcp_sequence[] is used to generate TCP sequence numbers */
static uint8_t g_tcp_sequence[4];
/****************************************************************************
* Private Functions
****************************************************************************/
@ -469,7 +465,7 @@ struct uip_conn *uip_tcpaccept(struct uip_tcpip_hdr *buf)
uip_ipaddr_copy(conn->ripaddr, uip_ip4addr_conv(buf->srcipaddr));
conn->tcpstateflags = UIP_SYN_RCVD;
memcpy(conn->snd_nxt, g_tcp_sequence, 4);
uip_tcpinitsequence(conn->snd_nxt);
conn->len = 1;
/* rcv_nxt should be the seqno from the incoming packet + 1. */
@ -491,33 +487,6 @@ struct uip_conn *uip_tcpaccept(struct uip_tcpip_hdr *buf)
return conn;
}
/****************************************************************************
* Name: uip_tcpnextsequence()
*
* Description:
* Increment the TCP/IP sequence number
*
* Assumptions:
* This function is called from the interrupt level
*
****************************************************************************/
void uip_tcpnextsequence(void)
{
/* This inplements a byte-by-byte big-endian increment */
if (++g_tcp_sequence[3] == 0)
{
if (++g_tcp_sequence[2] == 0)
{
if (++g_tcp_sequence[1] == 0)
{
++g_tcp_sequence[0];
}
}
}
}
/****************************************************************************
* Name: uip_tcpbind()
*
@ -626,7 +595,7 @@ int uip_tcpconnect(struct uip_conn *conn, const struct sockaddr_in *addr)
/* Initialize and return the connection structure, bind it to the port number */
conn->tcpstateflags = UIP_SYN_SENT;
memcpy(conn->snd_nxt, g_tcp_sequence, 4);
uip_tcpinitsequence(conn->snd_nxt);
conn->initialmss = conn->mss = UIP_TCP_MSS;
conn->len = 1; /* TCP length of the SYN is one. */

View file

@ -328,16 +328,28 @@ found:
if ((pbuf->flags & TCP_ACK) && uip_outstanding(conn))
{
/* Temporary variables. */
uint32_t seqno;
uint32_t ackno;
uint8_t acc32[4];
uip_add32(conn->snd_nxt, conn->len, acc32);
/* The next sequence number is equal to the current sequence
* number (snd_nxt) plus the size of the oustanding data (len).
*/
if (memcmp(pbuf->ackno, acc32, 4) == 0)
seqno = uip_tcpaddsequence(conn->snd_nxt, conn->len);
/* Check if all of the outstanding bytes have been acknowledged. For
* a "generic" send operation, this should always be true. However,
* the send() API sends data ahead when it can without waiting for
* the ACK. In this case, the 'ackno' could be less than then the
* new sequence number.
*/
ackno = uip_tcpgetsequence(pbuf->ackno);
if (ackno <= seqno)
{
/* Update sequence number. */
memcpy(conn->snd_nxt, acc32, 4);
uip_tcpsetsequence(conn->snd_nxt, seqno);
/* Do RTT estimation, unless we have done retransmissions. */

173
net/uip/uip_tcpseqno.c Executable file
View file

@ -0,0 +1,173 @@
/****************************************************************************
* net/uip/uip_tcpseqno.c
*
* Copyright (C) 2007-2009 Gregory Nutt. All rights reserved.
* Author: Gregory Nutt <spudmonkey@racsa.co.cr>
*
* Large parts of this file were leveraged from uIP logic:
*
* Copyright (c) 2001-2003, Adam Dunkels.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
* 3. The name of the author may not be used to endorse or promote
* products derived from this software without specific prior
* written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS
* OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
* WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
* DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE
* GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
* INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
* WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
* NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
****************************************************************************/
/****************************************************************************
* Compilation Switches
****************************************************************************/
/****************************************************************************
* Included Files
****************************************************************************/
#include <nuttx/config.h>
#if defined(CONFIG_NET) && defined(CONFIG_NET_TCP)
#include <stdint.h>
#include <debug.h>
#include <net/uip/uipopt.h>
#include <net/uip/uip.h>
#include <net/uip/uip-arch.h>
#include "uip_internal.h"
/****************************************************************************
* Public Data
****************************************************************************/
/****************************************************************************
* Private Data
****************************************************************************/
/* g_tcpsequence is used to generate initial TCP sequence numbers */
static uint32_t g_tcpsequence;
/****************************************************************************
* Private Functions
****************************************************************************/
/****************************************************************************
* Public Functions
****************************************************************************/
/****************************************************************************
* Name: uip_tcpsetsequence
*
* Description:
* Set the TCP/IP sequence number
*
* Assumptions:
* This function may called from the interrupt level
*
****************************************************************************/
void uip_tcpsetsequence(FAR uint8_t *seqno, uint32_t value)
{
/* Copy the sequence number in network (big-endian) order */
*seqno++ = value >> 24;
*seqno++ = (value >> 16) & 0xff;
*seqno++ = (value >> 8) & 0xff;
*seqno = value & 0xff;
}
/****************************************************************************
* Name: uip_tcpgetsequence
*
* Description:
* Get the TCP/IP sequence number
*
* Assumptions:
* This function may called from the interrupt level
*
****************************************************************************/
uint32_t uip_tcpgetsequence(FAR uint8_t *seqno)
{
uint32_t value;
/* Combine the sequence number from network (big-endian) order */
value = (uint32_t)seqno[0] << 24 |
(uint32_t)seqno[1] << 16 |
(uint32_t)seqno[2] << 8 |
(uint32_t)seqno[3];
return value;
}
/****************************************************************************
* Name: uip_tcpaddsequence
*
* Description:
* Add the length to get the next TCP sequence number.
*
* Assumptions:
* This function may called from the interrupt level
*
****************************************************************************/
uint32_t uip_tcpaddsequence(FAR uint8_t *seqno, uint16_t len)
{
return uip_tcpgetsequence(seqno) + (uint32_t)len;
}
/****************************************************************************
* Name: uip_tcpinitsequence
*
* Description:
* Set the (initial) the TCP/IP sequence number when a TCP connection is
* established.
*
* Assumptions:
* This function may called from the interrupt level
*
****************************************************************************/
void uip_tcpinitsequence(FAR uint8_t *seqno)
{
uip_tcpsetsequence(seqno, g_tcpsequence);
}
/****************************************************************************
* Name: uip_tcpnextsequence
*
* Description:
* Increment the TCP/IP sequence number
*
* Assumptions:
* This function is called from the interrupt level
*
****************************************************************************/
void uip_tcpnextsequence(void)
{
g_tcpsequence++;
}
#endif /* CONFIG_NET && CONFIG_NET_TCP */