change aggregation flow map to hashmap instead perCPU hashmap (#118)

Signed-off-by: msherif1234 <[email protected]>

Author: msherif1234

bpf/flows.c

@@ -58,11 +58,12 @@ struct {
 } direct_flows SEC(".maps");
 
 // Key: the flow identifier. Value: the flow metrics for that identifier.
-// The userspace will aggregate them into a single flow.
 struct {
-    __uint(type, BPF_MAP_TYPE_PERCPU_HASH);
+    __uint(type, BPF_MAP_TYPE_HASH);
     __type(key, flow_id);
     __type(value, flow_metrics);
+    __uint(max_entries, 1 << 24);
+    __uint(map_flags, BPF_F_NO_PREALLOC);
 } aggregated_flows SEC(".maps");
 
 // Constant definitions, to be overridden by the invoker
@@ -260,11 +261,6 @@ static inline int flow_monitor(struct __sk_buff *skb, u8 direction) {
         aggregate_flow->packets += 1;
         aggregate_flow->bytes += skb->len;
         aggregate_flow->end_mono_time_ts = current_time;
-        // it might happen that start_mono_time hasn't been set due to
-        // the way percpu hashmap deal with concurrent map entries
-        if (aggregate_flow->start_mono_time_ts == 0) {
-            aggregate_flow->start_mono_time_ts = current_time;
-        }
         aggregate_flow->flags |= flags;
         long ret = bpf_map_update_elem(&aggregated_flows, &id, aggregate_flow, BPF_ANY);
         if (trace_messages && ret != 0) {

docs/architecture.md

@@ -11,7 +11,7 @@ flowchart TD
     E(ebpf.FlowFetcher) --> |"pushes via<br/>RingBuffer"| RB(flow.RingBufTracer)
     style E fill:#990
 
-    E --> |"polls<br/>PerCPUHashMap"| M(flow.MapTracer)
+    E --> |"polls<br/>HashMap"| M(flow.MapTracer)
     RB --> |chan *flow.Record| ACC(flow.Accounter)
     RB -.-> |flushes| M
     ACC --> |"chan []*flow.Record"| DD(flow.Deduper)

pkg/agent/agent.go

@@ -78,7 +78,7 @@ type ebpfFlowFetcher interface {
 	io.Closer
 	Register(iface ifaces.Interface) error
 
-	LookupAndDeleteMap() map[ebpf.BpfFlowId][]ebpf.BpfFlowMetrics
+	LookupAndDeleteMap() map[ebpf.BpfFlowId]ebpf.BpfFlowMetrics
 	ReadRingBuf() (ringbuf.Record, error)
 }
 

pkg/agent/agent_test.go

@@ -49,11 +49,6 @@ var (
 		DstPort: 456,
 		IfIndex: 3,
 	}
-	key1Dupe = ebpf.BpfFlowId{
-		SrcPort: 123,
-		DstPort: 456,
-		IfIndex: 4,
-	}
 
 	key2 = ebpf.BpfFlowId{
 		SrcPort: 333,
@@ -71,7 +66,7 @@ func TestFlowsAgent_Deduplication(t *testing.T) {
 	})
 
 	exported := export.Get(t, timeout)
-	assert.Len(t, exported, 2)
+	assert.Len(t, exported, 1)
 
 	receivedKeys := map[ebpf.BpfFlowId]struct{}{}
 
@@ -81,21 +76,11 @@ func TestFlowsAgent_Deduplication(t *testing.T) {
 		receivedKeys[f.Id] = struct{}{}
 		switch f.Id {
 		case key1:
-			assert.EqualValues(t, 4, f.Metrics.Packets)
-			assert.EqualValues(t, 66, f.Metrics.Bytes)
+			assert.EqualValues(t, 3, f.Metrics.Packets)
+			assert.EqualValues(t, 44, f.Metrics.Bytes)
 			assert.False(t, f.Duplicate)
 			assert.Equal(t, "foo", f.Interface)
 			key1Flows = append(key1Flows, f)
-		case key1Dupe:
-			assert.EqualValues(t, 4, f.Metrics.Packets)
-			assert.EqualValues(t, 66, f.Metrics.Bytes)
-			assert.False(t, f.Duplicate)
-			assert.Equal(t, "bar", f.Interface)
-			key1Flows = append(key1Flows, f)
-		case key2:
-			assert.EqualValues(t, 7, f.Metrics.Packets)
-			assert.EqualValues(t, 33, f.Metrics.Bytes)
-			assert.False(t, f.Duplicate)
 		}
 	}
 	assert.Lenf(t, key1Flows, 1, "only one flow should have been forwarded: %#v", key1Flows)
@@ -112,33 +97,22 @@ func TestFlowsAgent_DeduplicationJustMark(t *testing.T) {
 	exported := export.Get(t, timeout)
 	receivedKeys := map[ebpf.BpfFlowId]struct{}{}
 
-	assert.Len(t, exported, 3)
+	assert.Len(t, exported, 1)
 	duplicates := 0
 	for _, f := range exported {
 		require.NotContains(t, receivedKeys, f.Id)
 		receivedKeys[f.Id] = struct{}{}
 		switch f.Id {
 		case key1:
-			assert.EqualValues(t, 4, f.Metrics.Packets)
-			assert.EqualValues(t, 66, f.Metrics.Bytes)
+			assert.EqualValues(t, 3, f.Metrics.Packets)
+			assert.EqualValues(t, 44, f.Metrics.Bytes)
 			if f.Duplicate {
 				duplicates++
 			}
 			assert.Equal(t, "foo", f.Interface)
-		case key1Dupe:
-			assert.EqualValues(t, 4, f.Metrics.Packets)
-			assert.EqualValues(t, 66, f.Metrics.Bytes)
-			if f.Duplicate {
-				duplicates++
-			}
-			assert.Equal(t, "bar", f.Interface)
-		case key2:
-			assert.EqualValues(t, 7, f.Metrics.Packets)
-			assert.EqualValues(t, 33, f.Metrics.Bytes)
-			assert.False(t, f.Duplicate)
 		}
 	}
-	assert.Equalf(t, 1, duplicates, "exported flows should have only one duplicate: %#v", exported)
+	assert.Equalf(t, 0, duplicates, "exported flows should have only one duplicate: %#v", exported)
 }
 
 func TestFlowsAgent_Deduplication_None(t *testing.T) {
@@ -149,7 +123,7 @@ func TestFlowsAgent_Deduplication_None(t *testing.T) {
 	})
 
 	exported := export.Get(t, timeout)
-	assert.Len(t, exported, 3)
+	assert.Len(t, exported, 1)
 	receivedKeys := map[ebpf.BpfFlowId]struct{}{}
 
 	var key1Flows []*flow.Record
@@ -158,24 +132,14 @@ func TestFlowsAgent_Deduplication_None(t *testing.T) {
 		receivedKeys[f.Id] = struct{}{}
 		switch f.Id {
 		case key1:
-			assert.EqualValues(t, 4, f.Metrics.Packets)
-			assert.EqualValues(t, 66, f.Metrics.Bytes)
+			assert.EqualValues(t, 3, f.Metrics.Packets)
+			assert.EqualValues(t, 44, f.Metrics.Bytes)
 			assert.False(t, f.Duplicate)
 			assert.Equal(t, "foo", f.Interface)
 			key1Flows = append(key1Flows, f)
-		case key1Dupe:
-			assert.EqualValues(t, 4, f.Metrics.Packets)
-			assert.EqualValues(t, 66, f.Metrics.Bytes)
-			assert.False(t, f.Duplicate)
-			assert.Equal(t, "bar", f.Interface)
-			key1Flows = append(key1Flows, f)
-		case key2:
-			assert.EqualValues(t, 7, f.Metrics.Packets)
-			assert.EqualValues(t, 33, f.Metrics.Bytes)
-			assert.False(t, f.Duplicate)
 		}
 	}
-	assert.Lenf(t, key1Flows, 2, "both key1 flows should have been forwarded: %#v", key1Flows)
+	assert.Lenf(t, key1Flows, 1, "both key1 flows should have been forwarded: %#v", key1Flows)
 }
 
 func TestFlowsAgent_Decoration(t *testing.T) {
@@ -185,7 +149,7 @@ func TestFlowsAgent_Decoration(t *testing.T) {
 	})
 
 	exported := export.Get(t, timeout)
-	assert.Len(t, exported, 3)
+	assert.Len(t, exported, 1)
 
 	// Tests that the decoration stage has been properly executed. It should
 	// add the interface name and the agent IP
@@ -219,18 +183,10 @@ func testAgent(t *testing.T, cfg *Config) *test.ExporterFake {
 	})
 
 	now := uint64(monotime.Now())
-	key1Metrics := []ebpf.BpfFlowMetrics{
-		{Packets: 3, Bytes: 44, StartMonoTimeTs: now + 1000, EndMonoTimeTs: now + 1_000_000_000},
-		{Packets: 1, Bytes: 22, StartMonoTimeTs: now, EndMonoTimeTs: now + 3000},
-	}
-	key2Metrics := []ebpf.BpfFlowMetrics{
-		{Packets: 7, Bytes: 33, StartMonoTimeTs: now, EndMonoTimeTs: now + 2_000_000_000},
-	}
+	key1Metrics := ebpf.BpfFlowMetrics{Packets: 3, Bytes: 44, StartMonoTimeTs: now + 1000, EndMonoTimeTs: now + 1_000_000_000}
 
-	ebpfTracer.AppendLookupResults(map[ebpf.BpfFlowId][]ebpf.BpfFlowMetrics{
-		key1:     key1Metrics,
-		key1Dupe: key1Metrics,
-		key2:     key2Metrics,
+	ebpfTracer.AppendLookupResults(map[ebpf.BpfFlowId]ebpf.BpfFlowMetrics{
+		key1: key1Metrics,
 	})
 	return export
 }

pkg/ebpf/bpf_bpfeb.go

@@ -308,25 +308,25 @@ func (m *FlowFetcher) ReadRingBuf() (ringbuf.Record, error) {
 // TODO: detect whether BatchLookupAndDelete is supported (Kernel>=5.6) and use it selectively
 // Supported Lookup/Delete operations by kernel: https://github.com/iovisor/bcc/blob/master/docs/kernel-versions.md
 // Race conditions here causes that some flows are lost in high-load scenarios
-func (m *FlowFetcher) LookupAndDeleteMap() map[BpfFlowId][]BpfFlowMetrics {
+func (m *FlowFetcher) LookupAndDeleteMap() map[BpfFlowId]BpfFlowMetrics {
 	flowMap := m.objects.AggregatedFlows
 
 	iterator := flowMap.Iterate()
-	flows := make(map[BpfFlowId][]BpfFlowMetrics, m.cacheMaxSize)
+	var flow = make(map[BpfFlowId]BpfFlowMetrics, m.cacheMaxSize)
 
 	id := BpfFlowId{}
-	var metrics []BpfFlowMetrics
+	var metric BpfFlowMetrics
 	// Changing Iterate+Delete by LookupAndDelete would prevent some possible race conditions
 	// TODO: detect whether LookupAndDelete is supported (Kernel>=4.20) and use it selectively
-	for iterator.Next(&id, &metrics) {
+	for iterator.Next(&id, &metric) {
 		if err := flowMap.Delete(id); err != nil {
 			log.WithError(err).WithField("flowId", id).
 				Warnf("couldn't delete flow entry")
 		}
 		// We observed that eBFP PerCPU map might insert multiple times the same key in the map
 		// (probably due to race conditions) so we need to re-join metrics again at userspace
 		// TODO: instrument how many times the keys are is repeated in the same eviction
-		flows[id] = append(flows[id], metrics...)
+		flow[id] = metric
 	}
-	return flows
+	return flow
 }

pkg/ebpf/bpf_bpfeb.o

@@ -65,9 +65,7 @@ func (c *Accounter) Account(in <-chan *RawRecord, out chan<- []*Record) {
 				alog.Debug("exiting account routine")
 				return
 			}
-			if stored, ok := c.entries[record.Id]; ok {
-				Accumulate(stored, &record.Metrics)
-			} else {
+			if _, ok := c.entries[record.Id]; !ok {
 				if len(c.entries) >= c.maxEntries {
 					evictingEntries := c.entries
 					c.entries = map[ebpf.BpfFlowId]*ebpf.BpfFlowMetrics{}