Memfault Firmware SDK 1.0.1 (Build 2436)

Memfault Inc · Memfault Inc · commit 0e579c285a53 · 2023-06-10T12:10:59.000Z
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,3 +1,22 @@
+### Changes between Memfault SDK 1.0.1 and 1.0.0 - June 9, 2023
+
+#### :chart_with_upwards_trend: Improvements
+
+- Zephyr:
+
+  - Improve the quality of Zephyr stack usage detection when capturing less than
+    the full stack contents for each thread. This feature was originally
+    released in SDK `0.43.0`.
+
+- ESP-IDF:
+
+  - Add Memfault Metrics tracking LwIP runtime statistics (packet counts). A new
+    Kconfig flag `CONFIG_MEMFAULT_LWIP_METRICS` controls this feature, which is
+    enabled by default. The LwIP metrics helper is available for non-ESP-IDF
+    projects using LwIP, see
+    [`ports/lwip/memfault_lwip_metrics.c`](ports/lwip/memfault_lwip_metrics.c)
+    for details
+
 ### Changes between Memfault SDK 1.0.0 and 0.43.3 - June 1, 2023
 
 🎉🎉🎉
diff --git a/VERSION b/VERSION
@@ -1,2 +1,2 @@
-BUILD ID: 2329
-GIT COMMIT: 03cc59b56
+BUILD ID: 2436
+GIT COMMIT: 6273359e7
diff --git a/components/include/memfault/version.h b/components/include/memfault/version.h
@@ -19,7 +19,7 @@ typedef struct {
   uint8_t patch;
 } sMfltSdkVersion;
 
-#define MEMFAULT_SDK_VERSION   { .major = 1, .minor = 0, .patch = 0 }
+#define MEMFAULT_SDK_VERSION   { .major = 1, .minor = 0, .patch = 1 }
 
 #ifdef __cplusplus
 }
diff --git a/examples/esp32/README.md b/examples/esp32/README.md
@@ -228,7 +228,7 @@ I (116450) connect: Connecting to '<SSID>'
 Then post the data:
 
 ```plaintext
-esp32> post_core
+esp32> post_chunks
 I (12419) mflt: Posting Memfault Data...
 I (12419) mflt: Result: 0
 ```
diff --git a/examples/zephyr/qemu/qemu-app/CMakeLists.txt b/examples/zephyr/qemu/qemu-app/CMakeLists.txt
@@ -13,15 +13,17 @@ zephyr_include_directories(config)
 project(qemu-app LANGUAGES C VERSION 1.0.0)
 
 # Generate a git hash that's used as part of the software_version, eg
