portlist: refactor, introduce OS-specific types

Add an osImpl interface that can be stateful and thus more efficient
between calls. It will later be implemented by all OSes but for now
this change only adds a Linux implementation.

Remove Port.inode. It was only used by Linux and moves into its osImpl.

Don't reopen /proc/net/* files on each run. Turns out you can just
keep then open and seek to the beginning and reread and the contents
are fresh.

    name                    old time/op    new time/op    delta
    GetListIncremental-8    7.29ms ± 2%    6.53ms ± 1%  -10.50%  (p=0.000 n=9+9)

    name                   old alloc/op   new alloc/op   delta
    GetListIncremental-8    1.30kB ±13%    0.70kB ± 5%  -46.38%  (p=0.000 n=9+10)

    name                  old allocs/op  new allocs/op  delta
    GetListIncremental-8      33.2 ±11%      18.0 ± 0%  -45.82%  (p=0.000 n=9+10)

Updates #5958

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

portlist/netstat_test.go

@@ -71,19 +71,19 @@ udp4       0      0  *.5553                 *.*
 
 func TestParsePortsNetstat(t *testing.T) {
 	want := List{
-		Port{"tcp", 22, "", ""},
-		Port{"tcp", 23, "", ""},
-		Port{"tcp", 24, "", ""},
-		Port{"tcp", 32, "sshd", ""},
-		Port{"udp", 53, "chrome", ""},
-		Port{"udp", 53, "funball", ""},
-		Port{"udp", 5050, "CDPSvc", ""},
-		Port{"udp", 5353, "", ""},
-		Port{"udp", 5354, "", ""},
-		Port{"udp", 5453, "", ""},
-		Port{"udp", 5553, "", ""},
-		Port{"tcp", 8185, "", ""}, // but not 8186, 8187, 8188 on localhost
-		Port{"udp", 9353, "iTunes", ""},
+		Port{"tcp", 22, ""},
+		Port{"tcp", 23, ""},
+		Port{"tcp", 24, ""},
+		Port{"tcp", 32, "sshd"},
+		Port{"udp", 53, "chrome"},
+		Port{"udp", 53, "funball"},
+		Port{"udp", 5050, "CDPSvc"},
+		Port{"udp", 5353, ""},
+		Port{"udp", 5354, ""},
+		Port{"udp", 5453, ""},
+		Port{"udp", 5553, ""},
+		Port{"tcp", 8185, ""}, // but not 8186, 8187, 8188 on localhost
+		Port{"udp", 9353, "iTunes"},
 	}
 
 	pl := appendParsePortsNetstat(nil, netstatOutput)

portlist/poller.go

@@ -11,6 +11,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"sync"
 	"time"
 
 	"tailscale.com/envknob"
@@ -24,6 +25,18 @@ var debugDisablePortlist = envknob.RegisterBool("TS_DEBUG_DISABLE_PORTLIST")
 type Poller struct {
 	c chan List // unbuffered
 
+	// os, if non-nil, is an OS-specific implementation of the portlist getting
+	// code. When non-nil, it's responsible for getting the complete list of
+	// cached ports complete with the process name. That is, when set,
+	// addProcesses is not used.
+	//
+	// This is part of a multi-step migration (starting 2022-10-22) to move to
+	// using osImpl for all of Linux, macOS (unsandboxed), and Windows. But
+	// during the transition period, we support this being nil.
+	// TODO(bradfitz): finish that migration.
+	os     osImpl
+	osOnce sync.Once // guards init of os
+
 	// closeCtx is the context that's canceled on Close.
 	closeCtx       context.Context
 	closeCtxCancel context.CancelFunc
@@ -33,9 +46,26 @@ type Poller struct {
 	// scatch is memory for Poller.getList to reuse between calls.
 	scratch []Port
 
-	prev List // most recent data
+	prev List // most recent data, not aliasing scratch
+}
+
+// osImpl is the OS-specific implementation of getting the open listening ports.
+type osImpl interface {
+	Close() error
+
+	// AppendListeningPorts appends to base (which must have length 0 but
+	// optional capacity) the list of listening ports. The Port struct should be
+	// populated as completely as possible. Another pass will not add anything
+	// to it.
+	//
+	// The appended ports should be in a sorted (or at least stable) order so
+	// the caller can cheaply detect when there are no changes.
+	AppendListeningPorts(base []Port) ([]Port, error)
 }
 
+// newOSImpl, if non-nil, constructs a new osImpl.
+var newOSImpl func() osImpl
+
 // NewPoller returns a new portlist Poller. It returns an error
 // if the portlist couldn't be obtained.
 func NewPoller() (*Poller, error) {
@@ -50,6 +80,7 @@ func NewPoller() (*Poller, error) {
 		runDone: make(chan struct{}),
 	}
 	p.closeCtx, p.closeCtxCancel = context.WithCancel(context.Background())
+	p.osOnce.Do(p.initOSField)
 
 	// Do one initial poll synchronously so we can return an error
 	// early.
@@ -61,6 +92,12 @@ func NewPoller() (*Poller, error) {
 	return p, nil
 }
 
+func (p *Poller) initOSField() {
+	if newOSImpl != nil {
+		p.os = newOSImpl()
+	}
+}
+
 // Updates return the channel that receives port list updates.
 //
 // The channel is closed when the Poller is closed.
@@ -71,6 +108,9 @@ func (p *Poller) Updates() <-chan List { return p.c }
 func (p *Poller) Close() error {
 	p.closeCtxCancel()
 	<-p.runDone
+	if p.os != nil {
+		p.os.Close()
+	}
 	return nil
 }
 
@@ -109,10 +149,12 @@ func (p *Poller) Run(ctx context.Context) error {
 			if err != nil {
 				return err
 			}
-			if pl.sameInodes(p.prev) {
+			if pl.equal(p.prev) {
 				continue
 			}
-			p.prev = pl
+			// New value. Make a copy, as pl might alias pl.scratch
+			// and prev must not.
+			p.prev = append([]Port(nil), pl...)
 			if sent, err := p.send(ctx, p.prev); !sent {
 				return err
 			}
@@ -128,13 +170,21 @@ func (p *Poller) getList() (List, error) {
 	if debugDisablePortlist() {
 		return nil, nil
 	}
+	p.osOnce.Do(p.initOSField)
 	var err error
+	if p.os != nil {
+		p.scratch, err = p.os.AppendListeningPorts(p.scratch[:0])
+		return p.scratch, err
+	}
+
+	// Old path for OSes that don't have osImpl yet.
+	// TODO(bradfitz): delete these when macOS and Windows are converted.
 	p.scratch, err = appendListeningPorts(p.scratch[:0])
 	if err != nil {
 		return nil, fmt.Errorf("listPorts: %s", err)
 	}
 	pl := sortAndDedup(p.scratch)
-	if pl.sameInodes(p.prev) {
+	if pl.equal(p.prev) {
 		// Nothing changed, skip inode lookup
 		return p.prev, nil
 	}

portlist/portlist.go

@@ -19,48 +19,33 @@ type Port struct {
 	Proto   string // "tcp" or "udp"
 	Port    uint16 // port number
 	Process string // optional process name, if found
-
-	inode string // OS-specific; "socket:[165614651]" on Linux
 }
 
 // List is a list of Ports.
 type List []Port
 
 func (a *Port) lessThan(b *Port) bool {
-	if a.Port < b.Port {
-		return true
-	} else if a.Port > b.Port {
-		return false
-	}
-
-	if a.Proto < b.Proto {
-		return true
-	} else if a.Proto > b.Proto {
-		return false
+	if a.Port != b.Port {
+		return a.Port < b.Port
 	}
-
-	if a.inode < b.inode {
-		return true
-	} else if a.inode > b.inode {
-		return false
+	if a.Proto != b.Proto {
+		return a.Proto < b.Proto
 	}
+	return a.Process < b.Process
+}
 
-	if a.Process < b.Process {
-		return true
-	} else if a.Process > b.Process {
-		return false
-	}
-	return false
+func (a *Port) equal(b *Port) bool {
+	return a.Port == b.Port &&
+		a.Proto == b.Proto &&
+		a.Process == b.Process
 }
 
-func (a List) sameInodes(b List) bool {
+func (a List) equal(b List) bool {
 	if len(a) != len(b) {
 		return false
 	}
 	for i := range a {
-		if a[i].Proto != b[i].Proto ||
-			a[i].Port != b[i].Port ||
-			a[i].inode != b[i].inode {
+		if !a[i].equal(&b[i]) {
 			return false
 		}
 	}
@@ -70,8 +55,8 @@ func (a List) sameInodes(b List) bool {
 func (pl List) String() string {
 	var sb strings.Builder
 	for _, v := range pl {
-		fmt.Fprintf(&sb, "%-3s %5d %-17s %#v\n",
-			v.Proto, v.Port, v.inode, v.Process)
+		fmt.Fprintf(&sb, "%-3s %5d %#v\n",
+			v.Proto, v.Port, v.Process)
 	}
 	return strings.TrimRight(sb.String(), "\n")
 }