Skip to content

fix(control): send the advertised endpoints on every map request - #662

Merged
GeiserX merged 1 commit into
GeiserX:mainfrom
kasper0406:fix/control-carry-endpoints
Oct 29, 2026
Merged

GeiserX merged 1 commit into
GeiserX:mainfrom
kasper0406:fix/control-carry-endpoints

Conversation

@kasper0406

Copy link
Copy Markdown
Contributor

Problem. Control takes the endpoints of every endpoint update as the node's endpoints. An endpoint update is a map request with OmitPeers, without Stream and without ReadOnly (tailcfg MapRequest.OmitPeers). In ts_control/src/tokio/client.rs, only SetEndpoints set them.

  • These requests sent none, so control dropped the node's endpoints:
    • the Hostinfo update after each (re)connect;
    • SetDerpHomeRegion, which follows every reconnect because the reconnect brings a derp map;
    • SetRoutableIPs and SetHostname.
  • The runtime advertises endpoints again only when the set changes. On a stable network they stay gone.
  • A peer that starts from a fresh netmap then has no candidate to ping. Go sends a CallMeMaybe only after a ping (endpoint.go:1446-1462), so that peer cannot start a direct path.

Seen on an exit node. After a map-poll reconnect, a Go client's netmap listed the node with HomeDERP but no Endpoints. A peer whose Tailscale had restarted did not get back to a direct path until the node was restarted. Its first packet after the restart was a direct ping to the re-advertised endpoint. That restart also reset other state, so this is supporting context, not proof of the whole outage.

Fix:

  • CarriedNetInfo keeps the endpoints from the last SetEndpoints, and apply attaches them to every request. Go attaches c.endpoints to each request too (direct.go:1048-1104). Because the carried state outlives the poll, endpoints whose SetEndpoints request was lost are sent again on the next reconnect.
  • A side command records its state, then waits for any still-pending post-connect Hostinfo update before sending its own. That update carries the state from the start of the poll; without the wait it could land last and clear the endpoints again. Go sends these updates one at a time from updateRoutine (auto.go:61-109).

Tests:

  • every_request_carries_the_last_advertised_endpoints fails if apply drops the endpoints or set_endpoints does not store them.
  • startup_update_is_applied_before_a_side_update fails without the wait.
  • side_command_state_survives_a_watchdog_restart_while_waiting_for_startup (real watchdog, paused clock) fails if the state is recorded after the wait.
  • On main, the SetDerpHomeRegion request after a SetEndpoints serializes no Endpoints.
  • cargo test -p geiserx_ts_control --all-features, clippy -D warnings and fmt pass.

Not in this PR:

  • Runtime SetRoutableIPs and SetHostname values are not carried, so later updates revert them to the start-up config.
  • register sends no NetInfo.
  • Side updates still block reading the map stream (Go runs them in updateRoutine).

Go references are at tailscale/tailscale 3bb8a906.

Created using Claude Code. Reviewed with Codex (GPT-6.1) before opening; the tests above were run locally.

🤖 Generated with Claude Code

Control takes the endpoints of each endpoint update (a map request with
OmitPeers, without Stream and ReadOnly; tailcfg MapRequest.OmitPeers) as
the node's endpoints. Only SetEndpoints sent them. The Hostinfo update
after each (re)connect, SetDerpHomeRegion, SetRoutableIPs and SetHostname
went without, so control dropped the node's endpoints, and they came back
only when the endpoint set changed.

After a map-poll reconnect, which brings a derp map and so a
SetDerpHomeRegion, a node on a stable network could therefore have no
endpoints at control. A peer that starts from a fresh netmap, with no
cached candidate for the node, then has nothing to ping, and Go sends a
CallMeMaybe only after a ping (endpoint.go:1446-1462), so it cannot start
a direct path to the node.

Keep the endpoints from the last SetEndpoints with the carried NetInfo
and attach them to every request, as Go attaches c.endpoints to each one
(direct.go:1048-1104). The carried state outlives the poll, so a
SetEndpoints whose own request is lost is sent again after a reconnect.

The post-connect Hostinfo update carries the state from when the poll
started, and it could still be unsent or in flight when a side command
sent newer state. Control could then apply it last and clear the
endpoints again. A side command now records its state, then waits for
that update before sending its own, as Go sends all updates one at a
time from updateRoutine (auto.go:61-109). Recording first means a
watchdog restart during the wait cannot lose the command's state.

Go references are at tailscale/tailscale 3bb8a906.

Signed-off-by: Kasper Nielsen <kasper0406@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 29, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@GeiserX

GeiserX commented Oct 29, 2026

Copy link
Copy Markdown
Owner

Thanks, this looks right to us and we'll merge it as is.

We checked it against the Go client. Go's Direct.sendMapRequest copies the current endpoints into every map request, streaming and lite alike, and tailcfg treats a request with OmitPeers set and neither Stream nor ReadOnly as an endpoint update. So a lite request without endpoints does clear them, and carrying the last SetEndpoints value in every request fixes that. Holding side commands until the post-connect Hostinfo update has gone out matches Go's one-at-a-time update loop. Recording the endpoints before that wait means a watchdog restart doesn't lose them, and your test covers that case.

We aren't changing anything here:

  • The CHANGELOG entry stays. The file and CONTRIBUTING.md still ask for it. Whether release-please makes the hand-written Unreleased section redundant is a repo housekeeping question for us, not something this PR should settle.
  • While a side command waits for the startup Hostinfo request, the map stream isn't read. If that request hangs, the 120 s watchdog restarts the poll and the carried endpoints are resent. That's a bounded cost, and main already stops reading the stream during every side request.

We'll take this up separately: routes and hostname set at runtime still aren't carried, the same way endpoints weren't. SetRoutableIPs and SetHostname record nothing (

Command::SetRoutableIPs { .. } | Command::SetHostname { .. } => {}
), and the other requests rebuild from config.advertised_routes() and config.hostname (
.routable_ips(config.advertised_routes());
,
.routable_ips(config.advertised_routes())
). We'll open an issue to move them into the carried state, at which point CarriedNetInfo (
struct CarriedNetInfo {
) should probably get a broader name.

@GeiserX
GeiserX merged commit 4168e0a into GeiserX:main Oct 29, 2026
18 checks passed
@GeiserX

GeiserX commented Oct 29, 2026

Copy link
Copy Markdown
Owner

Merged as 4168e0a. Thanks for the careful write-up and the tests, @kasper0406. The follow-ups you listed (carrying runtime routes and hostname) will go in a separate issue.

GeiserX added a commit to GeiserX/tailscaled-rs that referenced this pull request Oct 30, 2026
…ngine 0.58.4 (#662)

Engine 0.58.4 (GeiserX/tailscale-rs#662) sends the advertised endpoints on every map request. Before it, the Hostinfo update after a map-poll reconnect, and DERP-home, route and hostname updates, went out without endpoints, so control dropped them until the endpoint set changed and peers could not open a direct path.

Pin 0631a5fc (0.58.2) -> d92b8598 (0.58.4). Capver stays 125. Only engine crates move in Cargo.lock.

Signed-off-by: Sergio Fernández <9169332+GeiserX@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants