-
Notifications
You must be signed in to change notification settings - Fork 716
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
cmd/bpf2go: enable building custom tools (wip) #1320
Conversation
lmb once said: My personal preference would be to make the parts of bpf2go that are useful available via Go API so that power users can build their own tools. Turning bpf2go code into a library of reusable components is a massive effort. The resulting API surface would be quite large and it will be hard to commit to API stability. Explore the next best option: - expose bpf2go Run for use by custom tools built on top; - provide hooks to make it possible to customise certain aspects of bpf2go. Hook interface is purposedly minimal, based on demand, and gives no API stability guarantees. It will enable the community to experiment with new powerful features and immediately benefit from these new features in their projects. Hopefully, users will still work towards getting their features accepted. If it so happens that a feature is not in line with cilium/ebpf goals users could stick to their custom tool while benefitting from the upstream effort going into improving bpf2go.
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.
My initial impression: this is going to be a pain to support long-term. As this abstraction grows, it will grow tendrils throughout the codebase, as we're basically invoking caller-supplied logic everywhere. Any modifications will need to take into account that "oh, we always need to invoke the user's specified Compile() that's Docker-aware", which will end up severly limiting what this tool can become over time.
Also, this model implies all contributors are aware of the user's intentions at all times, lest we break something, which is not something we should sign ourselves up for IMO. This is where the term 'callback hell' originates from. You're no longer really in control over your program's execution flow, since the caller can inject behaviour everywhere, and you can no longer assume the state of things on the filesystem, etc.
Turning bpf2go into a library is probably the way forward. Composition over injection/inheritance.
} | ||
|
||
// Customisations enables building custom tools on top of bpf2go | ||
type Customisations interface { |
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.
Drive-by nit: isn't this basically a variation of *Options
, but with an interface instead of a struct?
} | ||
|
||
// TargetCustomisations enables building custom tools on top of bpf2go | ||
type TargetCustomisations interface { |
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.
What's the difference with a Customizations
? Why are they split?
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.
They are split to enable storing target-specific state in TargetCustomisations
. E.g. read DWARF in .Compile
, later use the data in .AugmentSpec
.
Thank you for the feedback! I'd like to understand the issues you see in The model I had in mind is summarised in the commit description:
If the tool evolution necessitates breaking
I am happy to explore that route as well. My concerns:
By unergonomic I refer to e.g. splitting planning and execution. E.g. |
Sorry for the long radio silence. I agree with Timo that hook based approaches are hard to maintain. I'm guessing that there are specific changes you'd like to make to bpf2go, could you describe them? That will make it easier to discuss solutions. |
I'd like to make it more of a turn-key solution. In order of priority (some of the items are highly specific to our use-case):
1. Ensure that everyone running
|
Very interesting, thanks! Sorry for the long wait, this slipped through the cracks and then I went on PTO.
Okay, seems like there are two goals here:
What if we allowed passing an OCI container spec for
Do you build for many arches? We could speed up builds by compiling in parallel. Or do you just compile lots of different files? I'm not sure about doing caching, it's complicated with a lot of edge cases. Maybe there are other things we can do to speed up builds? So far that has never been much of a priority.
I've thought about something similar, except that my idea was to ship pre-compiled libraries in go modules. eBPF code would use
I agree with the goal, but I'm worried about the complexity of parsing DWARF. Maybe this is something where we need to work with upstream clang / gcc developers (or just libbpf? not sure what generates BTF these days). Or invest time into finding a better work around. |
lmb once said: My personal preference would be to make the parts of bpf2go that are useful available via Go API so that power users can build their own tools.
Turning bpf2go code into a library of reusable components is a massive effort. The resulting API surface would be quite large and it will be hard to commit to API stability.
Explore the next best option:
expose bpf2go Run for use by custom tools built on top;
provide hooks to make it possible to customise certain aspects of bpf2go.
Hook interface is purposedly minimal, based on demand, and gives no API stability guarantees.
It will enable the community to experiment with new powerful features and immediately benefit from these new features in their projects. Hopefully, users will still work towards getting their features accepted.
If it so happens that a feature is not in line with cilium/ebpf goals users could stick to their custom tool while benefitting from the upstream effort going into improving bpf2go.