From 14ec75e7fcb43db72a70c0b8eeefe7b9843a5596 Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 1 Jun 2021 16:57:00 +0900 Subject: [PATCH] tcp: window update improvements * Fixes the case where the window was small but not zero. * tcp_recvfrom: Remove tcp_ackhandler. Instead, simply schedule TX for a possible window update and make tcp_appsend decide. * Replace rcv_wnd (the last advertized window size value) with rcv_adv. (the window edge sequence number advertized to the peer) rcv_wnd was complicated to deal with because its base (rcvseq) is also moving. * tcp_appsend: Send a window update even if there are no other reasons to send an ack. Namely, send an update if it increases the window by * 2 * mss * or the half of the max possible window size --- net/sixlowpan/sixlowpan_tcpsend.c | 3 +- net/tcp/tcp.h | 24 +++++-- net/tcp/tcp_appsend.c | 12 +++- net/tcp/tcp_conn.c | 1 + net/tcp/tcp_input.c | 1 + net/tcp/tcp_recvfrom.c | 57 +--------------- net/tcp/tcp_recvwindow.c | 104 ++++++++++++++++++++++++++++++ net/tcp/tcp_send.c | 3 +- 8 files changed, 140 insertions(+), 65 deletions(-) diff --git a/net/sixlowpan/sixlowpan_tcpsend.c b/net/sixlowpan/sixlowpan_tcpsend.c index 593bbc4be2b..47f9c5a833d 100644 --- a/net/sixlowpan/sixlowpan_tcpsend.c +++ b/net/sixlowpan/sixlowpan_tcpsend.c @@ -257,6 +257,7 @@ static int sixlowpan_tcp_header(FAR struct tcp_conn_s *conn, { /* Update the TCP received window based on I/O buffer availability */ + uint32_t rcvseq = tcp_getsequence(conn->rcvseq); uint16_t recvwndo = tcp_get_recvwindow(dev, conn); /* Set the TCP Window */ @@ -266,7 +267,7 @@ static int sixlowpan_tcp_header(FAR struct tcp_conn_s *conn, /* Update the Receiver Window */ - conn->rcv_wnd = recvwndo; + conn->rcv_adv = rcvseq + recvwndo; } /* Calculate TCP checksum. */ diff --git a/net/tcp/tcp.h b/net/tcp/tcp.h index b71c01272c3..5cbf8f4efb1 100644 --- a/net/tcp/tcp.h +++ b/net/tcp/tcp.h @@ -188,7 +188,7 @@ struct tcp_conn_s * connection */ uint16_t snd_wnd; /* Sequence and acknowledgement numbers of last * window update */ - uint16_t rcv_wnd; /* Receiver window available */ + uint32_t rcv_adv; /* The right edge of the recv window advertized */ #ifdef CONFIG_NET_TCP_WRITE_BUFFERS uint32_t tx_unacked; /* Number bytes sent but not yet ACKed */ #else @@ -267,12 +267,6 @@ struct tcp_conn_s FAR struct devif_callback_s *connevents; - /* Receiver callback to indicate that the data has been consumed and that - * an ACK should be send. - */ - - FAR struct devif_callback_s *rcv_ackcb; - /* accept() is called when the TCP logic has created a connection * * accept_private: This is private data that will be available to the @@ -1468,6 +1462,22 @@ int tcp_getsockopt(FAR struct socket *psock, int option, uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn); +/**************************************************************************** + * Name: tcp_should_send_recvwindow + * + * Description: + * Determine if we should advertize the new recv window to the peer. + * + * Input Parameters: + * conn - The TCP connection structure holding connection information. + * + * Returned Value: + * If we should send an update. + * + ****************************************************************************/ + +bool tcp_should_send_recvwindow(FAR struct tcp_conn_s *conn); + /**************************************************************************** * Name: psock_tcp_cansend * diff --git a/net/tcp/tcp_appsend.c b/net/tcp/tcp_appsend.c index cff0740ed9b..d2a05bd175d 100644 --- a/net/tcp/tcp_appsend.c +++ b/net/tcp/tcp_appsend.c @@ -89,10 +89,20 @@ void tcp_appsend(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn, ninfo("result: %04x d_sndlen: %d conn->tx_unacked: %" PRId32 "\n", result, dev->d_sndlen, (uint32_t)conn->tx_unacked); + /* Need to update the recv window? */ + + if (tcp_should_send_recvwindow(conn)) + { + result |= TCP_SNDACK; +#ifdef CONFIG_NET_TCP_DELAYED_ACK + conn->rx_unackseg = 0; +#endif + } + #ifdef CONFIG_NET_TCP_DELAYED_ACK /* Did the caller request that an ACK be sent? */ - if ((result & TCP_SNDACK) != 0) + else if ((result & TCP_SNDACK) != 0) { /* Yes.. Handle delayed acknowledgments */ diff --git a/net/tcp/tcp_conn.c b/net/tcp/tcp_conn.c index af1515fafd9..d2b3c34b64a 100644 --- a/net/tcp/tcp_conn.c +++ b/net/tcp/tcp_conn.c @@ -966,6 +966,7 @@ FAR struct tcp_conn_s *tcp_alloc_accept(FAR struct net_driver_s *dev, /* rcvseq should be the seqno from the incoming packet + 1. */ memcpy(conn->rcvseq, tcp->seqno, 4); + conn->rcv_adv = tcp_getsequence(conn->rcvseq); /* Initialize the list of TCP read-ahead buffers */ diff --git a/net/tcp/tcp_input.c b/net/tcp/tcp_input.c index 5823f24769b..2d8735fe246 100644 --- a/net/tcp/tcp_input.c +++ b/net/tcp/tcp_input.c @@ -704,6 +704,7 @@ found: conn->tcpstateflags = TCP_ESTABLISHED; memcpy(conn->rcvseq, tcp->seqno, 4); + conn->rcv_adv = tcp_getsequence(conn->rcvseq); net_incr32(conn->rcvseq, 1); conn->tx_unacked = 0; diff --git a/net/tcp/tcp_recvfrom.c b/net/tcp/tcp_recvfrom.c index 973a5c56a97..c6b79d79cdc 100644 --- a/net/tcp/tcp_recvfrom.c +++ b/net/tcp/tcp_recvfrom.c @@ -510,53 +510,6 @@ static uint16_t tcp_recvhandler(FAR struct net_driver_s *dev, return flags; } -/**************************************************************************** - * Name: tcp_ackhandler - * - * Description: - * This function is called with the network locked to send the ACK in - * response by the lower, device interfacing layer. - * - * Input Parameters: - * dev The structure of the network driver that generated the event. - * pvconn The connection structure associated with the socket - * flags Set of events describing why the callback was invoked - * - * Returned Value: - * ACK should be send in the response. - * - * Assumptions: - * The network is locked. - * - ****************************************************************************/ - -static uint16_t tcp_ackhandler(FAR struct net_driver_s *dev, - FAR void *pvconn, FAR void *pvpriv, - uint16_t flags) -{ - FAR struct tcp_conn_s *conn = (FAR struct tcp_conn_s *)pvconn; - - ninfo("flags: %04x\n", flags); - - if (conn != NULL && (flags & TCP_POLL) != 0) - { - /* Indicate that the data has been consumed and that an ACK - * should be send. - */ - - if (tcp_get_recvwindow(dev, conn) != 0 && - conn->rcv_wnd == 0) - { - flags |= TCP_SNDACK; - } - - tcp_callback_free(conn, conn->rcv_ackcb); - conn->rcv_ackcb = NULL; - } - - return flags; -} - /**************************************************************************** * Name: tcp_recvfrom_initialize * @@ -816,15 +769,9 @@ ssize_t psock_tcp_recvfrom(FAR struct socket *psock, FAR void *buf, * not only this particular connection. */ - if (conn->rcv_wnd == 0 && conn->rcv_ackcb == NULL) + if (tcp_should_send_recvwindow(conn)) { - conn->rcv_ackcb = tcp_callback_alloc(conn); - if (conn->rcv_ackcb) - { - conn->rcv_ackcb->flags = TCP_POLL; - conn->rcv_ackcb->event = tcp_ackhandler; - netdev_txnotify_dev(conn->dev); - } + netdev_txnotify_dev(conn->dev); } net_unlock(); diff --git a/net/tcp/tcp_recvwindow.c b/net/tcp/tcp_recvwindow.c index b8afa3ba0b9..00083821eda 100644 --- a/net/tcp/tcp_recvwindow.c +++ b/net/tcp/tcp_recvwindow.c @@ -37,6 +37,45 @@ #include "tcp/tcp.h" +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: tcp_maxrcvwin + * + * Description: + * Calculate the possible max TCP receive window for the connection. + * + * Input Parameters: + * conn - The TCP connection. + * + * Returned Value: + * The value of the TCP receive window. + ****************************************************************************/ + +static uint16_t tcp_maxrcvwin(FAR struct tcp_conn_s *conn) +{ + size_t maxiob; + uint16_t maxwin; + + /* Calculate the max possible window size for the connection. + * This needs to be in sync with tcp_get_recvwindow(). + */ + + maxiob = (CONFIG_IOB_NBUFFERS - CONFIG_IOB_THROTTLE) * CONFIG_IOB_BUFSIZE; + if (maxiob >= UINT16_MAX) + { + maxwin = UINT16_MAX; + } + else + { + maxwin = maxiob; + } + + return maxwin; +} + /**************************************************************************** * Public Functions ****************************************************************************/ @@ -163,3 +202,68 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev, return recvwndo; } + +bool tcp_should_send_recvwindow(FAR struct tcp_conn_s *conn) +{ + FAR struct net_driver_s *dev = conn->dev; + uint16_t win; + uint16_t maxwin; + uint16_t oldwin; + uint32_t rcvseq; + uint16_t adv; + uint16_t mss; + + /* Note: rcv_adv can be smaller than rcvseq. + * For examples, when: + * + * - we shrunk the window + * - zero window probes advanced rcvseq + */ + + rcvseq = tcp_getsequence(conn->rcvseq); + if (TCP_SEQ_GT(conn->rcv_adv, rcvseq)) + { + oldwin = TCP_SEQ_SUB(conn->rcv_adv, rcvseq); + } + else + { + oldwin = 0; + } + + win = tcp_get_recvwindow(dev, conn); + + /* If the window doesn't extend, don't send. */ + + if (win <= oldwin) + { + return false; + } + + adv = win - oldwin; + + /* The following conditions are inspired from NetBSD TCP stack. + * + * - If we can extend the window by the half of the max possible size, + * send it. + * + * - If we can extend the window by 2 * mss, send it. + */ + + maxwin = tcp_maxrcvwin(conn); + if (2 * adv >= maxwin) + { + return true; + } + + /* Revisit: the real expected size should be used instead. + * E.g. consider the path MTU + */ + + mss = tcp_rx_mss(dev); + if (adv >= 2 * mss) + { + return true; + } + + return false; +} diff --git a/net/tcp/tcp_send.c b/net/tcp/tcp_send.c index 1079b137a8e..a17fe9719ea 100644 --- a/net/tcp/tcp_send.c +++ b/net/tcp/tcp_send.c @@ -362,6 +362,7 @@ static void tcp_sendcommon(FAR struct net_driver_s *dev, { /* Update the TCP received window based on I/O buffer availability */ + uint32_t rcvseq = tcp_getsequence(conn->rcvseq); uint16_t recvwndo = tcp_get_recvwindow(dev, conn); /* Set the TCP Window */ @@ -371,7 +372,7 @@ static void tcp_sendcommon(FAR struct net_driver_s *dev, /* Update the Receiver Window */ - conn->rcv_wnd = recvwndo; + conn->rcv_adv = rcvseq + recvwndo; } /* Finish the IP portion of the message and calculate checksums */