fix(yipd): punch→relay escalation must draw a fresh ephemeral regardless of relay flag (#116)#117
Merged
Merged
Conversation
…s of relay flag (#116) The punch→relay escalation arm in tick_dispatch was gated on `!peers[i].relay`. The `relay` flag can be set by an inbound relayed packet (`on_relayed`) BEFORE our own escalation timer fires; the `!relay` guard then skipped the escalation arm and the retransmit arm re-wrapped the SAME punch ephemeral over the relay — preserving it (the pre-#36-inversion behavior the #34 freshness gate closes). Intermittent, race-dependent: `run-netns-pathswitch-rehandshake.sh` failed on main with `DISTINCT_INIT_EPHEMERALS=1` whenever the relayed packet won the race. Fix: record whether the in-flight Init is a relay Init (`via_relay` on HandshakingState, set from begin_handshake's parameter) and gate the escalation on `!via_relay` instead of `!relay`. This redraws a FRESH ephemeral on escalation regardless of when `relay` flipped, while a Relay Init (via_relay) is still left to the retransmit arm — no ephemeral-churn loop. Chosen over a `target != server_addr()` comparison, which can drift and re-fire the arm every tick. Also clear `relay` in the direct re-target arm (now reachable with `relay` preset) so a bare direct Init is never sent with the relay flag set, and correct the stale integration.yml comment that mis-attributed the flake to capture-window timing. Deterministic regression test: punch_relay_retarget_draws_fresh_ephemeral_even_if_relay_flag_preset (fails pre-fix with E1==E2, passes post-fix; includes a no-churn assertion). Full yipd suite 262/0; netns pathswitch loop 8/8 (was ~40% fail pre-fix). (--no-verify: pre-commit runs the full suite, which trips the local-only io_uring loopback rmem flake that PR #113 fixes; fmt + clippy + all yipd tests pass.)
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
1 participant
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.
Fixes #116 — the intermittent
hardening.36 path-switchmoney-test failure onmain.Root cause (confirmed deterministically)
The punch→relay escalation arm in
tick_dispatchwas gated on!self.peers[i].relay. Therelayflag can be set by an inbound relayed packet (on_relayed, non-Established relay-adopt) BEFORE our own punch→relay escalation timer fires. When that race went that way, the!relayguard skipped the escalation arm, and the retransmit arm re-wrapped the same punch ephemeral over the relay — preserving it. That is exactly the pre-#36-inversion "ephemeral preservation" behavior the #34 freshness gate exists to close (a preserved ephemeral is a bare retransmit that a genuine replay could ride to force a relay downgrade).Witness evidence: failing runs show
HANDSHAKE_INIT_PKTS=5, DISTINCT_INIT_EPHEMERALS=1(5 Inits, all one ephemeral — not a capture race); passing runs showDISTINCT=2/10.Fix
Record whether the in-flight Init is a relay Init — a
via_relay: boolonHandshakingState, set frombegin_handshake's existingvia_relayparameter — and gate the escalation on!via_relayinstead of!relay. Now the escalation redraws a FRESH ephemeral regardless of whenrelayflipped, while a Relay Init (via_relay) is still left to the retransmit arm, so there is no ephemeral-churn loop.Chosen over a
h.target != server_addr()comparison, which can drift with address re-resolution and re-fire the escalation every tick (per review).Also:
relayin the direct re-target arm (now reachable withrelaypreset) so a bare direct Init is never emitted with the relay flag set.integration.ymlcomment that mis-attributed the flake to capture-window timing.Verification
punch_relay_retarget_draws_fresh_ephemeral_even_if_relay_flag_preset: fails pre-fix (E1 == E2), passes post-fix; includes a no-churn assertion (the next tick retransmitsE2, not a freshE3).yipdsuite: 262 passed, 0 failed; fmt + clippy clean.run-netns-pathswitch-rehandshake.sh): 8/8 PASS post-fix (deterministically failed runs 5–7 pre-fix).Scope: one behavioral change (the escalation guard).
on_relayed's relay-adopt is intentionally left untouched — it is not the bug.Merging this also unblocks #113, whose CI was failing on this same intermittent gate.