From 4c27e2fa22ed9aa466eec362ba8caabe5bf41dc6 Mon Sep 17 00:00:00 2001
From: David Anderson <danderson@tailscale.com>
Date: Tue, 31 Aug 2021 18:09:52 -0700
Subject: [PATCH] wgengine/magicsock: remove Start method from Conn.

Over time, other magicsock refactors have made Start effectively a
no-op, except that some other functions choose to panic if called
before Start.

Signed-off-by: David Anderson <danderson@tailscale.com>
---
 wgengine/magicsock/magicsock.go      | 25 +++----------------------
 wgengine/magicsock/magicsock_test.go |  3 ---
 wgengine/userspace.go                |  1 -
 3 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go
index bb2db50d0..13f1acb8c 100644
--- a/wgengine/magicsock/magicsock.go
+++ b/wgengine/magicsock/magicsock.go
@@ -276,8 +276,7 @@ type Conn struct {
 	mu     sync.Mutex
 	muCond *sync.Cond
 
-	started bool // Start was called
-	closed  bool // Close was called
+	closed bool // Close was called
 
 	// derpCleanupTimer is the timer that fires to occasionally clean
 	// up idle DERP connections. It's only used when there is a non-home
@@ -546,17 +545,6 @@ func NewConn(opts Options) (*Conn, error) {
 	return c, nil
 }
 
-func (c *Conn) Start() {
-	c.mu.Lock()
-	if c.started {
-		panic("duplicate Start call")
-	}
-	c.started = true
-	c.mu.Unlock()
-
-	c.ReSTUN("initial")
-}
-
 // ignoreSTUNPackets sets a STUN packet processing func that does nothing.
 func (c *Conn) ignoreSTUNPackets() {
 	c.stunReceiveFunc.Store(func([]byte, netaddr.IPPort) {})
@@ -1981,9 +1969,7 @@ func (c *Conn) SetPrivateKey(privateKey wgkey.Private) error {
 	if oldKey.IsZero() {
 		c.everHadKey = true
 		c.logf("magicsock: SetPrivateKey called (init)")
-		if c.started {
-			go c.ReSTUN("set-private-key")
-		}
+		go c.ReSTUN("set-private-key")
 	} else if newKey.IsZero() {
 		c.logf("magicsock: SetPrivateKey called (zeroed)")
 		c.closeAllDerpLocked("zero-private-key")
@@ -2050,9 +2036,7 @@ func (c *Conn) SetDERPMap(dm *tailcfg.DERPMap) {
 		return
 	}
 
-	if c.started {
-		go c.ReSTUN("derp-map-update")
-	}
+	go c.ReSTUN("derp-map-update")
 }
 
 func nodesEqual(x, y []*tailcfg.Node) bool {
@@ -2476,9 +2460,6 @@ func (c *Conn) onPortMapChanged() { c.ReSTUN("portmap-changed") }
 func (c *Conn) ReSTUN(why string) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
-	if !c.started {
-		panic("call to ReSTUN before Start")
-	}
 	if c.closed {
 		// raced with a shutdown.
 		return
diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go
index cc401d933..56a157d93 100644
--- a/wgengine/magicsock/magicsock_test.go
+++ b/wgengine/magicsock/magicsock_test.go
@@ -154,7 +154,6 @@ func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, der
 	if err != nil {
 		t.Fatalf("constructing magicsock: %v", err)
 	}
-	conn.Start()
 	conn.SetDERPMap(derpMap)
 	if err := conn.SetPrivateKey(privateKey); err != nil {
 		t.Fatalf("setting private key in magicsock: %v", err)
@@ -353,7 +352,6 @@ func TestNewConn(t *testing.T) {
 	defer conn.Close()
 	conn.SetDERPMap(stuntest.DERPMapOf(stunAddr.String()))
 	conn.SetPrivateKey(wgkey.Private(key.NewPrivate()))
-	conn.Start()
 
 	go func() {
 		var pkt [64 << 10]byte
@@ -465,7 +463,6 @@ func TestDeviceStartStop(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	conn.Start()
 	defer conn.Close()
 
 	tun := tuntest.NewChannelTUN()
diff --git a/wgengine/userspace.go b/wgengine/userspace.go
index 6f37f2cd2..7e901d9d4 100644
--- a/wgengine/userspace.go
+++ b/wgengine/userspace.go
@@ -403,7 +403,6 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 	e.logf("Starting link monitor...")
 	e.linkMon.Start()
 	e.logf("Starting magicsock...")
-	e.magicConn.Start()
 	close(e.magicConnStarted)
 
 	go e.pollResolver()