diff --git a/probe/endpoint/procspy/benchmark_internal_test.go b/probe/endpoint/procspy/benchmark_internal_test.go index ec1dea59c..1e48aab06 100644 --- a/probe/endpoint/procspy/benchmark_internal_test.go +++ b/probe/endpoint/procspy/benchmark_internal_test.go @@ -6,13 +6,16 @@ import ( ) func BenchmarkParseConnectionsBaseline(b *testing.B) { - readFile = func(string, *bytes.Buffer) error { return nil } + readFile = func(string, *bytes.Buffer) (int64, error) { return 0, nil } benchmarkConnections(b) // 333 ns/op, 0 allocs/op } func BenchmarkParseConnectionsFixture(b *testing.B) { - readFile = func(_ string, buf *bytes.Buffer) error { _, err := buf.Write(fixture); return err } + readFile = func(_ string, buf *bytes.Buffer) (int64, error) { + n, err := buf.Write(fixture) + return int64(n), err + } benchmarkConnections(b) // 15553 ns/op, 12 allocs/op } diff --git a/probe/endpoint/procspy/proc.go b/probe/endpoint/procspy/proc.go index 84c5a0086..dd8c47ae1 100644 --- a/probe/endpoint/procspy/proc.go +++ b/probe/endpoint/procspy/proc.go @@ -24,10 +24,10 @@ func SetProcRoot(root string) { // walkProcPid walks over all numerical (PID) /proc entries, and sees if their // ./fd/* files are symlink to sockets. Returns a map from socket ID (inode) // to PID. Will return an error if /proc isn't there. -func walkProcPid(buf *bytes.Buffer, walker process.Walker) (map[uint64]Proc, error) { +func walkProcPid(buf *bytes.Buffer, walker process.Walker) (map[uint64]*Proc, error) { var ( - res = map[uint64]Proc{} - namespaces = map[uint64]struct{}{} + res = map[uint64]*Proc{} + namespaces = map[uint64]bool{} // map namespace id -> has connections statT syscall.Stat_t ) @@ -40,9 +40,14 @@ func walkProcPid(buf *bytes.Buffer, walker process.Walker) (map[uint64]Proc, err if err := fs.Lstat(filepath.Join(procRoot, dirName, "/ns/net"), &statT); err != nil { return } - if _, ok := namespaces[statT.Ino]; !ok { - namespaces[statT.Ino] = struct{}{} - readFile(filepath.Join(procRoot, dirName, "/net/tcp"), buf) + hasConns, ok := namespaces[statT.Ino] + if !ok { + read, err := readFile(filepath.Join(procRoot, dirName, "/net/tcp"), buf) + hasConns = err == nil && read > 0 + namespaces[statT.Ino] = hasConns + } + if !hasConns { + return } fds, err := fs.ReadDirNames(fdBase) @@ -50,6 +55,7 @@ func walkProcPid(buf *bytes.Buffer, walker process.Walker) (map[uint64]Proc, err // Process is be gone by now, or we don't have access. return } + var proc *Proc for _, fd := range fds { // Direct use of syscall.Stat() to save garbage. err = fs.Stat(filepath.Join(fdBase, fd), &statT) @@ -61,11 +67,13 @@ func walkProcPid(buf *bytes.Buffer, walker process.Walker) (map[uint64]Proc, err if statT.Mode&syscall.S_IFMT != syscall.S_IFSOCK { continue } - - res[statT.Ino] = Proc{ - PID: uint(p.PID), - Name: p.Comm, + if proc == nil { + proc = &Proc{ + PID: uint(p.PID), + Name: p.Comm, + } } + res[statT.Ino] = proc } }) @@ -75,12 +83,11 @@ func walkProcPid(buf *bytes.Buffer, walker process.Walker) (map[uint64]Proc, err // readFile reads an arbitrary file into a buffer. It's a variable so it can // be overwritten for benchmarks. That's bad practice and we should change it // to be a dependency. -var readFile = func(filename string, buf *bytes.Buffer) error { +var readFile = func(filename string, buf *bytes.Buffer) (int64, error) { f, err := fs.Open(filename) if err != nil { - return err + return -1, err } - _, err = buf.ReadFrom(f) - f.Close() - return err + defer f.Close() + return buf.ReadFrom(f) } diff --git a/probe/endpoint/procspy/proc_internal_test.go b/probe/endpoint/procspy/proc_internal_test.go index 4ebd00552..19c9aa381 100644 --- a/probe/endpoint/procspy/proc_internal_test.go +++ b/probe/endpoint/procspy/proc_internal_test.go @@ -33,6 +33,12 @@ var mockFS = fs.Dir("", FStat: syscall.Stat_t{}, }, ), + fs.Dir("net", + fs.File{ + FName: "tcp", + FContents: "I'm a little teapot", + }, + ), fs.File{ FName: "stat", FContents: "1 na R 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1", @@ -50,7 +56,7 @@ func TestWalkProcPid(t *testing.T) { if err != nil { t.Fatal(err) } - want := map[uint64]Proc{ + want := map[uint64]*Proc{ 45: { PID: 1, Name: "foo", diff --git a/probe/endpoint/procspy/spy_linux.go b/probe/endpoint/procspy/spy_linux.go index 29634edb0..68b0d7c0b 100644 --- a/probe/endpoint/procspy/spy_linux.go +++ b/probe/endpoint/procspy/spy_linux.go @@ -16,7 +16,7 @@ var bufPool = sync.Pool{ type pnConnIter struct { pn *ProcNet buf *bytes.Buffer - procs map[uint64]Proc + procs map[uint64]*Proc } func (c *pnConnIter) Next() *Connection { @@ -27,7 +27,7 @@ func (c *pnConnIter) Next() *Connection { return nil } if proc, ok := c.procs[n.inode]; ok { - n.Proc = proc + n.Proc = *proc } return n } @@ -38,7 +38,7 @@ var cbConnections = func(processes bool, walker process.Walker) (ConnIter, error buf := bufPool.Get().(*bytes.Buffer) buf.Reset() - var procs map[uint64]Proc + var procs map[uint64]*Proc if processes { var err error if procs, err = walkProcPid(buf, walker); err != nil {