EMAC memory manager improvements & fixes

Author: multiplemonomials

connectivity/drivers/wifi/TARGET_STM/COMPONENT_EMW3080B/mx_wifi_mbed_os.cpp

@@ -60,7 +60,7 @@ void mx_buf_set_size(mx_buf_t *p, int32_t n)
 
     mx_buf_t   *mx_buf_alloc(uint32_t len)
     {
-        emac_mem_buf_t *p = emac3080b_global_memory_manager->alloc_pool(len, 0);
+        emac_mem_buf_t *p = emac3080b_global_memory_manager->alloc_heap(len, 0);
         return (mx_buf_t *) p;
     }
 

connectivity/libraries/nanostack-libservice/CMakeLists.txt

@@ -22,6 +22,10 @@ target_sources(mbed-nanostack-libservice
         source/nvmHelper/ns_nvm_helper.c
 )
 
+target_link_libraries(mbed-nanostack-libservice
+    PUBLIC
+        mbed-core-flags)
+
 # The definition, source files and include directories below
 # are needed by mbed-trace which is part of the mbed-core CMake target
 target_compile_definitions(mbed-core-flags

connectivity/libraries/nanostack-libservice/source/nsdynmemLIB/nsdynmemLIB.c

@@ -21,6 +21,7 @@
 #include "platform/arm_hal_interrupt.h"
 #include <stdlib.h>
 #include "ns_list.h"
+#include "mbed_toolchain.h"
 
 #ifndef STANDARD_MALLOC
 typedef enum mem_stat_update_t {
@@ -598,6 +599,10 @@ static bool pointer_address_validate(ns_mem_book_t *book, ns_mem_word_size_t *pt
     return false;
 }
 
+// Hook from nanostack to the Mbed EMAC memory manager when memory becomes free.
+MBED_WEAK void mbed_ns_heap_free_hook(void)
+{}
+
 void ns_mem_free(ns_mem_book_t *book, void *block)
 {
 #ifndef STANDARD_MALLOC
@@ -633,6 +638,8 @@ void ns_mem_free(ns_mem_book_t *book, void *block)
     free(block);
     platform_exit_critical();
 #endif
+
+    mbed_ns_heap_free_hook();
 }
 
 void ns_dyn_mem_free(void *block)

connectivity/lwipstack/include/lwipstack/LWIPMemoryManager.h

@@ -19,151 +19,47 @@
 
 #include "EMACMemoryManager.h"
 
+// Hook called from LwIP whenever a pbuf chain containing at least one pool pbuf has been freed
+extern "C" void mbed_lwip_on_pbuf_pool_free_hook();
 
 class LWIPMemoryManager final : public EMACMemoryManager {
 public:
 
-    /**
-     * Allocates memory buffer from the heap
-     *
-     * Memory buffer allocated from heap is always contiguous and can be arbitrary size.
-     *
-     * @param size     Size of the memory to allocate in bytes
-     * @param align    Memory alignment requirement in bytes
-     * @return         Allocated memory buffer, or NULL in case of error
-     */
     net_stack_mem_buf_t *alloc_heap(uint32_t size, uint32_t align) override;
 
-    /**
-     * Allocates memory buffer chain from a pool
-     *
-     * Memory allocated from pool is contiguous if size is equal or less than
-     * (aligned) allocation unit, otherwise may be chained. Will typically come from
-     * fixed-size packet pool memory.
-     *
-     * @param  size    Total size of the memory to allocate in bytes
-     * @param  align   Memory alignment requirement for each buffer in bytes
-     * @return         Allocated memory buffer chain, or NULL in case of error
-     */
     net_stack_mem_buf_t *alloc_pool(uint32_t size, uint32_t align) override;
 
-    /**
-     * Get memory buffer pool allocation unit
-     *
-     * Returns the maximum size of contiguous memory that can be allocated from a pool.
-     *
-     * @param align    Memory alignment requirement in bytes
-     * @return         Contiguous memory size
-     */
     uint32_t get_pool_alloc_unit(uint32_t align) const override;
 
-    /**
-     * Free memory buffer chain
-     *
-     * If memory buffer is chained must point to the start of the chain. Frees all buffers
-     * from the chained list.
-     *
-     * @param buf      Memory buffer chain to be freed.
-     */
+    uint32_t get_pool_size() const override;
+
     void free(net_stack_mem_buf_t *buf) override;
 
-    /**
-     * Return total length of a memory buffer chain
-     *
-     * Returns a total length of this buffer and any following buffers in the chain.
-     *
-     * @param buf      Memory buffer chain
-     * @return         Total length in bytes
-     */
     uint32_t get_total_len(const net_stack_mem_buf_t *buf) const override;
 
-    /**
-     * Copy a memory buffer chain
-     *
-     * Copies data from one buffer chain to another. Copy operation does not adjust the lengths
-     * of the copied-to memory buffer chain, so chain total lengths must be the same.
-     *
-     * @param to_buf    Memory buffer chain to copy to
-     * @param from_buf  Memory buffer chain to copy from
-     */
     void copy(net_stack_mem_buf_t *to_buf, const net_stack_mem_buf_t *from_buf) override;
 
-    /**
-     * Copy to a memory buffer chain
-     *
-     * Copies data to a buffer chain. Copy operation does not adjust the lengths
-     * of the copied-to memory buffer chain, so chain total length must match the
-     * copied length.
-     *
-     * @param to_buf    Memory buffer chain to copy to
-     * @param ptr       Pointer to data
-     * @param len       Data length
-     */
     void copy_to_buf(net_stack_mem_buf_t *to_buf, const void *ptr, uint32_t len) override;
 
-    /**
-     * Copy from a memory buffer chain
-     *
-     * Copies data from a memory buffer chain.
-     *
-     * @param len       Data length
-     * @param ptr       Pointer to data
-     * @param from_buf  Memory buffer chain to copy from
-     * @return          Length of the data that was copied
-     */
     uint32_t copy_from_buf(void *ptr, uint32_t len, const net_stack_mem_buf_t *from_buf) const override;
 
-    /**
-     * Concatenate two memory buffer chains
-     *
-     * Concatenates buffer chain to end of the other buffer chain. Concatenated-to buffer total length
-     * is adjusted accordingly. cat_buf must point to the start of a the chain. After concatenation
-     * to_buf's chain now owns those buffers, and they will be freed when the to_buf chain is freed.
-     *
-     * @param to_buf   Memory buffer chain to concatenate to
-     * @param cat_buf  Memory buffer chain to concatenate
-     */
     void cat(net_stack_mem_buf_t *to_buf, net_stack_mem_buf_t *cat_buf) override;
 
-    /**
-     * Returns the next buffer
-     *
-     * Returns the next buffer from the memory buffer chain.
-     *
-     * @param buf      Memory buffer
-     * @return         The next memory buffer, or NULL if last
-     */
     net_stack_mem_buf_t *get_next(const net_stack_mem_buf_t *buf) const override;
 
-    /**
-     * Return pointer to the payload of the buffer
-     *
-     * @param buf      Memory buffer
-     * @return         Pointer to the payload
-     */
     void *get_ptr(const net_stack_mem_buf_t *buf) const override;
 
-    /**
-     * Return payload size of the buffer
-     *
-     * @param buf      Memory buffer
-     * @return         Size in bytes
-     */
     uint32_t get_len(const net_stack_mem_buf_t *buf) const override;
 
-    /**
-     * Sets the payload size of the buffer
-     *
-     * The allocated payload size will not change. It is not permitted
-     * to change the length of a buffer that is not the first (or only) in a chain.
-     *
-     * @param buf      Memory buffer
-     * @param len      Payload size, must be less or equal allocated size
-     */
     void set_len(net_stack_mem_buf_t *buf, uint32_t len) override;
 
+    Lifetime get_lifetime(const net_stack_mem_buf_t *buf) const override;
+
 private:
 
+    // Allow callback to access private vars
+    friend void mbed_lwip_on_pbuf_pool_free_hook();
+
     /**
      * Returns a total memory alignment size
      *
@@ -192,7 +88,7 @@ class LWIPMemoryManager final : public EMACMemoryManager {
      * Sets total lengths of a memory buffer chain
      *
      * Sets total length fields for a memory buffer chain based on buffer
-     * length fields. All total lengths are calculated again.
+     * \c len fields. All total lengths are calculated again.
      *
      * @param pbuf     Memory buffer
      */

connectivity/lwipstack/include/lwipstack/LWIPStack.h

@@ -344,6 +344,12 @@ class LWIP : public OnboardNetworkStack, private mbed::NonCopyable<LWIP> {
       */
     void set_default_interface(OnboardNetworkStack::Interface *interface) override;
 
+    /// Get the memory manager for the LwIP stack
+    LWIPMemoryManager &get_memory_manager()
+    {
+        return memory_manager;
+    }
+
 protected:
     LWIP();
 

connectivity/lwipstack/include/lwipstack/lwipopts.h

@@ -136,11 +136,11 @@
 
 // Number of pool pbufs.
 // Each requires 684 bytes of RAM (if MSS=536 and PBUF_POOL_BUFSIZE defaulting to be based on MSS)
-#define PBUF_POOL_SIZE              MBED_CONF_LWIP_PBUF_POOL_SIZE
+#define PBUF_POOL_SIZE              MBED_CONF_NSAPI_EMAC_RX_POOL_NUM_BUFS
 
-#ifdef MBED_CONF_LWIP_PBUF_POOL_BUFSIZE
+#ifdef MBED_CONF_NSAPI_EMAC_RX_POOL_BUF_SIZE
 #undef PBUF_POOL_BUFSIZE
-#define PBUF_POOL_BUFSIZE           LWIP_MEM_ALIGN_SIZE(MBED_CONF_LWIP_PBUF_POOL_BUFSIZE)
+#define PBUF_POOL_BUFSIZE           LWIP_MEM_ALIGN_SIZE(MBED_CONF_NSAPI_EMAC_RX_POOL_BUF_SIZE)
 #else
 #ifndef PBUF_POOL_BUFSIZE
 #if LWIP_IPV6

connectivity/lwipstack/lwip/src/core/lwip_pbuf.c

@@ -739,6 +739,9 @@ pbuf_free(struct pbuf *p)
   PERF_START;
 
   count = 0;
+
+  bool pbuf_pool_freed = false;
+
   /* de-allocate all consecutive pbufs from the head of the chain that
    * obtain a zero reference count after decrementing*/
   while (p != NULL) {
@@ -771,6 +774,9 @@ pbuf_free(struct pbuf *p)
         /* is this a pbuf from the pool? */
         if (alloc_src == PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF_POOL) {
           memp_free(MEMP_PBUF_POOL, p);
+
+          // Mbed OS Patch: set flag that buffer was from the pool
+          pbuf_pool_freed = true;
           /* is this a ROM or RAM referencing pbuf? */
         } else if (alloc_src == PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF) {
           memp_free(MEMP_PBUF, p);
@@ -793,6 +799,13 @@ pbuf_free(struct pbuf *p)
       p = NULL;
     }
   }
+
+  // Mbed OS Patch: Hook into the memory manager when a POOL buffer is freed.
+  if(pbuf_pool_freed) {
+    extern void mbed_lwip_on_pbuf_pool_free_hook(void);
+    mbed_lwip_on_pbuf_pool_free_hook();
+  }
+
   PERF_STOP("pbuf_free");
   /* return number of de-allocated pbufs */
   return count;

connectivity/lwipstack/mbed_lib.json connectivity/lwipstack/mbed_lib.json5connectivity/lwipstack/mbed_lib.json renamed to connectivity/lwipstack/mbed_lib.json5

@@ -118,14 +118,6 @@
             "help": "Priority of lwip TCPIP thread",
             "value": "osPriorityNormal"
         },
-        "pbuf-pool-size": {
-            "help": "Number of pbufs in pool - usually used for received packets, so this determines how much data can be buffered between reception and the application reading, see LWIP's opt.h for more information. If a driver uses PBUF_RAM for reception, less pool may be needed. Current default  is 5.",
-            "value": 5
-        },
-        "pbuf-pool-bufsize": {
-            "help": "Size of pbufs in pool, see LWIP's opt.h for more information.",
-            "value": null
-        },
         "mem-size": {
             "help": "Size of heap (bytes) - used for outgoing packets, and also used by some drivers for reception, see LWIP's opt.h for more information. Current default is 1600.",
             "value": 1600
@@ -193,7 +185,6 @@
             "tcp-mss": 1440,
             "tcp-snd-buf": "(8 * TCP_MSS)",
             "tcp-wnd": "(TCP_MSS * 8)",
-            "pbuf-pool-size": 16,
             "mem-size": 51200
         },
         "RZ_A2XX": {
@@ -203,7 +194,6 @@
             "tcp-mss": 1440,
             "tcp-snd-buf": "(8 * TCP_MSS)",
             "tcp-wnd": "(TCP_MSS * 8)",
-            "pbuf-pool-size": 16,
             "mem-size": 51200
         },
         "MCU_PSOC6": {
@@ -216,7 +206,6 @@
             "tcp-mss": 1540,
             "tcp-snd-buf": "(6 * TCP_MSS)",
             "tcp-wnd": "(TCP_MSS * 6)",
-            "pbuf-pool-size": 14,
             "mem-size": 65536
         },
         "MIMXRT105X": {

connectivity/lwipstack/source/LWIPMemoryManager.cpp

@@ -17,6 +17,8 @@
 #include "pbuf.h"
 #include "LWIPMemoryManager.h"
 
+#include <LWIPStack.h>
+
 net_stack_mem_buf_t *LWIPMemoryManager::alloc_heap(uint32_t size, uint32_t align)
 {
     struct pbuf *pbuf = pbuf_alloc(PBUF_RAW, size + align, PBUF_RAM);
@@ -49,6 +51,11 @@ uint32_t LWIPMemoryManager::get_pool_alloc_unit(uint32_t align) const
     return alloc_unit;
 }
 
+uint32_t LWIPMemoryManager::get_pool_size() const
+{
+    return PBUF_POOL_SIZE;
+}
+
 void LWIPMemoryManager::free(net_stack_mem_buf_t *buf)
 {
     pbuf_free(static_cast<struct pbuf *>(buf));
@@ -105,6 +112,23 @@ void LWIPMemoryManager::set_len(net_stack_mem_buf_t *buf, uint32_t len)
     set_total_len(pbuf);
 }
 
+NetStackMemoryManager::Lifetime LWIPMemoryManager::get_lifetime(const net_stack_mem_buf_t *buf) const
+{
+    auto const *p = static_cast<const struct pbuf *>(buf);
+
+    uint8_t allocSrc = pbuf_get_allocsrc(p);
+
+    if (allocSrc == PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF && PBUF_NEEDS_COPY(p)) {
+        return Lifetime::VOLATILE;
+    } else if (allocSrc == PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF) {
+        return Lifetime::CONSTANT;
+    } else if (allocSrc == PBUF_TYPE_ALLOC_SRC_MASK_STD_MEMP_PBUF_POOL) {
+        return Lifetime::POOL_ALLOCATED;
+    } else {
+        return Lifetime::HEAP_ALLOCATED;
+    }
+}
+
 uint32_t LWIPMemoryManager::count_total_align(uint32_t size, uint32_t align)
 {
     uint32_t buffers = size / get_pool_alloc_unit(align);
@@ -162,3 +186,8 @@ void LWIPMemoryManager::set_total_len(struct pbuf *pbuf)
         pbuf = pbuf->next;
     }
 }
+
+void mbed_lwip_on_pbuf_pool_free_hook()
+{
+    LWIP::get_instance().get_memory_manager().onPoolSpaceAvailCallback();
+}

connectivity/lwipstack/source/LWIPStack.cpp

@@ -550,6 +550,9 @@ nsapi_size_or_error_t LWIP::socket_sendto_control(nsapi_socket_t handle, const S
 
     struct netbuf *buf = netbuf_new();
 
+    // Note: netbuf_ref() tells LwIP that it can reference the application-supplied buffer,
+    // but only until sendto() returns.  If it has to hold onto the buffer until later
+    // (e.g. due to an ARP packet that needs to be sent first), it must copy it.
     err_t err = netbuf_ref(buf, data, (u16_t)size);
     if (err != ERR_OK) {
         netbuf_free(buf);