cache binaries that have nothing to instrument

AcquireGlobalUprobe only cached successful attachments. When attach()
returned no links (e.g. a stripped Go binary such as kubectl, which has
no symbol table), nothing was remembered, so the binary was re-opened
and its symbols re-parsed for every new process that ran it. On nodes
that run such binaries frequently this is pure overhead.

Cache the negative result in a separate map keyed by dev+inode. The
binary's size and mtime are stored in the entry and re-checked on every
lookup, so a binary replaced in place or whose inode is recycled is
re-evaluated rather than served a stale verdict. Entries expire after a
TTL; expired ones are swept on insertion, but at most once per TTL, so a
burst of new binaries does not turn every insertion into a full-map scan
under the lock. Live (refcounted) attachments are unchanged.

Author: gecube

ebpftracer/tracer.go

@@ -91,6 +91,25 @@ type globalUprobe struct {
 	refcount int
 }
 
+// negativeUprobe records that a binary had nothing to instrument (e.g. a
+// stripped Go binary such as kubectl), so it is not re-opened and re-parsed for
+// every process that runs it. It is keyed by dev+inode; size and mtime are kept
+// in the value and re-checked on every lookup so an in-place replacement or a
+// recycled inode is re-evaluated instead of being served a stale verdict. The
+// entry also self-expires after negativeUprobeCacheTTL. This is a heuristic, not
+// tamper-proof: a replacement that preserves both size and mtime within the
+// filesystem's timestamp granularity is only re-evaluated once the TTL elapses.
+type negativeUprobe struct {
+	size      int64
+	mtime     int64
+	expiresAt time.Time
+}
+
+// negativeUprobeCacheTTL bounds how long a "nothing to instrument" verdict is
+// trusted, so a binary that is later replaced in a way size+mtime cannot detect
+// is still eventually re-evaluated. A var (not const) so tests can shorten it.
+var negativeUprobeCacheTTL = 10 * time.Minute
+
 type Tracer struct {
 	disableL7Tracing bool
 	hostNetNs        netns.NsHandle
@@ -103,7 +122,10 @@ type Tracer struct {
 	uprobes        map[string]*ebpf.Program
 
 	globalUprobes     map[UprobeKey]*globalUprobe
+	negativeUprobes   map[UprobeKey]negativeUprobe
+	negativeSweepAt   time.Time
 	globalUprobesLock sync.Mutex
+	now               func() time.Time // overridable in tests
 }
 
 func NewTracer(hostNetNs, selfNetNs netns.NsHandle, disableL7Tracing bool) *Tracer {
@@ -115,9 +137,11 @@ func NewTracer(hostNetNs, selfNetNs netns.NsHandle, disableL7Tracing bool) *Trac
 		hostNetNs:        hostNetNs,
 		selfNetNs:        selfNetNs,
 
-		readers:       map[string]*perf.Reader{},
-		uprobes:       map[string]*ebpf.Program{},
-		globalUprobes: map[UprobeKey]*globalUprobe{},
+		readers:         map[string]*perf.Reader{},
+		uprobes:         map[string]*ebpf.Program{},
+		globalUprobes:   map[UprobeKey]*globalUprobe{},
+		negativeUprobes: map[UprobeKey]negativeUprobe{},
+		now:             time.Now,
 	}
 }
 
@@ -154,6 +178,7 @@ func (t *Tracer) Close() {
 		}
 	}
 	t.globalUprobes = nil
+	t.negativeUprobes = nil
 	t.globalUprobesLock.Unlock()
 	t.collection.Close()
 }
@@ -164,6 +189,7 @@ func (t *Tracer) AcquireGlobalUprobe(path string, attach func() []link.Link) (Up
 		return UprobeKey{}, false
 	}
 	key := UprobeKey{Dev: stat.Dev, Ino: stat.Ino}
+	size, mtime := stat.Size, stat.Mtim.Nano()
 
 	t.globalUprobesLock.Lock()
 	defer t.globalUprobesLock.Unlock()
