wgengine: flesh out some docs

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

wgengine/faketun.go

@@ -5,9 +5,10 @@
 package wgengine
 
 import (
-	"github.com/tailscale/wireguard-go/tun"
 	"io"
 	"os"
+
+	"github.com/tailscale/wireguard-go/tun"
 )
 
 type fakeTun struct {
@@ -16,6 +17,9 @@ type fakeTun struct {
 	closechan chan struct{}
 }
 
+// NewFakeTun returns a fake TUN device that does not depend on the
+// operating system or any special permissions.
+// It primarily exists for testing.
 func NewFakeTun() tun.Device {
 	return &fakeTun{
 		datachan:  make(chan []byte),

wgengine/rusage.go

@@ -11,7 +11,10 @@ import (
 	"tailscale.com/logger"
 )
 
-func RusagePrefixLog(logf logger.Logf) func(f string, argv ...interface{}) {
+// RusagePrefixLog returns a Logf func wrapping the provided logf func that adds
+// a prefixed log message to each line with the current binary memory usage
+// and max RSS.
+func RusagePrefixLog(logf logger.Logf) logger.Logf {
 	return func(f string, argv ...interface{}) {
 		var m runtime.MemStats
 		runtime.ReadMemStats(&m)

wgengine/rusage_nowindows.go

@@ -23,7 +23,7 @@ func rusageMaxRSS() float64 {
 		rss /= 1 << 20 // ru_maxrss is bytes on darwin
 	} else {
 		// ru_maxrss is kilobytes elsewhere (linux, openbsd, etc)
-		rss /= 1024
+		rss /= 1 << 10
 	}
 	return rss
 }

wgengine/userspace.go

@@ -67,14 +67,14 @@ func NewUserspaceEngine(logf logger.Logf, tunname string, listenPort uint16, der
 
 	tuntap, err := tun.CreateTUN(tunname, device.DefaultMTU)
 	if err != nil {
-		log.Printf("CreateTUN: %v\n", err)
+		logf("CreateTUN: %v\n", err)
 		return nil, err
 	}
-	log.Printf("CreateTUN ok.\n")
+	logf("CreateTUN ok.\n")
 
 	e, err := NewUserspaceEngineAdvanced(logf, tuntap, NewUserspaceRouter, listenPort, derp)
 	if err != nil {
-		log.Printf("NewUserspaceEngineAdv: %v\n", err)
+		logf("NewUserspaceEngineAdv: %v\n", err)
 		return nil, err
 	}
 	return e, err

wgengine/watchdog.go

@@ -5,9 +5,9 @@
 package wgengine
 
 import (
-	"bytes"
 	"log"
 	"runtime/pprof"
+	"strings"
 	"time"
 
 	"github.com/tailscale/wireguard-go/wgcfg"
@@ -45,7 +45,7 @@ func (e *watchdogEngine) watchdogErr(name string, fn func() error) error {
 		t.Stop()
 		return err
 	case <-t.C:
-		buf := new(bytes.Buffer)
+		buf := new(strings.Builder)
 		pprof.Lookup("goroutine").WriteTo(buf, 1)
 		e.logf("wgengine watchdog stacks:\n%s", buf.String())
 		e.fatalf("wgengine: watchdog timeout on %s", name)

wgengine/wgengine.go

@@ -14,6 +14,10 @@ import (
 	"tailscale.com/wgengine/filter"
 )
 
+// ByteCount is the number of bytes that have been sent or received.
+//
+// TODO: why is this a type? remove?
+// TODO: document whether it's payload bytes only or if it includes framing overhead.
 type ByteCount int64
 
 type PeerStatus struct {
@@ -22,20 +26,29 @@ type PeerStatus struct {
 	NodeKey          tailcfg.NodeKey
 }
 
+// Status is the Engine status.
 type Status struct {
 	Peers      []PeerStatus
 	LocalAddrs []string // TODO(crawshaw): []wgcfg.Endpoint?
 }
 
-type StatusCallback func(s *Status, err error)
+// StatusCallback is the type of status callbacks used by
+// Engine.SetStatusCallback.
+//
+// Exactly one of Status or error is non-nil.
+type StatusCallback func(*Status, error)
 
+// RouteSettings is the full WireGuard config data (set of peers keys,
+// IP, etc in wgcfg.Config) plus the things that WireGuard doesn't do
+// itself, like DNS stuff.
 type RouteSettings struct {
-	LocalAddr  wgcfg.CIDR
+	LocalAddr  wgcfg.CIDR // TODO: why is this here? how does it differ from wgcfg.Config's info?
 	DNS        []net.IP
 	DNSDomains []string
-	Cfg        wgcfg.Config
+	Cfg        wgcfg.Config // TODO: value type here, but pointer below?
 }
 
+// OnlyRelevantParts returns a string minimally describing the route settings.
 func (rs *RouteSettings) OnlyRelevantParts() string {
 	var peers [][]wgcfg.CIDR
 	for _, p := range rs.Cfg.Peers {
@@ -45,31 +58,55 @@ func (rs *RouteSettings) OnlyRelevantParts() string {
 		rs.LocalAddr, rs.DNS, rs.DNSDomains, peers)
 }
 
+// Router is the TODO.
 type Router interface {
+	// Up brings the router up.
+	// TODO: more than once? after Close?
 	Up() error
-	SetRoutes(rs RouteSettings) error
+	// SetRoutes sets the routes.
+	// TODO: while running?
+	SetRoutes(RouteSettings) error
+	// Close closes the router.
+	// TODO: return an error? does this block?
 	Close()
 }
 
+// Engine is the Tailscale WireGuard engine interface.
 type Engine interface {
-	// Reconfigure wireguard and make sure it's running.
+	// Reconfig reconfigures WireGuard and makes sure it's running.
 	// This also handles setting up any kernel routes.
+	//
+	// The provided DNS domains are not part of wgcfg.Config, as
+	// WireGuard itself doesn't care about such things.
+	//
+	// This is called whenever the tailcontrol (control plane)
+	// sends an updated network map.
 	Reconfig(cfg *wgcfg.Config, dnsDomains []string) error
-	// Update the packet filter.
-	SetFilter(filt *filter.Filter)
-	// Set the function to call when wireguard status changes.
-	SetStatusCallback(cb StatusCallback)
-	// Request a wireguard status update right away, sent to the callback.
+
+	// SetFilter updates the packet filter.
+	SetFilter(*filter.Filter)
+
+	// SetStatusCallback sets the function to call when the
+	// WireGuard status changes.
+	SetStatusCallback(StatusCallback)
+
+	// RequestStatus requests a WireGuard status update right
+	// away, sent to the callback registered via SetStatusCallback.
 	RequestStatus()
-	// Shut down this wireguard instance, remove any routes it added, etc.
-	// To bring it up again later, you'll need a new Engine.
+
+	// Close shuts down this wireguard instance, remove any routes
+	// it added, etc. To bring it up again later, you'll need a
+	// new Engine.
 	Close()
-	// Wait until the Engine is .Close()ed or aborts with an error.
-	// You don't have to call this.
+
+	// Wait waits until the Engine's Close method is called or the
+	// engine aborts with an error. You don't have to call this.
+	// TODO: return an error?
 	Wait()
+
 	// LinkChange informs the engine that the system network
 	// link has changed. The isExpensive parameter is set on links
-	// where sending packets uses substantial power or dollars
-	// (such as LTE on a phone).
+	// where sending packets uses substantial power or money,
+	// such as mobile data on a phone.
 	LinkChange(isExpensive bool)
 }