ipn/ipnserver: move Windows-specific code to tailscaled_windows.go

We'll eventually remove it entirely, but for now move get it out of ipnserver
where it's distracting and move it to its sole caller.

Updates #6522

Change-Id: I9c6f6a91bf9a8e3c5ea997952b7c08c81723d447
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/tailscaled/depaware.txt

@@ -305,7 +305,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
      💣 tailscale.com/wgengine/magicsock                             from tailscale.com/ipn/ipnlocal+
         tailscale.com/wgengine/monitor                               from tailscale.com/control/controlclient+
         tailscale.com/wgengine/netlog                                from tailscale.com/wgengine
-        tailscale.com/wgengine/netstack                              from tailscale.com/cmd/tailscaled+
+        tailscale.com/wgengine/netstack                              from tailscale.com/cmd/tailscaled
         tailscale.com/wgengine/router                                from tailscale.com/ipn/ipnlocal+
         tailscale.com/wgengine/wgcfg                                 from tailscale.com/ipn/ipnlocal+
         tailscale.com/wgengine/wgcfg/nmcfg                           from tailscale.com/ipn/ipnlocal

cmd/tailscaled/tailscaled_windows.go

@@ -26,6 +26,7 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"net"
 	"net/netip"
 	"os"
 	"os/exec"
@@ -40,6 +41,7 @@ import (
 	"golang.org/x/sys/windows/svc/eventlog"
 	"golang.zx2c4.com/wireguard/windows/tunnel/winipcfg"
 	"tailscale.com/envknob"
+	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnserver"
 	"tailscale.com/ipn/store"
 	"tailscale.com/logpolicy"
@@ -403,7 +405,7 @@ func startIPNServer(ctx context.Context, logid string) error {
 		return fmt.Errorf("safesocket.Listen: %v", err)
 	}
 
-	err = ipnserver.Run(ctx, logf, ln, store, linkMon, dialer, logid, getEngine, ipnServerOpts())
+	err = ipnServerRunWithRetries(ctx, logf, ln, store, linkMon, dialer, logid, getEngine, ipnServerOpts())
 	if err != nil {
 		logf("ipnserver.Run: %v", err)
 	}
@@ -556,3 +558,116 @@ func babysitProc(ctx context.Context, args []string, logf logger.Logf) {
 		}
 	}
 }
