Skip to content

Commit 559da95

Browse files
committed
fix(network/tracer): fix data race on conntrackerTelemetry.lastRegisters
Collect() did a non-atomic read-modify-write on the package-level conntrackerTelemetry.lastRegisters field. When two Prometheus Gather ticks overlapped, both goroutines raced on this field. Move lastRegisters to the ebpfConntracker instance and use a CAS loop (CompareAndSwap) so the read-modify-write is atomic and the stored counter only moves forward. Making the baseline per-instance also fixes a regression on module restart: a new conntracker gets a fresh telemetry map, so the package-level baseline (which retained the old map's counter) would have suppressed all deltas until the new counter exceeded the old total. Found via a race-detector-enabled build in staging (see #54333).
1 parent 768df0b commit 559da95

1 file changed

Lines changed: 27 additions & 4 deletions

File tree

pkg/network/tracer/ebpf_conntracker.go

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"fmt"
1414
"io"
1515
"net/netip"
16+
"sync/atomic"
1617
"time"
1718

1819
manager "github.com/DataDog/ebpf-manager"
@@ -62,14 +63,12 @@ var conntrackerTelemetry = struct {
6263
getsTotal telemetryComp.Counter
6364
unregistersTotal telemetryComp.Counter
6465
registersTotal *prometheus.Desc
65-
lastRegisters uint64
6666
}{
6767
telemetryimpl.GetCompatComponent().NewHistogram(ebpfConntrackerModuleName, "gets_duration_nanoseconds", []string{}, "Histogram measuring the time spent retrieving connection tuples from the EBPF map", defaultBuckets),
6868
telemetryimpl.GetCompatComponent().NewHistogram(ebpfConntrackerModuleName, "unregisters_duration_nanoseconds", []string{}, "Histogram measuring the time spent deleting connection tuples from the EBPF map", defaultBuckets),
6969
telemetryimpl.GetCompatComponent().NewCounter(ebpfConntrackerModuleName, "gets_total", []string{}, "Counter measuring the total number of attempts to get connection tuples from the EBPF map"),
7070
telemetryimpl.GetCompatComponent().NewCounter(ebpfConntrackerModuleName, "unregisters_total", []string{}, "Counter measuring the total number of attempts to delete connection tuples from the EBPF map"),
7171
prometheus.NewDesc(ebpfConntrackerModuleName+"__registers_total", "Counter measuring the total number of attempts to update/create connection tuples in the EBPF map", nil, nil),
72-
0,
7372
}
7473

7574
type ebpfConntracker struct {
@@ -83,6 +82,13 @@ type ebpfConntracker struct {
8382
stop chan struct{}
8483

8584
isPrebuilt bool
85+
86+
// lastRegisters is the baseline for the registers_total delta reported via
87+
// Collect. It is per-instance so that a module restart (which builds a new
88+
// ebpfConntracker with a fresh telemetry map) starts from zero instead of
89+
// inheriting the previous map's counter, which would suppress all deltas
90+
// until the new counter exceeds the old one.
91+
lastRegisters atomic.Uint64
8692
}
8793

8894
var ebpfConntrackerCORECreator func(cfg *config.Config) (*manager.Manager, error) = getCOREConntracker
@@ -411,12 +417,29 @@ func (e *ebpfConntracker) Collect(ch chan<- prometheus.Metric) {
411417
if err := e.telemetryMap.Lookup(&zero, ebpfTelemetry); err != nil {
412418
log.Tracef("error retrieving the telemetry struct: %s", err)
413419
} else {
414-
delta := ebpfTelemetry.Registers - conntrackerTelemetry.lastRegisters
415-
conntrackerTelemetry.lastRegisters = ebpfTelemetry.Registers
420+
delta := e.computeRegistersDelta(ebpfTelemetry.Registers)
416421
ch <- prometheus.MustNewConstMetric(conntrackerTelemetry.registersTotal, prometheus.CounterValue, float64(delta))
417422
}
418423
}
419424

425+
// computeRegistersDelta atomically computes the delta between the current and
426+
// last-seen register count, and updates the last-seen value. It is safe to call
427+
// concurrently (e.g. when two Prometheus Gather ticks overlap). A CAS loop
428+
// ensures the stored counter only moves forward, so a stale map lookup can
429+
// neither regress lastRegisters nor report a duplicate (or underflowing) delta.
430+
// The baseline is per-conntracker-instance so a module restart resets it.
431+
func (e *ebpfConntracker) computeRegistersDelta(currentRegisters uint64) uint64 {
432+
for {
433+
last := e.lastRegisters.Load()
434+
if currentRegisters <= last {
435+
return 0
436+
}
437+
if e.lastRegisters.CompareAndSwap(last, currentRegisters) {
438+
return currentRegisters - last
439+
}
440+
}
441+
}
442+
420443
func getManager(cfg *config.Config, buf io.ReaderAt, opts manager.Options, buildMode buildmode.Type) (*manager.Manager, error) {
421444
conntrackMaps := []*manager.Map{
422445
{Name: probes.ConntrackMap},

0 commit comments

Comments
 (0)