diff --git a/CHANGELOG b/CHANGELOG index d8fc2e89..c71431bd 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -42,6 +42,10 @@ HISTORY ++ Bug fixes: + 2007-03-04 Simon Goldschmidt + * etharp.c, etharp.h, memp.c, memp.h, opt.h: Fix BUG#11400 - don't corrupt + existing pbuf chain when enqueuing multiple pbufs to a pending ARP request + 2007-03-03 Frédéric Bernon * udp.c: remove obsolete line "static struct udp_pcb *pcb_cache = NULL;" Its is static, and never used in udp.c except udp_init(). diff --git a/src/core/memp.c b/src/core/memp.c index 74aac2b8..aad5e232 100644 --- a/src/core/memp.c +++ b/src/core/memp.c @@ -45,6 +45,10 @@ #include "lwip/sys.h" #include "lwip/stats.h" +#if ARP_QUEUEING +#include "netif/etharp.h" +#endif + struct memp { struct memp *next; }; @@ -63,6 +67,9 @@ static const u16_t memp_sizes[MEMP_MAX] = { MEM_ALIGN_SIZE(sizeof(struct netbuf)), MEM_ALIGN_SIZE(sizeof(struct netconn)), MEM_ALIGN_SIZE(sizeof(struct tcpip_msg)), +#if ARP_QUEUEING + MEM_ALIGN_SIZE(sizeof(struct etharp_q_entry)), +#endif MEM_ALIGN_SIZE(sizeof(struct sys_timeo)) }; @@ -76,6 +83,9 @@ static const u16_t memp_num[MEMP_MAX] = { MEMP_NUM_NETBUF, MEMP_NUM_NETCONN, MEMP_NUM_TCPIP_MSG, +#if ARP_QUEUEING + MEMP_NUM_ARP_QUEUE, +#endif MEMP_NUM_SYS_TIMEOUT }; @@ -92,6 +102,9 @@ static u8_t memp_memory[MEM_ALIGNMENT - 1 + MEMP_TYPE_SIZE(MEMP_NUM_NETBUF, struct netbuf) + MEMP_TYPE_SIZE(MEMP_NUM_NETCONN, struct netconn) + MEMP_TYPE_SIZE(MEMP_NUM_TCPIP_MSG, struct tcpip_msg) + +#if ARP_QUEUEING + MEMP_TYPE_SIZE(MEMP_NUM_TCPIP_MSG, struct etharp_q_entry) + +#endif MEMP_TYPE_SIZE(MEMP_NUM_SYS_TIMEOUT, struct sys_timeo)]; #if !SYS_LIGHTWEIGHT_PROT diff --git a/src/include/lwip/memp.h b/src/include/lwip/memp.h index 9f07391a..0ad91940 100644 --- a/src/include/lwip/memp.h +++ b/src/include/lwip/memp.h @@ -46,7 +46,9 @@ typedef enum { MEMP_NETBUF, MEMP_NETCONN, MEMP_TCPIP_MSG, - +#if ARP_QUEUEING + MEMP_ARP_QUEUE, +#endif MEMP_SYS_TIMEOUT, MEMP_MAX diff --git a/src/include/lwip/opt.h b/src/include/lwip/opt.h index c8bdb49f..eb66b2c3 100644 --- a/src/include/lwip/opt.h +++ b/src/include/lwip/opt.h @@ -107,6 +107,12 @@ a lot of data that needs to be copied, this should be set high. */ #ifndef MEMP_NUM_TCP_SEG #define MEMP_NUM_TCP_SEG 16 #endif +/* MEMP_NUM_ARP_QUEUE: the number of simulateously queued outgoing + packets (pbufs) that are waiting for an ARP request (to resolve + their destination address) to finish. */ +#ifndef MEMP_NUM_ARP_QUEUE +#define MEMP_NUM_ARP_QUEUE 30 +#endif /* MEMP_NUM_SYS_TIMEOUT: the number of simulateously active timeouts. */ #ifndef MEMP_NUM_SYS_TIMEOUT diff --git a/src/include/netif/etharp.h b/src/include/netif/etharp.h index d64f55e3..a01cce17 100644 --- a/src/include/netif/etharp.h +++ b/src/include/netif/etharp.h @@ -113,6 +113,15 @@ PACK_STRUCT_END #define ETHTYPE_ARP 0x0806 #define ETHTYPE_IP 0x0800 +#if ARP_QUEUEING +/** struct for queueing outgoing packets for unknown address + * defined here to be accessed by memp.h + */ +struct etharp_q_entry { + struct etharp_q_entry *next; + struct pbuf *p; +}; +#endif void etharp_init(void); void etharp_tmr(void); diff --git a/src/netif/etharp.c b/src/netif/etharp.c index 9787f6d8..8e284d99 100644 --- a/src/netif/etharp.c +++ b/src/netif/etharp.c @@ -93,7 +93,7 @@ struct etharp_entry { /** * Pointer to queue of pending outgoing packets on this ARP entry. */ - struct pbuf *p; + struct etharp_q_entry *q; #endif struct ip_addr ipaddr; struct eth_addr ethaddr; @@ -123,13 +123,29 @@ etharp_init(void) for(i = 0; i < ARP_TABLE_SIZE; ++i) { arp_table[i].state = ETHARP_STATE_EMPTY; #if ARP_QUEUEING - arp_table[i].p = NULL; + arp_table[i].q = NULL; #endif arp_table[i].ctime = 0; arp_table[i].netif = NULL; } } +#if ARP_QUEUEING +/** Free a complete queue of etharp entrys */ +static void free_etharp_q(struct etharp_q_entry *q) +{ + struct etharp_q_entry *r; + LWIP_ASSERT("q != NULL", q != NULL); + LWIP_ASSERT("q->p != NULL", q->p != NULL); + while(q) { + r = q; + q = q->next; + pbuf_free(r->p); + memp_free(MEMP_ARP_QUEUE, r); + } +} +#endif + /** * Clears expired entries in the ARP table. * @@ -158,7 +174,7 @@ etharp_tmr(void) LWIP_DEBUGF(ETHARP_DEBUG, ("etharp_timer: expired pending entry %"U16_F".\n", (u16_t)i)); arp_table[i].state = ETHARP_STATE_EXPIRED; #if ARP_QUEUEING - } else if (arp_table[i].p != NULL) { + } else if (arp_table[i].q != NULL) { /* resend an ARP query here */ #endif } @@ -169,11 +185,11 @@ etharp_tmr(void) snmp_delete_arpidx_tree(arp_table[i].netif, &arp_table[i].ipaddr); #if ARP_QUEUEING /* and empty packet queue */ - if (arp_table[i].p != NULL) { + if (arp_table[i].q != NULL) { /* remove all queued packets */ - LWIP_DEBUGF(ETHARP_DEBUG, ("etharp_timer: freeing entry %"U16_F", packet queue %p.\n", (u16_t)i, (void *)(arp_table[i].p))); - pbuf_free(arp_table[i].p); - arp_table[i].p = NULL; + LWIP_DEBUGF(ETHARP_DEBUG, ("etharp_timer: freeing entry %"U16_F", packet queue %p.\n", (u16_t)i, (void *)(arp_table[i].q))); + free_etharp_q(arp_table[i].q); + arp_table[i].q = NULL; } #endif /* recycle entry for re-use */ @@ -247,7 +263,7 @@ static s8_t find_entry(struct ip_addr *ipaddr, u8_t flags) return i; #if ARP_QUEUEING /* pending with queued packets? */ - } else if (arp_table[i].p != NULL) { + } else if (arp_table[i].q != NULL) { if (arp_table[i].ctime >= age_queue) { old_queue = i; age_queue = arp_table[i].ctime; @@ -304,7 +320,7 @@ static s8_t find_entry(struct ip_addr *ipaddr, u8_t flags) LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("find_entry: selecting oldest stable entry %"U16_F"\n", (u16_t)i)); #if ARP_QUEUEING /* no queued packets should exist on stable entries */ - LWIP_ASSERT("arp_table[i].p == NULL", arp_table[i].p == NULL); + LWIP_ASSERT("arp_table[i].q == NULL", arp_table[i].q == NULL); #endif /* 3) found recyclable pending entry without queued packets? */ } else if (old_pending < ARP_TABLE_SIZE) { @@ -316,9 +332,9 @@ static s8_t find_entry(struct ip_addr *ipaddr, u8_t flags) } else if (old_queue < ARP_TABLE_SIZE) { /* recycle oldest pending */ i = old_queue; - LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("find_entry: selecting oldest pending entry %"U16_F", freeing packet queue %p\n", (u16_t)i, (void *)(arp_table[i].p))); - pbuf_free(arp_table[i].p); - arp_table[i].p = NULL; + LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("find_entry: selecting oldest pending entry %"U16_F", freeing packet queue %p\n", (u16_t)i, (void *)(arp_table[i].q))); + free_etharp_q(arp_table[i].q); + arp_table[i].q = NULL; #endif /* no empty or recyclable entries found */ } else { @@ -405,14 +421,19 @@ update_arp_entry(struct netif *netif, struct ip_addr *ipaddr, struct eth_addr *e arp_table[i].ctime = 0; /* this is where we will send out queued packets! */ #if ARP_QUEUEING - while (arp_table[i].p != NULL) { - /* get the first packet on the queue */ - struct pbuf *p = arp_table[i].p; + while (arp_table[i].q != NULL) { + struct pbuf *p; + struct eth_hdr *ethhdr; + /* remember remainder of queue */ + struct etharp_q_entry *q = arp_table[i].q; + /* pop first item off the queue */ + arp_table[i].q = q->next; + /* get the packet pointer */ + p = q->p; + /* now queue entry can be freed */ + memp_free(MEMP_ARP_QUEUE, q); /* Ethernet header */ - struct eth_hdr *ethhdr = p->payload; - /* remember (and reference) remainder of queue */ - /* note: this will also terminate the p pbuf chain */ - arp_table[i].p = pbuf_dequeue(p); + ethhdr = p->payload; /* fill-in Ethernet header */ k = netif->hwaddr_len; while(k > 0) { @@ -810,19 +831,29 @@ err_t etharp_query(struct netif *netif, struct ip_addr *ipaddr, struct pbuf *q) /* copy any PBUF_REF referenced payloads into PBUF_RAM */ /* (the caller of lwIP assumes the referenced payload can be * freed after it returns from the lwIP call that brought us here) */ + /* @todo: if q->type == PBUF_REF, do we have a memory leak (since we call pbuf_ref(p) later)? */ p = pbuf_take(q); /* packet could be taken over? */ if (p != NULL) { /* queue packet ... */ - if (arp_table[i].p == NULL) { - /* ... in the empty queue */ - pbuf_ref(p); - arp_table[i].p = p; -#if 0 /* multi-packet-queueing disabled, see bug #11400 */ + struct etharp_q_entry *newEntry; + /* allocate a new arp queue entry */ + newEntry = memp_malloc(MEMP_ARP_QUEUE); + LWIP_ASSERT("newEntry != NULL", newEntry != NULL); + newEntry->next = 0; + newEntry->p = p; + pbuf_ref(p); + if(arp_table[i].q != NULL) { + /* queue was already existent, append the new entry to the end */ + struct etharp_q_entry *r; + r = arp_table[i].q; + while(r->next != NULL) { + r = r->next; + } + r->next = newEntry; } else { - /* ... at tail of non-empty queue */ - pbuf_queue(arp_table[i].p, p); -#endif + /* queue did not exist, first item in queue */ + arp_table[i].q = newEntry; } LWIP_DEBUGF(ETHARP_DEBUG | DBG_TRACE, ("etharp_query: queued packet %p on ARP entry %"S16_F"\n", (void *)q, (s16_t)i)); result = ERR_OK;