@@ -172,14 +198,39 @@ func (t *Tracer) AcquireGlobalUprobe(path string, attach func() []link.Link) (Up
 		gu.refcount++
 		return key, true
 	}
+	if neg, ok := t.negativeUprobes[key]; ok {
+		if t.now().Before(neg.expiresAt) && neg.size == size && neg.mtime == mtime {
+			return UprobeKey{}, false // known-empty, unchanged, not expired
+		}
+		delete(t.negativeUprobes, key) // expired or replaced: re-evaluate below
+	}
 	links := attach()
 	if len(links) == 0 {
+		t.cacheNegativeUprobe(key, size, mtime)
 		return UprobeKey{}, false
 	}
 	t.globalUprobes[key] = &globalUprobe{links: links, refcount: 1}
 	return key, true
 }
 
+// cacheNegativeUprobe remembers that key has nothing to instrument. Expired
+// entries are swept here, but at most once per TTL (negativeSweepAt), so a burst
+// of distinct binaries does not turn every insert into a full-map scan under the
+// lock. Stale entries are also dropped lazily on lookup in AcquireGlobalUprobe.
+// The caller must hold globalUprobesLock.
+func (t *Tracer) cacheNegativeUprobe(key UprobeKey, size, mtime int64) {
+	now := t.now()
+	if now.Sub(t.negativeSweepAt) >= negativeUprobeCacheTTL {
+		for k, neg := range t.negativeUprobes {
+			if now.After(neg.expiresAt) {
+				delete(t.negativeUprobes, k)
+			}
+		}
+		t.negativeSweepAt = now
+	}
+	t.negativeUprobes[key] = negativeUprobe{size: size, mtime: mtime, expiresAt: now.Add(negativeUprobeCacheTTL)}
+}
+
 func (t *Tracer) ReleaseGlobalUprobes(keys ...UprobeKey) {
 	t.globalUprobesLock.Lock()
 	defer t.globalUprobesLock.Unlock()

ebpftracer/tracer_cache_test.go

@@ -0,0 +1,147 @@
+package ebpftracer
+
+import (
+	"os"
+	"path/filepath"
+	"strconv"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/cilium/ebpf/link"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func newTestTracer() *Tracer {
+	return &Tracer{
+		globalUprobes:   map[UprobeKey]*globalUprobe{},
+		negativeUprobes: map[UprobeKey]negativeUprobe{},
+		now:             time.Now,
+	}
+}
+
+func writeFile(t *testing.T, content string) string {
+	t.Helper()
+	p := filepath.Join(t.TempDir(), "bin")
+	require.NoError(t, os.WriteFile(p, []byte(content), 0755))
+	return p
+}
+
+// A binary with nothing to instrument must be parsed once and then served from
+// the cache, instead of being re-opened for every process that runs it.
+func TestAcquireGlobalUprobeCachesEmptyResult(t *testing.T) {
+	tr := newTestTracer()
+	path := writeFile(t, "binary")
+
+	calls := 0
+	attach := func() []link.Link { calls++; return nil }
+
+	for i := 0; i < 5; i++ {
+		key, ok := tr.AcquireGlobalUprobe(path, attach)
+		assert.False(t, ok)
+		assert.Equal(t, UprobeKey{}, key)
+	}
+	assert.Equal(t, 1, calls, "attach must be called only once for an unchanged binary")
+}
+
+// A replaced binary (different size/mtime) must be evaluated again rather than
+// reusing a stale negative result.
+func TestAcquireGlobalUprobeReevaluatesReplacedBinary(t *testing.T) {
+	tr := newTestTracer()
+	path := writeFile(t, "v1")
+
+	calls := 0
+	attach := func() []link.Link { calls++; return nil }
+
+	_, ok := tr.AcquireGlobalUprobe(path, attach)
+	require.False(t, ok)
+	require.Equal(t, 1, calls)
+
+	require.NoError(t, os.WriteFile(path, []byte("v2-longer"), 0755))
+	_, ok = tr.AcquireGlobalUprobe(path, attach)
+	require.False(t, ok)
+	assert.Equal(t, 2, calls, "a replaced binary must be re-evaluated")
+}
+
+// A binary repeatedly overwritten in place keeps the same inode, so the negative
+// cache must hold exactly one entry, not one per spawn.
+func TestNegativeCacheDoesNotGrowOnInPlaceOverwrite(t *testing.T) {
+	tr := newTestTracer()
+	path := writeFile(t, "v1")
+	attach := func() []link.Link { return nil }
+
+	for i := 0; i < 20; i++ {
+		require.NoError(t, os.WriteFile(path, []byte("v"+strconv.Itoa(i)), 0755))
+		_, ok := tr.AcquireGlobalUprobe(path, attach)
+		require.False(t, ok)
+	}
+	assert.Len(t, tr.negativeUprobes, 1, "in-place overwrite must reuse one inode-keyed slot")
+}
+
+// An expired negative entry must be re-evaluated and replaced, not duplicated.
+// Expiry is driven by an injected clock rather than by poking internals.
+func TestAcquireGlobalUprobeExpiresEmptyResult(t *testing.T) {
+	tr := newTestTracer()
+	clock := time.Unix(0, 0)
+	tr.now = func() time.Time { return clock }
+	path := writeFile(t, "binary")
+
+	calls := 0
+	attach := func() []link.Link { calls++; return nil }
+
+	_, ok := tr.AcquireGlobalUprobe(path, attach)
+	require.False(t, ok)
+	require.Equal(t, 1, calls)
+	require.Len(t, tr.negativeUprobes, 1)
+
+	clock = clock.Add(negativeUprobeCacheTTL + time.Second)
+	_, ok = tr.AcquireGlobalUprobe(path, attach)
+	require.False(t, ok)
+	assert.Equal(t, 2, calls, "an expired negative entry must be re-evaluated")
+	assert.Len(t, tr.negativeUprobes, 1, "the stale entry must be replaced, not duplicated")
+}
+
+// Expired negative entries must not accumulate; they are swept when a new
+// negative entry is added (at most once per TTL).
+func TestNegativeCacheSweepsExpiredEntries(t *testing.T) {
+	tr := newTestTracer()
+	clock := time.Unix(0, 0)
+	tr.now = func() time.Time { return clock }
+
+	for i := 0; i < 10; i++ {
+		tr.negativeUprobes[UprobeKey{Ino: uint64(i + 1)}] = negativeUprobe{
+			expiresAt: clock.Add(time.Minute),
+		}
+	}
+	// A live (refcounted) entry must be untouched by the negative-cache sweep.
+	tr.globalUprobes[UprobeKey{Ino: 100}] = &globalUprobe{refcount: 1}
+
+	clock = clock.Add(negativeUprobeCacheTTL + time.Minute)
+	_, ok := tr.AcquireGlobalUprobe(writeFile(t, "binary"), func() []link.Link { return nil })
+	require.False(t, ok)
+
+	assert.Len(t, tr.negativeUprobes, 1, "expired negative entries must be swept; only the fresh one remains")
+	require.Len(t, tr.globalUprobes, 1, "live entries must be kept")
+	_, live := tr.globalUprobes[UprobeKey{Ino: 100}]
+	assert.True(t, live)
+}
+
+// The race detector must stay clean under concurrent acquisition of the same
+// binary, and the result must be a single negative entry.
+func TestNegativeCacheConcurrent(t *testing.T) {
+	tr := newTestTracer()
+	path := writeFile(t, "binary")
+	attach := func() []link.Link { return nil }
+
+	var wg sync.WaitGroup
+	for i := 0; i < 50; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			tr.AcquireGlobalUprobe(path, attach)
+		}()
+	}
+	wg.Wait()
+	assert.Len(t, tr.negativeUprobes, 1)
+}