From ac259c156506d3d48a1842bfc5c77db9e08993ef Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Mon, 10 Aug 2026 22:33:58 -0500 Subject: [PATCH] [rp2] Address review: correct the sizing numbers and pin the invariants Segment pool entries are 20 bytes on this build, not the nominal 16 that struct tcp_seg suggests; the comment now quotes the measured figure. The MEM_SIZE justification did not close as written: two connections at ~6KB each is ~12KB, which fits inside the old 16KB heap. The missing term is that mem.c is first-fit, so what has to be free is a contiguous 1.5KB block, and at 75% occupancy interleaved with ARP, DHCP/DNS and mDNS allocations the largest run collapses long before the total does. That is also why the failure looked intermittent. Spelled out in both the Python comment and the header template. Hoist the lwIP sizing values to module constants so the two numeric invariants can be tested: the segment pool must stay above the per-PCB send queue, and MEM_SIZE must stay under 64000 or lwIP widens mem_size_t to u32_t. Both were previously enforced by prose alone, and the first would silently rebuild the starvation this change removes. The generated header is byte identical. --- esphome/components/rp2/__init__.py | 92 +++++++++++++++---------- esphome/components/rp2/lwipopts.h.jinja | 4 +- tests/unit_tests/components/test_rp2.py | 26 +++++++ 3 files changed, 83 insertions(+), 39 deletions(-) diff --git a/esphome/components/rp2/__init__.py b/esphome/components/rp2/__init__.py index ea37134e27..5d1d3e20ae 100644 --- a/esphome/components/rp2/__init__.py +++ b/esphome/components/rp2/__init__.py @@ -388,6 +388,53 @@ 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: segment pool shared by every PCB, so it cannot be the +# per-PCB queue length — lwIP's sanity check only demands >=, which is the +# floor for a single connection. 2× lets two PCBs queue fully before the rest +# start seeing ERR_MEM. Measured at 20 bytes per entry on this build, so the +# whole pool is under 700 bytes. +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: the lwIP heap that backs PBUF_RAM, which is where tcp_write() +# copies outgoing data. +# +# TCP_OVERSIZE defaults to TCP_MSS, so every queued segment takes a full +# MSS-sized block no matter how little was written: struct pbuf (16) + +# PBUF_TRANSPORT offset (54) + TCP_MSS (1460) + the heap block header, about +# 1.5KB each. A PCB at a full TCP_SND_BUF holds four of them, ~6KB. +# +# Two of those is ~12KB of arduino-pico's 16KB heap, and that is already +# failing: mem.c is first-fit, so what has to be free is a *contiguous* 1.5KB +# block, and at 75% occupancy interleaved with the small allocations (ARP +# output queue, DHCP/DNS, mDNS TX) the largest run collapses long before the +# total does. That is why the failure looks intermittent. With api's +# max_connections on rp2 at 4, a third sender has nothing left to take. It +# surfaces as ERR_MEM from tcp_write(), then a refused send_message(). +# +# 32KB is arduino-pico's own next tier (its __LWIP_MEMMULT=2 boards). +# Must stay under 64000 or lwIP widens mem_size_t to u32_t. +LWIP_MEM_SIZE = 32768 + + def _configure_lwip() -> None: """Configure lwIP options for RP2040 by generating a custom lwipopts.h. @@ -458,37 +505,6 @@ 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 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 - tcp_snd_queuelen = 17 - # MEMP_NUM_TCP_SEG: segment pool shared by every PCB, so it cannot be the - # per-PCB queue length — lwIP's sanity check only demands >=, which is the - # floor for a single connection. 2× lets two PCBs queue fully before the - # rest start seeing ERR_MEM. Each entry is ~16 bytes. - memp_num_tcp_seg = 2 * 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. - pbuf_pool_size = 16 - - # MEM_SIZE: the lwIP heap that backs PBUF_RAM, which is where tcp_write() - # copies outgoing data. TCP_OVERSIZE defaults to TCP_MSS, so one PCB - # holding a full TCP_SND_BUF pins ~4 × 1.5KB. At arduino-pico's 16KB, two - # busy connections exhausted it while api's max_connections on rp2 is 4; - # that surfaces as ERR_MEM from tcp_write(), then a refused send_message(). - # 32KB is arduino-pico's own next tier (its __LWIP_MEMMULT=2 boards). - # Must stay under 64000 or lwIP widens mem_size_t to u32_t. - mem_size = 32768 - # 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. @@ -497,12 +513,12 @@ def _configure_lwip() -> None: # 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. lwip_defines: dict[str, str] = { - "TCP_SND_BUF": tcp_snd_buf, - "TCP_WND": tcp_wnd, - "TCP_SND_QUEUELEN": str(tcp_snd_queuelen), - "MEM_SIZE": str(mem_size), - "MEMP_NUM_TCP_SEG": str(memp_num_tcp_seg), - "PBUF_POOL_SIZE": str(pbuf_pool_size), + "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), @@ -522,7 +538,7 @@ def _configure_lwip() -> None: listen_min = " (min)" if listening_tcp > sc.tcp_listen else "" _LOGGER.info( "Configuring lwIP: %d byte heap; TCP=%d%s [%s], UDP=%d%s [%s], TCP_LISTEN=%d%s [%s]", - mem_size, + LWIP_MEM_SIZE, tcp_sockets, tcp_min, sc.tcp_details, diff --git a/esphome/components/rp2/lwipopts.h.jinja b/esphome/components/rp2/lwipopts.h.jinja index 6563489d2e..2da4f467a9 100644 --- a/esphome/components/rp2/lwipopts.h.jinja +++ b/esphome/components/rp2/lwipopts.h.jinja @@ -32,7 +32,9 @@ // 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 and two connections exhausted it. +// 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 }} diff --git a/tests/unit_tests/components/test_rp2.py b/tests/unit_tests/components/test_rp2.py index 023d926dc4..537a8d5234 100644 --- a/tests/unit_tests/components/test_rp2.py +++ b/tests/unit_tests/components/test_rp2.py @@ -93,3 +93,29 @@ 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 equality would rebuild + the starvation this sizing exists to prevent, and nothing in the build would + complain. + """ + from esphome.components import rp2 + + assert rp2.LWIP_MEMP_NUM_TCP_SEG >= rp2.LWIP_TCP_SND_QUEUELEN + assert rp2.LWIP_MEMP_NUM_TCP_SEG >= 2 * rp2.LWIP_TCP_SND_QUEUELEN + + +def test_lwip_mem_size_keeps_mem_size_t_narrow() -> None: + """``MEM_SIZE`` above 64000 silently widens lwIP's ``mem_size_t`` to + ``u32_t`` (``lwip/mem.h``), 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. + """ + from esphome.components import rp2 + + assert rp2.LWIP_MEM_SIZE < 64000