diff --git a/CHANGELOG b/CHANGELOG index 5e836ec1..c6044eaa 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -117,6 +117,15 @@ HISTORY ++ Bug fixes: + 2007-04-23 Simon Goldschmidt + * loopif.c, loopif.h, opt.h, src/netif/FILES: fix bug #2595: "loopif results + in NULL reference for incoming TCP packets". Loopif has to be configured + (using LWIP_LOOPIF_MULTITHREADING) to directly call netif->input() + (multithreading environments, e.g. netif->input() = tcpip_input()) or + putting packets on a list that is fed to the stack by calling loopif_poll() + (single-thread / NO_SYS / polling environment where e.g. + netif->input() = ip_input). + 2007-04-17 Jonathan Larmour * pbuf.c: Use s32_t in pbuf_realloc(), as an s16_t can't reliably hold the difference between two u16_t's. diff --git a/src/include/lwip/opt.h b/src/include/lwip/opt.h index 39085184..b9c7cee8 100644 --- a/src/include/lwip/opt.h +++ b/src/include/lwip/opt.h @@ -408,6 +408,17 @@ a lot of data that needs to be copied, this should be set high. */ #define LWIP_HAVE_LOOPIF 0 #endif +/* This switches between directly calling netif->input() (=1 for multithreading + * environments like tcpip.c) or putting the packets on a list and calling + * loopif_poll() in the main application loop (=0 for polling / NO_SYS + * environments). + * Setting this is needed to avoid reentering non-reentrant functions like + * tcp_input(). + */ +#ifndef LWIP_LOOPIF_MULTITHREADING +#define LWIP_LOOPIF_MULTITHREADING 1 +#endif + #ifndef LWIP_EVENT_API #define LWIP_EVENT_API 0 #define LWIP_CALLBACK_API 1 diff --git a/src/include/netif/loopif.h b/src/include/netif/loopif.h index 97b3c676..7d856c9e 100644 --- a/src/include/netif/loopif.h +++ b/src/include/netif/loopif.h @@ -34,6 +34,10 @@ #include "lwip/netif.h" +#if !LWIP_LOOPIF_MULTITHREADING +void loopif_poll(struct netif *netif); +#endif + err_t loopif_init(struct netif *netif); #endif /* __NETIF_LOOPIF_H__ */ diff --git a/src/netif/FILES b/src/netif/FILES index 825d4071..c7da7971 100644 --- a/src/netif/FILES +++ b/src/netif/FILES @@ -15,10 +15,8 @@ ethernetif.c network device drivers. It uses the etharp.c ARP code. loopif.c - An example network interface that shows how a "loopback" - interface would work. This is not really intended for actual - use, but as a very basic example of how initialization and - output functions work. + A "loopback" network interface driver. It requires configuration + through the define LWIP_LOOPIF_MULTITHREADING (see opt.h). slipif.c A generic implementation of the SLIP (Serial Line IP) diff --git a/src/netif/loopif.c b/src/netif/loopif.c index 0464cb8e..092655f0 100644 --- a/src/netif/loopif.c +++ b/src/netif/loopif.c @@ -36,75 +36,121 @@ #include "netif/loopif.h" #include "lwip/mem.h" -#if defined(LWIP_DEBUG) && defined(LWIP_TCPDUMP) -#include "netif/tcpdump.h" -#endif /* LWIP_DEBUG && LWIP_TCPDUMP */ +#if !LWIP_LOOPIF_MULTITHREADING -#include "lwip/tcp.h" -#include "lwip/ip.h" +#include "lwip/sys.h" -static void -loopif_input( void * arg ) +/* helper struct for the linked list of pbufs */ +struct loopif_private { + struct pbuf *first; + struct pbuf *last; +}; + +/* Call loopif_poll() in the main loop of your application. This is to prevent + * reentering non-reentrant functions like tcp_input(). Packets passed to + * loopif_output() are put on a list that is passed to netif->input() by + * loopif_poll(). + */ +void +loopif_poll(struct netif *netif) { - struct netif *netif = (struct netif *)( ((void **)arg)[ 0 ] ); - struct pbuf *r = (struct pbuf *)( ((void **)arg)[ 1 ] ); + SYS_ARCH_DECL_PROTECT(lev); + struct pbuf *in = NULL; + struct loopif_private *priv = (struct loopif_private*)netif->state; - mem_free( arg ); - netif -> input( r, netif ); + do { + /* Get a packet from the list. With SYS_LIGHTWEIGHT_PROT=1, this is protected */ + SYS_ARCH_PROTECT(lev); + in = priv->first; + if(priv->first) { + if(priv->first == priv->last) { + /* this was the last pbuf in the list */ + priv->first = priv->last = NULL; + } else { + /* pop the pbuf off the list */ + priv->first = priv->first->next; + LWIP_ASSERT("should not be null since first != last!", priv->first != NULL); + } + } + SYS_ARCH_UNPROTECT(lev); + + if(in != NULL) { + if(in->next != NULL) { + in->next = NULL; + LWIP_ASSERT("packet must not consist of multiple pbufs!", in->len == in->tot_len); + } + netif->input(in, netif); + } + /* go on while there is a packet on the list */ + } while(in != NULL); } +#endif /* LWIP_LOOPIF_MULTITHREADING */ static err_t loopif_output(struct netif *netif, struct pbuf *p, struct ip_addr *ipaddr) { +#if !LWIP_LOOPIF_MULTITHREADING + SYS_ARCH_DECL_PROTECT(lev); + struct loopif_private *priv; +#endif /* LWIP_LOOPIF_MULTITHREADING */ struct pbuf *q, *r; u8_t *ptr; - void **arg; -#if defined(LWIP_DEBUG) && defined(LWIP_TCPDUMP) - tcpdump(p); -#endif /* LWIP_DEBUG && LWIP_TCPDUMP */ - + LWIP_UNUSED_ARG(ipaddr); + + /* Allocate a new pbuf */ r = pbuf_alloc(PBUF_RAW, p->tot_len, PBUF_RAM); - if (r != NULL) { - ptr = r->payload; - - for(q = p; q != NULL; q = q->next) { - memcpy(ptr, q->payload, q->len); - ptr += q->len; - } - - arg = mem_malloc( sizeof( void *[2])); - if( NULL == arg ) { - return ERR_MEM; - } - - arg[0] = netif; - arg[1] = r; - /** - * workaround (patch #1779) to try to prevent bug #2595: - * When connecting to "localhost" with the loopif interface, - * tcp_output doesn't get the opportunity to finnish sending the - * segment before tcp_process gets it, resulting in tcp_process - * referencing pcb->unacked-> which still is NULL. - * - * TODO: Is there still a race condition here? Leon - */ - sys_timeout( 1, loopif_input, arg ); - - return ERR_OK; + if (r == NULL) { + return ERR_MEM; } - return ERR_MEM; + + /* Copy the whole pbuf queue p into the single pbuf r */ + ptr = r->payload; + for(q = p; q != NULL; q = q->next) { + memcpy(ptr, q->payload, q->len); + ptr += q->len; + } + +#if LWIP_LOOPIF_MULTITHREADING + /* Multithreading environment, netif->input() is supposed to put the packet + into a mailbox, so we can safely call it here without risking to re-enter + functions that are not reentrant (TCP!!!) */ + netif->input(r, netif); +#else /* LWIP_LOOPIF_MULTITHREADING */ + /* Raw API without threads: put the packet on a linked list which gets emptied + through calling loopif_poll(). */ + priv = (struct loopif_private*)netif->state; + + SYS_ARCH_PROTECT(lev); + if(priv->first != NULL) { + LWIP_ASSERT("if first!=NULL, last must also be != NULL", priv->last != NULL); + priv->last->next = r; + priv->last = r; + } else { + priv->first = priv->last = r; + } + SYS_ARCH_UNPROTECT(lev); +#endif /* LWIP_LOOPIF_MULTITHREADING */ + + return ERR_OK; } err_t loopif_init(struct netif *netif) { +#if !LWIP_LOOPIF_MULTITHREADING + struct loopif_private *priv; + + priv = (struct loopif_private*)mem_malloc(sizeof(struct loopif_private)); + if(priv == NULL) + return ERR_MEM; + priv->first = priv->last = NULL; + netif->state = priv; +#endif /* LWIP_LOOPIF_MULTITHREADING */ + netif->name[0] = 'l'; netif->name[1] = 'o'; -#if 0 /** TODO: I think this should be enabled, or not? Leon */ - netif->input = loopif_input; -#endif netif->output = loopif_output; return ERR_OK; }