From f61b80ca6ac40006061b95a84b38ccd8dfa40cb1 Mon Sep 17 00:00:00 2001 From: goldsimon Date: Mon, 21 Jun 2010 18:50:16 +0000 Subject: [PATCH] Fixed bug #29361 (ip_frag has problems with zero-copy DMA MACs) by adding custom pbufs and implementing custom pbufs that reference other (original) pbufs. Additionally set IP_FRAG_USES_STATIC_BUF=0 as default to be on the safe side. --- CHANGELOG | 6 +++ src/core/ipv4/ip_frag.c | 49 ++++++++++++++++-- src/core/pbuf.c | 89 +++++++++++++++++++++++++++++---- src/include/ipv4/lwip/ip_frag.h | 12 +++++ src/include/lwip/memp_std.h | 3 ++ src/include/lwip/opt.h | 19 +++++-- src/include/lwip/pbuf.h | 36 ++++++++++--- 7 files changed, 192 insertions(+), 22 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 349964eb..40662472 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -225,6 +225,12 @@ HISTORY ++ Bugfixes: + 2010-06-21: Simon Goldschmidt + * pbuf.c/.h, ip_frag.c/.h, opt.h, memp_std.h: Fixed bug #29361 (ip_frag has + problems with zero-copy DMA MACs) by adding custom pbufs and implementing + custom pbufs that reference other (original) pbufs. Additionally set + IP_FRAG_USES_STATIC_BUF=0 as default to be on the safe side. + 2010-06-15: Simon Goldschmidt * dhcp.c: Fixed bug #29970: DHCP endian issue parsing option responses diff --git a/src/core/ipv4/ip_frag.c b/src/core/ipv4/ip_frag.c index 1ffdfacd..a8e70662 100644 --- a/src/core/ipv4/ip_frag.c +++ b/src/core/ipv4/ip_frag.c @@ -616,6 +616,38 @@ nullreturn: #if IP_FRAG #if IP_FRAG_USES_STATIC_BUF static u8_t buf[LWIP_MEM_ALIGN_SIZE(IP_FRAG_MAX_MTU + MEM_ALIGNMENT - 1)]; +#else /* IP_FRAG_USES_STATIC_BUF */ + +#if !LWIP_NETIF_TX_SINGLE_PBUF +/** Allocate a new struct pbuf_custom_ref */ +static struct pbuf_custom_ref* +ip_frag_alloc_pbuf_custom_ref() +{ + return (struct pbuf_custom_ref*)memp_malloc(MEMP_FRAG_PBUF); +} + +/** Free a struct pbuf_custom_ref */ +static void +ip_frag_free_pbuf_custom_ref(struct pbuf_custom_ref* p) +{ + LWIP_ASSERT("p != NULL", p != NULL); + memp_free(MEMP_FRAG_PBUF, p); +} + +/** Free-callback function to free a 'struct pbuf_custom_ref', called by + * pbuf_free. */ +static void +ipfrag_free_pbuf_custom(struct pbuf *p) +{ + struct pbuf_custom_ref *pcr = (struct pbuf_custom_ref*)p; + LWIP_ASSERT("pcr != NULL", pcr != NULL); + LWIP_ASSERT("pcr == p", (void*)pcr == (void*)p); + if (pcr->original != NULL) { + pbuf_free(pcr->original); + } + ip_frag_free_pbuf_custom_ref(pcr); +} +#endif /* !LWIP_NETIF_TX_SINGLE_PBUF */ #endif /* IP_FRAG_USES_STATIC_BUF */ /** @@ -739,20 +771,29 @@ ip_frag(struct pbuf *p, struct netif *netif, ip_addr_t *dest) left_to_copy = cop; while (left_to_copy) { + struct pbuf_custom_ref *pcr; newpbuflen = (left_to_copy < p->len) ? left_to_copy : p->len; /* Is this pbuf already empty? */ if (!newpbuflen) { p = p->next; continue; } - newpbuf = pbuf_alloc(PBUF_RAW, 0, PBUF_REF); - if (newpbuf == NULL) { + pcr = ip_frag_alloc_pbuf_custom_ref(); + if (pcr == NULL) { pbuf_free(rambuf); return ERR_MEM; } /* Mirror this pbuf, although we might not need all of it. */ - newpbuf->payload = p->payload; - newpbuf->len = newpbuf->tot_len = newpbuflen; + newpbuf = pbuf_alloced_custom(PBUF_RAW, newpbuflen, PBUF_REF, &pcr->pc, p->payload, newpbuflen); + if (newpbuf == NULL) { + ip_frag_free_pbuf_custom_ref(pcr); + pbuf_free(rambuf); + return ERR_MEM; + } + pbuf_ref(p); + pcr->original = p; + pcr->pc.custom_free_function = ipfrag_free_pbuf_custom; + /* Add it to end of rambuf's chain, but using pbuf_cat, not pbuf_chain * so that it is removed when pbuf_dechain is later called on rambuf. */ diff --git a/src/core/pbuf.c b/src/core/pbuf.c index ffe354d2..319aadba 100644 --- a/src/core/pbuf.c +++ b/src/core/pbuf.c @@ -326,6 +326,67 @@ pbuf_alloc(pbuf_layer layer, u16_t length, pbuf_type type) return p; } +#if LWIP_SUPPORT_CUSTOM_PBUF +/** Initialize a custom pbuf (already allocated). + * + * @param layer flag to define header size + * @param length size of the pbuf's payload + * @param type type of the pbuf (only used to treat the pbuf accordingly, as + * this function allocates no memory) + * @param p pointer to the custom pbuf to initialize (already allocated) + * @param payload_mem pointer to the buffer that is used for payload and headers, + * must be at least big enough to hold 'length' plus the header size, + * may be NULL if set later + * @param payload_mem_len the size of the 'payload_mem' buffer, must be at least + * big enough to hold 'length' plus the header size + */ +struct pbuf* +pbuf_alloced_custom(pbuf_layer l, u16_t length, pbuf_type type, struct pbuf_custom *p, + void *payload_mem, u16_t payload_mem_len) +{ + u16_t offset; + LWIP_DEBUGF(PBUF_DEBUG | LWIP_DBG_TRACE, ("pbuf_alloced_custom(length=%"U16_F")\n", length)); + + /* determine header offset */ + offset = 0; + switch (l) { + case PBUF_TRANSPORT: + /* add room for transport (often TCP) layer header */ + offset += PBUF_TRANSPORT_HLEN; + /* FALLTHROUGH */ + case PBUF_IP: + /* add room for IP layer header */ + offset += PBUF_IP_HLEN; + /* FALLTHROUGH */ + case PBUF_LINK: + /* add room for link layer header */ + offset += PBUF_LINK_HLEN; + break; + case PBUF_RAW: + break; + default: + LWIP_ASSERT("pbuf_alloced_custom: bad pbuf layer", 0); + return NULL; + } + + if (LWIP_MEM_ALIGN_SIZE(offset) + length < payload_mem_len) { + LWIP_DEBUGF(PBUF_DEBUG | LWIP_DBG_LEVEL_WARNING, ("pbuf_alloced_custom(length=%"U16_F") buffer too short\n", length)); + return NULL; + } + + p->pbuf.next = NULL; + if (payload_mem != NULL) { + p->pbuf.payload = LWIP_MEM_ALIGN((void *)((u8_t *)payload_mem + offset)); + } else { + p->pbuf.payload = NULL; + } + p->pbuf.flags = PBUF_FLAG_IS_CUSTOM; + p->pbuf.len = p->pbuf.tot_len = length; + p->pbuf.type = type; + p->pbuf.ref = 1; + return &p->pbuf; +} +#endif /* LWIP_SUPPORT_CUSTOM_PBUF */ /** * Shrink a pbuf chain to a desired length. @@ -573,15 +634,25 @@ pbuf_free(struct pbuf *p) q = p->next; LWIP_DEBUGF( PBUF_DEBUG | LWIP_DBG_TRACE, ("pbuf_free: deallocating %p\n", (void *)p)); type = p->type; - /* is this a pbuf from the pool? */ - if (type == PBUF_POOL) { - memp_free(MEMP_PBUF_POOL, p); - /* is this a ROM or RAM referencing pbuf? */ - } else if (type == PBUF_ROM || type == PBUF_REF) { - memp_free(MEMP_PBUF, p); - /* type == PBUF_RAM */ - } else { - mem_free(p); +#if LWIP_SUPPORT_CUSTOM_PBUF + /* is this a custom pbuf? */ + if ((p->flags & PBUF_FLAG_IS_CUSTOM) != 0) { + struct pbuf_custom *pc = (struct pbuf_custom*)p; + LWIP_ASSERT("pc->custom_free_function != NULL", pc->custom_free_function != NULL); + pc->custom_free_function(p); + } else +#endif /* LWIP_SUPPORT_CUSTOM_PBUF */ + { + /* is this a pbuf from the pool? */ + if (type == PBUF_POOL) { + memp_free(MEMP_PBUF_POOL, p); + /* is this a ROM or RAM referencing pbuf? */ + } else if (type == PBUF_ROM || type == PBUF_REF) { + memp_free(MEMP_PBUF, p); + /* type == PBUF_RAM */ + } else { + mem_free(p); + } } count++; /* proceed to next pbuf */ diff --git a/src/include/ipv4/lwip/ip_frag.h b/src/include/ipv4/lwip/ip_frag.h index 97ab7416..77b5eb1e 100644 --- a/src/include/ipv4/lwip/ip_frag.h +++ b/src/include/ipv4/lwip/ip_frag.h @@ -66,6 +66,18 @@ struct pbuf * ip_reass(struct pbuf *p); #endif /* IP_REASSEMBLY */ #if IP_FRAG +#if !IP_FRAG_USES_STATIC_BUF && !LWIP_NETIF_TX_SINGLE_PBUF +/** A custom pbuf that holds a reference to another pbuf, which is freed + * when this custom pbuf is freed. This is used to create a custom PBUF_REF + * that points into the original pbuf. */ +struct pbuf_custom_ref { + /** 'base class' */ + struct pbuf_custom pc; + /** pointer to the original pbuf that is referenced */ + struct pbuf *original; +}; +#endif /* !IP_FRAG_USES_STATIC_BUF && !LWIP_NETIF_TX_SINGLE_PBUF */ + err_t ip_frag(struct pbuf *p, struct netif *netif, ip_addr_t *dest); #endif /* IP_FRAG */ diff --git a/src/include/lwip/memp_std.h b/src/include/lwip/memp_std.h index c64f7b88..15d27823 100644 --- a/src/include/lwip/memp_std.h +++ b/src/include/lwip/memp_std.h @@ -47,6 +47,9 @@ LWIP_MEMPOOL(TCP_SEG, MEMP_NUM_TCP_SEG, sizeof(struct tcp_seg), #if IP_REASSEMBLY LWIP_MEMPOOL(REASSDATA, MEMP_NUM_REASSDATA, sizeof(struct ip_reassdata), "REASSDATA") #endif /* IP_REASSEMBLY */ +#if IP_FRAG && !IP_FRAG_USES_STATIC_BUF && !LWIP_NETIF_TX_SINGLE_PBUF +LWIP_MEMPOOL(FRAG_PBUF, MEMP_NUM_FRAG_PBUF, sizeof(struct pbuf_custom_ref),"FRAG_PBUF") +#endif /* IP_FRAG && !IP_FRAG_USES_STATIC_BUF && !LWIP_NETIF_TX_SINGLE_PBUF */ #if LWIP_NETCONN LWIP_MEMPOOL(NETBUF, MEMP_NUM_NETBUF, sizeof(struct netbuf), "NETBUF") diff --git a/src/include/lwip/opt.h b/src/include/lwip/opt.h index d4e74d4a..968713e9 100644 --- a/src/include/lwip/opt.h +++ b/src/include/lwip/opt.h @@ -260,13 +260,24 @@ #endif /** - * MEMP_NUM_REASSDATA: the number of simultaneously IP packets queued for + * MEMP_NUM_REASSDATA: the number of IP packets simultaneously queued for * reassembly (whole packets, not fragments!) */ #ifndef MEMP_NUM_REASSDATA #define MEMP_NUM_REASSDATA 5 #endif +/** + * MEMP_NUM_FRAG_PBUF: the number of IP fragments simultaneously sent + * (fragments, not whole packets!). + * This is only used with IP_FRAG_USES_STATIC_BUF==0 and + * LWIP_NETIF_TX_SINGLE_PBUF==0 and only has to be > 1 with DMA-enabled MACs + * where the packet is not yet sent when netif->output returns. + */ +#ifndef MEMP_NUM_FRAG_PBUF +#define MEMP_NUM_FRAG_PBUF 15 +#endif + /** * MEMP_NUM_ARP_QUEUE: the number of simulateously queued outgoing * packets (pbufs) that are waiting for an ARP request (to resolve @@ -531,10 +542,12 @@ /** * IP_FRAG_USES_STATIC_BUF==1: Use a static MTU-sized buffer for IP * fragmentation. Otherwise pbufs are allocated and reference the original - * packet data to be fragmented. + * packet data to be fragmented (or with LWIP_NETIF_TX_SINGLE_PBUF==1, + * new PBUF_RAM pbufs are used for fragments). + * ATTENTION: IP_FRAG_USES_STATIC_BUF==1 may not be used for DMA-enabled MACs! */ #ifndef IP_FRAG_USES_STATIC_BUF -#define IP_FRAG_USES_STATIC_BUF 1 +#define IP_FRAG_USES_STATIC_BUF 0 #endif /** diff --git a/src/include/lwip/pbuf.h b/src/include/lwip/pbuf.h index 9ab75afd..b9227c58 100644 --- a/src/include/lwip/pbuf.h +++ b/src/include/lwip/pbuf.h @@ -40,6 +40,10 @@ extern "C" { #endif +/** Currently, the pbuf_custom code is only needed for one specific configuration + * of IP_FRAG */ +#define LWIP_SUPPORT_CUSTOM_PBUF (IP_FRAG && !IP_FRAG_USES_STATIC_BUF && !LWIP_NETIF_TX_SINGLE_PBUF) + #define PBUF_TRANSPORT_HLEN 20 #define PBUF_IP_HLEN 20 @@ -59,7 +63,10 @@ typedef enum { /** indicates this packet's data should be immediately passed to the application */ -#define PBUF_FLAG_PUSH 0x01U +#define PBUF_FLAG_PUSH 0x01U +/** indicates this is a custom pbuf: pbuf_free and pbuf_header handle such a + a pbuf differently */ +#define PBUF_FLAG_IS_CUSTOM 0x02U struct pbuf { /** next pbuf in singly linked pbuf chain */ @@ -67,7 +74,7 @@ struct pbuf { /** pointer to the actual data in the buffer */ void *payload; - + /** * total length of this buffer and all next buffers in chain * belonging to the same packet. @@ -76,9 +83,9 @@ struct pbuf { * p->tot_len == p->len + (p->next? p->next->tot_len: 0) */ u16_t tot_len; - + /** length of this buffer */ - u16_t len; + u16_t len; /** pbuf_type as u8_t instead of enum to save space */ u8_t /*pbuf_type*/ type; @@ -92,13 +99,30 @@ struct pbuf { * the stack itself, or pbuf->next pointers from a chain. */ u16_t ref; - }; +#if LWIP_SUPPORT_CUSTOM_PBUF +/** Prototype for a function to free a custom pbuf */ +typedef void (*pbuf_free_custom_fn)(struct pbuf *p); + +/** A custom pbuf: like a pbuf, but following a function pointer to free it. */ +struct pbuf_custom { + /** The actual pbuf */ + struct pbuf pbuf; + /** This function is called when pbuf_free deallocates this pbuf(_custom) */ + pbuf_free_custom_fn custom_free_function; +}; +#endif /* LWIP_SUPPORT_CUSTOM_PBUF */ + /* Initializes the pbuf module. This call is empty for now, but may not be in future. */ #define pbuf_init() -struct pbuf *pbuf_alloc(pbuf_layer l, u16_t size, pbuf_type type); +struct pbuf *pbuf_alloc(pbuf_layer l, u16_t length, pbuf_type type); +#if LWIP_SUPPORT_CUSTOM_PBUF +struct pbuf *pbuf_alloced_custom(pbuf_layer l, u16_t length, pbuf_type type, + struct pbuf_custom *p, void *payload_mem, + u16_t payload_mem_len); +#endif /* LWIP_SUPPORT_CUSTOM_PBUF */ void pbuf_realloc(struct pbuf *p, u16_t size); u8_t pbuf_header(struct pbuf *p, s16_t header_size); void pbuf_ref(struct pbuf *p);