From 30308514aa948bec8a5ca63b7a2035ef86773cb7 Mon Sep 17 00:00:00 2001 From: DanielLavie Date: Thu, 19 Dec 2024 14:40:45 +0200 Subject: [PATCH 1/3] Using ddebpf.Manager instead of manager.Manager in consumer_test.go --- pkg/network/protocols/events/consumer_test.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/network/protocols/events/consumer_test.go b/pkg/network/protocols/events/consumer_test.go index 5e9864b5a342a..1892d6e55b364 100644 --- a/pkg/network/protocols/events/consumer_test.go +++ b/pkg/network/protocols/events/consumer_test.go @@ -16,6 +16,7 @@ import ( "time" "unsafe" + ebpftelemetry "github.com/DataDog/datadog-agent/pkg/ebpf/telemetry" manager "github.com/DataDog/ebpf-manager" "github.com/cilium/ebpf" "github.com/cilium/ebpf/features" @@ -53,7 +54,7 @@ func TestConsumer(t *testing.T) { } } - consumer, err := NewConsumer("test", program, callback) + consumer, err := NewConsumer("test", program.Manager, callback) require.NoError(t, err) consumer.Start() @@ -61,7 +62,7 @@ func TestConsumer(t *testing.T) { require.NoError(t, err) // generate test events - generator := newEventGenerator(program, t) + generator := newEventGenerator(program.Manager, t) for i := 0; i < numEvents; i++ { generator.Generate(uint64(i)) } @@ -92,7 +93,7 @@ func TestInvalidBatchCountMetric(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { program.Stop(manager.CleanAll) }) - consumer, err := NewConsumer("test", program, func([]uint64) {}) + consumer, err := NewConsumer("test", program.Manager, func([]uint64) {}) require.NoError(t, err) // We are creating a raw sample with a data length of 4, which is smaller than sizeOfBatch @@ -172,7 +173,7 @@ func (e *eventGenerator) Stop() { e.testFile.Close() } -func newEBPFProgram(c *config.Config) (*manager.Manager, error) { +func newEBPFProgram(c *config.Config) (*ddebpf.Manager, error) { bc, err := bytecode.GetReader(c.BPFDir, "usm_events_test-debug.o") if err != nil { return nil, err @@ -208,11 +209,13 @@ func newEBPFProgram(c *config.Config) (*manager.Manager, error) { }, } - Configure(config.New(), "test", m, &options) + ddEbpfManager := ddebpf.NewManager(m, "usm", &ebpftelemetry.ErrorsTelemetryModifier{}) + + Configure(config.New(), "test", ddEbpfManager.Manager, &options) err = m.InitWithOptions(bc, options) if err != nil { return nil, err } - return m, nil + return ddEbpfManager, nil } From 4fa39101eda87722007d5c3a96040aff5e66b8fc Mon Sep 17 00:00:00 2001 From: DanielLavie Date: Thu, 19 Dec 2024 15:38:25 +0200 Subject: [PATCH 2/3] Fixed InitWithOptions usage --- pkg/network/protocols/events/consumer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/network/protocols/events/consumer_test.go b/pkg/network/protocols/events/consumer_test.go index 1892d6e55b364..5981e776711e6 100644 --- a/pkg/network/protocols/events/consumer_test.go +++ b/pkg/network/protocols/events/consumer_test.go @@ -212,7 +212,7 @@ func newEBPFProgram(c *config.Config) (*ddebpf.Manager, error) { ddEbpfManager := ddebpf.NewManager(m, "usm", &ebpftelemetry.ErrorsTelemetryModifier{}) Configure(config.New(), "test", ddEbpfManager.Manager, &options) - err = m.InitWithOptions(bc, options) + err = ddEbpfManager.InitWithOptions(bc, &options) if err != nil { return nil, err } From 2d27f50e8fe58294f8cea1f4ecea588f8f53ba2a Mon Sep 17 00:00:00 2001 From: DanielLavie Date: Thu, 19 Dec 2024 16:03:31 +0200 Subject: [PATCH 3/3] Using ddebpf.Manager instead of manager.Manager in all relevant locations --- pkg/network/protocols/events/configuration.go | 12 ++++++------ pkg/network/protocols/events/consumer.go | 6 ++---- pkg/network/protocols/events/consumer_test.go | 10 +++++----- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/pkg/network/protocols/events/configuration.go b/pkg/network/protocols/events/configuration.go index 0888c8d2fde7d..55b35e9cb5153 100644 --- a/pkg/network/protocols/events/configuration.go +++ b/pkg/network/protocols/events/configuration.go @@ -40,7 +40,7 @@ const defaultPerfHandlerSize = 100 // This essentially instantiates the perf map/ring buffers and configure the // eBPF maps where events are enqueued. // Note this must be called *before* manager.InitWithOptions -func Configure(cfg *config.Config, proto string, m *manager.Manager, o *manager.Options) { +func Configure(cfg *config.Config, proto string, m *ddebpf.Manager, o *manager.Options) { if alreadySetUp(proto, m) { return } @@ -63,7 +63,7 @@ func Configure(cfg *config.Config, proto string, m *manager.Manager, o *manager. } } -func setupPerfMap(proto string, m *manager.Manager) { +func setupPerfMap(proto string, m *ddebpf.Manager) { handler := ddebpf.NewPerfHandler(defaultPerfHandlerSize) mapName := eventMapName(proto) pm := &manager.PerfMap{ @@ -90,7 +90,7 @@ func setupPerfMap(proto string, m *manager.Manager) { setHandler(proto, handler) } -func setupPerfRing(proto string, m *manager.Manager, o *manager.Options, numCPUs int) { +func setupPerfRing(proto string, m *ddebpf.Manager, o *manager.Options, numCPUs int) { handler := ddebpf.NewRingBufferHandler(defaultPerfHandlerSize) mapName := eventMapName(proto) ringBufferSize := toPowerOf2(numCPUs * defaultPerfEventBufferSize) @@ -155,12 +155,12 @@ func eventMapName(proto string) string { // *However* in some instances this is not working on 4.14, so here we // essentially replace `bpf_ringbuf_output` helper calls by a noop operation so // they don't result in verifier errors even when deadcode elimination fails. -func removeRingBufferHelperCalls(m *manager.Manager) { +func removeRingBufferHelperCalls(m *ddebpf.Manager) { // TODO: this is not the intended API usage of a `ebpf.Modifier`. // Once we have access to the `ddebpf.Manager`, add this modifier to its list of // `EnabledModifiers` and let it control the execution of the callbacks patcher := ddebpf.NewHelperCallRemover(asm.FnRingbufOutput) - err := patcher.BeforeInit(m, names.NewModuleName("usm"), nil) + err := patcher.BeforeInit(m.Manager, names.NewModuleName("usm"), nil) if err != nil { // Our production code is actually loading on all Kernels we test on CI @@ -178,7 +178,7 @@ func removeRingBufferHelperCalls(m *manager.Manager) { } } -func alreadySetUp(proto string, m *manager.Manager) bool { +func alreadySetUp(proto string, m *ddebpf.Manager) bool { // check if we already have configured this perf map this can happen in the // context of a failed program load succeeded by another attempt mapName := eventMapName(proto) diff --git a/pkg/network/protocols/events/consumer.go b/pkg/network/protocols/events/consumer.go index eb00d29678b65..2cda5b137bafa 100644 --- a/pkg/network/protocols/events/consumer.go +++ b/pkg/network/protocols/events/consumer.go @@ -13,8 +13,6 @@ import ( "sync" "unsafe" - manager "github.com/DataDog/ebpf-manager" - ddebpf "github.com/DataDog/datadog-agent/pkg/ebpf" "github.com/DataDog/datadog-agent/pkg/ebpf/maps" "github.com/DataDog/datadog-agent/pkg/network/protocols/telemetry" @@ -57,9 +55,9 @@ type Consumer[V any] struct { // `callback` is executed once for every "event" generated on eBPF and must: // 1) copy the data it wishes to hold since the underlying byte array is reclaimed; // 2) be thread-safe, as the callback may be executed concurrently from multiple go-routines; -func NewConsumer[V any](proto string, ebpf *manager.Manager, callback func([]V)) (*Consumer[V], error) { +func NewConsumer[V any](proto string, ebpf *ddebpf.Manager, callback func([]V)) (*Consumer[V], error) { batchMapName := proto + batchMapSuffix - batchMap, err := maps.GetMap[batchKey, batch](ebpf, batchMapName) + batchMap, err := maps.GetMap[batchKey, batch](ebpf.Manager, batchMapName) if err != nil { return nil, fmt.Errorf("unable to find map %s: %s", batchMapName, err) } diff --git a/pkg/network/protocols/events/consumer_test.go b/pkg/network/protocols/events/consumer_test.go index 5981e776711e6..b4c8348e6ec09 100644 --- a/pkg/network/protocols/events/consumer_test.go +++ b/pkg/network/protocols/events/consumer_test.go @@ -54,7 +54,7 @@ func TestConsumer(t *testing.T) { } } - consumer, err := NewConsumer("test", program.Manager, callback) + consumer, err := NewConsumer("test", program, callback) require.NoError(t, err) consumer.Start() @@ -62,7 +62,7 @@ func TestConsumer(t *testing.T) { require.NoError(t, err) // generate test events - generator := newEventGenerator(program.Manager, t) + generator := newEventGenerator(program, t) for i := 0; i < numEvents; i++ { generator.Generate(uint64(i)) } @@ -93,7 +93,7 @@ func TestInvalidBatchCountMetric(t *testing.T) { require.NoError(t, err) t.Cleanup(func() { program.Stop(manager.CleanAll) }) - consumer, err := NewConsumer("test", program.Manager, func([]uint64) {}) + consumer, err := NewConsumer("test", program, func([]uint64) {}) require.NoError(t, err) // We are creating a raw sample with a data length of 4, which is smaller than sizeOfBatch @@ -132,7 +132,7 @@ func recordSample(c *config.Config, consumer *Consumer[uint64], sampleData []byt } } -func newEventGenerator(program *manager.Manager, t *testing.T) *eventGenerator { +func newEventGenerator(program *ddebpf.Manager, t *testing.T) *eventGenerator { m, _, _ := program.GetMap("test") require.NotNilf(t, m, "couldn't find test map") @@ -211,7 +211,7 @@ func newEBPFProgram(c *config.Config) (*ddebpf.Manager, error) { ddEbpfManager := ddebpf.NewManager(m, "usm", &ebpftelemetry.ErrorsTelemetryModifier{}) - Configure(config.New(), "test", ddEbpfManager.Manager, &options) + Configure(config.New(), "test", ddEbpfManager, &options) err = ddEbpfManager.InitWithOptions(bc, &options) if err != nil { return nil, err