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 @@ type InOut struct {
 
 func TestParsePortsNetstat(t *testing.T) {
 	want := List{
-		Port{"tcp", 22, "", ""},
+		Port{"tcp", 22, ""},
-		Port{"tcp", 23, "", ""},
+		Port{"tcp", 23, ""},
-		Port{"tcp", 24, "", ""},
+		Port{"tcp", 24, ""},
-		Port{"tcp", 32, "sshd", ""},
+		Port{"tcp", 32, "sshd"},
-		Port{"udp", 53, "chrome", ""},
+		Port{"udp", 53, "chrome"},
-		Port{"udp", 53, "funball", ""},
+		Port{"udp", 53, "funball"},
-		Port{"udp", 5050, "CDPSvc", ""},
+		Port{"udp", 5050, "CDPSvc"},
-		Port{"udp", 5353, "", ""},
+		Port{"udp", 5353, ""},
-		Port{"udp", 5354, "", ""},
+		Port{"udp", 5354, ""},
-		Port{"udp", 5453, "", ""},
+		Port{"udp", 5453, ""},
-		Port{"udp", 5553, "", ""},
+		Port{"udp", 5553, ""},
-		Port{"tcp", 8185, "", ""}, // but not 8186, 8187, 8188 on localhost
+		Port{"tcp", 8185, ""}, // but not 8186, 8187, 8188 on localhost
-		Port{"udp", 9353, "iTunes", ""},
+		Port{"udp", 9353, "iTunes"},
 	}
 
 	pl := appendParsePortsNetstat(nil, netstatOutput)

portlist/poller.go

@@ -11,6 +11,7 @@
 	"context"
 	"errors"
 	"fmt"
+	"sync"
 	"time"
 
 	"tailscale.com/envknob"
@@ -24,6 +25,18 @@
 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 {
+	if a.Port != b.Port {
-		return true
+		return a.Port < b.Port
-	} else if a.Port > b.Port {
-		return false
 	}
-
+	if a.Proto != b.Proto {
-	if a.Proto < b.Proto {
+		return a.Proto < b.Proto
-		return true
-	} else if a.Proto > b.Proto {
-		return false
 	}
-
+	return a.Process < b.Process
-	if a.inode < b.inode {
-		return true
-	} else if a.inode > b.inode {
-		return false
-	}
-
-	if a.Process < b.Process {
-		return true
-	} else if a.Process > b.Process {
-		return false
-	}
-	return false
 }
 
-func (a List) sameInodes(b List) bool {
+func (a *Port) equal(b *Port) bool {
+	return a.Port == b.Port &&
+		a.Proto == b.Proto &&
+		a.Process == b.Process
+}
+
+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 ||
+		if !a[i].equal(&b[i]) {
-			a[i].Port != b[i].Port ||
-			a[i].inode != b[i].inode {
 			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",
+		fmt.Fprintf(&sb, "%-3s %5d %#v\n",
-			v.Proto, v.Port, v.inode, v.Process)
+			v.Proto, v.Port, v.Process)
 	}
 	return strings.TrimRight(sb.String(), "\n")
 }

portlist/portlist_linux.go

@@ -7,15 +7,15 @@
 import (
 	"bufio"
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
+	"log"
 	"os"
 	"path/filepath"
 	"runtime"
 	"strconv"
 	"strings"
-	"sync"
-	"sync/atomic"
 	"syscall"
 	"time"
 	"unsafe"
@@ -25,13 +25,62 @@
 	"tailscale.com/util/mak"
 )
 
+func init() {
+	newOSImpl = newLinuxImpl
+}
+
+type linuxImpl struct {
+	procNetFiles []*os.File // seeked to start & reused between calls
+
+	known map[string]*portMeta // inode string => metadata
+	br    *bufio.Reader
+}
+
+type portMeta struct {
+	port          Port
+	keep          bool
+	needsProcName bool
+}
+
+func newLinuxImplBase() *linuxImpl {
+	return &linuxImpl{
+		br:    bufio.NewReader(eofReader),
+		known: map[string]*portMeta{},
+	}
+}
+
+func newLinuxImpl() osImpl {
+	li := newLinuxImplBase()
+	for _, name := range []string{
+		"/proc/net/tcp",
+		"/proc/net/tcp6",
+		"/proc/net/udp",
+		"/proc/net/udp6",
+	} {
+		f, err := os.Open(name)
+		if err != nil {
+			if os.IsNotExist(err) {
+				continue
+			}
+			log.Printf("portlist warning; ignoring: %v", err)
+			continue
+		}
+		li.procNetFiles = append(li.procNetFiles, f)
+	}
+	return li
+}
+
+func (li *linuxImpl) Close() error {
+	for _, f := range li.procNetFiles {
+		f.Close()
+	}
+	li.procNetFiles = nil
+	return nil
+}
+
 // Reading the sockfiles on Linux is very fast, so we can do it often.
 const pollInterval = 1 * time.Second
 
-var sockfiles = []string{"/proc/net/tcp", "/proc/net/tcp6", "/proc/net/udp", "/proc/net/udp6"}
-
-var sawProcNetPermissionErr atomic.Bool
-
 const (
 	v6Localhost = "00000000000000000000000001000000:"
 	v6Any       = "00000000000000000000000000000000:0000"
@@ -41,80 +90,68 @@
 
 var eofReader = bytes.NewReader(nil)
 
-var bufioReaderPool = &sync.Pool{
+func (li *linuxImpl) AppendListeningPorts(base []Port) ([]Port, error) {
-	New: func() any { return bufio.NewReader(eofReader) },
+	if runtime.GOOS == "android" {
-}
-
-type internedStrings struct {
-	m map[string]string
-}
-
-func (v *internedStrings) get(b []byte) string {
-	if s, ok := v.m[string(b)]; ok {
-		return s
-	}
-	s := string(b)
-	mak.Set(&v.m, s, s)
-	return s
-}
-
-var internedStringsPool = &sync.Pool{
-	New: func() any { return new(internedStrings) },
-}
-
-func appendListeningPorts(base []Port) ([]Port, error) {
-	ret := base
-	if sawProcNetPermissionErr.Load() {
-		return ret, nil
-	}
-
-	br := bufioReaderPool.Get().(*bufio.Reader)
-	defer bufioReaderPool.Put(br)
-	defer br.Reset(eofReader)
-
-	stringCache := internedStringsPool.Get().(*internedStrings)
-	defer internedStringsPool.Put(stringCache)
-
-	for _, fname := range sockfiles {
 		// Android 10+ doesn't allow access to this anymore.
 		// https://developer.android.com/about/versions/10/privacy/changes#proc-net-filesystem
 		// Ignore it rather than have the system log about our violation.
-		if runtime.GOOS == "android" && syscall.Access(fname, unix.R_OK) != nil {
+		return nil, nil
-			sawProcNetPermissionErr.Store(true)
+	}
-			return nil, nil
-		}
 
-		f, err := os.Open(fname)
+	br := li.br
-		if os.IsPermission(err) {
+	defer br.Reset(eofReader)
-			sawProcNetPermissionErr.Store(true)
+
-			return nil, nil
+	// Start by marking all previous known ports as gone. If this mark
-		}
+	// bit is still false later, we'll remove them.
+	for _, pm := range li.known {
+		pm.keep = false
+	}
+
+	for _, f := range li.procNetFiles {
+		name := f.Name()
+		_, err := f.Seek(0, io.SeekStart)
 		if err != nil {
-			return nil, fmt.Errorf("%s: %s", fname, err)
+			return nil, err
 		}
 		br.Reset(f)
-		ret, err = appendParsePorts(ret, stringCache, br, filepath.Base(fname))
+		err = li.parseProcNetFile(br, filepath.Base(name))
-		f.Close()
 		if err != nil {
-			return nil, fmt.Errorf("parsing %q: %w", fname, err)
+			return nil, fmt.Errorf("parsing %q: %w", name, err)
 		}
 	}
-	if len(stringCache.m) >= len(ret)*2 {
+
-		// Prevent unbounded growth of the internedStrings map.
+	// Delete ports that aren't open any longer.
-		stringCache.m = nil
+	// And see if there are any process names we need to look for.
+	var needProc map[string]*portMeta
+	for inode, pm := range li.known {
+		if !pm.keep {
+			delete(li.known, inode)
+			continue
+		}
+		if pm.needsProcName {
+			mak.Set(&needProc, inode, pm)
+		}
 	}
-	return ret, nil
+	err := li.findProcessNames(needProc)
+	if err != nil {
+		return nil, err
+	}
+
+	ret := base
+	for _, pm := range li.known {
+		ret = append(ret, pm.port)
+	}
+	return sortAndDedup(ret), nil
 }
 
 // fileBase is one of "tcp", "tcp6", "udp", "udp6".
-func appendParsePorts(base []Port, stringCache *internedStrings, r *bufio.Reader, fileBase string) ([]Port, error) {
+func (li *linuxImpl) parseProcNetFile(r *bufio.Reader, fileBase string) error {
 	proto := strings.TrimSuffix(fileBase, "6")
-	ret := base
 
 	// skip header row
 	_, err := r.ReadSlice('\n')
 	if err != nil {
-		return nil, err
+		return err
 	}
 
 	fields := make([]mem.RO, 0, 20) // 17 current fields + some future slop
@@ -144,7 +181,7 @@ func appendParsePorts(base []Port, stringCache *internedStrings, r *bufio.Reader
 			break
 		}
 		if err != nil {
-			return nil, err
+			return err
 		}
 		rows++
 		if rows >= maxRows {
@@ -191,30 +228,48 @@ func appendParsePorts(base []Port, stringCache *internedStrings, r *bufio.Reader
 		// allocations significant enough to show up in profiles.
 		i := mem.IndexByte(local, ':')
 		if i == -1 {
-			return nil, fmt.Errorf("%q unexpectedly didn't have a colon", local.StringCopy())
+			return fmt.Errorf("%q unexpectedly didn't have a colon", local.StringCopy())
 		}
 		portv, err := mem.ParseUint(local.SliceFrom(i+1), 16, 16)
 		if err != nil {
-			return nil, fmt.Errorf("%#v: %s", local.SliceFrom(9).StringCopy(), err)
+			return fmt.Errorf("%#v: %s", local.SliceFrom(9).StringCopy(), err)
 		}
 		inoBuf = append(inoBuf[:0], "socket:["...)
 		inoBuf = mem.Append(inoBuf, inode)
 		inoBuf = append(inoBuf, ']')
-		ret = append(ret, Port{
+
-			Proto: proto,
+		if pm, ok := li.known[string(inoBuf)]; ok {
-			Port:  uint16(portv),
+			pm.keep = true
-			inode: stringCache.get(inoBuf),
+			// Rest should be unchanged.
-		})
+		} else {
+			li.known[string(inoBuf)] = &portMeta{
+				needsProcName: true,
+				keep:          true,
+				port: Port{
+					Proto: proto,
+					Port:  uint16(portv),
+				},
+			}
+		}
 	}
 
-	return ret, nil
+	return nil
 }
 
-func addProcesses(pl []Port) ([]Port, error) {
+// errDone is an internal sentinel error that we found everything we were looking for.
-	pm := map[string]*Port{} // by Port.inode
+var errDone = errors.New("done")
-	for i := range pl {
+
-		pm[pl[i].inode] = &pl[i]
+// need is keyed by inode string.
+func (li *linuxImpl) findProcessNames(need map[string]*portMeta) error {
+	if len(need) == 0 {
+		return nil
 	}
+	defer func() {
+		// Anything we didn't find, give up on and don't try to look for it later.
+		for _, pm := range need {
+			pm.needsProcName = false
+		}
+	}()
 
 	var pathBuf []byte
 
@@ -262,7 +317,7 @@ func addProcesses(pl []Port) ([]Port, error) {
 					continue
 				}
 
-				pe := pm[string(targetBuf[:n])] // m[string([]byte)] avoids alloc
+				pe := need[string(targetBuf[:n])] // m[string([]byte)] avoids alloc
 				if pe != nil {
 					bs, err := os.ReadFile(fmt.Sprintf("/proc/%s/cmdline", pid))
 					if err != nil {
@@ -272,15 +327,20 @@ func addProcesses(pl []Port) ([]Port, error) {
 					}
 
 					argv := strings.Split(strings.TrimSuffix(string(bs), "\x00"), "\x00")
-					pe.Process = argvSubject(argv...)
+					pe.port.Process = argvSubject(argv...)
+					pe.needsProcName = false
+					delete(need, string(targetBuf[:n]))
+					if len(need) == 0 {
+						return errDone
+					}
 				}
 			}
 		}
 	})
-	if err != nil {
+	if err == errDone {
-		return nil, err
+		return nil
 	}
-	return pl, nil
+	return err
 }
 
 func foreachPID(fn func(pidStr string) error) error {
@@ -360,3 +420,11 @@ func readlink(path, buf []byte) (n int, ok bool) {
 	}
 	return n, true
 }
+
+func appendListeningPorts([]Port) ([]Port, error) {
+	panic("unused on linux; needed to compile for now")
+}
+
+func addProcesses([]Port) ([]Port, error) {
+	panic("unused on linux; needed to compile for now")
+}

portlist/portlist_linux_test.go

@@ -41,12 +41,12 @@ func TestParsePorts(t *testing.T) {
 		name string
 		in   string
 		file string
-		want []Port
+		want map[string]*portMeta
 	}{
 		{
 			name: "empty",
 			in:   "header line (ignored)\n",
-			want: nil,
+			want: map[string]*portMeta{},
 		},
 		{
 			name: "ipv4",
@@ -56,8 +56,10 @@ func TestParsePorts(t *testing.T) {
   1: 00000000:0016 00000000:0000 0A 00000000:00000000 00:00000000 00000000     0        0 34062 1 0000000000000000 100 0 0 10 0
   2: 5501A8C0:ADD4 B25E9536:01BB 01 00000000:00000000 02:00000B2B 00000000  1000        0 155276677 2 0000000000000000 22 4 30 10 -1
 `,
-			want: []Port{
+			want: map[string]*portMeta{
-				{Proto: "tcp", Port: 22, inode: "socket:[34062]"},
+				"socket:[34062]": &portMeta{
+					port: Port{Proto: "tcp", Port: 22},
+				},
 			},
 		},
 		{
@@ -69,14 +71,17 @@ func TestParsePorts(t *testing.T) {
    2: 00000000000000000000000000000000:0016 00000000000000000000000000000000:0000 0A 00000000:00000000 00:00000000 00000000     0        0 34064 1 0000000000000000 100 0 0 10 0
    3: 69050120005716BC64906EBE009ECD4D:D506 0047062600000000000000006E171268:01BB 01 00000000:00000000 02:0000009E 00000000  1000        0 151042856 2 0000000000000000 21 4 28 10 -1
 `,
-			want: []Port{
+			want: map[string]*portMeta{
-				{Proto: "tcp", Port: 8081, inode: "socket:[142240557]"},
+				"socket:[142240557]": &portMeta{
-				{Proto: "tcp", Port: 22, inode: "socket:[34064]"},
+					port: Port{Proto: "tcp", Port: 8081},
+				},
+				"socket:[34064]": &portMeta{
+					port: Port{Proto: "tcp", Port: 22},
+				},
 			},
 		},
 	}
 
-	stringCache := new(internedStrings)
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			buf := bytes.NewBufferString(tt.in)
@@ -85,12 +90,16 @@ func TestParsePorts(t *testing.T) {
 			if tt.file != "" {
 				file = tt.file
 			}
-			got, err := appendParsePorts(nil, stringCache, r, file)
+			li := newLinuxImplBase()
+			err := li.parseProcNetFile(r, file)
 			if err != nil {
 				t.Fatal(err)
 			}
-
+			for _, pm := range tt.want {
-			if diff := cmp.Diff(got, tt.want, cmp.AllowUnexported(Port{})); diff != "" {
+				pm.keep = true
+				pm.needsProcName = true
+			}
+			if diff := cmp.Diff(li.known, tt.want, cmp.AllowUnexported(Port{}), cmp.AllowUnexported(portMeta{})); diff != "" {
 				t.Errorf("unexpected parsed ports (-got+want):\n%s", diff)
 			}
 		})
@@ -110,36 +119,20 @@ func BenchmarkParsePorts(b *testing.B) {
 		contents.WriteString("   3: 69050120005716BC64906EBE009ECD4D:D506 0047062600000000000000006E171268:01BB 01 00000000:00000000 02:0000009E 00000000  1000        0 151042856 2 0000000000000000 21 4 28 10 -1\n")
 	}
 
-	want := []Port{
+	li := newLinuxImplBase()
-		{Proto: "tcp", Port: 8081, inode: "socket:[142240557]"},
-		{Proto: "tcp", Port: 22, inode: "socket:[34064]"},
-	}
 
 	r := bytes.NewReader(contents.Bytes())
 	br := bufio.NewReader(&contents)
-	stringCache := new(internedStrings)
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
 		r.Seek(0, io.SeekStart)
 		br.Reset(r)
-		got, err := appendParsePorts(nil, stringCache, br, "tcp6")
+		err := li.parseProcNetFile(br, "tcp6")
 		if err != nil {
 			b.Fatal(err)
 		}
-		if len(got) != 2 || got[0].Port != 8081 || got[1].Port != 22 {
+		if len(li.known) != 2 {
-			b.Fatalf("wrong result:\n got %+v\nwant %+v", got, want)
+			b.Fatalf("wrong results; want 2 parsed got %d", len(li.known))
-		}
-	}
-}
-
-func BenchmarkListPorts(b *testing.B) {
-	b.ReportAllocs()
-	var base []Port
-	for i := 0; i < b.N; i++ {
-		var err error
-		base, err = appendListeningPorts(base[:0])
-		if err != nil {
-			b.Fatal(err)
 		}
 	}
 }

portlist/portlist_test.go

@@ -47,7 +47,7 @@ func TestIgnoreLocallyBoundPorts(t *testing.T) {
 	}
 }
 
-func TestLessThan(t *testing.T) {
+func TestEqualLessThan(t *testing.T) {
 	tests := []struct {
 		name string
 		a, b Port
@@ -55,80 +55,62 @@ func TestLessThan(t *testing.T) {
 	}{
 		{
 			"Port a < b",
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc1"},
-			Port{Proto: "tcp", Port: 101, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 101, Process: "proc1"},
 			true,
 		},
 		{
 			"Port a > b",
-			Port{Proto: "tcp", Port: 101, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 101, Process: "proc1"},
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc1"},
 			false,
 		},
 		{
 			"Proto a < b",
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc1"},
-			Port{Proto: "udp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "udp", Port: 100, Process: "proc1"},
 			true,
 		},
 		{
 			"Proto a < b",
-			Port{Proto: "udp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "udp", Port: 100, Process: "proc1"},
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc1"},
-			false,
-		},
-		{
-			"inode a < b",
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode2"},
-			true,
-		},
-		{
-			"inode a > b",
-			Port{Proto: "tcp", Port: 100, Process: "proc2", inode: "inode2"},
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
 			false,
 		},
 		{
 			"Process a < b",
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc1"},
-			Port{Proto: "tcp", Port: 100, Process: "proc2", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc2"},
 			true,
 		},
 		{
 			"Process a > b",
-			Port{Proto: "tcp", Port: 100, Process: "proc2", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc2"},
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc1"},
 			false,
 		},
 		{
 			"Port evaluated first",
-			Port{Proto: "udp", Port: 100, Process: "proc2", inode: "inode2"},
+			Port{Proto: "udp", Port: 100, Process: "proc2"},
-			Port{Proto: "tcp", Port: 101, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 101, Process: "proc1"},
 			true,
 		},
 		{
 			"Proto evaluated second",
-			Port{Proto: "tcp", Port: 100, Process: "proc2", inode: "inode2"},
+			Port{Proto: "tcp", Port: 100, Process: "proc2"},
-			Port{Proto: "udp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "udp", Port: 100, Process: "proc1"},
-			true,
-		},
-		{
-			"inode evaluated third",
-			Port{Proto: "tcp", Port: 100, Process: "proc2", inode: "inode1"},
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode2"},
 			true,
 		},
 		{
 			"Process evaluated fourth",
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc1"},
-			Port{Proto: "tcp", Port: 100, Process: "proc2", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc2"},
 			true,
 		},
 		{
 			"equal",
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc1"},
-			Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"},
+			Port{Proto: "tcp", Port: 100, Process: "proc1"},
 			false,
 		},
 	}
@@ -138,58 +120,14 @@ func TestLessThan(t *testing.T) {
 		if got != tt.want {
 			t.Errorf("%s: Equal = %v; want %v", tt.name, got, tt.want)
 		}
-	}
+		lessBack := tt.b.lessThan(&tt.a)
-}
+		if got && lessBack {
-
+			t.Errorf("%s: both a and b report being less than each other", tt.name)
-func TestSameInodes(t *testing.T) {
+		}
-	port1 := Port{Proto: "tcp", Port: 100, Process: "proc", inode: "inode1"}
+		wantEqual := !got && !lessBack
-	port2 := Port{Proto: "tcp", Port: 100, Process: "proc", inode: "inode1"}
+		gotEqual := tt.a.equal(&tt.b)
-	portProto := Port{Proto: "udp", Port: 100, Process: "proc", inode: "inode1"}
+		if gotEqual != wantEqual {
-	portPort := Port{Proto: "tcp", Port: 101, Process: "proc", inode: "inode1"}
+			t.Errorf("%s: equal = %v; want %v", tt.name, gotEqual, wantEqual)
-	portInode := Port{Proto: "tcp", Port: 100, Process: "proc", inode: "inode2"}
-	portProcess := Port{Proto: "tcp", Port: 100, Process: "other", inode: "inode1"}
-
-	tests := []struct {
-		name string
-		a, b List
-		want bool
-	}{
-		{
-			"identical",
-			List{port1, port1},
-			List{port2, port2},
-			true,
-		},
-		{
-			"proto differs",
-			List{port1, port1},
-			List{port2, portProto},
-			false,
-		},
-		{
-			"port differs",
-			List{port1, port1},
-			List{port2, portPort},
-			false,
-		},
-		{
-			"inode differs",
-			List{port1, port1},
-			List{port2, portInode},
-			false,
-		},
-		{
-			// SameInodes does not check the Process field
-			"Process differs",
-			List{port1, port1},
-			List{port2, portProcess},
-			true,
-		},
-	}
-	for _, tt := range tests {
-		got := tt.a.sameInodes(tt.b)
-		if got != tt.want {
-			t.Errorf("%s: Equal = %v; want %v", tt.name, got, tt.want)
 		}
 	}
 }