Work on bug #3031/task #7896: change pbuf->type/enum pbuf_type to be more generic (see UPGRADING)

UPGRADING

@@ -9,7 +9,12 @@ with newer versions.
   * [Enter new changes just after this line - do not remove this line]
 
   ++ Application changes:
-  
+
+  * pbuf API: pbuf->type (an u8_t holding the enum 'pbuf_type') has changed to only hold a
+    description of the pbuf (e.g. data following pbuf struct, data volatile, allocation
+    source heap/pool/etc.). As a consequence, applications can't test pbuf->type any more.
+    Use pbuf_match_type(pbuf, type) instead.
+
   * slipif: The way to pass serial port number has changed. netif->num is not
     supported any more, netif->state is interpreted as an u8_t port number now
     (it's not a POINTER to an u8_t any more!)

src/core/pbuf.c

@@ -180,7 +180,7 @@ pbuf_init_alloced_pbuf(struct pbuf *p, void* payload, u16_t tot_len, u16_t len,
   p->payload = payload;
   p->tot_len = tot_len;
   p->len = len;
-  p->type = type;
+  p->type_internal = (u8_t)type;
   p->flags = 0;
   p->ref = 1;
   p->if_idx = NETIF_NO_INDEX;
@@ -428,7 +428,7 @@ pbuf_alloced_custom(pbuf_layer l, u16_t length, pbuf_type type, struct pbuf_cust
   }
   p->pbuf.flags = PBUF_FLAG_IS_CUSTOM;
   p->pbuf.len = p->pbuf.tot_len = length;
-  p->pbuf.type = type;
+  p->pbuf.type_internal = (u8_t)type;
   p->pbuf.ref = 1;
   return &p->pbuf;
 }
@@ -458,10 +458,6 @@ pbuf_realloc(struct pbuf *p, u16_t new_len)
   s32_t grow;
 
   LWIP_ASSERT("pbuf_realloc: p != NULL", p != NULL);
-  LWIP_ASSERT("pbuf_realloc: sane p->type", p->type == PBUF_POOL ||
-              p->type == PBUF_ROM ||
-              p->type == PBUF_RAM ||
-              p->type == PBUF_REF);
 
   /* desired length larger than current length? */
   if (new_len >= p->tot_len) {
@@ -492,7 +488,7 @@ pbuf_realloc(struct pbuf *p, u16_t new_len)
 
   /* shrink allocated memory for PBUF_RAM */
   /* (other types merely adjust their length fields */
-  if ((q->type == PBUF_RAM) && (rem_len != q->len)
+  if (pbuf_match_allocsrc(q, PBUF_TYPE_ALLOC_SRC_MASK_STD_HEAP) && (rem_len != q->len)
 #if LWIP_SUPPORT_CUSTOM_PBUF
       && ((q->flags & PBUF_FLAG_IS_CUSTOM) == 0)
 #endif /* LWIP_SUPPORT_CUSTOM_PBUF */
@@ -529,7 +525,7 @@ pbuf_realloc(struct pbuf *p, u16_t new_len)
 static u8_t
 pbuf_header_impl(struct pbuf *p, s16_t header_size_increment, u8_t force)
 {
-  u16_t type;
+  u16_t type_internal;
   void *payload;
   u16_t increment_magnitude;
 
@@ -560,12 +556,12 @@ pbuf_header_impl(struct pbuf *p, s16_t header_size_increment, u8_t force)
 #endif
   }
 
-  type = p->type;
+  type_internal = p->type_internal;
   /* remember current payload pointer */
   payload = p->payload;
 
   /* pbuf types containing payloads? */
-  if (type == PBUF_RAM || type == PBUF_POOL) {
+  if (type_internal & PBUF_TYPE_FLAG_STRUCT_DATA_CONTIGUOUS) {
     /* set new payload pointer */
     p->payload = (u8_t *)p->payload - header_size_increment;
     /* boundary check fails? */
@@ -579,7 +575,7 @@ pbuf_header_impl(struct pbuf *p, s16_t header_size_increment, u8_t force)
       return 1;
     }
   /* pbuf types referring to external payloads? */
-  } else if (type == PBUF_REF || type == PBUF_ROM) {
+  } else {
     /* hide a header in the payload? */
     if ((header_size_increment < 0) && (increment_magnitude <= p->len)) {
       /* increase payload pointer */
@@ -591,10 +587,6 @@ pbuf_header_impl(struct pbuf *p, s16_t header_size_increment, u8_t force)
        * bail out unsuccessfully */
       return 1;
     }
-  } else {
-    /* Unknown type */
-    LWIP_ASSERT("bad pbuf type", 0);
-    return 1;
   }
   /* modify pbuf length fields */
   p->len += header_size_increment;
@@ -709,7 +701,7 @@ pbuf_free_header(struct pbuf *q, u16_t size)
 u8_t
 pbuf_free(struct pbuf *p)
 {
-  u16_t type;
+  u8_t alloc_src;
   struct pbuf *q;
   u8_t count;
 
@@ -724,10 +716,6 @@ pbuf_free(struct pbuf *p)
 
   PERF_START;
 
-  LWIP_ASSERT("pbuf_free: sane type",
-    p->type == PBUF_RAM || p->type == PBUF_ROM ||
-    p->type == PBUF_REF || p->type == PBUF_POOL);
-
   count = 0;
   /* de-allocate all consecutive pbufs from the head of the chain that
    * obtain a zero reference count after decrementing*/
@@ -748,7 +736,7 @@ pbuf_free(struct pbuf *p)
       /* remember next pbuf in chain for next iteration */
       q = p->next;
       LWIP_DEBUGF( PBUF_DEBUG | LWIP_DBG_TRACE, ("pbuf_free: deallocating %p\n", (void *)p));
-      type = p->type;
+      alloc_src = pbuf_get_allocsrc(p);
 #if LWIP_SUPPORT_CUSTOM_PBUF
       /* is this a custom pbuf? */
       if ((p->flags & PBUF_FLAG_IS_CUSTOM) != 0) {
@@ -759,14 +747,17 @@ pbuf_free(struct pbuf *p)
 #endif /* LWIP_SUPPORT_CUSTOM_PBUF */
       {
         /* is this a pbuf from the pool? */
-        if (type == PBUF_POOL) {
+        if (alloc_src == PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF_POOL) {
           memp_free(MEMP_PBUF_POOL, p);
         /* is this a ROM or RAM referencing pbuf? */
-        } else if (type == PBUF_ROM || type == PBUF_REF) {
+        } else if (alloc_src == PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF) {
           memp_free(MEMP_PBUF, p);
         /* type == PBUF_RAM */
-        } else {
+        } else if (alloc_src == PBUF_TYPE_ALLOC_SRC_MASK_STD_HEAP) {
           mem_free(p);
+        } else {
+          /* @todo: support freeing other types */
+          LWIP_ASSERT("invalid pbuf type", 0);
         }
       }
       count++;

src/core/tcp_out.c

@@ -538,7 +538,7 @@ tcp_write(struct tcp_pcb *pcb, const void *arg, u16_t len, u8_t apiflags)
         /* If the last unsent pbuf is of type PBUF_ROM, try to extend it. */
         struct pbuf *p;
         for (p = last_unsent->p; p->next != NULL; p = p->next);
-        if (p->type == PBUF_ROM &&
+        if (((p->type_internal & (PBUF_TYPE_FLAG_STRUCT_DATA_CONTIGUOUS|PBUF_TYPE_FLAG_DATA_VOLATILE)) == 0) &&
             (const u8_t *)p->payload + p->len == (const u8_t *)arg) {
           LWIP_ASSERT("tcp_write: ROM pbufs cannot be oversized", pos == 0);
           extendlen = seglen;

src/include/lwip/pbuf.h

@@ -69,7 +69,7 @@ extern "C" {
  * for other purposes. For more background information on this, see tasks #6735
  * and #7896, and bugs #11400 and #49914. */
 #ifndef PBUF_NEEDS_COPY
-#define PBUF_NEEDS_COPY(p)  ((p)->type != PBUF_ROM)
+#define PBUF_NEEDS_COPY(p)  ((p)->type_internal & PBUF_TYPE_FLAG_DATA_VOLATILE)
 #endif /* PBUF_NEEDS_COPY */
 
 /* @todo: We need a mechanism to prevent wasting memory in every pbuf
@@ -111,6 +111,33 @@ typedef enum {
   PBUF_RAW
 } pbuf_layer;
 
+
+/* Base flags for pbuf_type definitions: */
+
+/** Indicates that the payload directly follows the struct pbuf.
+ *  This makes @ref pbuf_header work in both directions. */
+#define PBUF_TYPE_FLAG_STRUCT_DATA_CONTIGUOUS       0x80
+/** Indicates the data stored in this pbuf can change. If this pbuf needs
+ * to be queued, it must be copied/duplicated. */
+#define PBUF_TYPE_FLAG_DATA_VOLATILE                0x40
+/** 4 bits are reserved for 16 allocation sources (e.g. heap, pool1, pool2, etc)
+ * Internally, we use: 0=heap, 1=MEMP_PBUF, 2=MEMP_PBUF_POOL -> 13 types free*/
+#define PBUF_TYPE_ALLOC_SRC_MASK                    0x0F
+/** Indicates this pbuf is used for RX (if not set, indicates use for TX).
+ * This information can be used to keep some spare RX buffers e.g. for
+ * receiving TCP ACKs to unblock a connection) */
+#define PBUF_ALLOC_FLAG_RX                          0x0100
+/** Indicates the application needs the pbuf payload to be in one piece */
+#define PBUF_ALLOC_FLAG_DATA_CONTIGUOUS             0x0200
+
+#define PBUF_TYPE_ALLOC_SRC_MASK_STD_HEAP           0x00
+#define PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF      0x01
+#define PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF_POOL 0x02
+/** First pbuf allocation type for applications */
+#define PBUF_TYPE_ALLOC_SRC_MASK_APP_MIN            0x03
+/** Last pbuf allocation type for applications */
+#define PBUF_TYPE_ALLOC_SRC_MASK_APP_MAX            PBUF_TYPE_ALLOC_SRC_MASK
+
 /**
  * @ingroup pbuf
  * Enumeration of pbuf types
@@ -122,22 +149,22 @@ typedef enum {
       pbuf_alloc() allocates PBUF_RAM pbufs as unchained pbufs (although that might
       change in future versions).
       This should be used for all OUTGOING packets (TX).*/
-  PBUF_RAM,
+  PBUF_RAM = (PBUF_ALLOC_FLAG_DATA_CONTIGUOUS | PBUF_TYPE_FLAG_STRUCT_DATA_CONTIGUOUS | PBUF_TYPE_ALLOC_SRC_MASK_STD_HEAP),
   /** pbuf data is stored in ROM, i.e. struct pbuf and its payload are located in
       totally different memory areas. Since it points to ROM, payload does not
       have to be copied when queued for transmission. */
-  PBUF_ROM,
+  PBUF_ROM = PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF,
   /** pbuf comes from the pbuf pool. Much like PBUF_ROM but payload might change
       so it has to be duplicated when queued before transmitting, depending on
       who has a 'ref' to it. */
-  PBUF_REF,
+  PBUF_REF = (PBUF_TYPE_FLAG_DATA_VOLATILE | PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF),
   /** pbuf payload refers to RAM. This one comes from a pool and should be used
       for RX. Payload can be chained (scatter-gather RX) but like PBUF_RAM, struct
       pbuf and its payload are allocated in one piece of contiguous memory (so
       the first payload byte can be calculated from struct pbuf).
       Don't use this for TX, if the pool becomes empty e.g. because of TCP queuing,
       you are unable to receive TCP acks! */
-  PBUF_POOL
+  PBUF_POOL = (PBUF_ALLOC_FLAG_RX | PBUF_TYPE_FLAG_STRUCT_DATA_CONTIGUOUS | PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF_POOL)
 } pbuf_type;
 
 
@@ -175,8 +202,10 @@ struct pbuf {
   /** length of this buffer */
   u16_t len;
 
-  /** pbuf_type as u8_t instead of enum to save space */
-  u8_t /*pbuf_type*/ type;
+  /** a bit field indicating pbuf type and allocation sources
+      (see PBUF_TYPE_FLAG_*, PBUF_ALLOC_FLAG_* and PBUF_TYPE_ALLOC_SRC_MASK)
+    */
+  u8_t type_internal;
 
   /** misc flags */
   u8_t flags;
@@ -248,6 +277,9 @@ struct pbuf *pbuf_alloced_custom(pbuf_layer l, u16_t length, pbuf_type type,
                                  u16_t payload_mem_len);
 #endif /* LWIP_SUPPORT_CUSTOM_PBUF */
 void pbuf_realloc(struct pbuf *p, u16_t size);
+#define pbuf_get_allocsrc(p)          ((p)->type_internal & PBUF_TYPE_ALLOC_SRC_MASK)
+#define pbuf_match_allocsrc(p, type)  (pbuf_get_allocsrc(p) == ((type) & PBUF_TYPE_ALLOC_SRC_MASK))
+#define pbuf_match_type(p, type)      pbuf_match_allocsrc(p, type)
 u8_t pbuf_header(struct pbuf *p, s16_t header_size);
 u8_t pbuf_header_force(struct pbuf *p, s16_t header_size);
 struct pbuf *pbuf_free_header(struct pbuf *q, u16_t size);