From 036f70b7b45ab78b07ff0c691d749c2c6045ac33 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 23 Oct 2022 18:02:02 -0700 Subject: [PATCH] portlist: refactor, introduce OS-specific types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- portlist/netstat_test.go | 26 ++-- portlist/poller.go | 58 ++++++++- portlist/portlist.go | 43 ++----- portlist/portlist_linux.go | 220 +++++++++++++++++++++----------- portlist/portlist_linux_test.go | 55 ++++---- portlist/portlist_test.go | 120 +++++------------ 6 files changed, 278 insertions(+), 244 deletions(-) diff --git a/portlist/netstat_test.go b/portlist/netstat_test.go index 04a093fd68407..ecf9f8c110549 100644 --- a/portlist/netstat_test.go +++ b/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) diff --git a/portlist/poller.go b/portlist/poller.go index 56dfcb8674b5f..3476b4075ce12 100644 --- a/portlist/poller.go +++ b/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 } diff --git a/portlist/portlist.go b/portlist/portlist.go index 4daf83baa7741..fe4a1af4d0813 100644 --- a/portlist/portlist.go +++ b/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") } diff --git a/portlist/portlist_linux.go b/portlist/portlist_linux.go index 4d5faabbfdea1..6ddd1c662a247 100644 --- a/portlist/portlist_linux.go +++ b/portlist/portlist_linux.go @@ -7,15 +7,15 @@ package portlist import ( "bufio" "bytes" + "errors" "fmt" "io" + "log" "os" "path/filepath" "runtime" "strconv" "strings" - "sync" - "sync/atomic" "syscall" "time" "unsafe" @@ -25,96 +25,133 @@ import ( "tailscale.com/util/mak" ) -// 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" - v4Localhost = "0100007F:" - v4Any = "00000000:0000" -) +func init() { + newOSImpl = newLinuxImpl +} -var eofReader = bytes.NewReader(nil) +type linuxImpl struct { + procNetFiles []*os.File // seeked to start & reused between calls -var bufioReaderPool = &sync.Pool{ - New: func() any { return bufio.NewReader(eofReader) }, + known map[string]*portMeta // inode string => metadata + br *bufio.Reader } -type internedStrings struct { - m map[string]string +type portMeta struct { + port Port + keep bool + needsProcName bool } -func (v *internedStrings) get(b []byte) string { - if s, ok := v.m[string(b)]; ok { - return s +func newLinuxImplBase() *linuxImpl { + return &linuxImpl{ + br: bufio.NewReader(eofReader), + known: map[string]*portMeta{}, } - s := string(b) - mak.Set(&v.m, s, s) - return s } -var internedStringsPool = &sync.Pool{ - New: func() any { return new(internedStrings) }, +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 appendListeningPorts(base []Port) ([]Port, error) { - ret := base - if sawProcNetPermissionErr.Load() { - return ret, nil +func (li *linuxImpl) Close() error { + for _, f := range li.procNetFiles { + f.Close() } + li.procNetFiles = nil + return nil +} - br := bufioReaderPool.Get().(*bufio.Reader) - defer bufioReaderPool.Put(br) - defer br.Reset(eofReader) +// Reading the sockfiles on Linux is very fast, so we can do it often. +const pollInterval = 1 * time.Second - stringCache := internedStringsPool.Get().(*internedStrings) - defer internedStringsPool.Put(stringCache) +const ( + v6Localhost = "00000000000000000000000001000000:" + v6Any = "00000000000000000000000000000000:0000" + v4Localhost = "0100007F:" + v4Any = "00000000:0000" +) + +var eofReader = bytes.NewReader(nil) - for _, fname := range sockfiles { +func (li *linuxImpl) AppendListeningPorts(base []Port) ([]Port, error) { + if runtime.GOOS == "android" { // 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 { - sawProcNetPermissionErr.Store(true) - return nil, nil - } + return nil, nil + } - f, err := os.Open(fname) - if os.IsPermission(err) { - sawProcNetPermissionErr.Store(true) - return nil, nil - } + br := li.br + defer br.Reset(eofReader) + + // 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)) - f.Close() + err = li.parseProcNetFile(br, filepath.Base(name)) 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. - stringCache.m = nil + + // Delete ports that aren't open any longer. + // 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) + } + } + err := li.findProcessNames(needProc) + if err != nil { + return nil, err + } + + ret := base + for _, pm := range li.known { + ret = append(ret, pm.port) } - return ret, nil + 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, - Port: uint16(portv), - inode: stringCache.get(inoBuf), - }) + + if pm, ok := li.known[string(inoBuf)]; ok { + pm.keep = true + // 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) { - pm := map[string]*Port{} // by Port.inode - for i := range pl { - pm[pl[i].inode] = &pl[i] +// errDone is an internal sentinel error that we found everything we were looking for. +var errDone = errors.New("done") + +// 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 { - return nil, err + if err == errDone { + 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") +} diff --git a/portlist/portlist_linux_test.go b/portlist/portlist_linux_test.go index d48c004ec02fe..a0b8e1dc50137 100644 --- a/portlist/portlist_linux_test.go +++ b/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{ - {Proto: "tcp", Port: 22, inode: "socket:[34062]"}, + want: map[string]*portMeta{ + "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{ - {Proto: "tcp", Port: 8081, inode: "socket:[142240557]"}, - {Proto: "tcp", Port: 22, inode: "socket:[34064]"}, + want: map[string]*portMeta{ + "socket:[142240557]": &portMeta{ + 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) } - - if diff := cmp.Diff(got, tt.want, cmp.AllowUnexported(Port{})); diff != "" { + for _, pm := range tt.want { + 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{ - {Proto: "tcp", Port: 8081, inode: "socket:[142240557]"}, - {Proto: "tcp", Port: 22, inode: "socket:[34064]"}, - } + li := newLinuxImplBase() 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 { - b.Fatalf("wrong result:\n got %+v\nwant %+v", got, want) - } - } -} - -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) + if len(li.known) != 2 { + b.Fatalf("wrong results; want 2 parsed got %d", len(li.known)) } } } diff --git a/portlist/portlist_test.go b/portlist/portlist_test.go index 1987c91d5d140..a02e870e04a70 100644 --- a/portlist/portlist_test.go +++ b/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: 101, Process: "proc1", inode: "inode1"}, + Port{Proto: "tcp", Port: 100, Process: "proc1"}, + Port{Proto: "tcp", Port: 101, Process: "proc1"}, true, }, { "Port a > b", - Port{Proto: "tcp", Port: 101, Process: "proc1", inode: "inode1"}, - Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"}, + Port{Proto: "tcp", Port: 101, Process: "proc1"}, + Port{Proto: "tcp", Port: 100, Process: "proc1"}, false, }, { "Proto a < b", - Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"}, - Port{Proto: "udp", Port: 100, Process: "proc1", inode: "inode1"}, + Port{Proto: "tcp", Port: 100, Process: "proc1"}, + Port{Proto: "udp", Port: 100, Process: "proc1"}, true, }, { "Proto a < b", - Port{Proto: "udp", Port: 100, Process: "proc1", inode: "inode1"}, - Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"}, - 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"}, + Port{Proto: "udp", Port: 100, Process: "proc1"}, + Port{Proto: "tcp", Port: 100, Process: "proc1"}, false, }, { "Process a < b", - Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"}, - Port{Proto: "tcp", Port: 100, Process: "proc2", inode: "inode1"}, + Port{Proto: "tcp", Port: 100, Process: "proc1"}, + 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: "proc1", inode: "inode1"}, + Port{Proto: "tcp", Port: 100, Process: "proc2"}, + Port{Proto: "tcp", Port: 100, Process: "proc1"}, false, }, { "Port evaluated first", - Port{Proto: "udp", Port: 100, Process: "proc2", inode: "inode2"}, - Port{Proto: "tcp", Port: 101, Process: "proc1", inode: "inode1"}, + Port{Proto: "udp", Port: 100, Process: "proc2"}, + Port{Proto: "tcp", Port: 101, Process: "proc1"}, true, }, { "Proto evaluated second", - Port{Proto: "tcp", Port: 100, Process: "proc2", inode: "inode2"}, - Port{Proto: "udp", Port: 100, Process: "proc1", inode: "inode1"}, - true, - }, - { - "inode evaluated third", - Port{Proto: "tcp", Port: 100, Process: "proc2", inode: "inode1"}, - Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode2"}, + Port{Proto: "tcp", Port: 100, Process: "proc2"}, + Port{Proto: "udp", Port: 100, Process: "proc1"}, true, }, { "Process evaluated fourth", - Port{Proto: "tcp", Port: 100, Process: "proc1", inode: "inode1"}, - Port{Proto: "tcp", Port: 100, Process: "proc2", inode: "inode1"}, + Port{Proto: "tcp", Port: 100, Process: "proc1"}, + 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", inode: "inode1"}, + Port{Proto: "tcp", Port: 100, Process: "proc1"}, + 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) } - } -} - -func TestSameInodes(t *testing.T) { - port1 := Port{Proto: "tcp", Port: 100, Process: "proc", inode: "inode1"} - port2 := Port{Proto: "tcp", Port: 100, Process: "proc", inode: "inode1"} - portProto := Port{Proto: "udp", Port: 100, Process: "proc", inode: "inode1"} - portPort := Port{Proto: "tcp", Port: 101, Process: "proc", inode: "inode1"} - 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) + lessBack := tt.b.lessThan(&tt.a) + if got && lessBack { + t.Errorf("%s: both a and b report being less than each other", tt.name) + } + wantEqual := !got && !lessBack + gotEqual := tt.a.equal(&tt.b) + if gotEqual != wantEqual { + t.Errorf("%s: equal = %v; want %v", tt.name, gotEqual, wantEqual) } } }