diff --git a/CHANGELOG.md b/CHANGELOG.md index f941ab89..8d1ae765 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- A tunnel whose remote becomes unreachable, such as an IPv6 remote after moving to a network without IPv6, now + moves to another of the peer's addresses within a round trip or two instead of waiting 20-35s to be torn down + and rehandshaked. When a send fails because there is no route to the remote, the tunnel sends a test packet to + every other address it knows for the peer and asks the lighthouse to have the peer punch its new address. + Whichever path answers carries the tunnel. With `punchy.punch` enabled, idle tunnels notice on their next + keepalive punch. An idle tunnel whose remote is unreachable and none of whose other addresses answer is now + torn down by the connection manager rather than left in place. Not yet effective for relayed traffic or on + Windows with the default RIO backend. + ### Removed - The SSH debug server's `start-cpu-profile`, `stop-cpu-profile`, `save-heap-profile`, `save-mutex-profile`, and diff --git a/e2e/dual_stack_roam_test.go b/e2e/dual_stack_roam_test.go index e29f568c..44043be0 100644 --- a/e2e/dual_stack_roam_test.go +++ b/e2e/dual_stack_roam_test.go @@ -47,18 +47,24 @@ func TestDualStackRoamToV4Only(t *testing.T) { // TestDualStackRoamRecovers holds the line TestDualStackRoamToV4Only measures: a tunnel whose remote became unreachable // moves to the peer's other address instead of waiting out the connection manager to rehandshake, which takes 20s. func TestDualStackRoamRecovers(t *testing.T) { - for _, tc := range []dualStackRoamCase{ - {name: "both send", meSends: true, themSends: true}, - {name: "laptop sends, peer NAT", meSends: true, peerNAT: true}, + for _, tc := range []struct { + dualStackRoamCase + // within bounds recovery in each direction that has traffic + within time.Duration + }{ + // Measures 0.4s. Under the 5s connection manager check, so it fails if the data path stops probing. + {dualStackRoamCase{name: "both send", meSends: true, themSends: true}, 2 * time.Second}, + // Measures 2.6s or 3.8s, depending on whether a probe beats the peer's punch. Half the 20s it used to take. + {dualStackRoamCase{name: "laptop sends, peer NAT", meSends: true, peerNAT: true}, 10 * time.Second}, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - fwd, rev := measureDualStackRoam(t, tc) - if fwd == 0 || fwd > 5*time.Second { - t.Errorf("laptop->peer recovered in %v, want within 5s", fmtRecovery(fwd)) + fwd, rev := measureDualStackRoam(t, tc.dualStackRoamCase) + if fwd == 0 || fwd > tc.within { + t.Errorf("laptop->peer recovered in %v, want within %v", fmtRecovery(fwd), tc.within) } - if tc.themSends && (rev == 0 || rev > 5*time.Second) { - t.Errorf("peer->laptop recovered in %v, want within 5s", fmtRecovery(rev)) + if tc.themSends && (rev == 0 || rev > tc.within) { + t.Errorf("peer->laptop recovered in %v, want within %v", fmtRecovery(rev), tc.within) } }) } diff --git a/hostmap.go b/hostmap.go index 6dd29807..35548310 100644 --- a/hostmap.go +++ b/hostmap.go @@ -805,6 +805,13 @@ func (i *HostInfo) remoteUnreachable(ifce *Interface, dead netip.AddrPort) { ifce.sendTo(header.Test, header.TestRequest, i.ConnectionState, i, addr, []byte(""), nb, out) }) + if ifce.l.Enabled(context.Background(), slog.LevelDebug) { + i.logger(ifce.l).Debug("Remote unreachable, probing other remotes", + "udpAddr", dead, + "probed", probed, + ) + } + if probed == 0 { // With nothing else to try, the lighthouse is our only way to learn of another remote. The peer may not have // one, so ask on the usual requery schedule rather than every probe for as long as the tunnel sits here. @@ -813,6 +820,11 @@ func (i *HostInfo) remoteUnreachable(ifce *Interface, dead netip.AddrPort) { } i.nextLHQuery.Store(now + ifce.reQueryWait.Load()) } + + // With other remotes to probe we ask on every probe, without backing off. A repeat re-sends the peer its punch + // notification in case the last one or its punches were lost. If the tunnel stays up on inbound traffic while + // our sends to its remote keep failing (only one direction broken), this continues once a second for as long as + // that lasts. That is a bounded cost for a rare state, and the probe that would clear it has to keep running. ifce.lightHouse.QueryServer(i.vpnAddrs[0]) } diff --git a/interface.go b/interface.go index 9e7fcfdc..ca8cba74 100644 --- a/interface.go +++ b/interface.go @@ -508,6 +508,11 @@ func (f *Interface) flushSendBatch(sb *batch.SendBatch, q int) { f.metricTxDropped.Inc(int64(dropped)) } + if f.lightHouse == nil || f.lightHouse.amLighthouse { + // Lighthouses never probe, see remoteUnreachable, so skip finding the tunnels + return + } + // A tunnel's packets tend to fail together, so skip straight past a repeat of the one before. The first 8 bytes // of a header say whose packet it is, the counter after them changes with every packet. var last batch.Failure