fix(derp): retry-forever backoff + RX-liveness watchdog for the DERP task - #78
Merged
Merged
Conversation
…task ml_derp_tx_task()'s disconnected branch only ever ran the bounded 3-attempt/2s retries inside the CONNECT_REQ/RECONNECT event handlers -- once those were exhausted, nothing external re-armed a connect while coord sat in COORD_LONG_POLL, so the task just looped on a bare vTaskDelay(100ms) forever. New derp_wanted/derp_next_retry_ms/ derp_backoff_ms state keeps retrying with exponential backoff (ML_DERP_RETRY_MIN_MS 5s -> ML_DERP_RETRY_MAX_MS 60s, doubling on failure, reset on success) instead. Separately, ml->derp.last_recv_ms was already being written on every received frame and on connect (comment: "For keepalive watchdog") but nothing ever read it. Added the read side: if the socket still reports connected but nothing has arrived in ML_DERP_STALE_MS (90s), force a reconnect via ML_EVT_DERP_RECONNECT. Mirrors the already-landed control-plane watchdog (ctrl_stream_rx_ms/ML_CTRL_STREAM_STALE_MS). Adapted from Csontikka/microlink's `46e34917`, but the commit's bundled TLS-state-cleanup half wasn't ported -- issue #14's landed fix already has an equivalent fail_tls/derp_free_tls_state() path on every ml_derp_connect() failure branch, porting it again would be a no-op. Hardware-verified on a real tailnet: ~150s steady-state boot log, 15/15 DERP status lines connected=1, zero false-positive retry/watchdog triggers, zero crashes. Could not induce a real DERP-side outage from this sandbox to directly observe the recovery path firing -- noted as an open gap in FORK_PRS.md rather than silently assumed. Part of #37 (piece 4, the H2-reassembly fix for poll_map_update(), remains deferred -- separate, unmodified function from #38's already- landed do_fetch_peers() rewrite, needs a larger tailnet or a way to force a large incremental delta to verify). 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
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 2 of the remaining pieces of issue #37 (
FORK_PRS.mdrow 18) fromCsontikka/microlink's46e34917— the retry-forever DERP backoff and the RX-liveness watchdog. (Piece 4, the H2-reassembly fix forpoll_map_update(), remains deferred — see below.)ml_derp_tx_task()'s disconnected branch only ran the bounded 3-attempt/2s retries inside theCONNECT_REQ/RECONNECTevent handlers. Once those were exhausted, nothing external re-armed a connect while coord sat inCOORD_LONG_POLL— the task just looped on a barevTaskDelay(100ms)forever. Newderp_wanted/derp_next_retry_ms/derp_backoff_msstate keeps retrying with exponential backoff (ML_DERP_RETRY_MIN_MS5s →ML_DERP_RETRY_MAX_MS60s, doubling on failure, reset on success).ml->derp.last_recv_mswas already being written on every received frame and on connect (existing comment: "For keepalive watchdog") but nothing ever read it — half-wired from the start. Added the read side: if the socket still reports connected but nothing has arrived inML_DERP_STALE_MS(90s), force a reconnect viaML_EVT_DERP_RECONNECT. Mirrors the already-landed control-plane watchdog pattern (ctrl_stream_rx_ms/ML_CTRL_STREAM_STALE_MS) exactly.Not ported: the source commit's bundled TLS-state-cleanup half (a
fail_tlsgoto + free on everyml_derp_connect()failure path). Checkedml_derp.cfirst — issue #14's landed fix already has an equivalentfail_tls/derp_free_tls_state()path on every failure branch, including thembedtls_ssl_setup()check the source commit added. Porting it again would be a no-op.Credit to
Csontikka/microlinkfor both fixes.Test plan
pio runagainstzen-clockwithoverride_path— clean build,-Wall -Werrorpasses.DERP statuslines showedconnected=1, zeroDERP retry-forever/DERP RX silentfalse-positives, zero crashes — confirms the new code doesn't misfire during normal operation.ctrl_stream_rx_mspattern) but hasn't been proven end-to-end under a real failure.Part of #37 — not closing it, since piece 4 (H2-reassembly for
poll_map_update()) is still deferred. Investigated this session too: confirmed it targets a genuinely different, still-unmodified function fromdo_fetch_peers()(which #38 already rewrote and which already reassembles safely) — no overlap with #38's changes. Reproducing the bug needs a long-poll delta large enough to span anoise_recv()read boundary; no evidence this triggers on the current 5-peer test tailnet, so still deferred pending a larger tailnet or a way to force a large incremental delta.🤖 Generated with Claude Code