Skip to content
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

link: tracepoint support #186

Closed
wants to merge 3 commits into from
Closed

link: tracepoint support #186

wants to merge 3 commits into from

Conversation

lmb
Copy link
Collaborator

@lmb lmb commented Dec 15, 2020

link: add perf event tracepoint support

Allow attaching to perf event fd based tracepoints (which are
different than raw tracepoints).

Fixes #83

link: check program type for raw tracepoints

Return a more useful error message when passing a program with
the wrong type. This is especially useful here because it's
easy to mix up tracepoints and raw tracepoints.

link: allow Update to return ErrNotSupported

Raw tracepoints don't allow updating the program, yet they use
the bpf_link infrastructure. Allow Update to return ErrNotSupported
for that reason.

Raw tracepoints don't allow updating the program, yet they use
the bpf_link infrastructure. Allow Update to return ErrNotSupported
for that reason.
Return a more useful error message when passing a program with
the wrong type. This is especially useful here because it's
easy to mix up tracepoints and raw tracepoints.
Allow attaching to perf event fd based tracepoints (which are
different than raw tracepoints).

Fixes #83
@lmb lmb requested a review from florianl December 15, 2020 09:32
@lmb
Copy link
Collaborator Author

lmb commented Dec 15, 2020

cc @mythi @tqbf

@florianl
Copy link
Contributor

florianl commented Dec 15, 2020

Function-wise the PR looks fine for me.
Tracepoints are just one thing, where eBPF programs can be attached to using the perf API. And there are more things, like software and hardware events, that also use this API to attach eBPF programs.
So my concern is, will this package concentrate on the eBPF side or should it also be a wrapper for the perf API?

Personally I would prefer, if this package concentrates purely on the eBPF part. There is already example code that illustrates and documents how this eBPF package can be used with the perf API.
There are other packages like https://github.com/elastic/go-perf, that provide a nice interface to the perf API, if one doesn't want to use the low level calls.

@lmb
Copy link
Collaborator Author

lmb commented Dec 16, 2020

Personally I would prefer, if this package concentrates purely on the eBPF part.

Agreed. My main concern is to give a hand to newcomers when it comes to "how do I attach program XYZ". I really don't know much about tracepoints, I just figured that the example you added covers the use case. So maybe the better thing would be to rewrite the example to use the perf package you linked (which fork though?) and make sure it's prominent in the package documentation?

@florianl
Copy link
Contributor

The existing example on tracepoints is sufficient, I think. It is also linked with the related buzzword in the documentation of this package: https://pkg.go.dev/github.com/cilium/ebpf#example-package-Tracepoint
Maybe a second example which loads an eBPF specification from spec, instead of writing eBPF instructions, would help?

@lmb
Copy link
Collaborator Author

lmb commented Dec 17, 2020

I've split out the raw tracepoint cleanups into #189 while I ponder what to do here.

@florianl I'd prefer if the example were a bit safer, raw FDs flying around aren't really a recipe for success. At the same time I don't want to shoulder the burden of a complicated API, basically what is in this PR is the maximum. What I don't know, since I'm not using tracepoints: would this code be enough for 90% of BPF on tracepoint usage? If yes it's worth it to me, if not we have our answer as well.

@florianl
Copy link
Contributor

sorry for my late reply 🙏 is/was a busy week.

I can provide another example for tracepoints with the github.com/elastic/go-perf API.
I don't think the suggested code in this PR change would cover 90% of use cases.

@florianl
Copy link
Contributor

I have created PR #190 to demonstrate how github.com/elastic/go-perf can be used to attach an eBPF program to a tracepoint.

@lmb
Copy link
Collaborator Author

lmb commented Jan 13, 2021

Thanks! I've been pretty busy as well, so no worries at all.

Let's close this PR.

@lmb lmb closed this Jan 13, 2021
@ti-mo ti-mo removed the request for review from florianl March 19, 2021 14:13
@ti-mo ti-mo reopened this Mar 19, 2021
@ti-mo ti-mo marked this pull request as draft March 19, 2021 14:13
@ti-mo
Copy link
Collaborator

ti-mo commented Mar 19, 2021

Going to do another take on this with the intention of adding kprobe and uprobe support later on.

@ti-mo ti-mo mentioned this pull request Mar 25, 2021
2 tasks
@ti-mo
Copy link
Collaborator

ti-mo commented Mar 25, 2021

Continuing this in #265.

@ti-mo ti-mo closed this Mar 25, 2021
@ti-mo ti-mo deleted the lmb/tracepoint-support branch November 17, 2021 14:17
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants