[NPM-3378] Adding eBPF-less network tracer POC #27100
Conversation
hmahmood
added
changelog/no-changelog
qa/done
|
🚂 MergeQueue: waiting for PR to be ready This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals. Use |
|
This merge request was unqueued If you need support, contact us on Slack #devflow! |
| CORE BuildMode | ||
| Fentry BuildMode | ||
| Ebpfless BuildMode | ||
| ) |
There was a problem hiding this comment.
I feel like we should dedicate some time to the build modes. I don't think it's a blocker for this PR considering it's a PoC and the ebpfless mode isn't enabled by default (wdyt @brycekahle?), but we could probably find a way to reconciliate the actual eBPF build modes (prebuild/runtime/CORE) with fentry and ebpfless which are not something we'll want in the rest of test code.
There was a problem hiding this comment.
ebpfless feels OK because it is mutually exclusive from prebuilt/rc/co-re. fentry is the confusing one, because that could be any of the build modes.
| // NewTracer returns a new Tracer | ||
| func NewTracer(cfg *config.Config, telemetryComp telemetry.Component) (Tracer, error) { | ||
| if cfg.EnableEbpfless { | ||
| return newEbpfLessTracer(cfg) |
There was a problem hiding this comment.
Could the ebpf-less tracer be in a separate package and implement the Tracer interface? That was the original intent of that interface: to allow different implementations. Then you wouldn't have the git blame issue Lee was mentioning.
Creating pkg/network/tracer/ebpfless for the ebpf-less tracer.
There was a problem hiding this comment.
I did try that, and I could not really sort out the import cycles without making this PR bigger than it already is. Perhaps in a followup I can attempt that again.
The git blame issue is not as bad as we initially thought. I can see the blame showing up in github across the renames.
hmahmood
force-pushed
the
hasan.mahmood/ebpfless-tracer
branch
from
31b9b38 to
716ada1
Compare
hmahmood
force-pushed
the
hasan.mahmood/ebpfless-tracer
branch
from
716ada1 to
babc45d
Compare
hmahmood
force-pushed
the
hasan.mahmood/ebpfless-tracer
branch
from
babc45d to
ac937f7
Compare
Co-authored-by: Bryce Kahle <[email protected]>
| // PayloadLen returns the length of the application | ||
| // payload given the set of layers in `Layers` | ||
| func (l Layers) PayloadLen() (uint16, error) { | ||
| if l.UDP != nil { |
There was a problem hiding this comment.
this gets called in the tcp processing path as well.. I've noticed locally it triggers because the UDP layer is not nil but it is empty, which bails out of the TCP connection parsing. perhaps we should do
if len(l.UDP.Payload) > 0 || len(l.UDP.Contents) > 0
(or write a helper isEmpty() that checks all field values)
(or or just omit this check entirely if we know it's a TCP connection, and refactor this into two methods with a helper)
There was a problem hiding this comment.
I removed epbfless.Layers, so this should happen anymore.
|
/merge |
|
🚂 MergeQueue: waiting for PR to be ready This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals. Use |
|
🚂 MergeQueue: pull request added to the queue The median merge time in Use |
What does this PR do?
Adds a POC eBPF-less network tracer implementation. Only basic stats like bytes/packets sent/received and rudimentary TCP state tracking are currently supported. There is a new build mode to run tests under ebpf-less, but this only enabled if
TEST_EBPFLESS_OVERRIDE=trueis set in the environment.Motivation
Additional Notes
Possible Drawbacks / Trade-offs
Describe how to test/QA your changes