diff options
| -rwxr-xr-x | nuttx/ChangeLog | 13 | ||||
| -rw-r--r-- | nuttx/include/nuttx/net/iob.h | 26 | ||||
| -rw-r--r-- | nuttx/net/iob/iob.h | 23 | ||||
| -rw-r--r-- | nuttx/net/iob/iob_add_queue.c | 72 | ||||
| -rw-r--r-- | nuttx/net/iob/iob_alloc.c | 162 | ||||
| -rw-r--r-- | nuttx/net/iob/iob_alloc_qentry.c | 98 | ||||
| -rw-r--r-- | nuttx/net/iob/iob_copyin.c | 47 | ||||
| -rw-r--r-- | nuttx/net/tcp/tcp_callback.c | 18 |
8 files changed, 304 insertions, 155 deletions
diff --git a/nuttx/ChangeLog b/nuttx/ChangeLog index e6e6abfef..3ea6bd3fc 100755 --- a/nuttx/ChangeLog +++ b/nuttx/ChangeLog @@ -9538,4 +9538,15 @@ * net/local: Add initial support for local, Unix domain sockets. Initial support is limited to SOCK_STREAM and is untested on initial commit (2014-01-27). - + * net/iob, net/tcp/tcp_callback.c, and include/nuttx/net/iob.h: There + were issues with the TCP write-ahead buffering if CONFIG_NET_NOINTS was + enabled: There is a possibility of deadlocks in certain timing conditions. + I have not seen this with the Tiva driver that I have been users but + other people claim to see the issue on other platforms. Certainly it + is a logic error: The network should never wait for TCP read-ahead + buffering space to be available. It should drop the packets + immediately. This was fixed by duplicating most of the IOB interfaces: + The versions that waited are still present (like iob_alloc()), but now + there are non-waiting versions of the same interfaces (like + iob_tryalloc()). The TCP read-ahead logic now uses only these non- + waiting interfaces (2015-01-27). diff --git a/nuttx/include/nuttx/net/iob.h b/nuttx/include/nuttx/net/iob.h index 2cad4fa22..121d82ae1 100644 --- a/nuttx/include/nuttx/net/iob.h +++ b/nuttx/include/nuttx/net/iob.h @@ -220,6 +220,19 @@ int iob_add_queue(FAR struct iob_s *iob, FAR struct iob_queue_s *iobq); #endif /* CONFIG_IOB_NCHAINS > 0 */ /**************************************************************************** + * Name: iob_tryadd_queue + * + * Description: + * Add one I/O buffer chain to the end of a queue without waiting for + * resources to become free. + * + ****************************************************************************/ + +#if CONFIG_IOB_NCHAINS > 0 +int iob_tryadd_queue(FAR struct iob_s *iob, FAR struct iob_queue_s *iobq); +#endif /* CONFIG_IOB_NCHAINS > 0 */ + +/**************************************************************************** * Name: iob_remove_queue * * Description: @@ -278,6 +291,19 @@ int iob_copyin(FAR struct iob_s *iob, FAR const uint8_t *src, unsigned int len, unsigned int offset, bool throttled); /**************************************************************************** + * Name: iob_trycopyin + * + * Description: + * Copy data 'len' bytes from a user buffer into the I/O buffer chain, + * starting at 'offset', extending the chain as necessary BUT without + * waiting if buffers are not available. + * + ****************************************************************************/ + +int iob_trycopyin(FAR struct iob_s *iob, FAR const uint8_t *src, + unsigned int len, unsigned int offset, bool throttled); + +/**************************************************************************** * Name: iob_copyout * * Description: diff --git a/nuttx/net/iob/iob.h b/nuttx/net/iob/iob.h index 3b5eaf9e4..49222bec4 100644 --- a/nuttx/net/iob/iob.h +++ b/nuttx/net/iob/iob.h @@ -99,6 +99,29 @@ extern sem_t g_qentry_sem; /* Counts free I/O buffer queue containers */ FAR struct iob_qentry_s *iob_alloc_qentry(void); /**************************************************************************** + * Name: iob_tryalloc_qentry + * + * Description: + * Try to allocate an I/O buffer chain container by taking the buffer at + * the head of the free list without waiting for the container to become + * free. This function is intended only for internal use by the IOB module. + * + ****************************************************************************/ + +FAR struct iob_qentry_s *iob_tryalloc_qentry(void); + +/**************************************************************************** + * Name: iob_tryalloc + * + * Description: + * Try to allocate an I/O buffer by taking the buffer at the head of the + * free list without waiting for a buffer to become free. + * + ****************************************************************************/ + +FAR struct iob_s *iob_tryalloc(bool throttled); + +/**************************************************************************** * Name: iob_free_qentry * * Description: diff --git a/nuttx/net/iob/iob_add_queue.c b/nuttx/net/iob/iob_add_queue.c index 99e0f6ff4..c3da0dd68 100644 --- a/nuttx/net/iob/iob_add_queue.c +++ b/nuttx/net/iob/iob_add_queue.c @@ -65,11 +65,11 @@ #endif /**************************************************************************** - * Public Functions + * Private Functions ****************************************************************************/ /**************************************************************************** - * Name: iob_add_queue + * Name: iob_add_queue_internal * * Description: * Add one I/O buffer chain to the end of a queue. May fail due to lack @@ -77,19 +77,10 @@ * ****************************************************************************/ -int iob_add_queue(FAR struct iob_s *iob, FAR struct iob_queue_s *iobq) +static int iob_add_queue_internal(FAR struct iob_s *iob, + FAR struct iob_queue_s *iobq, + FAR struct iob_qentry_s *qentry) { - FAR struct iob_qentry_s *qentry; - - /* Allocate a container to hold the I/O buffer chain */ - - qentry = iob_alloc_qentry(); - if (!qentry) - { - ndbg("ERROR: Failed to allocate a container\n"); - return -ENOMEM; - } - /* Add the I/O buffer chain to the container */ qentry->qe_head = iob; @@ -112,4 +103,57 @@ int iob_add_queue(FAR struct iob_s *iob, FAR struct iob_queue_s *iobq) return 0; } +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: iob_add_queue + * + * Description: + * Add one I/O buffer chain to the end of a queue. May fail due to lack + * of resources. + * + ****************************************************************************/ + +int iob_add_queue(FAR struct iob_s *iob, FAR struct iob_queue_s *iobq) +{ + FAR struct iob_qentry_s *qentry; + + /* Allocate a container to hold the I/O buffer chain */ + + qentry = iob_alloc_qentry(); + if (!qentry) + { + ndbg("ERROR: Failed to allocate a container\n"); + return -ENOMEM; + } + + return iob_add_queue_internal(iob, iobq, qentry); +} + +/**************************************************************************** + * Name: iob_tryadd_queue + * + * Description: + * Add one I/O buffer chain to the end of a queue without waiting for + * resources to become free. + * + ****************************************************************************/ + +int iob_tryadd_queue(FAR struct iob_s *iob, FAR struct iob_queue_s *iobq) +{ + FAR struct iob_qentry_s *qentry; + + /* Allocate a container to hold the I/O buffer chain */ + + qentry = iob_tryalloc_qentry(); + if (!qentry) + { + ndbg("ERROR: Failed to allocate a container\n"); + return -ENOMEM; + } + + return iob_add_queue_internal(iob, iobq, qentry); +} #endif /* CONFIG_IOB_NCHAINS > 0 */ diff --git a/nuttx/net/iob/iob_alloc.c b/nuttx/net/iob/iob_alloc.c index 72da2c9ed..e6ad9de3c 100644 --- a/nuttx/net/iob/iob_alloc.c +++ b/nuttx/net/iob/iob_alloc.c @@ -75,87 +75,6 @@ ****************************************************************************/ /**************************************************************************** - * Name: iob_tryalloc - * - * Description: - * Try to allocate an I/O buffer by taking the buffer at the head of the - * free list. - * - ****************************************************************************/ - -static FAR struct iob_s *iob_tryalloc(bool throttled) -{ - FAR struct iob_s *iob; - irqstate_t flags; -#if CONFIG_IOB_THROTTLE > 0 - FAR sem_t *sem; -#endif - -#if CONFIG_IOB_THROTTLE > 0 - /* Select the semaphore count to check. */ - - sem = (throttled ? &g_throttle_sem : &g_iob_sem); -#endif - - /* We don't know what context we are called from so we use extreme measures - * to protect the free list: We disable interrupts very briefly. - */ - - flags = irqsave(); - -#if CONFIG_IOB_THROTTLE > 0 - /* If there are free I/O buffers for this allocation */ - - if (sem->semcount > 0) -#endif - { - /* Take the I/O buffer from the head of the free list */ - - iob = g_iob_freelist; - if (iob) - { - /* Remove the I/O buffer from the free list and decrement the - * counting semaphore(s) that tracks the number of available - * IOBs. - */ - - g_iob_freelist = iob->io_flink; - - /* Take a semaphore count. Note that we cannot do this in - * in the orthodox way by calling sem_wait() or sem_trywait() - * because this function may be called from an interrupt - * handler. Fortunately we know at at least one free buffer - * so a simple decrement is all that is needed. - */ - - g_iob_sem.semcount--; - DEBUGASSERT(g_iob_sem.semcount >= 0); - -#if CONFIG_IOB_THROTTLE > 0 - /* The throttle semaphore is a little more complicated because - * it can be negative! Decrementing is still safe, however. - */ - - g_throttle_sem.semcount--; - DEBUGASSERT(g_throttle_sem.semcount >= -CONFIG_IOB_THROTTLE); -#endif - irqrestore(flags); - - /* Put the I/O buffer in a known state */ - - iob->io_flink = NULL; /* Not in a chain */ - iob->io_len = 0; /* Length of the data in the entry */ - iob->io_offset = 0; /* Offset to the beginning of data */ - iob->io_pktlen = 0; /* Total length of the packet */ - return iob; - } - } - - irqrestore(flags); - return NULL; -} - -/**************************************************************************** * Name: iob_allocwait * * Description: @@ -246,3 +165,84 @@ FAR struct iob_s *iob_alloc(bool throttled) return iob_allocwait(throttled); } } + +/**************************************************************************** + * Name: iob_tryalloc + * + * Description: + * Try to allocate an I/O buffer by taking the buffer at the head of the + * free list without waiting for a buffer to become free. + * + ****************************************************************************/ + +FAR struct iob_s *iob_tryalloc(bool throttled) +{ + FAR struct iob_s *iob; + irqstate_t flags; +#if CONFIG_IOB_THROTTLE > 0 + FAR sem_t *sem; +#endif + +#if CONFIG_IOB_THROTTLE > 0 + /* Select the semaphore count to check. */ + + sem = (throttled ? &g_throttle_sem : &g_iob_sem); +#endif + + /* We don't know what context we are called from so we use extreme measures + * to protect the free list: We disable interrupts very briefly. + */ + + flags = irqsave(); + +#if CONFIG_IOB_THROTTLE > 0 + /* If there are free I/O buffers for this allocation */ + + if (sem->semcount > 0) +#endif + { + /* Take the I/O buffer from the head of the free list */ + + iob = g_iob_freelist; + if (iob) + { + /* Remove the I/O buffer from the free list and decrement the + * counting semaphore(s) that tracks the number of available + * IOBs. + */ + + g_iob_freelist = iob->io_flink; + + /* Take a semaphore count. Note that we cannot do this in + * in the orthodox way by calling sem_wait() or sem_trywait() + * because this function may be called from an interrupt + * handler. Fortunately we know at at least one free buffer + * so a simple decrement is all that is needed. + */ + + g_iob_sem.semcount--; + DEBUGASSERT(g_iob_sem.semcount >= 0); + +#if CONFIG_IOB_THROTTLE > 0 + /* The throttle semaphore is a little more complicated because + * it can be negative! Decrementing is still safe, however. + */ + + g_throttle_sem.semcount--; + DEBUGASSERT(g_throttle_sem.semcount >= -CONFIG_IOB_THROTTLE); +#endif + irqrestore(flags); + + /* Put the I/O buffer in a known state */ + + iob->io_flink = NULL; /* Not in a chain */ + iob->io_len = 0; /* Length of the data in the entry */ + iob->io_offset = 0; /* Offset to the beginning of data */ + iob->io_pktlen = 0; /* Total length of the packet */ + return iob; + } + } + + irqrestore(flags); + return NULL; +} diff --git a/nuttx/net/iob/iob_alloc_qentry.c b/nuttx/net/iob/iob_alloc_qentry.c index 23bd8712e..cd92eb5de 100644 --- a/nuttx/net/iob/iob_alloc_qentry.c +++ b/nuttx/net/iob/iob_alloc_qentry.c @@ -77,55 +77,6 @@ ****************************************************************************/ /**************************************************************************** - * Name: iob_tryalloc_qentry - * - * Description: - * Try to allocate an I/O buffer chain container by taking the buffer at - * the head of the free list. This function is intended only for internal - * use by the IOB module. - * - ****************************************************************************/ - -static FAR struct iob_qentry_s *iob_tryalloc_qentry(void) -{ - FAR struct iob_qentry_s *iobq; - irqstate_t flags; - - /* We don't know what context we are called from so we use extreme measures - * to protect the free list: We disable interrupts very briefly. - */ - - flags = irqsave(); - iobq = g_iob_freeqlist; - if (iobq) - { - /* Remove the I/O buffer chain container from the free list and - * decrement the counting semaphore that tracks the number of free - * containers. - */ - - g_iob_freeqlist = iobq->qe_flink; - - /* Take a semaphore count. Note that we cannot do this in - * in the orthodox way by calling sem_wait() or sem_trywait() - * because this function may be called from an interrupt - * handler. Fortunately we know at at least one free buffer - * so a simple decrement is all that is needed. - */ - - g_qentry_sem.semcount--; - DEBUGASSERT(g_qentry_sem.semcount >= 0); - - /* Put the I/O buffer in a known state */ - - iobq->qe_head = NULL; /* Nothing is contained */ - } - - irqrestore(flags); - return iobq; -} - -/**************************************************************************** * Name: iob_allocwait_qentry * * Description: @@ -211,4 +162,53 @@ FAR struct iob_qentry_s *iob_alloc_qentry(void) } } +/**************************************************************************** + * Name: iob_tryalloc_qentry + * + * Description: + * Try to allocate an I/O buffer chain container by taking the buffer at + * the head of the free list without waiting for the container to become + * free. This function is intended only for internal use by the IOB module. + * + ****************************************************************************/ + +FAR struct iob_qentry_s *iob_tryalloc_qentry(void) +{ + FAR struct iob_qentry_s *iobq; + irqstate_t flags; + + /* We don't know what context we are called from so we use extreme measures + * to protect the free list: We disable interrupts very briefly. + */ + + flags = irqsave(); + iobq = g_iob_freeqlist; + if (iobq) + { + /* Remove the I/O buffer chain container from the free list and + * decrement the counting semaphore that tracks the number of free + * containers. + */ + + g_iob_freeqlist = iobq->qe_flink; + + /* Take a semaphore count. Note that we cannot do this in + * in the orthodox way by calling sem_wait() or sem_trywait() + * because this function may be called from an interrupt + * handler. Fortunately we know at at least one free buffer + * so a simple decrement is all that is needed. + */ + + g_qentry_sem.semcount--; + DEBUGASSERT(g_qentry_sem.semcount >= 0); + + /* Put the I/O buffer in a known state */ + + iobq->qe_head = NULL; /* Nothing is contained */ + } + + irqrestore(flags); + return iobq; +} + #endif /* CONFIG_IOB_NCHAINS > 0 */ diff --git a/nuttx/net/iob/iob_copyin.c b/nuttx/net/iob/iob_copyin.c index 75d82023c..924c0a645 100644 --- a/nuttx/net/iob/iob_copyin.c +++ b/nuttx/net/iob/iob_copyin.c @@ -68,6 +68,8 @@ * Private Types ****************************************************************************/ +typedef CODE struct iob_s *(*iob_alloc_t)(bool throttled); + /**************************************************************************** * Private Data ****************************************************************************/ @@ -81,7 +83,7 @@ ****************************************************************************/ /**************************************************************************** - * Name: iob_copyin + * Name: iob_copyin_internal * * Description: * Copy data 'len' bytes from a user buffer into the I/O buffer chain, @@ -89,8 +91,9 @@ * ****************************************************************************/ -int iob_copyin(FAR struct iob_s *iob, FAR const uint8_t *src, - unsigned int len, unsigned int offset, bool throttled) +static int iob_copyin_internal(FAR struct iob_s *iob, FAR const uint8_t *src, + unsigned int len, unsigned int offset, + bool throttled, iob_alloc_t allocator) { FAR struct iob_s *head = iob; FAR struct iob_s *next; @@ -206,7 +209,7 @@ int iob_copyin(FAR struct iob_s *iob, FAR const uint8_t *src, { /* Yes.. allocate a new buffer */ - next = iob_alloc(throttled); + next = allocator(throttled); if (next == NULL) { ndbg("ERROR: Failed to allocate I/O buffer\n"); @@ -225,3 +228,39 @@ int iob_copyin(FAR struct iob_s *iob, FAR const uint8_t *src, return OK; } + +/**************************************************************************** + * Public Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: iob_copyin + * + * Description: + * Copy data 'len' bytes from a user buffer into the I/O buffer chain, + * starting at 'offset', extending the chain as necessary. + * + ****************************************************************************/ + +int iob_copyin(FAR struct iob_s *iob, FAR const uint8_t *src, + unsigned int len, unsigned int offset, bool throttled) +{ + return iob_copyin_internal(iob, src, len, offset, throttled, iob_alloc); +} + +/**************************************************************************** + * Name: iob_trycopyin + * + * Description: + * Copy data 'len' bytes from a user buffer into the I/O buffer chain, + * starting at 'offset', extending the chain as necessary BUT without + * waiting if buffers are not available. + * + ****************************************************************************/ + +int iob_trycopyin(FAR struct iob_s *iob, FAR const uint8_t *src, + unsigned int len, unsigned int offset, bool throttled) +{ + return iob_copyin_internal(iob, src, len, offset, throttled, iob_tryalloc); +} + diff --git a/nuttx/net/tcp/tcp_callback.c b/nuttx/net/tcp/tcp_callback.c index ece5d87ab..69d89eb82 100644 --- a/nuttx/net/tcp/tcp_callback.c +++ b/nuttx/net/tcp/tcp_callback.c @@ -50,6 +50,7 @@ #include <nuttx/net/netstats.h> #include "devif/devif.h" +#include "iob/iob.h" #include "tcp/tcp.h" /**************************************************************************** @@ -239,18 +240,21 @@ uint16_t tcp_datahandler(FAR struct tcp_conn_s *conn, FAR uint8_t *buffer, FAR struct iob_s *iob; int ret; - /* Allocate on I/O buffer to start the chain (throttling as necessary) */ + /* Try to allocate on I/O buffer to start the chain without waiting (and + * throttling as necessary). If we would have to wait, then drop the + * packet. + */ - iob = iob_alloc(true); + iob = iob_tryalloc(true); if (iob == NULL) { nlldbg("ERROR: Failed to create new I/O buffer chain\n"); return 0; } - /* Copy the new appdata into the I/O buffer chain */ + /* Copy the new appdata into the I/O buffer chain (without waiting) */ - ret = iob_copyin(iob, buffer, buflen, 0, true); + ret = iob_trycopyin(iob, buffer, buflen, 0, true); if (ret < 0) { /* On a failure, iob_copyin return a negated error value but does @@ -262,9 +266,11 @@ uint16_t tcp_datahandler(FAR struct tcp_conn_s *conn, FAR uint8_t *buffer, return 0; } - /* Add the new I/O buffer chain to the tail of the read-ahead queue */ + /* Add the new I/O buffer chain to the tail of the read-ahead queue (again + * without waiting). + */ - ret = iob_add_queue(iob, &conn->readahead); + ret = iob_tryadd_queue(iob, &conn->readahead); if (ret < 0) { nlldbg("ERROR: Failed to queue the I/O buffer chain: %d\n", ret); |