EMAC memory manager improvements & fixes

.github/workflows/greentea_cmake.yml

@@ -146,7 +146,7 @@ jobs:
         # Note: We have to set a wifi network name and password so that the test will compile on devices that use wifi
         run: |
             rm -rf __build
-            cmake -S . -B __build -GNinja -DUPLOAD_METHOD=NONE -DMBED_GREENTEA_WIFI_SECURE_SSID=SomeNetwork -DMBED_GREENTEA_WIFI_SECURE_PASSWORD=SomePassword -DCMAKE_CTEST_ARGUMENTS="--output-on-failure;-V" -DMBED_BUILD_GREENTEA_TESTS=ON -DMBED_GREENTEA_TEST_BAREMETAL=ON -DMBED_GREENTEA_SERIAL_PORT=/dev/ttyDUMMY -DMBED_TARGET=${{ matrix.target }} -DMBED_APP_JSON_PATH=TESTS/configs/greentea_baremetal.json5
+            cmake -S . -B __build -GNinja -DUPLOAD_METHOD=NONE -DMBED_GREENTEA_WIFI_SECURE_SSID=SomeNetwork -DMBED_GREENTEA_WIFI_SECURE_PASSWORD=SomePassword -DCMAKE_CTEST_ARGUMENTS="--output-on-failure;-V" -DMBED_BUILD_GREENTEA_TESTS=ON -DMBED_GREENTEA_SERIAL_PORT=/dev/ttyDUMMY -DMBED_TARGET=${{ matrix.target }} -DMBED_APP_JSON_PATH=TESTS/configs/greentea_baremetal.json5
             cmake --build __build
 
       - name: Build ${{ matrix.target }} with full profile

TESTS/configs/greentea_full.json5

@@ -18,6 +18,14 @@
         "mbed-trace.enable": true,
 
         // Disable colored traces in tests, as the test runner does not like the terminal control chars
-        "mbed-trace.default-config": "TRACE_ACTIVE_LEVEL_INFO | TRACE_CARRIAGE_RETURN"
+        "mbed-trace.default-config": "TRACE_ACTIVE_LEVEL_INFO | TRACE_CARRIAGE_RETURN",
+
+        // Don't use any network stack as the default, so that we can manually select which one
+        // is used in the tests.
+        "nsapi.default-stack": null,
+
+        // Configure Nanostack for Ethernet mode only by default. This is sufficient for our tests and is
+        // needed because not every target has enough flash space for all of Nanostack (it can be more than 400kiB when you include the Mbed TLS deps it pulls in!)
+        "nanostack.configuration": "ethernet_host"
     }
 }

connectivity/drivers/emac/TARGET_NXP_EMAC/TARGET_LPCTarget/lpc17_emac.cpp

@@ -9,6 +9,7 @@
 *
 * Copyright(C) 2011, NXP Semiconductor
 * All rights reserved.
+* SPDX-License-Identifier: Apache-2.0
 *
 ***********************************************************************
 * Software that is described herein is for illustrative purposes only
@@ -127,7 +128,7 @@ struct lpc_enetdata {
 
 #if defined(TARGET_LPC17XX)
 #  if defined(TOOLCHAIN_GCC_ARM) || defined(TOOLCHAIN_ARM)
-#     define ETHMEM_SECTION __attribute__((section("AHBSRAM1"),aligned))
+#     define ETHMEM_SECTION __attribute__((section("AHBSRAM"),aligned))
 #  endif
 #endif
 

connectivity/drivers/wifi/TARGET_STM/COMPONENT_EMW3080B/CMakeLists.txt

@@ -22,6 +22,12 @@ target_sources(mbed-wifi
         mx_wifi/core/mx_wifi_slip.c
 )
 
+# This driver needs utility functions from LwIP
+target_link_libraries(mbed-wifi
+    PRIVATE
+        mbed-lwipstack
+)
+
 # Link override object file coming from static library anyway
 #
 # NOTE: This linker option is to pretend undefined symbol and won't cause

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

@@ -1,4 +1,5 @@
 /* Copyright (C) 2012 mbed.org, MIT License
+ * SPDX-License-Identifier: MIT
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy of this software
  * and associated documentation files (the "Software"), to deal in the Software without restriction,
@@ -136,11 +137,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-sys/arch/cc.h

@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2001-2003 Swedish Institute of Computer Science.
  * All rights reserved.
+ * SPDX-License-Identifier: BSD-3-Clause
  *
  * Redistribution and use in source and binary forms, with or without modification,
  * are permitted provided that the following conditions are met:
@@ -136,7 +137,7 @@ void trace_to_ascii_hex_dump(char* prefix, int len, char *data);
 #  elif defined(TOOLCHAIN_GCC_CR)
 #     define MEMP_SECTION __attribute__((section(".data.$RamPeriph32")))
 #  else
-#     define MEMP_SECTION __attribute__((section("AHBSRAM1"),aligned))
+#     define MEMP_SECTION __attribute__((section("AHBSRAM"),aligned))
 #  endif
 #endif
 #endif

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

@@ -1,6 +1,7 @@
 /**
  * @file
  * Packet buffer management
+ * SPDX-License-Identifier: BSD-3-Clause
  */
 
 /**
@@ -739,6 +740,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 +775,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 +800,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;