From 622942482cf79818396e2db38f6e7ea717b4a7eb Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 12 Aug 2026 00:30:17 -0500 Subject: [PATCH] [rp2] Size the lwIP segment pool and heap for concurrent senders (#18257) --- esphome/components/api/api_frame_helper.h | 4 +- esphome/components/rp2/__init__.py | 150 ++++++++++++++-------- esphome/components/rp2/lwipopts.h.jinja | 13 +- tests/unit_tests/components/test_rp2.py | 107 +++++++++++++-- 4 files changed, 207 insertions(+), 67 deletions(-) diff --git a/esphome/components/api/api_frame_helper.h b/esphome/components/api/api_frame_helper.h index 9cae6ba92ef..9c49956bbd8 100644 --- a/esphome/components/api/api_frame_helper.h +++ b/esphome/components/api/api_frame_helper.h @@ -149,7 +149,7 @@ class APIFrameHelper { // holding data too long waiting for Nagle's timer causes buffer exhaustion // and dropped messages. // - // ESP32 (TCP_SND_BUF=4×MSS+) / RP2040 (8×MSS) / LibreTiny (4×MSS): 4 logs per cycle + // ESP32 (TCP_SND_BUF=4×MSS+) / RP2040 (4×MSS) / LibreTiny (4×MSS): 4 logs per cycle // ESP8266 (2×MSS): 3 logs per cycle (tightest buffers) // // Flow (ESP32/RP2040/LT): Log 1 (Nagle on) -> Log 2 -> Log 3 -> Log 4 (NODELAY, flush) @@ -312,7 +312,7 @@ class APIFrameHelper { // Values 1..LOG_NAGLE_COUNT count log messages in the current Nagle batch. // After LOG_NAGLE_COUNT logs, we flush by re-enabling NODELAY and resetting to 0. // ESP8266 has the tightest TCP send buffer (2×MSS) and needs conservative batching. - // ESP32 (4×MSS+), RP2040 (8×MSS), and LibreTiny (4×MSS) can coalesce more. + // ESP32 (4×MSS+), RP2040 (4×MSS), and LibreTiny (4×MSS) can coalesce more. #ifdef USE_ESP8266 static constexpr uint8_t LOG_NAGLE_COUNT = 2; #else diff --git a/esphome/components/rp2/__init__.py b/esphome/components/rp2/__init__.py index 3bc2df7a613..87e78003ed6 100644 --- a/esphome/components/rp2/__init__.py +++ b/esphome/components/rp2/__init__.py @@ -388,6 +388,74 @@ async def to_code(config): _configure_lwip() +# --- lwIP sizing. See _configure_lwip() for the platform comparison table. --- + +# TCP_SND_BUF: 4×MSS=5,840 matches ESP32. Down from arduino-pico's 8×MSS. +# ESPAsyncWebServer allocates malloc(tcp_sndbuf()) per response chunk. +LWIP_TCP_SND_BUF = "(4*TCP_MSS)" + +# TCP_WND: receive window. 4×MSS matches ESP32. Down from arduino-pico's 8×MSS. +LWIP_TCP_WND = "(4*TCP_MSS)" + +# TCP_SND_QUEUELEN: max pbufs queued per PCB for the send buffer +# ESP-IDF formula: (4 * TCP_SND_BUF + (TCP_MSS - 1)) / TCP_MSS +# With 4×MSS: (4*5840 + 1459) / 1460 = 17 — match ESP32 +LWIP_TCP_SND_QUEUELEN = 17 + +# MEMP_NUM_TCP_SEG: pool shared by every PCB, so it must not be the per-PCB +# queue length — lwIP's sanity check only demands >=, the floor for a single +# connection. 2× lets two PCBs fill up before the rest see ERR_MEM. Measured +# at 20 bytes per entry, so under 700 bytes total. +LWIP_MEMP_NUM_TCP_SEG = 2 * LWIP_TCP_SND_QUEUELEN + +# PBUF_POOL_SIZE: RP2040 has 264KB RAM, more generous than LibreTiny. +# 16 matches ESP32 (vs arduino-pico's 24). Receive side only; the send path +# copies into PBUF_RAM out of MEM_SIZE. +LWIP_PBUF_POOL_SIZE = 16 + +# MEM_SIZE: lwIP heap backing PBUF_RAM, where tcp_write() copies outgoing +# data. TCP_OVERSIZE defaults to TCP_MSS, so each queued segment takes a full +# MSS block whatever was written (pbuf 16 + PBUF_TRANSPORT 54 + MSS 1460 + +# block header ≈ 1.5KB); a PCB at a full TCP_SND_BUF holds four, ~6KB. +# +# Two of those is ~12KB of arduino-pico's 16KB heap and already fails: mem.c +# is first-fit, so a *contiguous* 1.5KB block must be free, and at 75% +# occupancy interleaved with ARP/DHCP/DNS/mDNS the largest run collapses well +# before the total does — hence the intermittent failures. With rp2's +# max_connections of 4, a third sender has nothing left. +# +# 32KB is arduino-pico's own next tier (__LWIP_MEMMULT=2 boards). +# Must stay under 64000 or lwIP widens mem_size_t to u32_t. +LWIP_MEM_SIZE = 32768 + + +def build_lwip_defines( + tcp_sockets: int, udp_sockets: int, listening_tcp: int +) -> dict[str, str]: + """Render the lwIP override values for the Jinja2 template. + + The template uses #include_next to chain to the framework's original + lwipopts.h, then #undef/#define only these. Split out from + _configure_lwip() so the values that actually reach the generated header + can be checked without standing up CORE. + + Both malloc flags stay 0 (framework defaults); see _configure_lwip(). The + static pools are the only IRQ-safe allocator on this platform, so the fix + is to size them correctly rather than to make them dynamic. + """ + return { + "TCP_SND_BUF": LWIP_TCP_SND_BUF, + "TCP_WND": LWIP_TCP_WND, + "TCP_SND_QUEUELEN": str(LWIP_TCP_SND_QUEUELEN), + "MEM_SIZE": str(LWIP_MEM_SIZE), + "MEMP_NUM_TCP_SEG": str(LWIP_MEMP_NUM_TCP_SEG), + "PBUF_POOL_SIZE": str(LWIP_PBUF_POOL_SIZE), + "MEMP_NUM_TCP_PCB": str(tcp_sockets), + "MEMP_NUM_TCP_PCB_LISTEN": str(listening_tcp), + "MEMP_NUM_UDP_PCB": str(udp_sockets), + } + + def _configure_lwip() -> None: """Configure lwIP options for RP2040 by generating a custom lwipopts.h. @@ -407,25 +475,36 @@ def _configure_lwip() -> None: ──────────────────────────────────────────────────────────────── TCP_SND_BUF 2×MSS 4×MSS 8×MSS 4×MSS TCP_WND 4×MSS 4×MSS 8×MSS 4×MSS + TCP_SND_QUEUELEN ~8 17 32 17 MEM_LIBC_MALLOC 1 1 0 0* MEMP_MEM_MALLOC 1 1 0 0** - MEM_SIZE N/A*** N/A*** 16KB 16KB + MEM_SIZE N/A*** N/A*** 16KB 32KB PBUF_POOL_SIZE 10 16 24 16 - MEMP_NUM_TCP_SEG 10 16 32 17 + MEMP_NUM_TCP_SEG 10 16 32 34**** MEMP_NUM_TCP_PCB 5 16 5 dynamic - MEMP_NUM_TCP_PCB_LISTEN 4 16 8**** dynamic + MEMP_NUM_TCP_PCB_LISTEN 4 16 8***** dynamic MEMP_NUM_UDP_PCB 4 16 7 dynamic - TCP_SND_QUEUELEN ~8 17 32 17 * MEM_LIBC_MALLOC must stay 0: arduino-pico uses PICO_CYW43_ARCH_THREADSAFE_BACKGROUND which runs lwIP callbacks from a low-priority pendsv IRQ. The pico-sdk explicitly blocks MEM_LIBC_MALLOC=1 because libc malloc uses mutexes (unsafe in IRQ). - ** MEMP_MEM_MALLOC must stay 0: the dedicated lwIP heap (MEM_SIZE=16KB) - is too small to hold all pools dynamically. The PBUF_POOL alone needs - ~24KB (16 × 1524 bytes). Increasing MEM_SIZE would negate BSS savings. - *** ESP8266/ESP32 use MEM_LIBC_MALLOC=1 (system heap, no dedicated pool). - **** opt.h default; arduino-pico doesn't override MEMP_NUM_TCP_PCB_LISTEN. + ** MEMP_MEM_MALLOC must stay 0 for IRQ safety, not size. memp_malloc() + pops the pool free list inside SYS_ARCH_PROTECT, but lwIP's heap takes + its protection from LWIP_ALLOW_MEM_FREE_FROM_OTHER_CONTEXT (default 0), + so under NO_SYS=1 mem_malloc()/mem_free() are unprotected — and memp.c + calls mem_malloc() outside the guard anyway. RX pbufs would then be + allocated from the pendsv IRQ on the same unguarded free list the main + loop uses for tcp_write(). Tried on hardware: faults within seconds on + CYW43. Ethernet survives only because it polls from the main loop. + *** ESP8266/ESP32 ship MEMP_MEM_MALLOC=1, so their pool entries come from + the heap on demand and MEMP_NUM_*/PBUF_POOL_SIZE are labels, not caps + (MEM_LIBC_MALLOC=1 points that heap at the system heap). Both flags are + 0 here, so ours are hard limits; don't copy their numbers. + **** MEMP_NUM_TCP_SEG is *global* while TCP_SND_QUEUELEN is *per-PCB*, so + sizing it to the per-PCB value lets one busy connection drain it for + every other. 2× covers two PCBs; MEM_SIZE is the real limit past that. + ***** opt.h default; arduino-pico doesn't override MEMP_NUM_TCP_PCB_LISTEN. "dynamic" = auto-calculated from component socket registrations via socket.get_socket_counts() with minimums of 8 TCP / 6 UDP / 2 TCP_LISTEN. """ @@ -444,48 +523,7 @@ def _configure_lwip() -> None: # UDP PCBs (2) are absorbed by the generous minimum of 6. listening_tcp = max(MIN_TCP_LISTEN_SOCKETS, sc.tcp_listen) - # TCP_SND_BUF: 4×MSS=5,840 matches ESP32. Down from arduino-pico's 8×MSS. - # ESPAsyncWebServer allocates malloc(tcp_sndbuf()) per response chunk. - tcp_snd_buf = "(4*TCP_MSS)" - - # TCP_WND: receive window. 4×MSS matches ESP32. Down from arduino-pico's 8×MSS. - tcp_wnd = "(4*TCP_MSS)" - - # TCP_SND_QUEUELEN: max pbufs queued for send buffer - # ESP-IDF formula: (4 * TCP_SND_BUF + (TCP_MSS - 1)) / TCP_MSS - # With 4×MSS: (4*5840 + 1459) / 1460 = 17 — match ESP32 - tcp_snd_queuelen = 17 - # MEMP_NUM_TCP_SEG: segment pool, must be >= TCP_SND_QUEUELEN (lwIP sanity check) - memp_num_tcp_seg = tcp_snd_queuelen - - # PBUF_POOL_SIZE: RP2040 has 264KB RAM, more generous than LibreTiny. - # 16 matches ESP32 (vs arduino-pico's 24). With MEMP_MEM_MALLOC=1, - # this is a max count (allocated on demand from heap). - pbuf_pool_size = 16 - - # Build the lwIP override defines for the Jinja2 template. - # The template uses #include_next to chain to the framework's original - # lwipopts.h, then #undef/#define only the values we need to change. - # - # Note: MEMP_MEM_MALLOC stays 0 (framework default). While the memp - # allocations use the dedicated lwIP heap (IRQ-safe), the 16KB MEM_SIZE - # is too small to hold all pools dynamically under stress. The PBUF_POOL - # alone needs ~24KB (16 × 1524 bytes). Increasing MEM_SIZE would negate - # the BSS savings. - # - # MEM_LIBC_MALLOC stays 0 (framework default): arduino-pico uses - # PICO_CYW43_ARCH_THREADSAFE_BACKGROUND which runs lwIP callbacks from - # a low-priority pendsv IRQ where libc malloc (mutex-based) is unsafe. - lwip_defines: dict[str, str] = { - "TCP_SND_BUF": tcp_snd_buf, - "TCP_WND": tcp_wnd, - "TCP_SND_QUEUELEN": str(tcp_snd_queuelen), - "MEMP_NUM_TCP_SEG": str(memp_num_tcp_seg), - "PBUF_POOL_SIZE": str(pbuf_pool_size), - "MEMP_NUM_TCP_PCB": str(tcp_sockets), - "MEMP_NUM_TCP_PCB_LISTEN": str(listening_tcp), - "MEMP_NUM_UDP_PCB": str(udp_sockets), - } + lwip_defines = build_lwip_defines(tcp_sockets, udp_sockets, listening_tcp) # Store for copy_files() to generate the header CORE.data[KEY_RP2][KEY_LWIP_OPTS] = lwip_defines @@ -500,7 +538,8 @@ def _configure_lwip() -> None: udp_min = " (min)" if udp_sockets > sc.udp else "" listen_min = " (min)" if listening_tcp > sc.tcp_listen else "" _LOGGER.info( - "Configuring lwIP: TCP=%d%s [%s], UDP=%d%s [%s], TCP_LISTEN=%d%s [%s]", + "Configuring lwIP: %d byte heap; TCP=%d%s [%s], UDP=%d%s [%s], TCP_LISTEN=%d%s [%s]", + LWIP_MEM_SIZE, tcp_sockets, tcp_min, sc.tcp_details, @@ -521,7 +560,7 @@ def _generate_lwipopts_h() -> None: in the build directory, and a pre-build script injects this directory into the compiler include path before the framework's own include dir. """ - from jinja2 import Environment + from jinja2 import Environment, StrictUndefined lwip_defines = CORE.data[KEY_RP2].get(KEY_LWIP_OPTS) if not lwip_defines: @@ -534,7 +573,10 @@ def _generate_lwipopts_h() -> None: template_text = (Path(__file__).parent / "lwipopts.h.jinja").read_text( encoding="utf-8" ) - jinja_env = Environment(keep_trailing_newline=True) + # StrictUndefined: a placeholder with no value would otherwise render + # empty, emitting a bare #define that compiles and silently means + # something else in lwIP's config. + jinja_env = Environment(keep_trailing_newline=True, undefined=StrictUndefined) template = jinja_env.from_string(template_text) content = template.render(**lwip_defines) diff --git a/esphome/components/rp2/lwipopts.h.jinja b/esphome/components/rp2/lwipopts.h.jinja index 36d7d4da140..2da4f467a92 100644 --- a/esphome/components/rp2/lwipopts.h.jinja +++ b/esphome/components/rp2/lwipopts.h.jinja @@ -20,13 +20,24 @@ #undef TCP_WND #define TCP_WND {{ TCP_WND }} -// Queued segment limits: derived from 4xMSS buffer size, matching ESP32 +// Per-PCB send queue: derived from 4xMSS buffer size, matching ESP32 #undef TCP_SND_QUEUELEN #define TCP_SND_QUEUELEN {{ TCP_SND_QUEUELEN }} +// Segment pool: global across every PCB, so it is sized above the per-PCB +// queue length rather than equal to it. lwIP's sanity check only requires +// >= TCP_SND_QUEUELEN, which is the floor for a single connection. #undef MEMP_NUM_TCP_SEG #define MEMP_NUM_TCP_SEG {{ MEMP_NUM_TCP_SEG }} +// lwIP heap backing PBUF_RAM, which is what tcp_write() copies into. +// Raised from arduino-pico's 16KB: TCP_OVERSIZE is TCP_MSS, so a single PCB +// at a full TCP_SND_BUF pins about 6KB. Two of those left the 16KB heap at +// 75%, and mem.c is first-fit, so the largest contiguous run ran out well +// before the total did. +#undef MEM_SIZE +#define MEM_SIZE {{ MEM_SIZE }} + // Packet buffer pool: 16 matches ESP32 (down from 24) #undef PBUF_POOL_SIZE #define PBUF_POOL_SIZE {{ PBUF_POOL_SIZE }} diff --git a/tests/unit_tests/components/test_rp2.py b/tests/unit_tests/components/test_rp2.py index 023d926dc4a..cd92bc24fae 100644 --- a/tests/unit_tests/components/test_rp2.py +++ b/tests/unit_tests/components/test_rp2.py @@ -13,25 +13,24 @@ itself (Python imports, YAML key rename, deprecation warning) is covered by the framework tests under ``tests/unit_tests/``. """ +from pathlib import Path +import re + +from esphome.components import rp2 + def test_board_id_has_wifi_for_known_wifi_board() -> None: """``rpipicow`` is the canonical Pico W → True.""" - from esphome.components import rp2 - assert rp2.board_id_has_wifi("rpipicow") is True def test_board_id_has_wifi_for_known_non_wifi_board() -> None: """Plain ``rpipico`` has no CYW43 → False.""" - from esphome.components import rp2 - assert rp2.board_id_has_wifi("rpipico") is False def test_board_id_has_wifi_for_rp2350_w_variant() -> None: """``rpipico2w`` is the RP2350 Pico 2 W → True.""" - from esphome.components import rp2 - assert rp2.board_id_has_wifi("rpipico2w") is True @@ -43,8 +42,6 @@ def test_board_id_has_wifi_for_unknown_board_returns_true() -> None: block and any genuinely-unsupported config trips the existing "no CYW43" guard at compile time. """ - from esphome.components import rp2 - assert rp2.board_id_has_wifi("not-a-real-board-id") is True @@ -55,8 +52,6 @@ def test_rp2_declares_rp2040_as_alias() -> None: opts in via ``ALIASES``; without this declaration the rename framework wouldn't route legacy configs. """ - from esphome.components import rp2 - assert "rp2040" in rp2.ALIASES assert rp2.ALIAS_REMOVAL_VERSION == "2027.7.0" @@ -93,3 +88,95 @@ def test_rp2040_submodule_imports_resolve_to_rp2_submodules() -> None: assert rp2040_boards is rp2_boards assert rp2040_generate is rp2_generate + + +def test_lwip_segment_pool_exceeds_per_pcb_queue() -> None: + """The segment pool is global while the send queue is per-PCB. + + lwIP's sanity check only requires ``MEMP_NUM_TCP_SEG >= TCP_SND_QUEUELEN``, + which is the floor for a *single* connection: at equality one busy PCB can + drain the pool for every other PCB. Dropping back to that floor would + rebuild the starvation this sizing exists to prevent, and nothing in the + build would complain. + """ + assert rp2.LWIP_MEMP_NUM_TCP_SEG >= 2 * rp2.LWIP_TCP_SND_QUEUELEN + + +def test_lwip_mem_size_keeps_mem_size_t_narrow() -> None: + """``lwip/mem.h`` widens ``mem_size_t`` to ``u32_t`` on + ``MEM_SIZE > 64000L``, growing the header on every heap block. Raising the + heap past that bound is a real option, but it should be a deliberate one + rather than a side effect of tuning. + """ + assert rp2.LWIP_MEM_SIZE <= 64000 + + +def test_lwip_mem_size_holds_the_concurrent_senders_it_claims() -> None: + """Pin the floor as well as the ceiling. + + The ceiling above is satisfied by arduino-pico's own 16 KB, which is the + value this change exists to move off, so on its own it would let a revert + through. Derive the floor from the sizing comment on the constant: with + TCP_OVERSIZE at TCP_MSS every queued segment takes a full MSS-sized block + (pbuf header + PBUF_TRANSPORT offset + 1460 + heap block header, ~1.5 KB), + a PCB at a full 4xMSS TCP_SND_BUF holds four of them, and api's + max_connections on rp2 is 4. Room for three concurrent senders is the + minimum that makes the change worth making; 16 KB does not reach it. + """ + segments_per_full_send_buf = 4 + bytes_per_mss_block = 1536 + concurrent_senders = 3 + + assert ( + concurrent_senders * segments_per_full_send_buf * bytes_per_mss_block + <= rp2.LWIP_MEM_SIZE + ) + + +def test_lwip_defines_carry_the_sizing_into_the_header() -> None: + """The constants above only matter if they reach the generated header. + + ``build_lwip_defines()`` is what feeds lwipopts.h.jinja, so assert on it + rather than on the constants alone: dropping a key here would silently + fall back to arduino-pico's own value while every other assertion in this + file stayed green. + """ + defines = rp2.build_lwip_defines(tcp_sockets=8, udp_sockets=6, listening_tcp=2) + + assert defines["MEM_SIZE"] == str(rp2.LWIP_MEM_SIZE) + assert defines["MEMP_NUM_TCP_SEG"] == str(rp2.LWIP_MEMP_NUM_TCP_SEG) + assert defines["TCP_SND_QUEUELEN"] == str(rp2.LWIP_TCP_SND_QUEUELEN) + # Socket-derived counts pass through untouched. + assert defines["MEMP_NUM_TCP_PCB"] == "8" + assert defines["MEMP_NUM_UDP_PCB"] == "6" + assert defines["MEMP_NUM_TCP_PCB_LISTEN"] == "2" + + +def test_lwipopts_template_renders_every_sizing_value() -> None: + """Render the template the way _generate_lwipopts_h() does and check the + header that actually ships. + + Covers both directions. A ``#define`` block deleted from the template + leaves the value at arduino-pico's own, which for MEM_SIZE is the 16 KB + heap this change exists to move off, and the loop below catches that. A + placeholder with no dict key would otherwise render empty and emit a bare + ``#define FOO``; StrictUndefined turns that into an error instead. + Matching on text also survives a filter or conditional appearing in the + template later, which a placeholder regex would not. + """ + from jinja2 import Environment, StrictUndefined + + defines = rp2.build_lwip_defines(tcp_sockets=8, udp_sockets=6, listening_tcp=2) + template_text = (Path(rp2.__file__).parent / "lwipopts.h.jinja").read_text( + encoding="utf-8" + ) + rendered = ( + Environment(keep_trailing_newline=True, undefined=StrictUndefined) + .from_string(template_text) + .render(**defines) + ) + + for name, value in defines.items(): + assert re.search( + rf"^#define {re.escape(name)} +{re.escape(value)}$", rendered, re.MULTILINE + ), f"{name} did not reach the generated header as {value!r}"