-
Notifications
You must be signed in to change notification settings - Fork 271
feat(packetparser): add configurable BPF Ring Buffer support #1981
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
7549b97 to
0f47a08
Compare
0f47a08 to
60dd31f
Compare
|
Can you get a bpf map dump for each scenario into the PR as well? |
|
What are the cons for switching over to ring buffer as default? IMO, either make this default or make this decision during runtime (packetparser makes the decision based on node specifications). This seems like a node specific decision and maybe too complex for user to determine. |
rbtr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i like it being configurable at runtime. can you update the docs to explain when this should be changed?
| MonitorSockPath string `yaml:"monitorSockPath"` | ||
| TelemetryInterval time.Duration `yaml:"telemetryInterval"` | ||
| DataSamplingRate uint32 `yaml:"dataSamplingRate"` | ||
| EnablePacketParserRingBuffer bool `yaml:"enablePacketParserRingBuffer"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should have two options - auto and false (default auto).
Packetparser should make runtime decision on whether to use ring buffer or not .


Description
This PR introduces support for
BPF_MAP_TYPE_RINGBUFin thepacketparserplugin as a configurable alternative toBPF_MAP_TYPE_PERF_EVENT_ARRAY. Ring Buffers offer better performance and memory efficiency, especially on high-core systems, by using a shared buffer across CPUs rather than per-CPU buffers.Changes
enablePacketParserRingBuffer(bool) andpacketParserRingBufferSize(uint32) to the Retina configuration and Helm charts.packetparser.cto conditionally compile withBPF_MAP_TYPE_RINGBUFwhen enabled.packetparser_linux.goto:-DUSE_RING_BUFFERand-DRING_BUFFER_SIZEflags during BPF compilation.ringBufReaderWrapperto adapt thecilium/ebpf/ringbufreader to the existing reader interface.Verification
go test -v ./pkg/plugin/packetparser/...enablePacketParserRingBuffer: true.ringbufusingbpftool.Related Issues/PRs
BPF_MAP_TYPE_RINGBUF) impl in packetparser #1966Checklist
git commit -S -s ...). See this documentation on signing commits.Screenshots (if applicable) or Testing Completed
Additional Notes
Once this PR gets reviewed and merged, I will update the docs accordingly.
Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.