+
+// getEngineUntilItWorksWrapper returns a getEngine wrapper that does
+// not call getEngine concurrently and stops calling getEngine once
+// it's returned a working engine.
+func getEngineUntilItWorksWrapper(getEngine func() (wgengine.Engine, *netstack.Impl, error)) func() (wgengine.Engine, *netstack.Impl, error) {
+	var mu sync.Mutex
+	var engGood wgengine.Engine
+	var nsGood *netstack.Impl
+	return func() (wgengine.Engine, *netstack.Impl, error) {
+		mu.Lock()
+		defer mu.Unlock()
+		if engGood != nil {
+			return engGood, nsGood, nil
+		}
+		e, ns, err := getEngine()
+		if err != nil {
+			return nil, nil, err
+		}
+		engGood = e
+		nsGood = ns
+		return e, ns, nil
+	}
+}
+
+// listenerWithReadyConn is a net.Listener wrapper that has
+// one net.Conn ready to be accepted already.
+type listenerWithReadyConn struct {
+	net.Listener
+
+	mu sync.Mutex
+	c  net.Conn // if non-nil, ready to be Accepted
+}
+
+func (ln *listenerWithReadyConn) Accept() (net.Conn, error) {
+	ln.mu.Lock()
+	c := ln.c
+	ln.c = nil
+	ln.mu.Unlock()
+	if c != nil {
+		return c, nil
+	}
+	return ln.Listener.Accept()
+}
+
+// ipnServerRunWithRetries runs a Tailscale backend service.
+//
+// The getEngine func is called repeatedly, once per connection, until it
+// returns an engine successfully.
+//
+// This works around issues on Windows with the wgengine.Engine/wintun creation
+// failing or hanging. See https://github.com/tailscale/tailscale/issues/6522.
+func ipnServerRunWithRetries(ctx context.Context, logf logger.Logf, ln net.Listener, store ipn.StateStore, linkMon *monitor.Mon, dialer *tsdial.Dialer, logid string, getEngine func() (wgengine.Engine, *netstack.Impl, error), opts ipnserver.Options) error {
+	getEngine = getEngineUntilItWorksWrapper(getEngine)
+	runDone := make(chan struct{})
+	defer close(runDone)
+
+	// When the context is closed or when we return, whichever is first, close our listener
+	// and all open connections.
+	go func() {
+		select {
+		case <-ctx.Done():
+		case <-runDone:
+		}
+		ln.Close()
+	}()
+	logf("Listening on %v", ln.Addr())
+
+	bo := backoff.NewBackoff("ipnserver", logf, 30*time.Second)
+	var unservedConn net.Conn // if non-nil, accepted, but hasn't served yet
+
+	eng, ns, err := getEngine()
+	if err != nil {
+		logf("ipnserver: initial getEngine call: %v", err)
+		for i := 1; ctx.Err() == nil; i++ {
+			c, err := ln.Accept()
+			if err != nil {
+				logf("%d: Accept: %v", i, err)
+				bo.BackOff(ctx, err)
+				continue
+			}
+			logf("ipnserver: try%d: trying getEngine again...", i)
+			eng, ns, err = getEngine()
+			if err == nil {
+				logf("%d: GetEngine worked; exiting failure loop", i)
+				unservedConn = c
+				break
+			}
+			logf("ipnserver%d: getEngine failed again: %v", i, err)
+			// TODO(bradfitz): queue this error up for the next IPN bus watcher call
+			// to get for the Windows GUI? We used to send it over the pre-HTTP
+			// protocol to the Windows GUI. Just close it.
+			c.Close()
+		}
+		if err := ctx.Err(); err != nil {
+			return err
+		}
+	}
+	if unservedConn != nil {
+		ln = &listenerWithReadyConn{
+			Listener: ln,
+			c:        unservedConn,
+		}
+	}
+
+	server, err := ipnserver.New(logf, logid, store, eng, dialer, opts)
+	if err != nil {
+		return err
+	}
+	if ns != nil {
+		ns.SetLocalBackend(server.LocalBackend())
+	}
+	return server.Run(ctx, ln)
+}

ipn/ipnserver/server.go

