fix(wg): rate-limit DISCO PONG replies once a direct path is trusted - #64
Merged
Conversation
Adapted from dj-oyu/microlink@bde4bff (issue #43, piece 4 of 4) -- hand-ported against process_disco_ping() since dj-oyu's pre-refactor disco_process_packet() no longer exists in this fork's layout. Once a peer has a trusted direct path, the remote tailscaled peer sends NAT-keepalive PINGs roughly every few seconds. This fork was answering every single one with a 3-path PONG (direct reply + all LAN endpoints + DERP relay) -- pure waste once the path is established. Now rate-limited to 1/5s per peer via a new last_pong_sent_ms field, gated on has_direct_path so it still answers immediately while searching (no direct path yet) or on the very first reply, since that's what establishes the path in the first place. This closes out issue #43's remaining bundle. The other 3 pieces needed no code change: - TAI64N: superseded by upstream PR #22's absorption (6a4447c). - DERP->direct re-handshake trilogy (a30160a, eb3dce0, c505539): investigated, not needed. wireguardif_connect() already forces an immediate handshake and wireguardif_connect_derp() already exists (landed via #34/#42). update_peer_addr() already retargets a peer's ip/port on every valid decrypted packet regardless of transport, so an established session's keypair keeps working transport-agnostically across a DERP->direct switch -- exactly what the trilogy's own final commit independently concludes. Issue #33's already-landed NAT-rebind logic implements the same reasoning with an added data-liveness safety net the trilogy never had. - LOCK_TCPIP_CORE no-op -> tcpip_try_callback: investigated, not needed. This fork's WG RX (direct + DERP) funnels through a single wg_rx_queue consumed only by wg_mgr task -- no concurrent caller of wireguardif_network_rx() ever exists to race. Same architectural class as issue #31's resolution. Closes #43 Co-authored-by: Adrian.Nguyen-Qualgo <tansolo.barn@gmail.com> 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
Adapted from
dj-oyu/microlink@bde4bff(issue #43, piece 4 of 4) — hand-ported againstprocess_disco_ping()since dj-oyu's pre-refactordisco_process_packet()no longer exists in this fork's layout.Once a peer has a trusted direct path, the remote tailscaled peer sends NAT-keepalive PINGs roughly every few seconds. This fork was answering every single one with a 3-path PONG (direct reply + all LAN endpoints + DERP relay) — pure waste once the path is established. Now rate-limited to 1/5s per peer via a new
last_pong_sent_msfield, gated onhas_direct_pathso it still answers immediately while searching (no direct path yet) or on the very first reply, since that's what establishes the path in the first place.This closes out issue #43's remaining bundle. The other 3 pieces needed no code change:
6a4447c5).a30160a8,eb3dce06,c505539d): investigated, not needed.wireguardif_connect()already forces an immediate handshake andwireguardif_connect_derp()already exists (landed via Throttle handshake retries to 1/tick round-robin (mined from Csontikka/microlink) #34/Extract DERP-routing-preservation fix for reply traffic (mined from caslavskola/microlink) #42).update_peer_addr()already retargets a peer's ip/port on every valid decrypted packet regardless of transport, so an established session's keypair keeps working transport-agnostically across a DERP→direct switch — exactly what the trilogy's own final commit independently concludes. Issue DISCO trust-expiry gate, NAT-rebind handshake-skip, SPIRAM pbuf headroom fix (mined from Csontikka/microlink) #33's already-landed NAT-rebind logic implements the same reasoning with an added data-liveness safety net the trilogy never had.LOCK_TCPIP_COREno-op →tcpip_try_callback: investigated, not needed. This fork's WG RX (direct + DERP) funnels through a singlewg_rx_queueconsumed only bywg_mgrtask — no concurrent caller ofwireguardif_network_rx()ever exists to race. Same architectural class as issue Compare LOCK_TCPIP_CORE fix against our netif->state workaround (mined from Csontikka/microlink) #31's resolution.Closes #43
Test plan
CLAUDE.md) — needs on-device verification: establish a direct path between two nodes, confirm PONG replies drop to ~1/5s cadence instead of matching every incoming PING, and that the direct path stays trusted (no spurious fallback to DERP from the reduced PONG traffic).has_direct_path,ml_get_time_ms()— both already used identically elsewhere in this file).ml_wg_mgr.c's DISCO ping handler, notml_derp.c).