-# 1.0.0+12345678
-EXECUTE_PROCESS(
-  WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
-  COMMAND git rev-parse --short HEAD
-  RESULT_VARIABLE commit_sha1
-  OUTPUT_VARIABLE ZEPHYR_MEMFAULT_EXAMPLE_GIT_SHA1
-  OUTPUT_STRIP_TRAILING_WHITESPACE
-  COMMAND_ERROR_IS_FATAL ANY
-)
+# 1.0.0+12345678. Permit overriding at command line for CI builds.
+if (NOT DEFINED ZEPHYR_MEMFAULT_EXAMPLE_GIT_SHA1)
+  EXECUTE_PROCESS(
+    WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+    COMMAND git rev-parse --short HEAD
+    RESULT_VARIABLE commit_sha1
+    OUTPUT_VARIABLE ZEPHYR_MEMFAULT_EXAMPLE_GIT_SHA1
+    OUTPUT_STRIP_TRAILING_WHITESPACE
+    COMMAND_ERROR_IS_FATAL ANY
+  )
+endif()
 
 zephyr_compile_definitions(
   ZEPHYR_MEMFAULT_EXAMPLE_GIT_SHA1=\"${ZEPHYR_MEMFAULT_EXAMPLE_GIT_SHA1}\"
diff --git a/examples/zephyr/qemu/qemu-app/prj.conf b/examples/zephyr/qemu/qemu-app/prj.conf
@@ -12,8 +12,9 @@ CONFIG_SHELL=y
 CONFIG_QEMU_ICOUNT=n
 
 CONFIG_LOG=y
-# LOG v2 option, only for pre zephyr-v3.1.0
-# CONFIG_LOG2_MODE_DEFERRED=y
+# Using immediate mode to avoid dropping bursts of log messages, eg when running
+# `mflt export` to dump base64-chunks to the console
+CONFIG_LOG_MODE_IMMEDIATE=y
 CONFIG_LOG_BACKEND_UART=y
 # Disable shell log output to prevent duplicate logs, since UART backend is
 # already enabled. Shell log backend enables later than UART backend, and won't
diff --git a/examples/zephyr/qemu/qemu-app/west.yml b/examples/zephyr/qemu/qemu-app/west.yml
@@ -14,7 +14,10 @@ manifest:
     - name: zephyr
       remote: zephyrproject-rtos
       revision: v3.3.0
-      import: true
+      import:
+        # Limit the Zephyr modules to the required set
+        name-allowlist:
+            - cmsis
 
     - name: memfault-firmware-sdk
       path: modules/lib/memfault-firmware-sdk
diff --git a/ports/esp_idf/memfault/CMakeLists.txt b/ports/esp_idf/memfault/CMakeLists.txt
@@ -82,9 +82,16 @@ list(APPEND MEMFAULT_COMPONENTS_SRCS
 list(APPEND MEMFAULT_COMPONENTS_INC_FOLDERS
    include
    include/${MEMFAULT_ESP_IDF_PORT}
+   ${MEMFAULT_SDK_ROOT}/ports/include
    $ENV{MEMFAULT_PLATFORM_EXTRA_INCLUDES}
+   config
 )
 
+if (CONFIG_MEMFAULT_LWIP_METRICS)
+  list(APPEND MEMFAULT_COMPONENTS_SRCS ${MEMFAULT_SDK_ROOT}/ports/lwip/memfault_lwip_metrics.c)
+  list(APPEND MEMFAULT_COMPONENTS_INC_FOLDERS ${MEMFAULT_SDK_ROOT}/ports/lwip/config)
+endif()
+
 # For version >= 4.4.3, we can collect smaller coredumps by default
 # by prioritizing active stack and FreeRTOS regions first. ESP-IDF < 4.4.3
 # uses a simpler scheme collecting all of DRAM. See
@@ -97,10 +104,6 @@ if (DEFINED ENV{IDF_VERSION})
       ${MEMFAULT_SDK_ROOT}/ports/freertos/src/memfault_freertos_ram_regions.c
     )
 
-    list(APPEND MEMFAULT_COMPONENTS_INC_FOLDERS
-      ${MEMFAULT_SDK_ROOT}/ports/include/
-    )
-
     # Add a linker fragment to place FreeRTOS timers and task objects in the same area of dram0.bss
     set(COMPONENT_ADD_LDFRAGMENTS "${MEMFAULT_ESP_IDF_PORT_COMMON}/memfault_esp_freertos.lf")
   endif()
@@ -122,6 +125,10 @@ list(APPEND COMPONENT_REQUIRES
     driver
 )
 
+if(CONFIG_MEMFAULT_LWIP_METRICS)
+  list(APPEND COMPONENT_REQUIRES lwip)
+endif()
+
 if($ENV{MEMFAULT_ESP_HTTP_CLIENT_ENABLE})
 list(APPEND COMPONENT_REQUIRES esp_http_client esp_https_ota)
 endif()
@@ -133,6 +140,7 @@ get_filename_component(this_directory_name ${this_directory_name} NAME)
 mflt_esp32_component_get_target(this_component ${this_directory_name})
 
 component_compile_options(-DMEMFAULT_ESP_HTTP_CLIENT_ENABLE=$ENV{MEMFAULT_ESP_HTTP_CLIENT_ENABLE})
+component_compile_options(-DMEMFAULT_METRICS_USER_HEARTBEAT_DEFS_FILE="memfault_esp_metrics_heartbeat_config.def")
 
 # We will intercept the panic handlers enabled by CONFIG_ESP32_ENABLE_COREDUMP_TO_FLASH
 # and run the Memfault Fault Handler instead.
diff --git a/ports/esp_idf/memfault/Kconfig b/ports/esp_idf/memfault/Kconfig
@@ -47,4 +47,9 @@ menu "Memfault"
             The Memfault HTTP client timeout in milliseconds. This is the
             maximum amount of time the HTTP client will wait for a response from
             the server.
+
+    config MEMFAULT_LWIP_METRICS
+        bool "Collect TCP/UDP metrics from LwIP"
+        default y
+        select LWIP_STATS
 endmenu
diff --git a/ports/esp_idf/memfault/common/memfault_platform_metrics.c b/ports/esp_idf/memfault/common/memfault_platform_metrics.c
@@ -9,13 +9,16 @@
 //! by using the following CFLAG:
 //!   -DMEMFAULT_METRICS_HEARTBEAT_INTERVAL_SECS=15
 
+#include "esp_timer.h"
 #include "memfault/core/debug_log.h"
 #include "memfault/core/reboot_tracking.h"
 #include "memfault/esp_port/metrics.h"
 #include "memfault/metrics/metrics.h"
 #include "memfault/metrics/platform/timer.h"
 
-#include "esp_timer.h"
+#if CONFIG_MEMFAULT_LWIP_METRICS
+  #include "memfault/ports/lwip/metrics.h"
+#endif  // CONFIG_MEMFAULT_LWIP_METRICS
 
 MEMFAULT_WEAK
 void memfault_esp_metric_timer_dispatch(MemfaultPlatformTimerCallback handler) {
@@ -56,3 +59,9 @@ bool memfault_platform_metrics_timer_boot(uint32_t period_sec,
 
   return true;
 }
+
+void memfault_metrics_heartbeat_collect_sdk_data(void) {
+#if CONFIG_MEMFAULT_LWIP_METRICS
+  memfault_lwip_heartbeat_collect_data();
+#endif  // CONFIG_MEMFAULT_LWIP_METRICS
+}
diff --git a/ports/esp_idf/memfault/config/memfault_esp_metrics_heartbeat_config.def b/ports/esp_idf/memfault/config/memfault_esp_metrics_heartbeat_config.def
@@ -0,0 +1,12 @@
+/// @file
+
+// Required to pull in Kconfig-generated definitions
+#include "sdkconfig.h"
+
+
+#if CONFIG_MEMFAULT_LWIP_METRICS
+#include "memfault_lwip_metrics_heartbeat_config.def"
+#endif  // CONFIG_MEMFAULT_LWIP_METRICS
+
+// Include user metrics
+#include "memfault_metrics_heartbeat_config.def"
diff --git a/ports/include/memfault/ports/lwip/metrics.h b/ports/include/memfault/ports/lwip/metrics.h
@@ -0,0 +1,17 @@
+//! @file
+//!
+//! Copyright (c) Memfault, Inc.
+//! See License.txt for details
+
+#pragma once
+
+#ifdef __cplusplus
+extern "C" {
+#endif  // _cplusplus
+
+//! Collects a round TCP/UDP metrics from LwIP
+void memfault_lwip_heartbeat_collect_data(void);
+
+#ifdef __cplusplus
+}
+#endif  // _cplusplus
diff --git a/ports/lwip/config/memfault_lwip_metrics_heartbeat_config.def b/ports/lwip/config/memfault_lwip_metrics_heartbeat_config.def
@@ -0,0 +1,13 @@
+#include "lwip/stats.h"
+
+#if TCP_STATS
+MEMFAULT_METRICS_KEY_DEFINE(Tcp_Tx_Count, kMemfaultMetricType_Unsigned)
+MEMFAULT_METRICS_KEY_DEFINE(Tcp_Rx_Count, kMemfaultMetricType_Unsigned)
+MEMFAULT_METRICS_KEY_DEFINE(Tcp_Drop_Count, kMemfaultMetricType_Unsigned)
+#endif  // TCP_STATS
+
+#if UDP_STATS
+MEMFAULT_METRICS_KEY_DEFINE(Udp_Tx_Count, kMemfaultMetricType_Unsigned)
+MEMFAULT_METRICS_KEY_DEFINE(Udp_Rx_Count, kMemfaultMetricType_Unsigned)
+MEMFAULT_METRICS_KEY_DEFINE(Udp_Drop_Count, kMemfaultMetricType_Unsigned)
+#endif  // UDP_STATS
diff --git a/ports/lwip/memfault_lwip_metrics.c b/ports/lwip/memfault_lwip_metrics.c
@@ -0,0 +1,51 @@
+//! @file
+//!
+//! Copyright (c) Memfault, Inc.
+//! See License.txt for details
+
+#include "lwip/stats.h"
+#include "memfault/metrics/metrics.h"
+#include "memfault/ports/lwip/metrics.h"
+
+// Macro used to calculate appropriate diff for the protocol stats, update the metric values, and
+// reset static values.
+//
+// Instead of resetting the counter values to 0, a difference is calculated to prevent concurrency
+// issues. We take the previous counter value and subtract from the current value. This way only
+// reads are performed on the stat counters in this context. The difference calculations rely on two
+// things:
+// 1. The counters are monotonically increasing
+// 2. The values will never wrap more than once before a heartbeat
+#define SET_PROTOCOL_METRICS(proto_struct, proto_name, proto_metric_prefix)                       \
+  do {                                                                                            \
+    uint32_t tx_diff = proto_struct.xmit - s_##proto_name##_tx_prev;                              \
+    uint32_t rx_diff = proto_struct.recv - s_##proto_name##_rx_prev;                              \
+    uint32_t drop_diff = proto_struct.drop - s_##proto_name##_drop_prev;                          \
+                                                                                                  \
+    memfault_metrics_heartbeat_set_unsigned(MEMFAULT_METRICS_KEY(proto_metric_prefix##_Tx_Count), \
+                                            tx_diff);                                             \
+    memfault_metrics_heartbeat_set_unsigned(MEMFAULT_METRICS_KEY(proto_metric_prefix##_Rx_Count), \
+                                            rx_diff);                                             \
+    memfault_metrics_heartbeat_set_unsigned(                                                      \
+      MEMFAULT_METRICS_KEY(proto_metric_prefix##_Drop_Count), drop_diff);                         \
+                                                                                                  \
+    s_##proto_name##_tx_prev += tx_diff;                                                          \
+    s_##proto_name##_rx_prev += rx_diff;                                                          \
+    s_##proto_name##_drop_prev += drop_diff;                                                      \
+  } while (0);
+
+void memfault_lwip_heartbeat_collect_data(void) {
+#if TCP_STATS
+  static uint32_t s_tcp_tx_prev = 0;
+  static uint32_t s_tcp_rx_prev = 0;
+  static uint32_t s_tcp_drop_prev = 0;
+  SET_PROTOCOL_METRICS(lwip_stats.tcp, tcp, Tcp);
+#endif  // TCP_STATS
+
+#if UDP_STATS
+  static uint32_t s_udp_tx_prev = 0;
+  static uint32_t s_udp_rx_prev = 0;
+  static uint32_t s_udp_drop_prev = 0;
+  SET_PROTOCOL_METRICS(lwip_stats.udp, udp, Udp);
+#endif  // UDP_STATS
+}
diff --git a/ports/zephyr/common/memfault_zephyr_ram_regions.c b/ports/zephyr/common/memfault_zephyr_ram_regions.c
@@ -8,6 +8,7 @@
 
 #include <kernel.h>
 #include <kernel_structs.h>
+#include <sys/types.h>
 #include <version.h>
 
 #include "memfault/components.h"
@@ -21,9 +22,16 @@
 
 static struct k_thread *s_task_tcbs[CONFIG_MEMFAULT_COREDUMP_MAX_TRACKED_TASKS];
 
+#if CONFIG_THREAD_STACK_INFO
+#if CONFIG_STACK_GROWS_UP
+    #error \
+      "Only full-descending stacks are supported by this implementation. Please contact support@memfault.com."
+#endif
+
 static struct MfltTaskWatermarks {
   uint32_t high_watermark;
 } s_task_watermarks[CONFIG_MEMFAULT_COREDUMP_MAX_TRACKED_TASKS];
+#endif
 
 #define EMPTY_SLOT 0
 
@@ -98,18 +106,25 @@ size_t memfault_platform_sanitize_address_range(void *start_addr, size_t desired
 
 //! Compute the high watermark of a task's stack. This is the amount of stack
 //! that has been written to since the task was created.
-static size_t prv_find_high_watermark(void *stack_start, size_t stack_size) {
-  const uint8_t *stack_ptr = (const uint8_t *)stack_start;
+static ssize_t prv_stack_bytes_unused(uintptr_t stack_start, size_t stack_size) {
+  // First confirm that it's safe to traverse the stack region. If the TCB has
+  // been corrupted, we don't want to trigger a memory error.
+  if (memfault_platform_sanitize_address_range((void *)stack_start, stack_size) != stack_size) {
+    return -1;
+  }
 
-  for (size_t i = 0; i < stack_size; i++) {
-    if (*stack_ptr != 0xAA) {
-      break;
-    }
-    stack_ptr++;
+  // This algorithm assumes the stack is full-descending. Start at the lowest
+  // address (highest stack index) and go up in addresses (down in the stack),
+  // looking for the first address that contains something other than the stack
+  // painting pattern, 0xAA.
+  const uint8_t *stack_max = (const uint8_t *)stack_start;
+  const uint8_t * const stack_bottom = (const uint8_t * const)(stack_start + stack_size);
+
+  for (; (stack_max < stack_bottom) && (*stack_max == 0xAA); stack_max++) {
   }
 
-  const uint8_t *stack_end = (const uint8_t *)stack_start + stack_size;
-  return stack_end - stack_ptr;
+  // return the "bytes unused" count for the stack
+  return (uintptr_t)stack_max - stack_start;
 }
 
 size_t memfault_zephyr_get_task_regions(sMfltCoredumpRegion *regions, size_t num_regions) {
@@ -155,7 +170,13 @@ size_t memfault_zephyr_get_task_regions(sMfltCoredumpRegion *regions, size_t num
     // coredump is truncated due to lack of space.
 #if !CONFIG_MEMFAULT_COREDUMP_FULL_THREAD_STACKS
     if ((uintptr_t)_kernel.cpus[0].current == (uintptr_t)thread) {
-      // when collecting partial stacks, thread context is only valid when task is _not_ running so we skip collecting it
+      // when collecting partial stacks, thread context is only valid when task
+      // is _not_ running so we skip collecting it. just update the watermark
+      // for the thread
+      #if CONFIG_THREAD_STACK_INFO
+      s_task_watermarks[i].high_watermark =
+        prv_stack_bytes_unused(thread->stack_info.start, thread->stack_info.size);
+      #endif
       continue;
     }
 #endif
@@ -185,16 +206,21 @@ size_t memfault_zephyr_get_task_regions(sMfltCoredumpRegion *regions, size_t num
       continue;
     }
 
+#if defined(CONFIG_THREAD_STACK_INFO)
     // compute high watermarks for each task
-    s_task_watermarks[i].high_watermark = prv_find_high_watermark(sp, stack_size_to_collect);
+    s_task_watermarks[i].high_watermark =
+      prv_stack_bytes_unused(thread->stack_info.start, thread->stack_info.size);
+#endif
 
     regions[region_idx] = MEMFAULT_COREDUMP_MEMORY_REGION_INIT(sp, stack_size_to_collect);
     region_idx++;
   }
 
+#if defined(CONFIG_THREAD_STACK_INFO)
   regions[region_idx] =
     MEMFAULT_COREDUMP_MEMORY_REGION_INIT(s_task_watermarks, sizeof(s_task_watermarks));
   region_idx++;
+#endif
 
   regions[region_idx] = MEMFAULT_COREDUMP_MEMORY_REGION_INIT(s_task_tcbs, sizeof(s_task_tcbs));
   region_idx++;
diff --git a/tests/comparators/comparator_memfault_metric_ids.hpp b/tests/comparators/comparator_memfault_metric_ids.hpp
@@ -0,0 +1,21 @@
+#pragma once
+
+#include "CppUTestExt/MockSupport.h"
+
+extern "C" {
+#include "memfault/metrics/metrics.h"
+}
+
+class MemfaultMetricIdsComparator : public MockNamedValueComparator {
+ public:
+  virtual bool isEqual(const void *object1, const void *object2) {
+    const MemfaultMetricId *a = (const MemfaultMetricId *)object1;
+    const MemfaultMetricId *b = (const MemfaultMetricId *)object2;
+
+    return a->_impl == b->_impl;
+  }
+  virtual SimpleString valueToString(const void *object) {
+    const MemfaultMetricId *o = (const MemfaultMetricId *)object;
+    return StringFromFormat("MemfaultMetricId: _impl: %d", o->_impl);
+  }
+};
diff --git a/tests/makefiles/Makefile_memfault_port_lwip_metrics.mk b/tests/makefiles/Makefile_memfault_port_lwip_metrics.mk
diff --git a/tests/mocks/mock_memfault_metrics.cpp b/tests/mocks/mock_memfault_metrics.cpp
diff --git a/tests/src/test_memfault_port_lwip_metrics.cpp b/tests/src/test_memfault_port_lwip_metrics.cpp
diff --git a/tests/stub_includes/lwip/stats.h b/tests/stub_includes/lwip/stats.h
diff --git a/tests/stub_includes/memfault_metrics_heartbeat_config.def b/tests/stub_includes/memfault_metrics_heartbeat_config.def

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ typedef struct {`
`19`	`19`	`uint8_t patch;`
`20`	`20`	`} sMfltSdkVersion;`
`21`	`21`
`22`		`-#define MEMFAULT_SDK_VERSION { .major = 1, .minor = 0, .patch = 0 }`
	`22`	`+#define MEMFAULT_SDK_VERSION { .major = 1, .minor = 0, .patch = 1 }`
`23`	`23`
`24`	`24`	`#ifdef __cplusplus`
`25`	`25`	`}`