From 59f777a06686672376f25bbe58cd98f4c573ccb7 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Fri, 2 Jun 2017 14:01:25 +0100 Subject: [PATCH] don't read all of /proc when probe.proc.spy=false Previously we were doing the reading even though we weren't looking at the result. --- probe/endpoint/connection_tracker.go | 8 +++--- probe/endpoint/procspy/fixture.go | 2 +- probe/endpoint/procspy/spy.go | 8 ++---- probe/endpoint/procspy/spy_darwin.go | 20 +++++++------- probe/endpoint/procspy/spy_linux.go | 26 ++++++++++++------- .../procspy/spy_linux_internal_test.go | 4 +-- 6 files changed, 36 insertions(+), 32 deletions(-) diff --git a/probe/endpoint/connection_tracker.go b/probe/endpoint/connection_tracker.go index c437d9719e..1db913f639 100644 --- a/probe/endpoint/connection_tracker.go +++ b/probe/endpoint/connection_tracker.go @@ -71,7 +71,7 @@ func flowToTuple(f flow) (ft fourTuple) { func (t *connectionTracker) useProcfs() { t.ebpfTracker = nil if t.conf.WalkProc && t.conf.Scanner == nil { - t.conf.Scanner = procspy.NewConnectionScanner(t.conf.ProcessCache) + t.conf.Scanner = procspy.NewConnectionScanner(t.conf.ProcessCache, t.conf.SpyProcs) } if t.flowWalker == nil { t.flowWalker = newConntrackFlowWalker(t.conf.UseConntrack, t.conf.ProcRoot, t.conf.BufferSize) @@ -112,7 +112,7 @@ func (t *connectionTracker) performFlowWalk(rpt *report.Report, seenTuples *map[ } func (t *connectionTracker) performWalkProc(rpt *report.Report, hostNodeID string, seenTuples *map[string]fourTuple) error { - conns, err := t.conf.Scanner.Connections(t.conf.SpyProcs) + conns, err := t.conf.Scanner.Connections() if err != nil { return err } @@ -159,7 +159,7 @@ func (t *connectionTracker) getInitialState() { processCache = process.NewCachingWalker(walker) processCache.Tick() - scanner := procspy.NewSyncConnectionScanner(processCache) + scanner := procspy.NewSyncConnectionScanner(processCache, t.conf.SpyProcs) seenTuples := map[string]fourTuple{} // Consult the flowWalker to get the initial state if err := IsConntrackSupported(t.conf.ProcRoot); t.conf.UseConntrack && err != nil { @@ -173,7 +173,7 @@ func (t *connectionTracker) getInitialState() { } } - conns, err := scanner.Connections(t.conf.SpyProcs) + conns, err := scanner.Connections() if err != nil { log.Errorf("Error initializing ebpfTracker while scanning /proc, continuing without initial connections: %s", err) } diff --git a/probe/endpoint/procspy/fixture.go b/probe/endpoint/procspy/fixture.go index 0731f624a0..59ac933688 100644 --- a/probe/endpoint/procspy/fixture.go +++ b/probe/endpoint/procspy/fixture.go @@ -18,7 +18,7 @@ func (f *fixedConnIter) Next() *Connection { type FixedScanner []Connection // Connections implements ConnectionsScanner.Connections -func (s FixedScanner) Connections(_ bool) (ConnIter, error) { +func (s FixedScanner) Connections() (ConnIter, error) { iter := fixedConnIter(s) return &iter, nil } diff --git a/probe/endpoint/procspy/spy.go b/probe/endpoint/procspy/spy.go index 04a9c3692b..1c09450abe 100644 --- a/probe/endpoint/procspy/spy.go +++ b/probe/endpoint/procspy/spy.go @@ -40,12 +40,8 @@ type ConnIter interface { // ConnectionScanner scans the system for established (TCP) connections type ConnectionScanner interface { - // Connections returns all established (TCP) connections. If processes is - // false we'll just list all TCP connections, and there is no need to be root. - // If processes is true it'll additionally try to lookup the process owning the - // connection, filling in the Proc field. You will need to run this as root to - // find all processes. - Connections(processes bool) (ConnIter, error) + // Connections returns all established (TCP) connections. + Connections() (ConnIter, error) // Stops the scanning Stop() } diff --git a/probe/endpoint/procspy/spy_darwin.go b/probe/endpoint/procspy/spy_darwin.go index 394407f39f..f7852c91ac 100644 --- a/probe/endpoint/procspy/spy_darwin.go +++ b/probe/endpoint/procspy/spy_darwin.go @@ -14,21 +14,21 @@ const ( ) // NewConnectionScanner creates a new Darwin ConnectionScanner -func NewConnectionScanner(_ process.Walker) ConnectionScanner { - return &darwinScanner{} +func NewConnectionScanner(_ process.Walker, processes bool) ConnectionScanner { + return &darwinScanner{processes} } // NewSyncConnectionScanner creates a new synchronous Darwin ConnectionScanner -func NewSyncConnectionScanner(_ process.Walker) ConnectionScanner { - return &darwinScanner{} +func NewSyncConnectionScanner(_ process.Walker, processes bool) ConnectionScanner { + return &darwinScanner{processes} } -type darwinScanner struct{} +type darwinScanner struct { + processes bool +} -// Connections returns all established (TCP) connections. No need to be root -// to run this. If processes is true it also tries to fill in the process -// fields of the connection. You need to be root to find all processes. -func (s *darwinScanner) Connections(processes bool) (ConnIter, error) { +// Connections returns all established (TCP) connections. +func (s *darwinScanner) Connections() (ConnIter, error) { out, err := exec.Command( netstatBinary, "-n", // no number resolving @@ -42,7 +42,7 @@ func (s *darwinScanner) Connections(processes bool) (ConnIter, error) { } connections := parseDarwinNetstat(string(out)) - if processes { + if s.processes { out, err := exec.Command( lsofBinary, "-i", // only Internet files diff --git a/probe/endpoint/procspy/spy_linux.go b/probe/endpoint/procspy/spy_linux.go index bdc80c66fc..08d4258c57 100644 --- a/probe/endpoint/procspy/spy_linux.go +++ b/probe/endpoint/procspy/spy_linux.go @@ -33,28 +33,34 @@ func (c *pnConnIter) Next() *Connection { } // NewConnectionScanner creates a new Linux ConnectionScanner -func NewConnectionScanner(walker process.Walker) ConnectionScanner { - br := newBackgroundReader(walker) - return &linuxScanner{br} +func NewConnectionScanner(walker process.Walker, processes bool) ConnectionScanner { + scanner := &linuxScanner{} + if processes { + scanner.r = newBackgroundReader(walker) + } + return scanner } // NewSyncConnectionScanner creates a new synchronous Linux ConnectionScanner -func NewSyncConnectionScanner(walker process.Walker) ConnectionScanner { - fr := newForegroundReader(walker) - return &linuxScanner{fr} +func NewSyncConnectionScanner(walker process.Walker, processes bool) ConnectionScanner { + scanner := &linuxScanner{} + if processes { + scanner.r = newForegroundReader(walker) + } + return scanner } type linuxScanner struct { r reader } -func (s *linuxScanner) Connections(processes bool) (ConnIter, error) { +func (s *linuxScanner) Connections() (ConnIter, error) { // buffer for contents of /proc//net/tcp buf := bufPool.Get().(*bytes.Buffer) buf.Reset() var procs map[uint64]*Proc - if processes { + if s.r != nil { var err error if procs, err = s.r.getWalkedProcPid(buf); err != nil { return nil, err @@ -74,5 +80,7 @@ func (s *linuxScanner) Connections(processes bool) (ConnIter, error) { } func (s *linuxScanner) Stop() { - s.r.stop() + if s.r != nil { + s.r.stop() + } } diff --git a/probe/endpoint/procspy/spy_linux_internal_test.go b/probe/endpoint/procspy/spy_linux_internal_test.go index d5e1fb1e1d..8614713391 100644 --- a/probe/endpoint/procspy/spy_linux_internal_test.go +++ b/probe/endpoint/procspy/spy_linux_internal_test.go @@ -14,13 +14,13 @@ import ( func TestLinuxConnections(t *testing.T) { fs_hook.Mock(mockFS) defer fs_hook.Restore() - scanner := NewConnectionScanner(process.NewWalker("/proc", false)) + scanner := NewConnectionScanner(process.NewWalker("/proc", false), true) defer scanner.Stop() // let the background scanner finish its first pass time.Sleep(1 * time.Second) - iter, err := scanner.Connections(true) + iter, err := scanner.Connections() if err != nil { t.Fatal(err) }