From cbed482747f112b438cfebf6e9c3b848e03890b5 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Thu, 17 Oct 2013 09:45:38 -0600 Subject: [PATCH] TCP: Change how initial MSS is handled. From Max Holtzberg --- ChangeLog | 3 ++ include/nuttx/net/uip/uip-tcp.h | 30 ++++++++------ net/send.c | 69 ++++++++++++++++++--------------- net/uip/uip_tcpconn.c | 3 +- net/uip/uip_tcpinput.c | 38 ++++++------------ 5 files changed, 72 insertions(+), 71 deletions(-) diff --git a/ChangeLog b/ChangeLog index becf0c0c63..03f4d57d03 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5782,4 +5782,7 @@ checking for timeout. From Max Holtzberg (2013-10-17). * net/net_sendfile.c: Correct paramter passed to netdev_txnotify() from Max Holtzberg (2013-10-17). + * include/nuttx/net/uip/uip-tcp.h, net/send.c, uip/uip_tcpconn.c, and + uip/uip_tcpinput.c: Change how the inital minimum MSS is calculated. + Max Holtzberg (2013-10-17). diff --git a/include/nuttx/net/uip/uip-tcp.h b/include/nuttx/net/uip/uip-tcp.h index b11de05d77..f9e6b742fa 100644 --- a/include/nuttx/net/uip/uip-tcp.h +++ b/include/nuttx/net/uip/uip-tcp.h @@ -105,6 +105,24 @@ #define UIP_IPTCPH_LEN (UIP_TCPH_LEN + UIP_IPH_LEN) /* Size of IP + TCP header */ #define UIP_TCPIP_HLEN UIP_IPTCPH_LEN +/* Initial minimum MSS according to RFC 879 + * + * There have been some assumptions made about using other than the + * default size for datagrams with some unfortunate results. + * + * HOSTS MUST NOT SEND DATAGRAMS LARGER THAN 576 OCTETS UNLESS THEY + * HAVE SPECIFIC KNOWLEDGE THAT THE DESTINATION HOST IS PREPARED TO + * ACCEPT LARGER DATAGRAMS. + * + * This is a long established rule. + */ + +#if UIP_TCP_MSS > 576 +# define UIP_TCP_INITIAL_MSS 576 +#else +# define UIP_TCP_INITIAL_MSS UIP_TCP_MSS +#endif + /**************************************************************************** * Public Type Definitions ****************************************************************************/ @@ -135,8 +153,6 @@ struct uip_conn uint16_t mss; /* Current maximum segment size for the * connection */ uint16_t winsize; /* Current window size of the connection */ - uint16_t initialmss; /* Initial maximum segment size for the - * connection */ uint8_t crefs; /* Reference counts on this instance */ uint8_t sa; /* Retransmission time-out calculation state * variable */ @@ -444,18 +460,8 @@ extern int uip_backlogdelete(FAR struct uip_conn *conn, FAR struct uip_conn *blc (conn)->tcpstateflags &= ~UIP_STOPPED; \ } while(0) -/* Get the initial maxium segment size (MSS) of the current - * connection. - */ - -#define uip_initialmss(conn) ((conn)->initialmss) - /* Get the current maximum segment size that can be sent on the current * connection. - * - * The current maxiumum segment size that can be sent on the connection is - * computed from the receiver's window and the MSS of the connection (which - * also is available by calling uip_initialmss()). */ #define uip_mss(conn) ((conn)->mss) diff --git a/net/send.c b/net/send.c index 4b6b34e735..07a36097af 100644 --- a/net/send.c +++ b/net/send.c @@ -356,45 +356,52 @@ static uint16_t send_interrupt(FAR struct uip_driver_s *dev, FAR void *pvconn, sndlen = uip_mss(conn); } - /* Set the sequence number for this packet. NOTE: uIP updates - * sndseq on recept of ACK *before* this function is called. In that - * case sndseq will point to the next unacknowledge byte (which might - * have already been sent). We will overwrite the value of sndseq - * here before the packet is sent. - */ + /* Check if we have "space" in the window */ - seqno = pstate->snd_sent + pstate->snd_isn; - nllvdbg("SEND: sndseq %08x->%08x\n", conn->sndseq, seqno); - uip_tcpsetsequence(conn->sndseq, seqno); + if ((pstate->snd_sent - pstate->snd_acked + sndlen) < conn->winsize) + { - /* Then set-up to send that amount of data. (this won't actually - * happen until the polling cycle completes). - */ + /* Set the sequence number for this packet. NOTE: uIP updates + * sndseq on recept of ACK *before* this function is called. In that + * case sndseq will point to the next unacknowledge byte (which might + * have already been sent). We will overwrite the value of sndseq + * here before the packet is sent. + */ - uip_send(dev, &pstate->snd_buffer[pstate->snd_sent], sndlen); + seqno = pstate->snd_sent + pstate->snd_isn; + nllvdbg("SEND: sndseq %08x->%08x\n", conn->sndseq, seqno); + uip_tcpsetsequence(conn->sndseq, seqno); - /* Check if the destination IP address is in the ARP table. If not, - * then the send won't actually make it out... it will be replaced with - * an ARP request. - * - * NOTE 1: This could be an expensive check if there are a lot of entries - * in the ARP table. Hence, we only check on the first packet -- when - * snd_sent is zero. - * - * NOTE 2: If we are actually harvesting IP addresses on incomming IP - * packets, then this check should not be necessary; the MAC mapping - * should already be in the ARP table. - */ + /* Then set-up to send that amount of data. (this won't actually + * happen until the polling cycle completes). + */ + + uip_send(dev, &pstate->snd_buffer[pstate->snd_sent], sndlen); + + /* Check if the destination IP address is in the ARP table. If not, + * then the send won't actually make it out... it will be replaced with + * an ARP request. + * + * NOTE 1: This could be an expensive check if there are a lot of entries + * in the ARP table. Hence, we only check on the first packet -- when + * snd_sent is zero. + * + * NOTE 2: If we are actually harvesting IP addresses on incomming IP + * packets, then this check should not be necessary; the MAC mapping + * should already be in the ARP table. + */ #if defined(CONFIG_NET_ETHERNET) && !defined(CONFIG_NET_ARP_IPIN) - if (pstate->snd_sent != 0 || uip_arp_find(conn->ripaddr) != NULL) + if (pstate->snd_sent != 0 || uip_arp_find(conn->ripaddr) != NULL) #endif - { - /* Update the amount of data sent (but not necessarily ACKed) */ + { + /* Update the amount of data sent (but not necessarily ACKed) */ - pstate->snd_sent += sndlen; - nllvdbg("SEND: acked=%d sent=%d buflen=%d\n", - pstate->snd_acked, pstate->snd_sent, pstate->snd_buflen); + pstate->snd_sent += sndlen; + nllvdbg("SEND: acked=%d sent=%d buflen=%d\n", + pstate->snd_acked, pstate->snd_sent, pstate->snd_buflen); + + } } } diff --git a/net/uip/uip_tcpconn.c b/net/uip/uip_tcpconn.c index 9ab3c38fe5..3482300105 100644 --- a/net/uip/uip_tcpconn.c +++ b/net/uip/uip_tcpconn.c @@ -496,6 +496,7 @@ struct uip_conn *uip_tcpaccept(struct uip_tcpip_hdr *buf) conn->nrtx = 0; conn->lport = buf->destport; conn->rport = buf->srcport; + conn->mss = UIP_TCP_INITIAL_MSS; uip_ipaddr_copy(conn->ripaddr, uip_ip4addr_conv(buf->srcipaddr)); conn->tcpstateflags = UIP_SYN_RCVD; @@ -632,7 +633,7 @@ int uip_tcpconnect(struct uip_conn *conn, const struct sockaddr_in *addr) conn->tcpstateflags = UIP_SYN_SENT; uip_tcpinitsequence(conn->sndseq); - conn->initialmss = conn->mss = UIP_TCP_MSS; + conn->mss = UIP_TCP_INITIAL_MSS; conn->unacked = 1; /* TCP length of the SYN is one. */ conn->nrtx = 0; conn->timer = 1; /* Send the SYN next time around. */ diff --git a/net/uip/uip_tcpinput.c b/net/uip/uip_tcpinput.c index e0f3a78194..54965918c9 100644 --- a/net/uip/uip_tcpinput.c +++ b/net/uip/uip_tcpinput.c @@ -2,7 +2,7 @@ * net/uip/uip_tcpinput.c * Handling incoming TCP input * - * Copyright (C) 2007-2012 Gregory Nutt. All rights reserved. + * Copyright (C) 2007-2013 Gregory Nutt. All rights reserved. * Author: Gregory Nutt * * Adapted for NuttX from logic in uIP which also has a BSD-like license: @@ -244,8 +244,7 @@ void uip_tcpinput(struct uip_driver_s *dev) tmp16 = ((uint16_t)dev->d_buf[UIP_TCPIP_HLEN + UIP_LLH_LEN + 2 + i] << 8) | (uint16_t)dev->d_buf[UIP_IPTCPH_LEN + UIP_LLH_LEN + 3 + i]; - conn->initialmss = conn->mss = - tmp16 > UIP_TCP_MSS? UIP_TCP_MSS: tmp16; + conn->mss = tmp16 > UIP_TCP_MSS ? UIP_TCP_MSS : tmp16; /* And we are done processing options. */ @@ -300,7 +299,7 @@ found: /* Update the connection's window size */ - conn->winsize = BUF->wnd[0] << 8 | BUF->wnd[1]; + conn->winsize = ((uint16_t)pbuf->wnd[0] << 8) + (uint16_t)pbuf->wnd[1]; flags = 0; @@ -522,9 +521,7 @@ found: tmp16 = (dev->d_buf[UIP_TCPIP_HLEN + UIP_LLH_LEN + 2 + i] << 8) | dev->d_buf[UIP_TCPIP_HLEN + UIP_LLH_LEN + 3 + i]; - conn->initialmss = - conn->mss = - tmp16 > UIP_TCP_MSS? UIP_TCP_MSS: tmp16; + conn->mss = tmp16 > UIP_TCP_MSS ? UIP_TCP_MSS : tmp16; /* And we are done processing options. */ @@ -595,10 +592,17 @@ found: if ((pbuf->flags & TCP_FIN) != 0 && (conn->tcpstateflags & UIP_STOPPED) == 0) { + /* Needs to be investigated further. + * Windows often sends FIN packets together with the last ACK for + * the received data. So the socket layer has to get this ACK even + * if the connection is going to be closed. + */ +#if 0 if (conn->unacked > 0) { goto drop; } +#endif /* Update the sequence number and indicate that the connection has * been closed. @@ -663,26 +667,6 @@ found: flags |= UIP_NEWDATA; } - /* Check if the available buffer space advertised by the other end - * is smaller than the initial MSS for this connection. If so, we - * set the current MSS to the window size to ensure that the - * application does not send more data than the other end can - * handle. - * - * If the remote host advertises a zero window, we set the MSS to - * the initial MSS so that the application will send an entire MSS - * of data. This data will not be acknowledged by the receiver, - * and the application will retransmit it. This is called the - * "persistent timer" and uses the retransmission mechanim. - */ - - tmp16 = ((uint16_t)pbuf->wnd[0] << 8) + (uint16_t)pbuf->wnd[1]; - if (tmp16 > conn->initialmss || tmp16 == 0) - { - tmp16 = conn->initialmss; - } - conn->mss = tmp16; - /* If this packet constitutes an ACK for outstanding data (flagged * by the UIP_ACKDATA flag), we should call the application since it * might want to send more data. If the incoming packet had data