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

@@ -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
 

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);
+      pcr = ip_frag_alloc_pbuf_custom_ref();
-      if (newpbuf == NULL) {
+      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 = pbuf_alloced_custom(PBUF_RAW, newpbuflen, PBUF_REF, &pcr->pc, p->payload, newpbuflen);
-      newpbuf->len = newpbuf->tot_len = 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.
        */

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 LWIP_SUPPORT_CUSTOM_PBUF
-      if (type == PBUF_POOL) {
+      /* is this a custom pbuf? */
-        memp_free(MEMP_PBUF_POOL, p);
+      if ((p->flags & PBUF_FLAG_IS_CUSTOM) != 0) {
-      /* is this a ROM or RAM referencing pbuf? */
+        struct pbuf_custom *pc = (struct pbuf_custom*)p;
-      } else if (type == PBUF_ROM || type == PBUF_REF) {
+        LWIP_ASSERT("pc->custom_free_function != NULL", pc->custom_free_function != NULL);
-        memp_free(MEMP_PBUF, p);
+        pc->custom_free_function(p);
-      /* type == PBUF_RAM */
+      } else
-      } else {
+#endif /* LWIP_SUPPORT_CUSTOM_PBUF */
-        mem_free(p);
+      {
+        /* 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 */

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 */
 

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")

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
 
 /**

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);