fix(wg): move magicsock TX pbuf allocation to SPIRAM - #77
Merged
Conversation
…zed window do_fetch_peers() previously allocated two fixed 512KB PSRAM buffers (H2 receive + a separate JSON parse buffer). Merge them into a single buffer, compacting the extracted MapResponse JSON in place via memmove instead of copying into a second buffer -- halves peak footprint (~1MB -> ~512KB at defaults). The buffer's size is now also clamped at connect time to the largest actually-free heap block (min 64KB, never above the configured ML_H2_BUFFER_SIZE_KB ceiling), via new choose_h2_rx_window_size(). CONFIG_ML_JSON_BUFFER_SIZE_KB is removed -- the merged buffer only needs one size knob. Adapted from djorr5/microlink's `67b230b2` piece (a) (dynamic H2 RX window sizing) -- piece (b), `ip4_route_src_hook`, is unrelated and untouched (tracked separately as #39). Kept this fork's existing frame_buf scratch-then-copy pattern in the initial receive loop rather than reading noise_recv() straight into the shrinking window, since this fork's noise_recv() doesn't drain the ciphertext off the socket when a frame doesn't fit the destination buffer -- reading directly into a near-full window risked desyncing the coordination stream. Verified with a from-clean `pio run` against zen-clock (a real downstream consumer, LilyGo T-Display-S3) using idf_component.yml's override_path pointed at this working tree: full firmware build + link succeeded. The adaptive clamp-under-heap-pressure path itself wasn't exercised on hardware since that board has ample free PSRAM. Closes #38 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Board became available mid-session -- re-flashed zen-clock (LilyGo T-Display-S3) against a real tailnet and captured the actual boot log through do_fetch_peers(): H2 rx window computed correctly (128KB, clamped to the configured ceiling since free heap was ample), a real 22KB/13-frame MapResponse reassembled and JSON-compacted in place without truncation or corruption, and all 5 real tailnet peers completed WireGuard handshakes normally afterward. Upgrades the earlier compile-only verification note. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
wg_udp_output_cb() (and its tcpip_try_callback-deferred counterpart wg_send_in_tcpip()) allocated every outbound WireGuard packet via pbuf_alloc(PBUF_TRANSPORT, len, PBUF_RAM) -- internal-DRAM-pool allocations that fragment under sustained high packet rate. New wg_alloc_tx_pbuf() replaces both call sites with a custom pbuf backed by SPIRAM (payload) + a small internal-heap control struct, freed via a custom_free_function so pbuf_free() keeps working normally. Adapted from Csontikka/microlink's `f1de3143`, but simplified after checking lwIP's actual pbuf.c/udp.c: the source fix hand-computes a PBUF_TRANSPORT header-offset reservation (the same arithmetic that shipped corrupted data once in that fork's own history) to leave room for lwIP to grow the pbuf in place. That growth path never fires here -- pbuf_add_header() unconditionally fails for PBUF_REF-type pbufs on every TX path (only the RX-only pbuf_header_force() respects reserved headroom), so udp_send() already falls back to chaining a small separate PBUF_RAM header pbuf on every send regardless. Using PBUF_RAW (offset 0) instead of PBUF_TRANSPORT gets identical runtime behavior without the fragile duplicated offset arithmetic. Hardware-verified on a LilyGo T-Display-S3 against a real tailnet: 0% loss over 400 pings at ~20pps (clean baseline), and 1038+ real WG UDP TX events under an 8-stream ~160pps overload stress test with zero crashes or heap-corruption signatures in the boot log. Closes #33 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports the last remaining piece of issue #33 (
FORK_PRS.mdrow 14) — the SPIRAM pbuf headroom fix fromCsontikka/microlink'sf1de3143. The other two pieces (DISCO trust-expiry gate, NAT-rebind handshake-skip) already landed in an earlier session.wg_udp_output_cb()(and itstcpip_try_callback-deferred counterpartwg_send_in_tcpip()) allocated every outbound WireGuard packet viapbuf_alloc(PBUF_TRANSPORT, len, PBUF_RAM)— internal-DRAM-pool allocations that fragment under sustained high packet rate (the source fork's own bug report: ~140pps causing periodicmem_mallocfailure on ~1312-byte WG packets). Newwg_alloc_tx_pbuf()replaces both call sites with a custom pbuf backed by SPIRAM for the payload + a small control struct on internal heap, freed via acustom_free_function.Improved on the source fix, not a literal port: the source commit hand-computes
hdr_offset = LWIP_MEM_ALIGN_SIZE(PBUF_TRANSPORT)to reserve header room in the SPIRAM buffer — this is the exact arithmetic that (per the source commit's own self-correcting comment) shipped corrupted data once already in that fork's history. Checked against lwIP's actualpbuf.c/udp.cbefore porting:pbuf_add_header()(the only header-growth call any TX path uses) unconditionally fails forPBUF_REF-type pbufs regardless of how much headroom is reserved — onlypbuf_header_force(), which lwIP only calls on RX, respects it.udp_send()already falls back to chaining a small separatePBUF_RAMheader pbuf in front on every send regardless of what layer we pass. So the reserved headroom was always dead weight — usingPBUF_RAW(offset always 0) gets identical runtime behavior while eliminating the fragile duplicated offset arithmetic entirely.Credit to
Csontikka/microlinkfor identifying and fixing the real underlying problem (DRAM pbuf pool fragmentation under load).Test plan
pio runagainstzen-clock(downstream consumer) withoverride_path— clean build, new symbols (wg_alloc_tx_pbuf,ml_spiram_pbuf_free_fn) present in the compiled object.wg_udp_output_cb, so no extra echo-listener firmware was needed):WG UDP TXevents through the new allocator path. Zero crashes, panics, or heap-corruption signatures in the boot log. The stress test's high packet loss (RTT growing to 4+ seconds before drops) is a DERP-relay congestion signature, not corruption — a corrupted checksum would drop immediately rather than after a growing queueing delay.Closes #33
🤖 Generated with Claude Code