From 587c3fcac4302c1fc2130e8a794043fdfffbb80e Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Fri, 14 Feb 2020 09:28:29 -0800
Subject: [PATCH] control/controlclient, wgengine/magicsock: misc cleanups

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 control/controlclient/auto.go   |  9 ++++-----
 control/controlclient/direct.go | 19 +++++++++----------
 wgengine/magicsock/magicsock.go | 33 ++++++++++++++++-----------------
 3 files changed, 29 insertions(+), 32 deletions(-)

diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go
index 7ff11926f..ba99884bc 100644
--- a/control/controlclient/auto.go
+++ b/control/controlclient/auto.go
@@ -2,7 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Package controlclient implements the client for the IPN control plane.
+// Package controlclient implements the client for the Tailscale
+// control plane.
 //
 // It handles authentication, port picking, and collects the local
 // network configuration.
@@ -565,10 +566,8 @@ func (c *Client) Logout() {
 }
 
 func (c *Client) UpdateEndpoints(localPort uint16, endpoints []string) {
-	changed, err := c.direct.SetEndpoints(localPort, endpoints)
-	if err != nil {
-		c.sendStatus("updateEndpoints", err, "", nil)
-	} else if changed {
+	changed := c.direct.SetEndpoints(localPort, endpoints)
+	if changed {
 		c.cancelMapSafely()
 	}
 }
diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go
index 890c9a129..c22787a0b 100644
--- a/control/controlclient/direct.go
+++ b/control/controlclient/direct.go
@@ -71,7 +71,7 @@ type Direct struct {
 	expiry       *time.Time
 	hostinfo     tailcfg.Hostinfo
 	endpoints    []string
-	localPort    uint16
+	localPort    uint16 // or zero to mean auto
 }
 
 type Options struct {
@@ -363,7 +363,11 @@ func sameStrings(a, b []string) bool {
 	return true
 }
 
-func (c *Direct) newEndpoints(localPort uint16, endpoints []string) bool {
+// newEndpoints acquires c.mu and sets the local port and endpoints and reports
+// whether they've changed.
+//
+// It does not retain the provided slice.
+func (c *Direct) newEndpoints(localPort uint16, endpoints []string) (changed bool) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
@@ -372,23 +376,18 @@ func (c *Direct) newEndpoints(localPort uint16, endpoints []string) bool {
 		return false // unchanged
 	}
 	c.logf("client.newEndpoints(%v, %v)\n", localPort, endpoints)
-	if len(c.endpoints) > 0 {
-		// empty the old list without deallocating it
-		c.endpoints = c.endpoints[:0]
-	}
 	c.localPort = localPort
-	c.endpoints = append(c.endpoints, endpoints...)
+	c.endpoints = append(c.endpoints[:0], endpoints...)
 	return true // changed
 }
 
 // SetEndpoints updates the list of locally advertised endpoints.
 // It won't be replicated to the server until a *fresh* call to PollNetMap().
 // You don't need to restart PollNetMap if we return changed==false.
-func (c *Direct) SetEndpoints(localPort uint16, endpoints []string) (changed bool, err error) {
+func (c *Direct) SetEndpoints(localPort uint16, endpoints []string) (changed bool) {
 	// (no log message on function entry, because it clutters the logs
 	//  if endpoints haven't changed. newEndpoints() will log it.)
-	changed = c.newEndpoints(localPort, endpoints)
-	return changed, nil
+	return c.newEndpoints(localPort, endpoints)
 }
 
 func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkMap)) error {
diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go
index d9d03a03d..2f9dbfb07 100644
--- a/wgengine/magicsock/magicsock.go
+++ b/wgengine/magicsock/magicsock.go
@@ -72,6 +72,8 @@ type indexedAddrSet struct {
 	index int // index of map key in addr.Addrs
 }
 
+// DefaultPort is the default port to listen on.
+// The current default (zero) means to auto-select a random free port.
 const DefaultPort = 0
 
 const DefaultDERP = "https://derp.tailscale.com/derp"
@@ -559,16 +561,22 @@ func (a *AddrSet) dst() *net.UDPAddr {
 	return &a.addrs[i]
 }
 
-func (a *AddrSet) DstToBytes() []byte {
-	dst := a.dst()
-	b := append([]byte(nil), dst.IP.To4()...)
-	if len(b) == 0 {
-		b = append([]byte(nil), dst.IP...)
+// packUDPAddr packs a UDPAddr in the form wanted by WireGuard.
+func packUDPAddr(ua *net.UDPAddr) []byte {
+	ip := ua.IP.To4()
+	if ip == nil {
+		ip = ua.IP
 	}
-	b = append(b, byte(dst.Port&0xff))
-	b = append(b, byte((dst.Port>>8)&0xff))
+	b := make([]byte, 0, len(ip)+2)
+	b = append(b, ip...)
+	b = append(b, byte(ua.Port))
+	b = append(b, byte(ua.Port>>8))
 	return b
 }
+
+func (a *AddrSet) DstToBytes() []byte {
+	return packUDPAddr(a.dst())
+}
 func (a *AddrSet) DstToString() string {
 	dst := a.dst()
 	return dst.String()
@@ -730,16 +738,7 @@ func (e *singleEndpoint) DstIP() net.IP       { return (*net.UDPAddr)(e).IP }
 func (e *singleEndpoint) SrcIP() net.IP       { return nil }
 func (e *singleEndpoint) SrcToString() string { return "" }
 func (e *singleEndpoint) DstToString() string { return (*net.UDPAddr)(e).String() }
-func (e *singleEndpoint) DstToBytes() []byte {
-	addr := (*net.UDPAddr)(e)
-	out := addr.IP.To4()
-	if out == nil {
-		out = addr.IP
-	}
-	out = append(out, byte(addr.Port&0xff))
-	out = append(out, byte((addr.Port>>8)&0xff))
-	return out
-}
+func (e *singleEndpoint) DstToBytes() []byte  { return packUDPAddr((*net.UDPAddr)(e)) }
 func (e *singleEndpoint) UpdateDst(dst *net.UDPAddr) error {
 	return fmt.Errorf("magicsock.singleEndpoint(%s).UpdateDst(%s): should never be called", (*net.UDPAddr)(e), dst)
 }