feat(packetparser): use ring buffer back-pressure#2208
feat(packetparser): use ring buffer back-pressure#2208holly-agyei wants to merge 1 commit intomicrosoft:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the packetparser plugin so that when packetParserRingBuffer=enabled, event dropping is governed by ring buffer back-pressure (reserve/submit) rather than the static dataSamplingRate, while keeping existing sampling behavior for the perf event array path.
Changes:
- Skip generating
DATA_SAMPLING_RATEindynamic.hwhen ring buffer mode is enabled; keep it for perf event arrays. - Switch ring buffer emission to
bpf_ringbuf_reserve()/bpf_ringbuf_submit()and disable sampling logic under ring buffer mode in the eBPF program. - Add/adjust tests and documentation to cover and explain the different behaviors in perf vs ring buffer mode.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/plugin/packetparser/packetparser_linux.go | Omits DATA_SAMPLING_RATE define in ring buffer mode; logs that sampling is ignored for ring buffer. |
| pkg/plugin/packetparser/packetparser_linux_test.go | Adds a header-generation test ensuring ring buffer mode ignores dataSamplingRate. |
| pkg/plugin/packetparser/packetparser_ebpf_test.go | Updates variant compilation to omit DATA_SAMPLING_RATE under ring buffer; adds tests for perf sampling vs ring buffer behavior. |
| pkg/plugin/packetparser/_cprog/packetparser.c | Introduces a unified emit helper and switches ring buffer path to reserve/submit; disables sampling when USE_RING_BUFFER is set. |
| docs/06-Troubleshooting/performance.md | Documents ring buffer back-pressure as a performance option; clarifies sampling behavior by mode. |
| docs/03-Metrics/plugins/Linux/packetparser.md | Updates performance considerations to recommend ring buffers; clarifies that sampling is ignored in ring buffer mode. |
| docs/02-Installation/03-Config.md | Clarifies dataSamplingRate applies only when ring buffers are disabled; documents ring buffer behavior. |
| docs/01-Introduction/01-intro.md | Updates guidance for high-core/high-throughput nodes to prefer ring buffers. |
| deploy/standard/manifests/controller/helm/retina/values.yaml | Adds comments clarifying dataSamplingRate is ignored in ring buffer mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func TestHighAggregationSamplingSuppressesPerfBufferEvents(t *testing.T) { | ||
| objs, reader := compileAndLoadVariant(t, compileOpts{ | ||
| bypassFilter: 1, | ||
| enableConntrack: false, | ||
| aggregationLevel: 1, // HIGH | ||
| samplingRate: 2147483647, // effectively never sampled | ||
| }) |
There was a problem hiding this comment.
TestHighAggregationSamplingSuppressesPerfBufferEvents relies on bpf_get_prandom_u32() for sampling and then asserts the ACK event is not emitted. Even with a very large samplingRate, there is still a non-zero chance the ACK is sampled and emitted, which can make this test flaky over enough CI runs. Consider making the assertion deterministic by avoiding a single random trial (e.g., send many ACK packets and assert fewer than N events were emitted, or add a test-only compile flag/hook to force sampling on/off).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
holly-agyei
force-pushed
the
feat-1967-adaptive-sampling
branch
from
9fda7df to
652930b
Compare
Signed-off-by: Agyei Holy <agyeiholy978@gmail.com>
holly-agyei
force-pushed
the
feat-1967-adaptive-sampling
branch
from
652930b to
0cff9d0
Compare
Description
This updates
packetparserso ring buffer mode uses ring buffer back-pressure instead of the staticdataSamplingRate.When
packetParserRingBuffer=enabled, Retina now:DATA_SAMPLING_RATEin the generated headerbpf_ringbuf_reserve()andbpf_ringbuf_submit()This also adds tests for both paths and updates the docs to explain how
dataSamplingRateandpacketParserRingBufferSizebehave in each mode.Related Issue
Related to #1966
Related to #655
Checklist
git commit -S -s ...). See this documentation on signing commits.Screenshots (if applicable) or Testing Completed
go generate ./pkg/plugin/...go generate ./...golangci-lintwithGOOS=linux GOARCH=amd64golangci-lintwithGOOS=linux GOARCH=arm64go build ./...withGOOS=linux GOARCH=amd64go build ./...withGOOS=linux GOARCH=arm64go test -c ./pkg/plugin/packetparserwithGOOS=linux GOARCH=amd64go test -c ./pkg/plugin/packetparserwithGOOS=linux GOARCH=arm64go test -tags=ebpf -c ./pkg/plugin/packetparserwithGOOS=linux GOARCH=amd64go test -tags=ebpf -c ./pkg/plugin/packetparserwithGOOS=linux GOARCH=arm64markdownlint-cli2on the updated docsAdditional Notes
dataSamplingRatestill applies to the perf event array path.packetParserRingBufferSizecontrols burst capacity in ring buffer mode.