From dec7ecbd5642ed63ec708e2780b0807806616edd Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sat, 22 Sep 2018 09:33:05 -0600 Subject: [PATCH] net/tcp/tcp_recvwindow.c: In order to receive data we must not only have IOBs available, but we must also have at least one IOB chain qentry available. Otherwise, we will advertise that we an buffer a lot of data when, in fact, we cannot. This is an expermental fix to a performance problem noted by Masayuki Ishikawa. --- include/nuttx/mm/iob.h | 4 ++-- mm/iob/iob_navail.c | 4 ++-- net/tcp/tcp_recvwindow.c | 31 ++++++++++++++++++------------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/include/nuttx/mm/iob.h b/include/nuttx/mm/iob.h index e47f4b55d56..0c075b117a0 100644 --- a/include/nuttx/mm/iob.h +++ b/include/nuttx/mm/iob.h @@ -198,14 +198,14 @@ FAR struct iob_s *iob_tryalloc(bool throttled); int iob_navail(bool throttled); /**************************************************************************** - * Name: iob_quentry_navail + * Name: iob_qentry_navail * * Description: * Return the number of available IOB chains. * ****************************************************************************/ -int iob_quentry_navail(void); +int iob_qentry_navail(void); /**************************************************************************** * Name: iob_free diff --git a/mm/iob/iob_navail.c b/mm/iob/iob_navail.c index 4f41cadfe70..2c3b157083f 100644 --- a/mm/iob/iob_navail.c +++ b/mm/iob/iob_navail.c @@ -93,14 +93,14 @@ int iob_navail(bool throttled) } /**************************************************************************** - * Name: iob_quentry_navail + * Name: iob_qentry_navail * * Description: * Return the number of available IOB chains. * ****************************************************************************/ -int iob_quentry_navail(void) +int iob_qentry_navail(void) { int navail = 0; int ret; diff --git a/net/tcp/tcp_recvwindow.c b/net/tcp/tcp_recvwindow.c index 30b5f352684..694b965682b 100644 --- a/net/tcp/tcp_recvwindow.c +++ b/net/tcp/tcp_recvwindow.c @@ -76,7 +76,8 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev) uint16_t mss; uint16_t recvwndo; #ifdef CONFIG_NET_TCP_READAHEAD - int navail; + int niob_avail; + int nqentry_avail; #endif #ifdef CONFIG_NET_IPv6 @@ -107,10 +108,14 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev) #ifdef CONFIG_NET_TCP_READAHEAD /* Update the TCP received window based on read-ahead I/O buffer - * availability. + * and IOB chain availability. At least one queue entry is required. + * If one queue entry is available, then the amount of read-ahead + * data that can be buffered is given by the number of IOBs available + * (ignoring competition with other IOB consumers). */ - navail = iob_navail(true); + niob_avail = iob_navail(true); + nqentry_avail = iob_qentry_navail(); /* Are the read-ahead allocations throttled? If so, then not all of these * IOBs are available for read-ahead buffering. @@ -120,19 +125,19 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev) */ #if CONFIG_IOB_THROTTLE > 0 - if (navail > CONFIG_IOB_THROTTLE) + if (niob_avail > CONFIG_IOB_THROTTLE) { - navail -= CONFIG_IOB_THROTTLE; + niob_avail -= CONFIG_IOB_THROTTLE; } else { - navail = 0; + niob_avail = 0; } #endif - /* Are there any IOBs available for read-ahead buffering? */ + /* Is there a a queue entry and IOBs available for read-ahead buffering? */ - if (navail > 0) + if (nqentry_avail > 0 && niob_avail > 0) { uint32_t rwnd; @@ -152,7 +157,7 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev) * buffering for this connection. */ - rwnd = (navail * CONFIG_IOB_BUFSIZE) + mss; + rwnd = (niob_avail * CONFIG_IOB_BUFSIZE) + mss; if (rwnd > UINT16_MAX) { rwnd = UINT16_MAX; @@ -162,12 +167,12 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev) recvwndo = (uint16_t)rwnd; } - else /* if (navail == 0) */ + else /* nqentry_avail == 0 || niob_avail == 0 */ #endif { - /* No IOBs are available. The only buffering available is within the - * packet buffer itself. We can buffer no more than the MSS (unless - * we are very fast). + /* No IOB chains or noIOBs are available. The only buffering + * available is within the packet buffer itself. We can buffer no + * more than the MSS (unless we are very fast). * * NOTE: If no IOBs are available, then the next packet will be * lost if there is no listener on the connection.