feat: Fixed eBPF Feature Detection

On working to migrate to cilium/ebpf I started to hit issues with
nil pointer dereferences that indicated that something odd was happening
between eBPF feature detection and metrics export.

I discovered that there were additional global variables in many of the
packages whose values are set before eBPF feature detection has
happened.

It appears that in these cases - through the various layers of
indirection - we may try and read/write metrics that do not exist
causing a nil pointer dereference.

There appear to be several places where we're accessing map keys
without checking they are nil - that will need to be fixed in a
follow up. This PR attempts to at least ensure that from the BPF
side that the keys we expect to exist, do actually exist in the
maps that matter.

eBPF feature detection how produces a bpf.SupportedMetrics
struct, which is now consumed in various places in the codebase
to ensure that the metrics we're exporting match those supported
by the eBPF exporter.

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>

cmd/exporter/exporter.go

@@ -110,7 +110,7 @@ func main() {
 	}
 	defer bpfExporter.Detach()
 
-	stats.InitAvailableParamAndMetrics(bpfExporter.GetEnabledBPFHWCounters(), bpfExporter.GetEnabledBPFSWCounters())
+	stats.InitAvailableParamAndMetrics()
 
 	if config.EnabledGPU {
 		klog.Infof("Initializing the GPU collector")

go.mod

@@ -21,7 +21,6 @@ require (
 	github.com/prometheus/common v0.48.0
 	github.com/prometheus/prometheus v0.48.1
 	github.com/sirupsen/logrus v1.9.0
-	golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842
 	golang.org/x/sys v0.20.0
 	gopkg.in/yaml.v3 v3.0.1
 	k8s.io/api v0.28.2
@@ -75,6 +74,7 @@ require (
 	github.com/prometheus/procfs v0.12.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/stretchr/testify v1.8.4 // indirect
+	golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 // indirect
 	golang.org/x/net v0.25.0 // indirect
 	golang.org/x/oauth2 v0.16.0 // indirect
 	golang.org/x/term v0.20.0 // indirect

pkg/bpf/exporter.go

@@ -33,7 +33,6 @@ import (
 	"github.com/jaypipes/ghw"
 	"github.com/sustainable-computing-io/kepler/pkg/config"
 	"github.com/sustainable-computing-io/kepler/pkg/utils"
-	"golang.org/x/exp/slices"
 	"golang.org/x/sys/unix"
 	"k8s.io/klog/v2"
 )
@@ -43,19 +42,10 @@ const (
 	bpfAssestsLocation = "/var/lib/kepler/bpfassets"
 	cpuOnline          = "/sys/devices/system/cpu/online"
 	bpfPerfArraySuffix = "_event_reader"
-
-	// Per /sys/kernel/debug/tracing/events/irq/softirq_entry/format
-	// { 0, "HI" }, { 1, "TIMER" }, { 2, "NET_TX" }, { 3, "NET_RX" }, { 4, "BLOCK" }, { 5, "IRQ_POLL" }, { 6, "TASKLET" }, { 7, "SCHED" }, { 8, "HRTIMER" }, { 9, "RCU" }
-
-	// IRQ vector to IRQ number
-	IRQNetTX = 2
-	IRQNetRX = 3
-	IRQBlock = 4
-
-	TableProcessName = "processes"
-	TableCPUFreqName = "cpu_freq_array"
-	MapSize          = 10240
-	CPUNumSize       = 128
+	TableProcessName   = "processes"
+	TableCPUFreqName   = "cpu_freq_array"
+	MapSize            = 10240
+	CPUNumSize         = 128
 )
 
 type exporter struct {
@@ -66,11 +56,10 @@ type exporter struct {
 	cpuCores              int
 	// due to performance reason we keep an empty struct to verify if a new read is also empty
 	emptyct                 ProcessBPFMetrics
-	hardwareCountersEnabled bool
 	byteOrder               binary.ByteOrder
 	perfEventFds            []int
-	enabledHardwareCounters []string
-	enabledSoftwareCounters []string
+	enabledHardwareCounters map[string]struct{}
+	enabledSoftwareCounters map[string]struct{}
 }
 
 func NewExporter() (Exporter, error) {
@@ -80,11 +69,10 @@ func NewExporter() (Exporter, error) {
 		ebpfBatchGetAndDelete:   true,
 		cpuCores:                getCPUCores(),
 		emptyct:                 ProcessBPFMetrics{},
-		hardwareCountersEnabled: true,
 		byteOrder:               utils.DetermineHostByteOrder(),
 		perfEventFds:            []int{},
-		enabledHardwareCounters: []string{},
-		enabledSoftwareCounters: []string{},
+		enabledHardwareCounters: map[string]struct{}{},
+		enabledSoftwareCounters: map[string]struct{}{},
 	}
 	err := e.attach()
 	if err != nil {
@@ -98,16 +86,11 @@ type perfCounter struct {
 	EvConfig int
 }
 
-func (e *exporter) GetEnabledBPFHWCounters() []string {
-	return e.enabledHardwareCounters
-}
-
-func (e *exporter) GetEnabledBPFSWCounters() []string {
-	return e.enabledSoftwareCounters
-}
-
-func (e *exporter) HardwareCountersEnabled() bool {
-	return e.hardwareCountersEnabled
+func (e *exporter) SupportedMetrics() SupportedMetrics {
+	return SupportedMetrics{
+		HardwareCounters: e.enabledHardwareCounters,
+		SoftwareCounters: e.enabledSoftwareCounters,
+	}
 }
 
 func getLibbpfObjectFilePath(byteOrder binary.ByteOrder) (string, error) {
@@ -185,23 +168,26 @@ func (e *exporter) attach() error {
 	if _, err = prog.AttachGeneric(); err != nil {
 		klog.Infof("failed to attach tracepoint/sched/sched_switch: %v", err)
 	} else {
-		e.enabledSoftwareCounters = append(e.enabledSoftwareCounters, config.CPUTime)
+		e.enabledSoftwareCounters[config.CPUTime] = struct{}{}
 	}
 
 	if config.ExposeIRQCounterMetrics {
-		// attach softirq_entry tracepoint to kepler_irq_trace function
-		irq_prog, err := e.module.GetProgram("kepler_irq_trace")
-		if err != nil {
-			klog.Warningf("could not get kepler_irq_trace: %v", err)
-			// disable IRQ metric
-			config.ExposeIRQCounterMetrics = false
-		} else {
+		err := func() error {
+			// attach softirq_entry tracepoint to kepler_irq_trace function
+			irq_prog, err := e.module.GetProgram("kepler_irq_trace")
+			if err != nil {
+				return fmt.Errorf("could not get kepler_irq_trace: %v", err)
+			}
 			if _, err := irq_prog.AttachGeneric(); err != nil {
-				klog.Warningf("could not attach irq/softirq_entry: %v", err)
-				// disable IRQ metric
-				config.ExposeIRQCounterMetrics = false
+				return fmt.Errorf("could not attach irq/softirq_entry: %v", err)
 			}
-			e.enabledSoftwareCounters = append(e.enabledSoftwareCounters, SoftIRQEvents...)
+			e.enabledSoftwareCounters[config.IRQNetTXLabel] = struct{}{}
+			e.enabledSoftwareCounters[config.IRQNetRXLabel] = struct{}{}
+			e.enabledSoftwareCounters[config.IRQBlockLabel] = struct{}{}
+			return nil
+		}()
+		if err != nil {
+			klog.Warningf("IRQ tracing disabled: %v", err)
 		}
 	}
 
@@ -214,7 +200,7 @@ func (e *exporter) attach() error {
 		if err != nil {
 			klog.Warningf("failed to attach tp/writeback/writeback_dirty_folio: %v. Kepler will not collect page cache write events. This will affect the DRAM power model estimation on VMs.", err)
 		} else {
-			e.enabledSoftwareCounters = append(e.enabledSoftwareCounters, config.PageCacheHit)
+			e.enabledSoftwareCounters[config.PageCacheHit] = struct{}{}
 		}
 	}
 
@@ -226,12 +212,15 @@ func (e *exporter) attach() error {
 		if _, err = page_read.AttachGeneric(); err != nil {
 			klog.Warningf("failed to attach fentry/mark_page_accessed: %v. Kepler will not collect page cache read events. This will affect the DRAM power model estimation on VMs.", err)
 		} else {
-			if !slices.Contains(e.enabledSoftwareCounters, config.PageCacheHit) {
-				e.enabledSoftwareCounters = append(e.enabledSoftwareCounters, config.PageCacheHit)
-			}
+			e.enabledSoftwareCounters[config.PageCacheHit] = struct{}{}
 		}
 	}
 
+	if !config.ExposeHardwareCounterMetrics {
+		klog.Infof("Hardware counter metrics are disabled")
+		return nil
+	}
+
 	// attach performance counter fd to BPF_PERF_EVENT_ARRAY
 	counters := map[string]perfCounter{
 		config.CPUCycle: {unix.PERF_TYPE_HARDWARE, unix.PERF_COUNT_HW_CPU_CYCLES},
@@ -243,30 +232,36 @@ func (e *exporter) attach() error {
 		config.TaskClock:      {unix.PERF_TYPE_SOFTWARE, unix.PERF_COUNT_SW_TASK_CLOCK},
 	}
 
+	cleanup := func() error {
+		unixClosePerfEvents(e.perfEventFds)
+		e.perfEventFds = []int{}
+		e.enabledHardwareCounters = map[string]struct{}{}
+		return nil
+	}
+
 	for arrayName, counter := range counters {
 		bpfPerfArrayName := arrayName + bpfPerfArraySuffix
 		bpfMap, perfErr := e.module.GetMap(bpfPerfArrayName)
 		if perfErr != nil {
 			klog.Warningf("could not get ebpf map for perf event %s: %v\n", bpfPerfArrayName, perfErr)
-			continue
-		} else {
-			fds, perfErr := unixOpenPerfEvent(counter.EvType, counter.EvConfig, e.cpuCores)
-			if perfErr != nil {
-				// some hypervisors don't expose perf counters
-				klog.Warningf("could not attach perf event %s: %v. Are you using a VM?\n", bpfPerfArrayName, perfErr)
-				// if any counter is not enabled, we need disable HardwareCountersEnabled
-				e.hardwareCountersEnabled = false
-			}
-			for i, fd := range fds {
-				err = bpfMap.Update(unsafe.Pointer(&i), unsafe.Pointer(&fd))
-				if err != nil {
-					return fmt.Errorf("failed to update bpf map: %v", err)
-				}
+			return cleanup()
+		}
+		fds, perfErr := unixOpenPerfEvent(counter.EvType, counter.EvConfig, e.cpuCores)
+		if perfErr != nil {
+			klog.Warningf("could not attach perf event %s: %v. Are you using a VM?\n", bpfPerfArrayName, perfErr)
+			return cleanup()
+		}
+		for i, fd := range fds {
+			err = bpfMap.Update(unsafe.Pointer(&i), unsafe.Pointer(&fd))
+			if err != nil {
+				klog.Warningf("failed to update bpf map: %v", err)
+				return cleanup()
 			}
-			e.perfEventFds = append(e.perfEventFds, fds...)
-			e.enabledHardwareCounters = append(e.enabledHardwareCounters, arrayName)
 		}
+		e.perfEventFds = append(e.perfEventFds, fds...)
+		e.enabledHardwareCounters[arrayName] = struct{}{}
 	}
+
 	klog.Infof("Successfully load eBPF module from libbpf object")
 	return nil
 }

pkg/bpf/exporter_stub.go

@@ -25,12 +25,11 @@ func NewExporter() (Exporter, error) {
 	return &stubAttacher{}, nil
 }
 
-func (a *stubAttacher) GetEnabledBPFHWCounters() []string {
-	return []string{}
-}
-
-func (a *stubAttacher) GetEnabledBPFSWCounters() []string {
-	return []string{}
+func (a *stubAttacher) SupportedMetrics() SupportedMetrics {
+	return SupportedMetrics{
+		HardwareCounters: map[string]struct{}{},
+		SoftwareCounters: map[string]struct{}{},
+	}
 }
 
 func (a *stubAttacher) Detach() {

pkg/bpf/test_utils.go

@@ -3,17 +3,43 @@ package bpf
 import "github.com/sustainable-computing-io/kepler/pkg/config"
 
 type mockExporter struct {
-	hardwareCountersEnabled bool
+	softwareCounters map[string]struct{}
+	hardwareCounters map[string]struct{}
 }
 
-func NewMockExporter(hardwareCountersEnabled bool) Exporter {
+func DefaultSupportedMetrics() SupportedMetrics {
+	return SupportedMetrics{
+		HardwareCounters: defaultHardwareCounters(),
+		SoftwareCounters: defaultSoftwareCounters(),
+	}
+}
+
+func defaultHardwareCounters() map[string]struct{} {
+	return map[string]struct{}{config.CPUCycle: {}, config.CPUInstruction: {}, config.CacheMiss: {}, config.TaskClock: {}}
+}
+
+func defaultSoftwareCounters() map[string]struct{} {
+	swCounters := map[string]struct{}{config.CPUTime: {}, config.PageCacheHit: {}}
+	if config.ExposeIRQCounterMetrics {
+		swCounters[config.IRQNetTXLabel] = struct{}{}
+		swCounters[config.IRQNetRXLabel] = struct{}{}
+		swCounters[config.IRQBlockLabel] = struct{}{}
+	}
+	return swCounters
+}
+
+func NewMockExporter(bpfSupportedMetrics SupportedMetrics) Exporter {
 	return &mockExporter{
-		hardwareCountersEnabled: hardwareCountersEnabled,
+		softwareCounters: bpfSupportedMetrics.SoftwareCounters,
+		hardwareCounters: bpfSupportedMetrics.HardwareCounters,
 	}
 }
 
-func (m *mockExporter) HardwareCountersEnabled() bool {
-	return m.hardwareCountersEnabled
+func (m *mockExporter) SupportedMetrics() SupportedMetrics {
+	return SupportedMetrics{
+		HardwareCounters: m.hardwareCounters,
+		SoftwareCounters: m.softwareCounters,
+	}
 }
 
 func (m *mockExporter) Detach() {}
@@ -39,18 +65,3 @@ func (m *mockExporter) CollectProcesses() ([]ProcessBPFMetrics, error) {
 func (m *mockExporter) CollectCPUFreq() (map[int32]uint64, error) {
 	return map[int32]uint64{0: 0}, nil
 }
-
-func (m *mockExporter) GetEnabledBPFHWCounters() []string {
-	if !m.hardwareCountersEnabled {
-		return []string{}
-	}
-	return []string{config.CPUCycle, config.CPUInstruction, config.CacheMiss, config.TaskClock}
-}
-
-func (m *mockExporter) GetEnabledBPFSWCounters() []string {
-	swCounters := []string{config.CPUTime, config.TaskClock, config.PageCacheHit}
-	if config.ExposeIRQCounterMetrics {
-		swCounters = append(swCounters, SoftIRQEvents...)
-	}
-	return swCounters
-}

pkg/bpf/types.go

@@ -20,15 +20,26 @@ import (
 	"github.com/sustainable-computing-io/kepler/pkg/config"
 )
 
-var SoftIRQEvents = []string{config.IRQNetTXLabel, config.IRQNetRXLabel, config.IRQBlockLabel}
+const (
+	// Per /sys/kernel/debug/tracing/events/irq/softirq_entry/format
+	// { 0, "HI" }, { 1, "TIMER" }, { 2, "NET_TX" }, { 3, "NET_RX" }, { 4, "BLOCK" }, { 5, "IRQ_POLL" }, { 6, "TASKLET" }, { 7, "SCHED" }, { 8, "HRTIMER" }, { 9, "RCU" }
+
+	// IRQ vector to IRQ number
+	IRQNetTX = 2
+	IRQNetRX = 3
+	IRQBlock = 4
+)
 
 type Exporter interface {
-	HardwareCountersEnabled() bool
+	SupportedMetrics() SupportedMetrics
 	Detach()
 	CollectProcesses() ([]ProcessBPFMetrics, error)
 	CollectCPUFreq() (map[int32]uint64, error)
-	GetEnabledBPFHWCounters() []string
-	GetEnabledBPFSWCounters() []string
+}
+
+type SupportedMetrics struct {
+	HardwareCounters map[string]struct{}
+	SoftwareCounters map[string]struct{}
 }
 
 // must be in sync with bpf program