@@ -26,7 +26,6 @@ import (
 	"tailscale.com/ipn/ipnauth"
 	"tailscale.com/ipn/ipnlocal"
 	"tailscale.com/ipn/localapi"
-	"tailscale.com/logtail/backoff"
 	"tailscale.com/net/dnsfallback"
 	"tailscale.com/net/tsdial"
 	"tailscale.com/smallzstd"
@@ -35,8 +34,6 @@ import (
 	"tailscale.com/util/systemd"
 	"tailscale.com/version/distro"
 	"tailscale.com/wgengine"
-	"tailscale.com/wgengine/monitor"
-	"tailscale.com/wgengine/netstack"
 )
 
 // Options is the configuration of the Tailscale node agent.
@@ -306,73 +303,6 @@ func (s *Server) addActiveHTTPRequest(req *http.Request, ci *ipnauth.ConnIdentit
 	return onDone, nil
 }
 
-// Run runs a Tailscale backend service.
-// The getEngine func is called repeatedly, once per connection, until it returns an engine successfully.
-//
-// Deprecated: use New and Server.Run instead.
-func Run(ctx context.Context, logf logger.Logf, ln net.Listener, store ipn.StateStore, linkMon *monitor.Mon, dialer *tsdial.Dialer, logid string, getEngine func() (wgengine.Engine, *netstack.Impl, error), opts Options) error {
-	getEngine = getEngineUntilItWorksWrapper(getEngine)
-	runDone := make(chan struct{})
-	defer close(runDone)
-
-	// When the context is closed or when we return, whichever is first, close our listener
-	// and all open connections.
-	go func() {
-		select {
-		case <-ctx.Done():
-		case <-runDone:
-		}
-		ln.Close()
-	}()
-	logf("Listening on %v", ln.Addr())
-
-	bo := backoff.NewBackoff("ipnserver", logf, 30*time.Second)
-	var unservedConn net.Conn // if non-nil, accepted, but hasn't served yet
-
-	eng, ns, err := getEngine()
-	if err != nil {
-		logf("ipnserver: initial getEngine call: %v", err)
-		for i := 1; ctx.Err() == nil; i++ {
-			c, err := ln.Accept()
-			if err != nil {
-				logf("%d: Accept: %v", i, err)
-				bo.BackOff(ctx, err)
-				continue
-			}
-			logf("ipnserver: try%d: trying getEngine again...", i)
-			eng, ns, err = getEngine()
-			if err == nil {
-				logf("%d: GetEngine worked; exiting failure loop", i)
-				unservedConn = c
-				break
-			}
-			logf("ipnserver%d: getEngine failed again: %v", i, err)
-			// TODO(bradfitz): queue this error up for the next IPN bus watcher call
-			// to get for the Windows GUI? We used to send it over the pre-HTTP
-			// protocol to the Windows GUI. Just close it.
-			c.Close()
-		}
-		if err := ctx.Err(); err != nil {
-			return err
-		}
-	}
-	if unservedConn != nil {
-		ln = &listenerWithReadyConn{
-			Listener: ln,
-			c:        unservedConn,
-		}
-	}
-
-	server, err := New(logf, logid, store, eng, dialer, opts)
-	if err != nil {
-		return err
-	}
-	if ns != nil {
-		ns.SetLocalBackend(server.LocalBackend())
-	}
-	return server.Run(ctx, ln)
-}
-
 // New returns a new Server.
 //
 // To start it, use the Server.Run method.
@@ -472,29 +402,6 @@ func (s *Server) Run(ctx context.Context, ln net.Listener) error {
 	return nil
 }
 
-// getEngineUntilItWorksWrapper returns a getEngine wrapper that does
-// not call getEngine concurrently and stops calling getEngine once
-// it's returned a working engine.
-func getEngineUntilItWorksWrapper(getEngine func() (wgengine.Engine, *netstack.Impl, error)) func() (wgengine.Engine, *netstack.Impl, error) {
-	var mu sync.Mutex
-	var engGood wgengine.Engine
-	var nsGood *netstack.Impl
-	return func() (wgengine.Engine, *netstack.Impl, error) {
-		mu.Lock()
-		defer mu.Unlock()
-		if engGood != nil {
-			return engGood, nsGood, nil
-		}
-		e, ns, err := getEngine()
-		if err != nil {
-			return nil, nil, err
-		}
-		engGood = e
-		nsGood = ns
-		return e, ns, nil
-	}
-}
-
 // ServeHTMLStatus serves an HTML status page at http://localhost:41112/ for
 // Windows and via $DEBUG_LISTENER/debug/ipn when tailscaled's --debug flag
 // is used to run a debug server.
@@ -515,26 +422,6 @@ func (s *Server) ServeHTMLStatus(w http.ResponseWriter, r *http.Request) {
 	st.WriteHTML(w)
 }
 
-// listenerWithReadyConn is a net.Listener wrapper that has
-// one net.Conn ready to be accepted already.
-type listenerWithReadyConn struct {
-	net.Listener
-
-	mu sync.Mutex
-	c  net.Conn // if non-nil, ready to be Accepted
-}
-
-func (ln *listenerWithReadyConn) Accept() (net.Conn, error) {
-	ln.mu.Lock()
-	c := ln.c
-	ln.c = nil
-	ln.mu.Unlock()
-	if c != nil {
-		return c, nil
-	}
-	return ln.Listener.Accept()
-}
-
 func findTaildropDir(dg distro.Distro) (string, error) {
 	const name = "Taildrop"
 	switch dg {