From fe53a714bd2693f4c8e4f153a3c6dc47c93a21c0 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 23 Apr 2021 10:26:25 -0700 Subject: [PATCH] ipn/ipnlocal: add a LocalBackend.Start fast path if already running Updates tailscale/corp#1621 Signed-off-by: Brad Fitzpatrick --- ipn/backend.go | 26 +++++++++++++++----------- ipn/ipnlocal/local.go | 40 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/ipn/backend.go b/ipn/backend.go index 102dc2c97..b5aa78c2d 100644 --- a/ipn/backend.go +++ b/ipn/backend.go @@ -55,17 +55,21 @@ type EngineStatus struct { // that they have not changed. // They are JSON-encoded on the wire, despite the lack of struct tags. type Notify struct { - _ structs.Incomparable - Version string // version number of IPN backend - ErrMessage *string // critical error message, if any; for InUseOtherUser, the details - LoginFinished *empty.Message // event: non-nil when login process succeeded - State *State // current IPN state has changed - Prefs *Prefs // preferences were changed - NetMap *netmap.NetworkMap // new netmap received - Engine *EngineStatus // wireguard engine stats - BrowseToURL *string // UI should open a browser right now - BackendLogID *string // public logtail id used by backend - PingResult *ipnstate.PingResult + _ structs.Incomparable + Version string // version number of IPN backend + + // ErrMessage, if non-nil, contains a critical error message. + // For State InUseOtherUser, ErrMessage is not critical and just contains the details. + ErrMessage *string + + LoginFinished *empty.Message // non-nil when/if the login process succeeded + State *State // if non-nil, the new or current IPN state + Prefs *Prefs // if non-nil, the new or current preferences + NetMap *netmap.NetworkMap // if non-nil, the new or current netmap + Engine *EngineStatus // if non-nil, the new or urrent wireguard stats + BrowseToURL *string // if non-nil, UI should open a browser right now + BackendLogID *string // if non-nil, the public logtail ID used by backend + PingResult *ipnstate.PingResult // if non-nil, a ping response arrived // FilesWaiting if non-nil means that files are buffered in // the Tailscale daemon and ready for local transfer to the diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 48841a8ea..b69ee54d7 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -590,6 +590,24 @@ func (b *LocalBackend) SetHTTPTestClient(c *http.Client) { b.httpTestClient = c } +// startIsNoopLocked reports whether a Start call on this LocalBackend +// with the provided Start Options would be a useless no-op. +// +// b.mu must be held. +func (b *LocalBackend) startIsNoopLocked(opts ipn.Options) bool { + // Options has 4 fields; check all of them: + // * FrontendLogID + // * StateKey + // * Prefs + // * AuthKey + return b.state == ipn.Running && + b.hostinfo != nil && + b.hostinfo.FrontendLogID == opts.FrontendLogID && + b.stateKey == opts.StateKey && + opts.Prefs == nil && + opts.AuthKey == "" +} + // Start applies the configuration specified in opts, and starts the // state machine. // @@ -612,12 +630,30 @@ func (b *LocalBackend) Start(opts ipn.Options) error { b.logf("Start") } + b.mu.Lock() + + // The iOS client sends a "Start" whenever its UI screen comes + // up, just because it wants a netmap. That should be fixed, + // but meanwhile we can make Start cheaper here for such a + // case and not restart the world (which takes a few seconds). + // Instead, just send a notify with the state that iOS needs. + if b.startIsNoopLocked(opts) { + b.logf("Start: already running; sending notify") + nm := b.netMap + state := b.state + b.mu.Unlock() + b.send(ipn.Notify{ + State: &state, + NetMap: nm, + LoginFinished: new(empty.Message), + }) + return nil + } + hostinfo := controlclient.NewHostinfo() hostinfo.BackendLogID = b.backendLogID hostinfo.FrontendLogID = opts.FrontendLogID - b.mu.Lock() - if b.cc != nil { // TODO(apenwarr): avoid the need to reinit controlclient. // This will trigger a full relogin/reconfigure cycle every