From d707e2f7e524a994ce38615d74f1793784705232 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Tue, 11 May 2021 11:10:07 -0700
Subject: [PATCH 01/16] wgengine/bench: skip flaky test

Updates tailscale/corp#1776

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 wgengine/bench/bench_test.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/wgengine/bench/bench_test.go b/wgengine/bench/bench_test.go
index 253209bb6..477ea17e5 100644
--- a/wgengine/bench/bench_test.go
+++ b/wgengine/bench/bench_test.go
@@ -42,6 +42,7 @@ func BenchmarkBatchTCP(b *testing.B) {
 }
 
 func BenchmarkWireGuardTest(b *testing.B) {
+	b.Skip("flaky; see https://github.com/tailscale/corp/issues/1776")
 	run(b, func(logf logger.Logf, traf *TrafficGen) {
 		setupWGTest(b, logf, traf, Addr1, Addr2)
 	})

From 68911f677864795751025175578953b24bc6cfd6 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Tue, 11 May 2021 11:14:53 -0700
Subject: [PATCH 02/16] wgengine/bench: ignore "engine closing" errors

On benchmark completion, we shut down the wgengine.
If we happen to poll for status during shutdown,
we get an "engine closing" error.
It doesn't hurt anything; ignore it.

Fixes tailscale/corp#1776

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 wgengine/bench/wg.go  | 7 +++++++
 wgengine/userspace.go | 4 +++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/wgengine/bench/wg.go b/wgengine/bench/wg.go
index 412799e15..f4275d674 100644
--- a/wgengine/bench/wg.go
+++ b/wgengine/bench/wg.go
@@ -5,6 +5,7 @@
 package main
 
 import (
+	"errors"
 	"io"
 	"log"
 	"os"
@@ -89,6 +90,9 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netadd
 
 	var e1waitDoneOnce sync.Once
 	e1.SetStatusCallback(func(st *wgengine.Status, err error) {
+		if errors.Is(err, wgengine.ErrEngineClosing) {
+			return
+		}
 		if err != nil {
 			log.Fatalf("e1 status err: %v", err)
 		}
@@ -124,6 +128,9 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netadd
 
 	var e2waitDoneOnce sync.Once
 	e2.SetStatusCallback(func(st *wgengine.Status, err error) {
+		if errors.Is(err, wgengine.ErrEngineClosing) {
+			return
+		}
 		if err != nil {
 			log.Fatalf("e2 status err: %v", err)
 		}
diff --git a/wgengine/userspace.go b/wgengine/userspace.go
index d90cb8533..4a7f56545 100644
--- a/wgengine/userspace.go
+++ b/wgengine/userspace.go
@@ -880,6 +880,8 @@ func (e *userspaceEngine) getStatusCallback() StatusCallback {
 
 var singleNewline = []byte{'\n'}
 
+var ErrEngineClosing = errors.New("engine closing; no status")
+
 func (e *userspaceEngine) getStatus() (*Status, error) {
 	// Grab derpConns before acquiring wgLock to not violate lock ordering;
 	// the DERPs method acquires magicsock.Conn.mu.
@@ -893,7 +895,7 @@ func (e *userspaceEngine) getStatus() (*Status, error) {
 	closing := e.closing
 	e.mu.Unlock()
 	if closing {
-		return nil, errors.New("engine closing; no status")
+		return nil, ErrEngineClosing
 	}
 
 	if e.wgdev == nil {

From 773fcfd0072483373ae7a77c72661f144f648982 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Tue, 11 May 2021 11:17:49 -0700
Subject: [PATCH 03/16] Revert "wgengine/bench: skip flaky test"

This reverts commit d707e2f7e524a994ce38615d74f1793784705232.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 wgengine/bench/bench_test.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/wgengine/bench/bench_test.go b/wgengine/bench/bench_test.go
index 477ea17e5..253209bb6 100644
--- a/wgengine/bench/bench_test.go
+++ b/wgengine/bench/bench_test.go
@@ -42,7 +42,6 @@ func BenchmarkBatchTCP(b *testing.B) {
 }
 
 func BenchmarkWireGuardTest(b *testing.B) {
-	b.Skip("flaky; see https://github.com/tailscale/corp/issues/1776")
 	run(b, func(logf logger.Logf, traf *TrafficGen) {
 		setupWGTest(b, logf, traf, Addr1, Addr2)
 	})

From cb97062baca55dc5ce7ca4bd5cbbc5cd738363ca Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Tue, 11 May 2021 11:33:04 -0700
Subject: [PATCH 04/16] go.mod: bump inet.af/netaddr

For IPPort.MarshalText optimizations.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 go.mod | 2 +-
 go.sum | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/go.mod b/go.mod
index 29e42c2ed..3378debf4 100644
--- a/go.mod
+++ b/go.mod
@@ -40,7 +40,7 @@ require (
 	golang.zx2c4.com/wireguard/windows v0.1.2-0.20201113162609-9b85be97fdf8
 	gopkg.in/yaml.v2 v2.2.8 // indirect
 	honnef.co/go/tools v0.1.0
-	inet.af/netaddr v0.0.0-20210508014949-da1c2a70a83d
+	inet.af/netaddr v0.0.0-20210511181906-37180328850c
 	inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22
 	inet.af/peercred v0.0.0-20210302202138-56e694897155
 	inet.af/wf v0.0.0-20210424212123-eaa011a774a4
diff --git a/go.sum b/go.sum
index 062e51705..d7017de21 100644
--- a/go.sum
+++ b/go.sum
@@ -256,6 +256,8 @@ honnef.co/go/tools v0.1.0/go.mod h1:XtegFAyX/PfluP4921rXU5IkjkqBCDnUq4W8VCIoKvM=
 inet.af/netaddr v0.0.0-20210222205655-a1ec2b7b8c44/go.mod h1:I2i9ONCXRZDnG1+7O8fSuYzjcPxHQXrIfzD/IkR87x4=
 inet.af/netaddr v0.0.0-20210508014949-da1c2a70a83d h1:9tuJMxDV7THGfXWirKBD/v9rbsBC21bHd2eEYsYuIek=
 inet.af/netaddr v0.0.0-20210508014949-da1c2a70a83d/go.mod h1:z0nx+Dh+7N7CC8V5ayHtHGpZpxLQZZxkIaaz6HN65Ls=
+inet.af/netaddr v0.0.0-20210511181906-37180328850c h1:rzDy/tC8LjEdN94+i0Bu22tTo/qE9cvhKyfD0HMU0NU=
+inet.af/netaddr v0.0.0-20210511181906-37180328850c/go.mod h1:z0nx+Dh+7N7CC8V5ayHtHGpZpxLQZZxkIaaz6HN65Ls=
 inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22 h1:DNtszwGa6w76qlIr+PbPEnlBJdiRV8SaxeigOy0q1gg=
 inet.af/netstack v0.0.0-20210317161235-a1bf4e56ef22/go.mod h1:GVx+5OZtbG4TVOW5ilmyRZAZXr1cNwfqUEkTOtWK0PM=
 inet.af/peercred v0.0.0-20210302202138-56e694897155 h1:KojYNEYqDkZ2O3LdyTstR1l13L3ePKTIEM2h7ONkfkE=

From 7891b34266728fcfd0c82f8d76a183ebb84981ef Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Fri, 7 May 2021 18:05:04 -0700
Subject: [PATCH 05/16] internal/deepprint: add BenchmarkHash

deepprint currently accounts for 15% of allocs in tailscaled.
This is a useful benchmark to have.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 internal/deepprint/deepprint_test.go | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/internal/deepprint/deepprint_test.go b/internal/deepprint/deepprint_test.go
index d7576f27a..0b2d25423 100644
--- a/internal/deepprint/deepprint_test.go
+++ b/internal/deepprint/deepprint_test.go
@@ -62,3 +62,11 @@ func getVal() []interface{} {
 		},
 	}
 }
+
+func BenchmarkHash(b *testing.B) {
+	b.ReportAllocs()
+	v := getVal()
+	for i := 0; i < b.N; i++ {
+		Hash(v)
+	}
+}

From 752f8c0f2fcdefb83e5a8bcbc53ff444b6fa69e4 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Mon, 10 May 2021 13:29:56 -0700
Subject: [PATCH 06/16] internal/deepprint: buffer writes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The sha256 hash writer doesn't implement WriteString.
(See https://github.com/golang/go/issues/38776.)
As a consequence, we end up converting many strings to []byte.

Wrapping a bufio.Writer around the hash writer lets us
avoid these conversions by using WriteString.

Using a bufio.Writer is, perhaps surprisingly, almost as cheap as using unsafe.
The reason is that the sha256 writer does internal buffering,
but doesn't do any when handed larger writers.
Using a bufio.Writer merely shifts the data copying from one buffer
to a different one.

Using a concrete type for Print and print cuts 10% off of the execution time.

name    old time/op    new time/op    delta
Hash-8    15.3µs ± 0%    11.5µs ± 0%  -24.84%  (p=0.000 n=10+10)

name    old alloc/op   new alloc/op   delta
Hash-8    2.82kB ± 0%    1.98kB ± 0%  -29.57%  (p=0.000 n=10+10)

name    old allocs/op  new allocs/op  delta
Hash-8       140 ± 0%        82 ± 0%  -41.43%  (p=0.000 n=10+10)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 internal/deepprint/deepprint.go      | 16 ++++++++++------
 internal/deepprint/deepprint_test.go |  5 -----
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/internal/deepprint/deepprint.go b/internal/deepprint/deepprint.go
index 75088d553..580b85237 100644
--- a/internal/deepprint/deepprint.go
+++ b/internal/deepprint/deepprint.go
@@ -12,15 +12,18 @@
 package deepprint
 
 import (
+	"bufio"
 	"crypto/sha256"
 	"fmt"
-	"io"
 	"reflect"
 )
 
 func Hash(v ...interface{}) string {
 	h := sha256.New()
-	Print(h, v)
+	// 64 matches the chunk size in crypto/sha256/sha256.go
+	b := bufio.NewWriterSize(h, 64)
+	Print(b, v)
+	b.Flush()
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
@@ -34,11 +37,11 @@ func UpdateHash(last *string, v ...interface{}) (changed bool) {
 	return false
 }
 
-func Print(w io.Writer, v ...interface{}) {
+func Print(w *bufio.Writer, v ...interface{}) {
 	print(w, reflect.ValueOf(v), make(map[uintptr]bool))
 }
 
-func print(w io.Writer, v reflect.Value, visited map[uintptr]bool) {
+func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool) {
 	if !v.IsValid() {
 		return
 	}
@@ -58,7 +61,8 @@ func print(w io.Writer, v reflect.Value, visited map[uintptr]bool) {
 		t := v.Type()
 		for i, n := 0, v.NumField(); i < n; i++ {
 			sf := t.Field(i)
-			fmt.Fprintf(w, "%s: ", sf.Name)
+			w.WriteString(sf.Name)
+			w.WriteString(": ")
 			print(w, v.Field(i), visited)
 			fmt.Fprintf(w, "\n")
 		}
@@ -88,7 +92,7 @@ func print(w io.Writer, v reflect.Value, visited map[uintptr]bool) {
 		fmt.Fprintf(w, "}\n")
 
 	case reflect.String:
-		fmt.Fprintf(w, "%s", v.String())
+		w.WriteString(v.String())
 	case reflect.Bool:
 		fmt.Fprintf(w, "%v", v.Bool())
 	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
diff --git a/internal/deepprint/deepprint_test.go b/internal/deepprint/deepprint_test.go
index 0b2d25423..796930fb8 100644
--- a/internal/deepprint/deepprint_test.go
+++ b/internal/deepprint/deepprint_test.go
@@ -5,7 +5,6 @@
 package deepprint
 
 import (
-	"bytes"
 	"testing"
 
 	"inet.af/netaddr"
@@ -18,10 +17,6 @@ func TestDeepPrint(t *testing.T) {
 	// Mostly we're just testing that we don't panic on handled types.
 	v := getVal()
 
-	var buf bytes.Buffer
-	Print(&buf, v)
-	t.Logf("Got: %s", buf.Bytes())
-
 	hash1 := Hash(v)
 	t.Logf("hash: %v", hash1)
 	for i := 0; i < 20; i++ {

From d4f805339e11d8b15c67b4e9a0da0cb2e2036c81 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Mon, 10 May 2021 14:15:31 -0700
Subject: [PATCH 07/16] internal/deepprint: special-case some common types
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

These show up a lot in our data structures.

name    old time/op    new time/op    delta
Hash-8    11.5µs ± 1%     7.8µs ± 1%  -32.17%  (p=0.000 n=10+10)

name    old alloc/op   new alloc/op   delta
Hash-8    1.98kB ± 0%    1.67kB ± 0%  -15.73%  (p=0.000 n=10+10)

name    old allocs/op  new allocs/op  delta
Hash-8      82.0 ± 0%      53.0 ± 0%  -35.37%  (p=0.000 n=10+10)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 internal/deepprint/deepprint.go | 75 +++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/internal/deepprint/deepprint.go b/internal/deepprint/deepprint.go
index 580b85237..f0e93980c 100644
--- a/internal/deepprint/deepprint.go
+++ b/internal/deepprint/deepprint.go
@@ -16,6 +16,10 @@
 	"crypto/sha256"
 	"fmt"
 	"reflect"
+
+	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
+	"tailscale.com/types/wgkey"
 )
 
 func Hash(v ...interface{}) string {
@@ -41,10 +45,81 @@ func Print(w *bufio.Writer, v ...interface{}) {
 	print(w, reflect.ValueOf(v), make(map[uintptr]bool))
 }
 
+var (
+	netaddrIPType       = reflect.TypeOf(netaddr.IP{})
+	netaddrIPPrefix     = reflect.TypeOf(netaddr.IPPrefix{})
+	wgkeyKeyType        = reflect.TypeOf(wgkey.Key{})
+	wgkeyPrivateType    = reflect.TypeOf(wgkey.Private{})
+	tailcfgDiscoKeyType = reflect.TypeOf(tailcfg.DiscoKey{})
+)
+
 func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool) {
 	if !v.IsValid() {
 		return
 	}
+
+	// Special case some common types.
+	if v.CanInterface() {
+		switch v.Type() {
+		case netaddrIPType:
+			var b []byte
+			var err error
+			if v.CanAddr() {
+				x := v.Addr().Interface().(*netaddr.IP)
+				b, err = x.MarshalText()
+			} else {
+				x := v.Interface().(netaddr.IP)
+				b, err = x.MarshalText()
+			}
+			if err == nil {
+				w.Write(b)
+				return
+			}
+		case netaddrIPPrefix:
+			var b []byte
+			var err error
+			if v.CanAddr() {
+				x := v.Addr().Interface().(*netaddr.IPPrefix)
+				b, err = x.MarshalText()
+			} else {
+				x := v.Interface().(netaddr.IPPrefix)
+				b, err = x.MarshalText()
+			}
+			if err == nil {
+				w.Write(b)
+				return
+			}
+		case wgkeyKeyType:
+			if v.CanAddr() {
+				x := v.Addr().Interface().(*wgkey.Key)
+				w.Write(x[:])
+			} else {
+				x := v.Interface().(wgkey.Key)
+				w.Write(x[:])
+			}
+			return
+		case wgkeyPrivateType:
+			if v.CanAddr() {
+				x := v.Addr().Interface().(*wgkey.Private)
+				w.Write(x[:])
+			} else {
+				x := v.Interface().(wgkey.Private)
+				w.Write(x[:])
+			}
+			return
+		case tailcfgDiscoKeyType:
+			if v.CanAddr() {
+				x := v.Addr().Interface().(*tailcfg.DiscoKey)
+				w.Write(x[:])
+			} else {
+				x := v.Interface().(tailcfg.DiscoKey)
+				w.Write(x[:])
+			}
+			return
+		}
+	}
+
+	// Generic handling.
 	switch v.Kind() {
 	default:
 		panic(fmt.Sprintf("unhandled kind %v for type %v", v.Kind(), v.Type()))

From dfa0c9095505f447e04ddea32939ee8c5f876699 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Mon, 10 May 2021 14:35:45 -0700
Subject: [PATCH 08/16] internal/deepprint: replace Fprintf(w, const) with
 w.WriteString
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

name    old time/op    new time/op    delta
Hash-8    7.77µs ± 0%    6.29µs ± 1%  -19.11%  (p=0.000 n=9+10)

name    old alloc/op   new alloc/op   delta
Hash-8    1.67kB ± 0%    1.67kB ± 0%     ~     (all equal)

name    old allocs/op  new allocs/op  delta
Hash-8      53.0 ± 0%      53.0 ± 0%     ~     (all equal)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 internal/deepprint/deepprint.go | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/internal/deepprint/deepprint.go b/internal/deepprint/deepprint.go
index f0e93980c..ddc48d2fc 100644
--- a/internal/deepprint/deepprint.go
+++ b/internal/deepprint/deepprint.go
@@ -132,14 +132,14 @@ func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool) {
 		print(w, v.Elem(), visited)
 		return
 	case reflect.Struct:
-		fmt.Fprintf(w, "struct{\n")
+		w.WriteString("struct{\n")
 		t := v.Type()
 		for i, n := 0, v.NumField(); i < n; i++ {
 			sf := t.Field(i)
 			w.WriteString(sf.Name)
 			w.WriteString(": ")
 			print(w, v.Field(i), visited)
-			fmt.Fprintf(w, "\n")
+			w.WriteString("\n")
 		}
 	case reflect.Slice, reflect.Array:
 		if v.Type().Elem().Kind() == reflect.Uint8 && v.CanInterface() {
@@ -150,9 +150,9 @@ func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool) {
 		for i, ln := 0, v.Len(); i < ln; i++ {
 			fmt.Fprintf(w, " [%d]: ", i)
 			print(w, v.Index(i), visited)
-			fmt.Fprintf(w, "\n")
+			w.WriteString("\n")
 		}
-		fmt.Fprintf(w, "}\n")
+		w.WriteString("}\n")
 	case reflect.Interface:
 		print(w, v.Elem(), visited)
 	case reflect.Map:
@@ -160,12 +160,11 @@ func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool) {
 		fmt.Fprintf(w, "map[%d]{\n", len(sm.Key))
 		for i, k := range sm.Key {
 			print(w, k, visited)
-			fmt.Fprintf(w, ": ")
+			w.WriteString(": ")
 			print(w, sm.Value[i], visited)
-			fmt.Fprintf(w, "\n")
+			w.WriteString("\n")
 		}
-		fmt.Fprintf(w, "}\n")
-
+		w.WriteString("}\n")
 	case reflect.String:
 		w.WriteString(v.String())
 	case reflect.Bool:

From 8368bac847c4cdae9d2ecd652a28a19662bdd50a Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Mon, 10 May 2021 17:51:25 -0700
Subject: [PATCH 09/16] internal/deepprint: stop printing struct field names
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The struct field names don't change within a single run,
so they are irrelevant. Use the field index instead.

name    old time/op    new time/op    delta
Hash-8    6.52µs ± 0%    6.64µs ± 0%   +1.91%  (p=0.000 n=6+9)

name    old alloc/op   new alloc/op   delta
Hash-8    1.67kB ± 0%    1.54kB ± 0%   -7.66%  (p=0.000 n=10+10)

name    old allocs/op  new allocs/op  delta
Hash-8      53.0 ± 0%      37.0 ± 0%  -30.19%  (p=0.000 n=10+10)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 internal/deepprint/deepprint.go | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/internal/deepprint/deepprint.go b/internal/deepprint/deepprint.go
index ddc48d2fc..ef846fce6 100644
--- a/internal/deepprint/deepprint.go
+++ b/internal/deepprint/deepprint.go
@@ -133,11 +133,8 @@ func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool) {
 		return
 	case reflect.Struct:
 		w.WriteString("struct{\n")
-		t := v.Type()
 		for i, n := 0, v.NumField(); i < n; i++ {
-			sf := t.Field(i)
-			w.WriteString(sf.Name)
-			w.WriteString(": ")
+			fmt.Fprintf(w, " [%d]: ", i)
 			print(w, v.Field(i), visited)
 			w.WriteString("\n")
 		}

From 712774a6979607214caf4ec6c1adee644fc6c2ec Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Mon, 10 May 2021 17:52:29 -0700
Subject: [PATCH 10/16] internal/deepprint: close struct curly parens
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Not that it matters, but we were missing a close parens.
It's cheap, so add it.

name    old time/op    new time/op    delta
Hash-8    6.64µs ± 0%    6.67µs ± 1%  +0.42%  (p=0.008 n=9+10)

name    old alloc/op   new alloc/op   delta
Hash-8    1.54kB ± 0%    1.54kB ± 0%    ~     (all equal)

name    old allocs/op  new allocs/op  delta
Hash-8      37.0 ± 0%      37.0 ± 0%    ~     (all equal)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 internal/deepprint/deepprint.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/internal/deepprint/deepprint.go b/internal/deepprint/deepprint.go
index ef846fce6..d2be04440 100644
--- a/internal/deepprint/deepprint.go
+++ b/internal/deepprint/deepprint.go
@@ -138,6 +138,7 @@ func print(w *bufio.Writer, v reflect.Value, visited map[uintptr]bool) {
 			print(w, v.Field(i), visited)
 			w.WriteString("\n")
 		}
+		w.WriteString("}\n")
 	case reflect.Slice, reflect.Array:
 		if v.Type().Elem().Kind() == reflect.Uint8 && v.CanInterface() {
 			fmt.Fprintf(w, "%q", v.Interface())

From 6ab2176dc75176a8b1a8efb17c444d9197f10be6 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Tue, 11 May 2021 09:45:12 -0700
Subject: [PATCH 11/16] internal/deepprint: improve benchmark

This more closely matches our real usage of deepprint.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 internal/deepprint/deepprint_test.go | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/internal/deepprint/deepprint_test.go b/internal/deepprint/deepprint_test.go
index 796930fb8..731548f3f 100644
--- a/internal/deepprint/deepprint_test.go
+++ b/internal/deepprint/deepprint_test.go
@@ -8,6 +8,8 @@
 	"testing"
 
 	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
+	"tailscale.com/util/dnsname"
 	"tailscale.com/wgengine/router"
 	"tailscale.com/wgengine/wgcfg"
 )
@@ -44,16 +46,17 @@ func getVal() []interface{} {
 				netaddr.MustParseIPPrefix("1234::/64"),
 			},
 		},
-		map[string]string{
-			"key1": "val1",
-			"key2": "val2",
-			"key3": "val3",
-			"key4": "val4",
-			"key5": "val5",
-			"key6": "val6",
-			"key7": "val7",
-			"key8": "val8",
-			"key9": "val9",
+		map[dnsname.FQDN][]netaddr.IP{
+			dnsname.FQDN("a."): {netaddr.MustParseIP("1.2.3.4"), netaddr.MustParseIP("4.3.2.1")},
+			dnsname.FQDN("b."): {netaddr.MustParseIP("8.8.8.8"), netaddr.MustParseIP("9.9.9.9")},
+		},
+		map[dnsname.FQDN][]netaddr.IPPort{
+			dnsname.FQDN("a."): {netaddr.MustParseIPPort("1.2.3.4:11"), netaddr.MustParseIPPort("4.3.2.1:22")},
+			dnsname.FQDN("b."): {netaddr.MustParseIPPort("8.8.8.8:11"), netaddr.MustParseIPPort("9.9.9.9:22")},
+		},
+		map[tailcfg.DiscoKey]bool{
+			{1: 1}: true,
+			{1: 2}: false,
 		},
 	}
 }

From 36a26e6a71a6351aa965501c9e184e01a4c43373 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Tue, 11 May 2021 12:09:25 -0700
Subject: [PATCH 12/16] internal/deephash: rename from deepprint

Yes, it printed, but that was an implementation detail for hashing.

And coming optimization will make it print even less.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
---
 cmd/tailscaled/depaware.txt                           |  2 +-
 .../{deepprint/deepprint.go => deephash/deephash.go}  | 11 +++--------
 .../deepprint_test.go => deephash/deephash_test.go}   |  2 +-
 internal/{deepprint => deephash}/fmtsort.go           |  2 +-
 ipn/ipnlocal/local.go                                 |  4 ++--
 wgengine/userspace.go                                 |  8 ++++----
 6 files changed, 12 insertions(+), 17 deletions(-)
 rename internal/{deepprint/deepprint.go => deephash/deephash.go} (91%)
 rename internal/{deepprint/deepprint_test.go => deephash/deephash_test.go} (98%)
 rename internal/{deepprint => deephash}/fmtsort.go (99%)

diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt
index c1183bf57..5104c8c8a 100644
--- a/cmd/tailscaled/depaware.txt
+++ b/cmd/tailscaled/depaware.txt
@@ -78,7 +78,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/derp/derpmap                                   from tailscale.com/cmd/tailscaled+
         tailscale.com/disco                                          from tailscale.com/derp+
         tailscale.com/health                                         from tailscale.com/control/controlclient+
-        tailscale.com/internal/deepprint                             from tailscale.com/ipn/ipnlocal+
+        tailscale.com/internal/deephash                              from tailscale.com/ipn/ipnlocal+
         tailscale.com/ipn                                            from tailscale.com/ipn/ipnserver+
         tailscale.com/ipn/ipnlocal                                   from tailscale.com/ipn/ipnserver+
         tailscale.com/ipn/ipnserver                                  from tailscale.com/cmd/tailscaled
diff --git a/internal/deepprint/deepprint.go b/internal/deephash/deephash.go
similarity index 91%
rename from internal/deepprint/deepprint.go
rename to internal/deephash/deephash.go
index d2be04440..a82d04ba9 100644
--- a/internal/deepprint/deepprint.go
+++ b/internal/deephash/deephash.go
@@ -2,14 +2,9 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Package deepprint walks a Go value recursively, in a predictable
-// order, without looping, and prints each value out to a given
-// Writer, which is assumed to be a hash.Hash, as this package doesn't
-// format things nicely.
-//
-// This is intended as a lighter version of go-spew, etc. We don't need its
-// features when our writer is just a hash.
-package deepprint
+// Package deephash hashes a Go value recursively, in a predictable
+// order, without looping.
+package deephash
 
 import (
 	"bufio"
diff --git a/internal/deepprint/deepprint_test.go b/internal/deephash/deephash_test.go
similarity index 98%
rename from internal/deepprint/deepprint_test.go
rename to internal/deephash/deephash_test.go
index 731548f3f..06faa32ec 100644
--- a/internal/deepprint/deepprint_test.go
+++ b/internal/deephash/deephash_test.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-package deepprint
+package deephash
 
 import (
 	"testing"
diff --git a/internal/deepprint/fmtsort.go b/internal/deephash/fmtsort.go
similarity index 99%
rename from internal/deepprint/fmtsort.go
rename to internal/deephash/fmtsort.go
index 861679153..f4d3674f9 100644
--- a/internal/deepprint/fmtsort.go
+++ b/internal/deephash/fmtsort.go
@@ -10,7 +10,7 @@
 
 // This is a slightly modified fork of Go's src/internal/fmtsort/sort.go
 
-package deepprint
+package deephash
 
 import (
 	"reflect"
diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 6df575621..00065623a 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -29,7 +29,7 @@
 	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/control/controlclient"
 	"tailscale.com/health"
-	"tailscale.com/internal/deepprint"
+	"tailscale.com/internal/deephash"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/ipn/policy"
@@ -903,7 +903,7 @@ func (b *LocalBackend) updateFilter(netMap *netmap.NetworkMap, prefs *ipn.Prefs)
 	localNets := localNetsB.IPSet()
 	logNets := logNetsB.IPSet()
 
-	changed := deepprint.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, localNets.Ranges(), logNets.Ranges(), shieldsUp)
+	changed := deephash.UpdateHash(&b.filterHash, haveNetmap, addrs, packetFilter, localNets.Ranges(), logNets.Ranges(), shieldsUp)
 	if !changed {
 		return
 	}
diff --git a/wgengine/userspace.go b/wgengine/userspace.go
index 4a7f56545..38002b218 100644
--- a/wgengine/userspace.go
+++ b/wgengine/userspace.go
@@ -27,7 +27,7 @@
 	"inet.af/netaddr"
 	"tailscale.com/control/controlclient"
 	"tailscale.com/health"
-	"tailscale.com/internal/deepprint"
+	"tailscale.com/internal/deephash"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/dns"
 	"tailscale.com/net/dns/resolver"
@@ -664,7 +664,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Publ
 		}
 	}
 
-	if !deepprint.UpdateHash(&e.lastEngineSigTrim, min, trimmedDisco, trackDisco, trackIPs) {
+	if !deephash.UpdateHash(&e.lastEngineSigTrim, min, trimmedDisco, trackDisco, trackIPs) {
 		// No changes
 		return nil
 	}
@@ -785,8 +785,8 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 	}
 	e.mu.Unlock()
 
-	engineChanged := deepprint.UpdateHash(&e.lastEngineSigFull, cfg)
-	routerChanged := deepprint.UpdateHash(&e.lastRouterSig, routerCfg, dnsCfg)
+	engineChanged := deephash.UpdateHash(&e.lastEngineSigFull, cfg)
+	routerChanged := deephash.UpdateHash(&e.lastRouterSig, routerCfg, dnsCfg)
 	if !engineChanged && !routerChanged {
 		return ErrNoChanges
 	}

From 93569120531345fb32d0d6333a1c8e8f5e0031bf Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Tue, 11 May 2021 12:09:51 -0700
Subject: [PATCH 13/16] wgengine/wglog: add BenchmarkSetPeer

Because it showed up on hello profiles.

Cycle through some moderate-sized sets of peers.
This should cover the "small tweaks to netmap"
and the "up/down cycle" cases.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 wgengine/wglog/wglog_test.go | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/wgengine/wglog/wglog_test.go b/wgengine/wglog/wglog_test.go
index b804b5959..f2aad667a 100644
--- a/wgengine/wglog/wglog_test.go
+++ b/wgengine/wglog/wglog_test.go
@@ -8,6 +8,7 @@
 	"fmt"
 	"testing"
 
+	"tailscale.com/types/logger"
 	"tailscale.com/types/wgkey"
 	"tailscale.com/wgengine/wgcfg"
 	"tailscale.com/wgengine/wglog"
@@ -70,3 +71,30 @@ func stringer(s string) stringerString {
 type stringerString string
 
 func (s stringerString) String() string { return string(s) }
+
+func BenchmarkSetPeers(b *testing.B) {
+	b.ReportAllocs()
+	x := wglog.NewLogger(logger.Discard)
+	peers := [][]wgcfg.Peer{genPeers(0), genPeers(15), genPeers(16), genPeers(15)}
+	for i := 0; i < b.N; i++ {
+		for _, p := range peers {
+			x.SetPeers(p)
+		}
+	}
+}
+
+func genPeers(n int) []wgcfg.Peer {
+	if n > 32 {
+		panic("too many peers")
+	}
+	if n == 0 {
+		return nil
+	}
+	peers := make([]wgcfg.Peer, n)
+	for i := range peers {
+		var k wgkey.Key
+		k[n] = byte(n)
+		peers[i].PublicKey = k
+	}
+	return peers
+}

From 98cae48e707c2ac9c712f040998f1c8c311b6914 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Tue, 11 May 2021 12:20:00 -0700
Subject: [PATCH 14/16] wgengine/wglog: optimize wireguardGoString
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The new code is ugly, but much faster and leaner.

name        old time/op    new time/op    delta
SetPeers-8    7.81µs ± 1%    3.59µs ± 1%  -54.04%  (p=0.000 n=9+10)

name        old alloc/op   new alloc/op   delta
SetPeers-8    7.68kB ± 0%    2.53kB ± 0%  -67.08%  (p=0.000 n=10+10)

name        old allocs/op  new allocs/op  delta
SetPeers-8       237 ± 0%        99 ± 0%  -58.23%  (p=0.000 n=10+10)

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 wgengine/wglog/wglog.go | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/wgengine/wglog/wglog.go b/wgengine/wglog/wglog.go
index 9e7cc5482..4a9c5a6a9 100644
--- a/wgengine/wglog/wglog.go
+++ b/wgengine/wglog/wglog.go
@@ -85,7 +85,7 @@ func (x *Logger) SetPeers(peers []wgcfg.Peer) {
 	// Construct a new peer public key log rewriter.
 	replace := make(map[string]string)
 	for _, peer := range peers {
-		old := "peer(" + wireguardGoString(peer.PublicKey) + ")"
+		old := wireguardGoString(peer.PublicKey)
 		new := peer.PublicKey.ShortString()
 		replace[old] = new
 	}
@@ -94,10 +94,17 @@ func (x *Logger) SetPeers(peers []wgcfg.Peer) {
 
 // wireguardGoString prints p in the same format used by wireguard-go.
 func wireguardGoString(k wgkey.Key) string {
-	base64Key := base64.StdEncoding.EncodeToString(k[:])
-	abbreviatedKey := "invalid"
-	if len(base64Key) == 44 {
-		abbreviatedKey = base64Key[0:4] + "…" + base64Key[39:43]
-	}
-	return abbreviatedKey
+	const prefix = "peer("
+	b := make([]byte, len(prefix)+44)
+	copy(b, prefix)
+	r := b[len(prefix):]
+	base64.StdEncoding.Encode(r, k[:])
+	r = r[4:]
+	copy(r, "…")
+	r = r[len("…"):]
+	copy(r, b[len(prefix)+39:len(prefix)+43])
+	r = r[4:]
+	r[0] = ')'
+	r = r[1:]
+	return string(b[:len(b)-len(r)])
 }

From aacb2107aeb19a9e91627d6a9da267ad32db3759 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josharian@gmail.com>
Date: Fri, 30 Apr 2021 16:45:36 -0700
Subject: [PATCH 15/16] all: add extra information to serialized endpoints

magicsock.Conn.ParseEndpoint requires a peer's public key,
disco key, and legacy ip/ports in order to do its job.
We currently accomplish that by:

* adding the public key in our wireguard-go fork
* encoding the disco key as magic hostname
* using a bespoke comma-separated encoding

It's a bit messy.

Instead, switch to something simpler: use a json-encoded struct
containing exactly the information we need, in the form we use it.

Our wireguard-go fork still adds the public key to the
address when it passes it to ParseEndpoint, but now the code
compensating for that is just a couple of simple, well-commented lines.
Once this commit is in, we can remove that part of the fork
and remove the compensating code.

Signed-off-by: Josh Bleecher Snyder <josharian@gmail.com>
---
 internal/deephash/deephash_test.go   |  4 +-
 wgengine/bench/wg.go                 | 17 ++++-
 wgengine/magicsock/legacy.go         | 21 +++---
 wgengine/magicsock/magicsock.go      | 52 +++++++--------
 wgengine/magicsock/magicsock_test.go | 32 ++++++++--
 wgengine/userspace.go                | 48 ++------------
 wgengine/userspace_test.go           |  2 +-
 wgengine/wgcfg/clone.go              | 48 ++++++++++++--
 wgengine/wgcfg/config.go             | 95 ++++++++++++++++++++++++++--
 wgengine/wgcfg/device_test.go        |  9 +--
 wgengine/wgcfg/nmcfg/nmcfg.go        | 27 +++-----
 wgengine/wgcfg/parser.go             | 24 ++-----
 wgengine/wgcfg/parser_test.go        | 18 ------
 wgengine/wgcfg/writer.go             | 29 ++-------
 14 files changed, 242 insertions(+), 184 deletions(-)

diff --git a/internal/deephash/deephash_test.go b/internal/deephash/deephash_test.go
index 06faa32ec..909c69ec0 100644
--- a/internal/deephash/deephash_test.go
+++ b/internal/deephash/deephash_test.go
@@ -36,7 +36,9 @@ func getVal() []interface{} {
 			Addresses: []netaddr.IPPrefix{{Bits: 5, IP: netaddr.IPFrom16([16]byte{3: 3})}},
 			Peers: []wgcfg.Peer{
 				{
-					Endpoints: "foo:5",
+					Endpoints: wgcfg.Endpoints{
+						IPPorts: wgcfg.NewIPPortSet(netaddr.MustParseIPPort("42.42.42.42:5")),
+					},
 				},
 			},
 		},
diff --git a/wgengine/bench/wg.go b/wgengine/bench/wg.go
index f4275d674..21554e814 100644
--- a/wgengine/bench/wg.go
+++ b/wgengine/bench/wg.go
@@ -9,7 +9,6 @@
 	"io"
 	"log"
 	"os"
-	"strings"
 	"sync"
 	"testing"
 
@@ -99,8 +98,14 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netadd
 		logf("e1 status: %v", *st)
 
 		var eps []string
+		var ipps []netaddr.IPPort
 		for _, ep := range st.LocalAddrs {
 			eps = append(eps, ep.Addr.String())
+			ipps = append(ipps, ep.Addr)
+		}
+		endpoint := wgcfg.Endpoints{
+			PublicKey: c1.PrivateKey.Public(),
+			IPPorts:   wgcfg.NewIPPortSet(ipps...),
 		}
 
 		n := tailcfg.Node{
@@ -119,7 +124,7 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netadd
 		p := wgcfg.Peer{
 			PublicKey:  c1.PrivateKey.Public(),
 			AllowedIPs: []netaddr.IPPrefix{a1},
-			Endpoints:  strings.Join(eps, ","),
+			Endpoints:  endpoint,
 		}
 		c2.Peers = []wgcfg.Peer{p}
 		e2.Reconfig(&c2, &router.Config{}, new(dns.Config))
@@ -137,8 +142,14 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netadd
 		logf("e2 status: %v", *st)
 
 		var eps []string
+		var ipps []netaddr.IPPort
 		for _, ep := range st.LocalAddrs {
 			eps = append(eps, ep.Addr.String())
+			ipps = append(ipps, ep.Addr)
+		}
+		endpoint := wgcfg.Endpoints{
+			PublicKey: c2.PrivateKey.Public(),
+			IPPorts:   wgcfg.NewIPPortSet(ipps...),
 		}
 
 		n := tailcfg.Node{
@@ -157,7 +168,7 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netadd
 		p := wgcfg.Peer{
 			PublicKey:  c2.PrivateKey.Public(),
 			AllowedIPs: []netaddr.IPPrefix{a2},
-			Endpoints:  strings.Join(eps, ","),
+			Endpoints:  endpoint,
 		}
 		c1.Peers = []wgcfg.Peer{p}
 		e1.Reconfig(&c1, &router.Config{}, new(dns.Config))
diff --git a/wgengine/magicsock/legacy.go b/wgengine/magicsock/legacy.go
index c07166235..57b4765dd 100644
--- a/wgengine/magicsock/legacy.go
+++ b/wgengine/magicsock/legacy.go
@@ -10,7 +10,6 @@
 	"crypto/subtle"
 	"encoding/binary"
 	"errors"
-	"fmt"
 	"hash"
 	"net"
 	"strings"
@@ -27,6 +26,7 @@
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/wgkey"
+	"tailscale.com/wgengine/wgcfg"
 )
 
 var (
@@ -34,7 +34,11 @@
 	errDisabled       = errors.New("magicsock: legacy networking disabled")
 )
 
-func (c *Conn) createLegacyEndpointLocked(pk key.Public, addrs string) (conn.Endpoint, error) {
+// createLegacyEndpointLocked creates a new wireguard-go endpoint for a legacy connection.
+// pk is the public key of the remote peer. addrs is the ordered set of addresses for the remote peer.
+// rawDest is the encoded wireguard-go endpoint string. It should be treated as a black box.
+// It is provided so that addrSet.DstToString can return it when requested by wireguard-go.
+func (c *Conn) createLegacyEndpointLocked(pk key.Public, addrs wgcfg.IPPortSet, rawDest string) (conn.Endpoint, error) {
 	if c.disableLegacy {
 		return nil, errDisabled
 	}
@@ -43,18 +47,9 @@ func (c *Conn) createLegacyEndpointLocked(pk key.Public, addrs string) (conn.End
 		Logf:      c.logf,
 		publicKey: pk,
 		curAddr:   -1,
-		rawdst:    addrs,
-	}
-
-	if addrs != "" {
-		for _, ep := range strings.Split(addrs, ",") {
-			ipp, err := netaddr.ParseIPPort(ep)
-			if err != nil {
-				return nil, fmt.Errorf("bogus address %q", ep)
-			}
-			a.ipPorts = append(a.ipPorts, ipp)
-		}
+		rawdst:    rawDest,
 	}
+	a.ipPorts = append(a.ipPorts, addrs.IPPorts()...)
 
 	// If this endpoint is being updated, remember its old set of
 	// endpoints so we can remove any (from c.addrsByUDP) that are
diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go
index 2a88c1d64..575dc8ce5 100644
--- a/wgengine/magicsock/magicsock.go
+++ b/wgengine/magicsock/magicsock.go
@@ -11,6 +11,7 @@
 	"context"
 	crand "crypto/rand"
 	"encoding/binary"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"hash/fnv"
@@ -27,7 +28,6 @@
 	"time"
 
 	"github.com/tailscale/wireguard-go/conn"
-	"go4.org/mem"
 	"golang.org/x/crypto/nacl/box"
 	"golang.org/x/time/rate"
 	"inet.af/netaddr"
@@ -2736,42 +2736,36 @@ func packIPPort(ua netaddr.IPPort) []byte {
 }
 
 // ParseEndpoint is called by WireGuard to connect to an endpoint.
-//
-// keyAddrs is the 32 byte public key of the peer followed by addrs.
-// Addrs is either:
-//
-//  1) a comma-separated list of UDP ip:ports (the peer doesn't have a discovery key)
-//  2) "<hex-discovery-key>.disco.tailscale:12345", a magic value that means the peer
-//     is running code that supports active discovery, so ParseEndpoint returns
-//     a discoEndpoint.
-func (c *Conn) ParseEndpoint(keyAddrs string) (conn.Endpoint, error) {
-	if len(keyAddrs) < 32 {
-		c.logf("[unexpected] ParseEndpoint keyAddrs too short: %q", keyAddrs)
-		return nil, errors.New("endpoint string too short")
+// endpointStr is a json-serialized wgcfg.Endpoints struct.
+// (It it currently prefixed by 32 bytes of junk, but that will change shortly.)
+// If those Endpoints contain an active discovery key, ParseEndpoint returns a discoEndpoint.
+// Otherwise it returns a legacy endpoint.
+func (c *Conn) ParseEndpoint(endpointStr string) (conn.Endpoint, error) {
+	// Our wireguard-go fork prepends the public key to endpointStr.
+	// We don't need it; strip it off.
+	// This code will be deleted soon.
+	endpointStr = endpointStr[32:]
+
+	var endpoints wgcfg.Endpoints
+	err := json.Unmarshal([]byte(endpointStr), &endpoints)
+	if err != nil {
+		return nil, fmt.Errorf("magicsock: ParseEndpoint: json.Unmarshal failed on %q: %w", endpointStr, err)
 	}
-	var pk key.Public
-	copy(pk[:], keyAddrs)
-	addrs := keyAddrs[len(pk):]
+	pk := key.Public(endpoints.PublicKey)
+	discoKey := endpoints.DiscoKey
+	c.logf("magicsock: ParseEndpoint: key=%s: disco=%s ipps=%s", pk.ShortString(), discoKey.ShortString(), derpStr(endpoints.IPPorts.String()))
+
 	c.mu.Lock()
 	defer c.mu.Unlock()
-
-	c.logf("magicsock: ParseEndpoint: key=%s: %s", pk.ShortString(), derpStr(addrs))
-
-	if !strings.HasSuffix(addrs, wgcfg.EndpointDiscoSuffix) {
-		return c.createLegacyEndpointLocked(pk, addrs)
-	}
-
-	discoHex := strings.TrimSuffix(addrs, wgcfg.EndpointDiscoSuffix)
-	discoKey, err := key.NewPublicFromHexMem(mem.S(discoHex))
-	if err != nil {
-		return nil, fmt.Errorf("magicsock: invalid discokey endpoint %q for %v: %w", addrs, pk.ShortString(), err)
+	if discoKey.IsZero() {
+		return c.createLegacyEndpointLocked(pk, endpoints.IPPorts, endpointStr)
 	}
 	de := &discoEndpoint{
 		c:             c,
 		publicKey:     tailcfg.NodeKey(pk),        // peer public key (for WireGuard + DERP)
 		discoKey:      tailcfg.DiscoKey(discoKey), // for discovery mesages
 		discoShort:    tailcfg.DiscoKey(discoKey).ShortString(),
-		wgEndpoint:    addrs,
+		wgEndpoint:    endpointStr,
 		sentPing:      map[stun.TxID]sentPing{},
 		endpointState: map[netaddr.IPPort]*endpointState{},
 	}
@@ -3115,7 +3109,7 @@ type discoEndpoint struct {
 	discoKey   tailcfg.DiscoKey // for discovery mesages
 	discoShort string           // ShortString of discoKey
 	fakeWGAddr netaddr.IPPort   // the UDP address we tell wireguard-go we're using
-	wgEndpoint string           // string from ParseEndpoint: "<hex-discovery-key>.disco.tailscale:12345"
+	wgEndpoint string           // string from ParseEndpoint, holds a JSON-serialized wgcfg.Endpoints
 
 	// Owned by Conn.mu:
 	lastPingFrom netaddr.IPPort
diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go
index a98d48a45..5b6dc2f8a 100644
--- a/wgengine/magicsock/magicsock_test.go
+++ b/wgengine/magicsock/magicsock_test.go
@@ -10,6 +10,7 @@
 	crand "crypto/rand"
 	"crypto/tls"
 	"encoding/binary"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io/ioutil"
@@ -468,10 +469,14 @@ func makeConfigs(t *testing.T, addrs []netaddr.IPPort) []wgcfg.Config {
 			if peerNum == i {
 				continue
 			}
+			publicKey := privKeys[peerNum].Public()
 			peer := wgcfg.Peer{
-				PublicKey:           privKeys[peerNum].Public(),
-				AllowedIPs:          addresses[peerNum],
-				Endpoints:           addr.String(),
+				PublicKey:  publicKey,
+				AllowedIPs: addresses[peerNum],
+				Endpoints: wgcfg.Endpoints{
+					PublicKey: publicKey,
+					IPPorts:   wgcfg.NewIPPortSet(addr),
+				},
 				PersistentKeepalive: 25,
 			}
 			cfg.Peers = append(cfg.Peers, peer)
@@ -1242,6 +1247,23 @@ func newNonLegacyTestConn(t testing.TB) *Conn {
 	return conn
 }
 
+func makeEndpoint(tb testing.TB, public tailcfg.NodeKey, disco tailcfg.DiscoKey) string {
+	tb.Helper()
+	ep := wgcfg.Endpoints{
+		PublicKey: wgkey.Key(public),
+		DiscoKey:  disco,
+	}
+	buf, err := json.Marshal(ep)
+	if err != nil {
+		tb.Fatal(err)
+	}
+	// Our wireguard-go fork prepends the public key when calling ParseEndpoint.
+	// We no longer use it; add some junk there to emulate wireguard-go.
+	// This will go away soon.
+	junk := strings.Repeat(" ", 32)
+	return junk + string(buf)
+}
+
 // addTestEndpoint sets conn's network map to a single peer expected
 // to receive packets from sendConn (or DERP), and returns that peer's
 // nodekey and discokey.
@@ -1261,7 +1283,7 @@ func addTestEndpoint(tb testing.TB, conn *Conn, sendConn net.PacketConn) (tailcf
 		},
 	})
 	conn.SetPrivateKey(wgkey.Private{0: 1})
-	_, err := conn.ParseEndpoint(string(nodeKey[:]) + "0000000000000000000000000000000000000000000000000000000000000001.disco.tailscale:12345")
+	_, err := conn.ParseEndpoint(makeEndpoint(tb, nodeKey, discoKey))
 	if err != nil {
 		tb.Fatal(err)
 	}
@@ -1435,7 +1457,7 @@ func TestSetNetworkMapChangingNodeKey(t *testing.T) {
 			},
 		},
 	})
-	_, err := conn.ParseEndpoint(string(nodeKey1[:]) + "0000000000000000000000000000000000000000000000000000000000000001.disco.tailscale:12345")
+	_, err := conn.ParseEndpoint(makeEndpoint(t, nodeKey1, discoKey))
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/wgengine/userspace.go b/wgengine/userspace.go
index 38002b218..c4f732d18 100644
--- a/wgengine/userspace.go
+++ b/wgengine/userspace.go
@@ -11,7 +11,6 @@
 	"errors"
 	"fmt"
 	"io"
-	"net"
 	"os"
 	"reflect"
 	"runtime"
@@ -502,15 +501,7 @@ func isTrimmablePeer(p *wgcfg.Peer, numPeers int) bool {
 	if forceFullWireguardConfig(numPeers) {
 		return false
 	}
-	if !isSingleEndpoint(p.Endpoints) {
-		return false
-	}
-
-	host, _, err := net.SplitHostPort(p.Endpoints)
-	if err != nil {
-		return false
-	}
-	if !strings.HasSuffix(host, ".disco.tailscale") {
+	if p.Endpoints.DiscoKey.IsZero() {
 		return false
 	}
 
@@ -580,26 +571,6 @@ func (e *userspaceEngine) isActiveSince(dk tailcfg.DiscoKey, ip netaddr.IP, t ti
 	return unixTime >= t.Unix()
 }
 
-// discoKeyFromPeer returns the DiscoKey for a wireguard config's Peer.
-//
-// Invariant: isTrimmablePeer(p) == true, so it should have 1 endpoint with
-// Host of form "<64-hex-digits>.disco.tailscale". If invariant is violated,
-// we return the zero value.
-func discoKeyFromPeer(p *wgcfg.Peer) tailcfg.DiscoKey {
-	if len(p.Endpoints) < 64 {
-		return tailcfg.DiscoKey{}
-	}
-	host, rest := p.Endpoints[:64], p.Endpoints[64:]
-	if !strings.HasPrefix(rest, ".disco.tailscale") {
-		return tailcfg.DiscoKey{}
-	}
-	k, err := key.NewPublicFromHexMem(mem.S(host))
-	if err != nil {
-		return tailcfg.DiscoKey{}
-	}
-	return tailcfg.DiscoKey(k)
-}
-
 // discoChanged are the set of peers whose disco keys have changed, implying they've restarted.
 // If a peer is in this set and was previously in the live wireguard config,
 // it needs to be first removed and then re-added to flush out its wireguard session key.
@@ -647,7 +618,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Publ
 			}
 			continue
 		}
-		dk := discoKeyFromPeer(p)
+		dk := p.Endpoints.DiscoKey
 		trackDisco = append(trackDisco, dk)
 		recentlyActive := false
 		for _, cidr := range p.AllowedIPs {
@@ -797,19 +768,19 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 	// and a second time with it.
 	discoChanged := make(map[key.Public]bool)
 	{
-		prevEP := make(map[key.Public]string)
+		prevEP := make(map[key.Public]tailcfg.DiscoKey)
 		for i := range e.lastCfgFull.Peers {
-			if p := &e.lastCfgFull.Peers[i]; isSingleEndpoint(p.Endpoints) {
-				prevEP[key.Public(p.PublicKey)] = p.Endpoints
+			if p := &e.lastCfgFull.Peers[i]; !p.Endpoints.DiscoKey.IsZero() {
+				prevEP[key.Public(p.PublicKey)] = p.Endpoints.DiscoKey
 			}
 		}
 		for i := range cfg.Peers {
 			p := &cfg.Peers[i]
-			if !isSingleEndpoint(p.Endpoints) {
+			if p.Endpoints.DiscoKey.IsZero() {
 				continue
 			}
 			pub := key.Public(p.PublicKey)
-			if old, ok := prevEP[pub]; ok && old != p.Endpoints {
+			if old, ok := prevEP[pub]; ok && old != p.Endpoints.DiscoKey {
 				discoChanged[pub] = true
 				e.logf("wgengine: Reconfig: %s changed from %q to %q", pub.ShortString(), old, p.Endpoints)
 			}
@@ -853,11 +824,6 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 	return nil
 }
 
-// isSingleEndpoint reports whether endpoints contains exactly one host:port pair.
-func isSingleEndpoint(s string) bool {
-	return s != "" && !strings.Contains(s, ",")
-}
-
 func (e *userspaceEngine) GetFilter() *filter.Filter {
 	return e.tundev.GetFilter()
 }
diff --git a/wgengine/userspace_test.go b/wgengine/userspace_test.go
index 34a331233..ea5f3ef08 100644
--- a/wgengine/userspace_test.go
+++ b/wgengine/userspace_test.go
@@ -104,7 +104,7 @@ func TestUserspaceEngineReconfig(t *testing.T) {
 					AllowedIPs: []netaddr.IPPrefix{
 						{IP: netaddr.IPv4(100, 100, 99, 1), Bits: 32},
 					},
-					Endpoints: discoHex + ".disco.tailscale:12345",
+					Endpoints: wgcfg.Endpoints{DiscoKey: dkFromHex(discoHex)},
 				},
 			},
 		}
diff --git a/wgengine/wgcfg/clone.go b/wgengine/wgcfg/clone.go
index 1b35a5d95..a29e0262d 100644
--- a/wgengine/wgcfg/clone.go
+++ b/wgengine/wgcfg/clone.go
@@ -2,12 +2,13 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// Code generated by tailscale.com/cmd/cloner -type Config,Peer; DO NOT EDIT.
+// Code generated by tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet; DO NOT EDIT.
 
 package wgcfg
 
 import (
 	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
 	"tailscale.com/types/wgkey"
 )
 
@@ -29,7 +30,7 @@ func (src *Config) Clone() *Config {
 }
 
 // A compilation failure here means this code must be regenerated, with command:
-//   tailscale.com/cmd/cloner -type Config,Peer
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
 var _ConfigNeedsRegeneration = Config(struct {
 	Name       string
 	PrivateKey wgkey.Private
@@ -48,14 +49,53 @@ func (src *Peer) Clone() *Peer {
 	dst := new(Peer)
 	*dst = *src
 	dst.AllowedIPs = append(src.AllowedIPs[:0:0], src.AllowedIPs...)
+	dst.Endpoints = *src.Endpoints.Clone()
 	return dst
 }
 
 // A compilation failure here means this code must be regenerated, with command:
-//   tailscale.com/cmd/cloner -type Config,Peer
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
 var _PeerNeedsRegeneration = Peer(struct {
 	PublicKey           wgkey.Key
 	AllowedIPs          []netaddr.IPPrefix
-	Endpoints           string
+	Endpoints           Endpoints
 	PersistentKeepalive uint16
 }{})
+
+// Clone makes a deep copy of Endpoints.
+// The result aliases no memory with the original.
+func (src *Endpoints) Clone() *Endpoints {
+	if src == nil {
+		return nil
+	}
+	dst := new(Endpoints)
+	*dst = *src
+	dst.IPPorts = *src.IPPorts.Clone()
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with command:
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
+var _EndpointsNeedsRegeneration = Endpoints(struct {
+	PublicKey wgkey.Key
+	DiscoKey  tailcfg.DiscoKey
+	IPPorts   IPPortSet
+}{})
+
+// Clone makes a deep copy of IPPortSet.
+// The result aliases no memory with the original.
+func (src *IPPortSet) Clone() *IPPortSet {
+	if src == nil {
+		return nil
+	}
+	dst := new(IPPortSet)
+	*dst = *src
+	dst.ipp = append(src.ipp[:0:0], src.ipp...)
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with command:
+//   tailscale.com/cmd/cloner -type Config,Peer,Endpoints,IPPortSet
+var _IPPortSetNeedsRegeneration = IPPortSet(struct {
+	ipp []netaddr.IPPort
+}{})
diff --git a/wgengine/wgcfg/config.go b/wgengine/wgcfg/config.go
index 3abaf48cc..31dc4b238 100644
--- a/wgengine/wgcfg/config.go
+++ b/wgengine/wgcfg/config.go
@@ -6,16 +6,15 @@
 package wgcfg
 
 import (
+	"encoding/json"
+	"strings"
+
 	"inet.af/netaddr"
+	"tailscale.com/tailcfg"
 	"tailscale.com/types/wgkey"
 )
 
-//go:generate go run tailscale.com/cmd/cloner -type=Config,Peer -output=clone.go
-
-// EndpointDiscoSuffix is appended to the hex representation of a peer's discovery key
-// and is then the sole wireguard endpoint for peers with a non-zero discovery key.
-// This form is then recognize by magicsock's ParseEndpoint.
-const EndpointDiscoSuffix = ".disco.tailscale:12345"
+//go:generate go run tailscale.com/cmd/cloner -type=Config,Peer,Endpoints,IPPortSet -output=clone.go
 
 // Config is a WireGuard configuration.
 // It only supports the set of things Tailscale uses.
@@ -31,10 +30,92 @@ type Config struct {
 type Peer struct {
 	PublicKey           wgkey.Key
 	AllowedIPs          []netaddr.IPPrefix
-	Endpoints           string // comma-separated host/port pairs: "1.2.3.4:56,[::]:80"
+	Endpoints           Endpoints
 	PersistentKeepalive uint16
 }
 
+// Endpoints represents the routes to reach a remote node.
+// It is serialized and provided to wireguard-go as a conn.Endpoint.
+type Endpoints struct {
+	// PublicKey is the public key for the remote node.
+	PublicKey wgkey.Key `json:"pk"`
+	// DiscoKey is the disco key associated with the remote node.
+	DiscoKey tailcfg.DiscoKey `json:"dk,omitempty"`
+	// IPPorts is a set of possible ip+ports the remote node can be reached at.
+	// This is used only for legacy connections to pre-disco (pre-0.100) peers.
+	IPPorts IPPortSet `json:"ipp,omitempty"`
+}
+
+func (e Endpoints) Equal(f Endpoints) bool {
+	if e.PublicKey != f.PublicKey {
+		return false
+	}
+	if e.DiscoKey != f.DiscoKey {
+		return false
+	}
+	return e.IPPorts.EqualUnordered(f.IPPorts)
+}
+
+// IPPortSet is an immutable slice of netaddr.IPPorts.
+type IPPortSet struct {
+	ipp []netaddr.IPPort
+}
+
+// NewIPPortSet returns an IPPortSet containing the ports in ipp.
+func NewIPPortSet(ipps ...netaddr.IPPort) IPPortSet {
+	return IPPortSet{ipp: append(ipps[:0:0], ipps...)}
+}
+
+// String returns a comma-separated list of all IPPorts in s.
+func (s IPPortSet) String() string {
+	buf := new(strings.Builder)
+	for i, ipp := range s.ipp {
+		if i > 0 {
+			buf.WriteByte(',')
+		}
+		buf.WriteString(ipp.String())
+	}
+	return buf.String()
+}
+
+// IPPorts returns a slice of netaddr.IPPorts containing the IPPorts in s.
+func (s IPPortSet) IPPorts() []netaddr.IPPort {
+	return append(s.ipp[:0:0], s.ipp...)
+}
+
+// EqualUnordered reports whether s and t contain the same IPPorts, regardless of order.
+func (s IPPortSet) EqualUnordered(t IPPortSet) bool {
+	if len(s.ipp) != len(t.ipp) {
+		return false
+	}
+	// Check whether the endpoints are the same, regardless of order.
+	ipps := make(map[netaddr.IPPort]int, len(s.ipp))
+	for _, ipp := range s.ipp {
+		ipps[ipp]++
+	}
+	for _, ipp := range t.ipp {
+		ipps[ipp]--
+	}
+	for _, n := range ipps {
+		if n != 0 {
+			return false
+		}
+	}
+	return true
+}
+
+// MarshalJSON marshals s into JSON.
+// It is necessary so that IPPortSet's fields can be unexported, to guarantee immutability.
+func (s IPPortSet) MarshalJSON() ([]byte, error) {
+	return json.Marshal(s.ipp)
+}
+
+// UnmarshalJSON unmarshals s from JSON.
+// It is necessary so that IPPortSet's fields can be unexported, to guarantee immutability.
+func (s *IPPortSet) UnmarshalJSON(b []byte) error {
+	return json.Unmarshal(b, &s.ipp)
+}
+
 // PeerWithKey returns the Peer with key k and reports whether it was found.
 func (config Config) PeerWithKey(k wgkey.Key) (Peer, bool) {
 	for _, p := range config.Peers {
diff --git a/wgengine/wgcfg/device_test.go b/wgengine/wgcfg/device_test.go
index 25be7f8ae..c3b8ffba4 100644
--- a/wgengine/wgcfg/device_test.go
+++ b/wgengine/wgcfg/device_test.go
@@ -10,6 +10,7 @@
 	"io"
 	"net"
 	"os"
+	"reflect"
 	"sort"
 	"strings"
 	"sync"
@@ -90,7 +91,7 @@ func TestDeviceConfig(t *testing.T) {
 				t.Errorf("on error, could not IpcGetOperation: %v", err)
 			}
 			w.Flush()
-			t.Errorf("cfg:\n%s\n---- want:\n%s\n---- uapi:\n%s", gotStr, wantStr, buf.String())
+			t.Errorf("config mismatch:\n---- got:\n%s\n---- want:\n%s\n---- uapi:\n%s", gotStr, wantStr, buf.String())
 		}
 	}
 
@@ -127,7 +128,7 @@ func TestDeviceConfig(t *testing.T) {
 	})
 
 	t.Run("device1 modify peer", func(t *testing.T) {
-		cfg1.Peers[0].Endpoints = "1.2.3.4:12345"
+		cfg1.Peers[0].Endpoints.IPPorts = NewIPPortSet(netaddr.MustParseIPPort("1.2.3.4:12345"))
 		if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil {
 			t.Fatal(err)
 		}
@@ -135,7 +136,7 @@ func TestDeviceConfig(t *testing.T) {
 	})
 
 	t.Run("device1 replace endpoint", func(t *testing.T) {
-		cfg1.Peers[0].Endpoints = "1.1.1.1:123"
+		cfg1.Peers[0].Endpoints.IPPorts = NewIPPortSet(netaddr.MustParseIPPort("1.1.1.1:123"))
 		if err := ReconfigDevice(device1, cfg1, t.Logf); err != nil {
 			t.Fatal(err)
 		}
@@ -176,7 +177,7 @@ func TestDeviceConfig(t *testing.T) {
 		}
 		peersEqual := func(p, q Peer) bool {
 			return p.PublicKey == q.PublicKey && p.PersistentKeepalive == q.PersistentKeepalive &&
-				p.Endpoints == q.Endpoints && cidrsEqual(p.AllowedIPs, q.AllowedIPs)
+				reflect.DeepEqual(p.Endpoints, q.Endpoints) && cidrsEqual(p.AllowedIPs, q.AllowedIPs)
 		}
 		if !peersEqual(peer0(origCfg), peer0(newCfg)) {
 			t.Error("reconfig modified old peer")
diff --git a/wgengine/wgcfg/nmcfg/nmcfg.go b/wgengine/wgcfg/nmcfg/nmcfg.go
index 8fe1c062a..938bd6826 100644
--- a/wgengine/wgcfg/nmcfg/nmcfg.go
+++ b/wgengine/wgcfg/nmcfg/nmcfg.go
@@ -8,8 +8,6 @@
 import (
 	"bytes"
 	"fmt"
-	"net"
-	"strconv"
 	"strings"
 
 	"inet.af/netaddr"
@@ -79,17 +77,19 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 			cpeer.PersistentKeepalive = 25 // seconds
 		}
 
-		if !peer.DiscoKey.IsZero() {
-			cpeer.Endpoints = fmt.Sprintf("%x.disco.tailscale:12345", peer.DiscoKey[:])
-		} else {
-			if err := appendEndpoint(cpeer, peer.DERP); err != nil {
+		cpeer.Endpoints = wgcfg.Endpoints{PublicKey: wgkey.Key(peer.Key), DiscoKey: peer.DiscoKey}
+		if peer.DiscoKey.IsZero() {
+			// Legacy connection. Add IP+port endpoints.
+			var ipps []netaddr.IPPort
+			if err := appendEndpoint(cpeer, &ipps, peer.DERP); err != nil {
 				return nil, err
 			}
 			for _, ep := range peer.Endpoints {
-				if err := appendEndpoint(cpeer, ep); err != nil {
+				if err := appendEndpoint(cpeer, &ipps, ep); err != nil {
 					return nil, err
 				}
 			}
+			cpeer.Endpoints.IPPorts = wgcfg.NewIPPortSet(ipps...)
 		}
 		didExitNodeWarn := false
 		for _, allowedIP := range peer.AllowedIPs {
@@ -136,21 +136,14 @@ func WGCfg(nm *netmap.NetworkMap, logf logger.Logf, flags netmap.WGConfigFlags,
 	return cfg, nil
 }
 
-func appendEndpoint(peer *wgcfg.Peer, epStr string) error {
+func appendEndpoint(peer *wgcfg.Peer, ipps *[]netaddr.IPPort, epStr string) error {
 	if epStr == "" {
 		return nil
 	}
-	_, port, err := net.SplitHostPort(epStr)
+	ipp, err := netaddr.ParseIPPort(epStr)
 	if err != nil {
 		return fmt.Errorf("malformed endpoint %q for peer %v", epStr, peer.PublicKey.ShortString())
 	}
-	_, err = strconv.ParseUint(port, 10, 16)
-	if err != nil {
-		return fmt.Errorf("invalid port in endpoint %q for peer %v", epStr, peer.PublicKey.ShortString())
-	}
-	if peer.Endpoints != "" {
-		peer.Endpoints += ","
-	}
-	peer.Endpoints += epStr
+	*ipps = append(*ipps, ipp)
 	return nil
 }
diff --git a/wgengine/wgcfg/parser.go b/wgengine/wgcfg/parser.go
index 318728a6b..55dc95012 100644
--- a/wgengine/wgcfg/parser.go
+++ b/wgengine/wgcfg/parser.go
@@ -7,6 +7,7 @@
 import (
 	"bufio"
 	"encoding/hex"
+	"encoding/json"
 	"fmt"
 	"io"
 	"net"
@@ -26,21 +27,6 @@ func (e *ParseError) Error() string {
 	return fmt.Sprintf("%s: %q", e.why, e.offender)
 }
 
-func validateEndpoints(s string) error {
-	if s == "" {
-		// Otherwise strings.Split of the empty string produces [""].
-		return nil
-	}
-	vals := strings.Split(s, ",")
-	for _, val := range vals {
-		_, _, err := parseEndpoint(val)
-		if err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func parseEndpoint(s string) (host string, port uint16, err error) {
 	i := strings.LastIndexByte(s, ':')
 	if i < 0 {
@@ -103,6 +89,7 @@ func FromUAPI(r io.Reader) (*Config, error) {
 		}
 		key := parts[0]
 		value := parts[1]
+		valueBytes := scanner.Bytes()[len(key)+1:]
 
 		if key == "public_key" {
 			if deviceConfig {
@@ -121,7 +108,7 @@ func FromUAPI(r io.Reader) (*Config, error) {
 		if deviceConfig {
 			err = cfg.handleDeviceLine(key, value)
 		} else {
-			err = cfg.handlePeerLine(peer, key, value)
+			err = cfg.handlePeerLine(peer, key, value, valueBytes)
 		}
 		if err != nil {
 			return nil, err
@@ -165,14 +152,13 @@ func (cfg *Config) handlePublicKeyLine(value string) (*Peer, error) {
 	return peer, nil
 }
 
-func (cfg *Config) handlePeerLine(peer *Peer, key, value string) error {
+func (cfg *Config) handlePeerLine(peer *Peer, key, value string, valueBytes []byte) error {
 	switch key {
 	case "endpoint":
-		err := validateEndpoints(value)
+		err := json.Unmarshal(valueBytes, &peer.Endpoints)
 		if err != nil {
 			return err
 		}
-		peer.Endpoints = value
 	case "persistent_keepalive_interval":
 		n, err := strconv.ParseUint(value, 10, 16)
 		if err != nil {
diff --git a/wgengine/wgcfg/parser_test.go b/wgengine/wgcfg/parser_test.go
index 9d4fe1992..e101a3a05 100644
--- a/wgengine/wgcfg/parser_test.go
+++ b/wgengine/wgcfg/parser_test.go
@@ -53,21 +53,3 @@ func TestParseEndpoint(t *testing.T) {
 		t.Error("Error was expected")
 	}
 }
-
-func TestValidateEndpoints(t *testing.T) {
-	tests := []struct {
-		in   string
-		want error
-	}{
-		{"", nil},
-		{"1.2.3.4:5", nil},
-		{"1.2.3.4:5,6.7.8.9:10", nil},
-		{",", &ParseError{why: "Missing port from endpoint", offender: ""}},
-	}
-	for _, tt := range tests {
-		got := validateEndpoints(tt.in)
-		if !reflect.DeepEqual(got, tt.want) {
-			t.Errorf("%q = %#v (%s); want %#v (%s)", tt.in, got, got, tt.want, tt.want)
-		}
-	}
-}
diff --git a/wgengine/wgcfg/writer.go b/wgengine/wgcfg/writer.go
index 9e3462c38..c8d09a52a 100644
--- a/wgengine/wgcfg/writer.go
+++ b/wgengine/wgcfg/writer.go
@@ -5,11 +5,10 @@
 package wgcfg
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
-	"sort"
 	"strconv"
-	"strings"
 
 	"inet.af/netaddr"
 	"tailscale.com/types/wgkey"
@@ -53,8 +52,12 @@ func (cfg *Config) ToUAPI(w io.Writer, prev *Config) error {
 		setPeer(p)
 		set("protocol_version", "1")
 
-		if !endpointsEqual(oldPeer.Endpoints, p.Endpoints) {
-			set("endpoint", p.Endpoints)
+		if !oldPeer.Endpoints.Equal(p.Endpoints) {
+			buf, err := json.Marshal(p.Endpoints)
+			if err != nil {
+				return err
+			}
+			set("endpoint", string(buf))
 		}
 
 		// TODO: replace_allowed_ips is expensive.
@@ -90,24 +93,6 @@ func (cfg *Config) ToUAPI(w io.Writer, prev *Config) error {
 	return stickyErr
 }
 
-func endpointsEqual(x, y string) bool {
-	// Cheap comparisons.
-	if x == y {
-		return true
-	}
-	xs := strings.Split(x, ",")
-	ys := strings.Split(y, ",")
-	if len(xs) != len(ys) {
-		return false
-	}
-	// Otherwise, see if they're the same, but out of order.
-	sort.Strings(xs)
-	sort.Strings(ys)
-	x = strings.Join(xs, ",")
-	y = strings.Join(ys, ",")
-	return x == y
-}
-
 func cidrsEqual(x, y []netaddr.IPPrefix) bool {
 	// TODO: re-implement using netaddr.IPSet.Equal.
 	if len(x) != len(y) {

From ebcd7ab89042fb660a96a75a360ce17140084435 Mon Sep 17 00:00:00 2001
From: Josh Bleecher Snyder <josh@tailscale.com>
Date: Tue, 11 May 2021 15:24:37 -0700
Subject: [PATCH 16/16] wgengine: remove wireguard-go DeviceOptions

We no longer need them.
This also removes the 32 bytes of prefix junk before endpoints.

Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
---
 wgengine/magicsock/magicsock.go      |  6 ------
 wgengine/magicsock/magicsock_test.go | 10 +++-------
 wgengine/userspace.go                |  4 +---
 3 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go
index 575dc8ce5..4291e13d9 100644
--- a/wgengine/magicsock/magicsock.go
+++ b/wgengine/magicsock/magicsock.go
@@ -2737,15 +2737,9 @@ func packIPPort(ua netaddr.IPPort) []byte {
 
 // ParseEndpoint is called by WireGuard to connect to an endpoint.
 // endpointStr is a json-serialized wgcfg.Endpoints struct.
-// (It it currently prefixed by 32 bytes of junk, but that will change shortly.)
 // If those Endpoints contain an active discovery key, ParseEndpoint returns a discoEndpoint.
 // Otherwise it returns a legacy endpoint.
 func (c *Conn) ParseEndpoint(endpointStr string) (conn.Endpoint, error) {
-	// Our wireguard-go fork prepends the public key to endpointStr.
-	// We don't need it; strip it off.
-	// This code will be deleted soon.
-	endpointStr = endpointStr[32:]
-
 	var endpoints wgcfg.Endpoints
 	err := json.Unmarshal([]byte(endpointStr), &endpoints)
 	if err != nil {
diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go
index 5b6dc2f8a..8545a39dc 100644
--- a/wgengine/magicsock/magicsock_test.go
+++ b/wgengine/magicsock/magicsock_test.go
@@ -168,7 +168,7 @@ func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, der
 	tsTun.SetFilter(filter.NewAllowAllForTest(logf))
 
 	wgLogger := wglog.NewLogger(logf)
-	dev := device.NewDevice(tsTun, conn.Bind(), wgLogger.DeviceLogger, new(device.DeviceOptions))
+	dev := device.NewDevice(tsTun, conn.Bind(), wgLogger.DeviceLogger)
 	dev.Up()
 
 	// Wait for magicsock to connect up to DERP.
@@ -509,7 +509,7 @@ func TestDeviceStartStop(t *testing.T) {
 
 	tun := tuntest.NewChannelTUN()
 	wgLogger := wglog.NewLogger(t.Logf)
-	dev := device.NewDevice(tun.TUN(), conn.Bind(), wgLogger.DeviceLogger, new(device.DeviceOptions))
+	dev := device.NewDevice(tun.TUN(), conn.Bind(), wgLogger.DeviceLogger)
 	dev.Up()
 	dev.Close()
 }
@@ -1257,11 +1257,7 @@ func makeEndpoint(tb testing.TB, public tailcfg.NodeKey, disco tailcfg.DiscoKey)
 	if err != nil {
 		tb.Fatal(err)
 	}
-	// Our wireguard-go fork prepends the public key when calling ParseEndpoint.
-	// We no longer use it; add some junk there to emulate wireguard-go.
-	// This will go away soon.
-	junk := strings.Repeat(" ", 32)
-	return junk + string(buf)
+	return string(buf)
 }
 
 // addTestEndpoint sets conn's network map to a single peer expected
diff --git a/wgengine/userspace.go b/wgengine/userspace.go
index c4f732d18..6e56bbd11 100644
--- a/wgengine/userspace.go
+++ b/wgengine/userspace.go
@@ -306,8 +306,6 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 	}
 
 	e.wgLogger = wglog.NewLogger(logf)
-	opts := &device.DeviceOptions{}
-
 	e.tundev.OnTSMPPongReceived = func(pong packet.TSMPPongReply) {
 		e.mu.Lock()
 		defer e.mu.Unlock()
@@ -320,7 +318,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 
 	// wgdev takes ownership of tundev, will close it when closed.
 	e.logf("Creating wireguard device...")
-	e.wgdev = device.NewDevice(e.tundev, e.magicConn.Bind(), e.wgLogger.DeviceLogger, opts)
+	e.wgdev = device.NewDevice(e.tundev, e.magicConn.Bind(), e.wgLogger.DeviceLogger)
 	closePool.addFunc(e.wgdev.Close)
 	closePool.addFunc(func() {
 		if err := e.magicConn.Close(); err != nil {