-
Notifications
You must be signed in to change notification settings - Fork 729
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: support bpf perf event link, add bpf_cookies #565
Conversation
7c690ea
to
4023616
Compare
Signed-off-by: Mattia Meleleo <mattia.meleleo@elastic.co>
Signed-off-by: Mattia Meleleo <mattia.meleleo@elastic.co>
Signed-off-by: Mattia Meleleo <mattia.meleleo@elastic.co>
4023616
to
5eb7ec2
Compare
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 did another pass, thanks for clarifying my questions re bpf.h
pointers. Regarding Link
and how to implement that: I think we should split the code that does perf event set up from the code that does perf event attachment. Rough outline:
- Add
type perfEventIoctl struct
andtype perfEventLink struct
. The former has stub functions to implementLink
, the latter just wraps a RawLink similar torawTracepoint
(maybe we can just use RawLink directly?) perfEvent.attach
becomes a free functionattachPerfEvent
or similar which returns aLink
, eitherperfEventIoctl
orperfEventLink
.
…ptions for kprobes Signed-off-by: Mattia Meleleo <mattia.meleleo@elastic.co>
Signed-off-by: Mattia Meleleo <mattia.meleleo@elastic.co>
Signed-off-by: Mattia Meleleo <mattia.meleleo@elastic.co>
5eb7ec2
to
ec60821
Compare
@lmb I did some changes, not sure if this is what you asked (I needed to reimplement certain RawLink methods). Also, do you know where I could check which operations for which link type are supported? I am asking because I kept getting EINVAL in all |
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.
Please take another look at the changes I outlined in #565 (review). My idea for the final state is as follows:
perfEvent
: encapsulates handling tracefs / pmu, creating the perf event really.perfEventLink
: attaches a bpf program to aperfEvent
using BPF_LINK_CREATE.perfEventIoctl
: attaches a bpf program to aperfEvent
using ioctl().
perfEventLink and perfEventIoctl will wrap perfEvent so that they can do clean up. perfEventLink will additionally embed RawLink.
perfEvent
should not implement Link
, only perfEventLink
and perfEventIoctl
do. This is because we want a Link
to always represent the result of attaching a program.
If we follow this schema we don't have to change kprobePmu
, etc. and can keep attachPerfEvent
more straighforward.
ec60821
to
c3744d7
Compare
@lmb I set this as draft as I am still figuring out what to do with pinning |
c3744d7
to
50ee383
Compare
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.
Looks good, I left a few nits. Thanks for working in the feedback.
I'm currently traveling, so I'll probably merge this Friday if you're not in a hurry.
Signed-off-by: Mattia Meleleo <mattia.meleleo@elastic.co>
50ee383
to
621e7c6
Compare
Thanks a lot! |
This PR adds
bpf_cookie
andbpf_perf_link
.bpf_perf_link
is used automatically, if supported, in place ofperfEvent
.bpf_cookie
can be specified only if the link is implemented viabpf_perf_link
; if this is not the case, an error will be returned