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
program: support tracing of kernel modules #737
Conversation
ce87a5f
to
7c3261d
Compare
btf/handle.go
Outdated
// | ||
// Requires CAP_SYS_ADMIN. | ||
// | ||
// Returns ErrNotFound if predicate never returns true or if there is no BTF |
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.
Nit: You return a wrapped version of ErrNotFound
, this might give users the impression they can use err != ErrNotFound
to check the output while they need to check with error.Is()
. So I would reword the comment, remove it or change the return statement. Which ever suites best
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.
Good point, I'll amend the doc.
btf/handle.go
Outdated
} | ||
|
||
if predicate(info) { | ||
tmp := handle |
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 is the point of copying handle
and setting it to nil before a return, it is a local var. Can't you just return handle directly?
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.
The point is to avoid defer handle.Close()
closing the handle we are returning. It is used in findTargetInModule
as well.
Not sure how to solve this without adding the same comment to every callsite. Maybe I need to change HandleIterator to own the handle after 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.
Ah, I see. That is a cool pattern. Changing ownership is difficult since you are returning the handle while the iterator goes out of scope.
Any reason for not using runtime.SetFinalizer
like with other objects that hold file descriptors like maps and programs?
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.
We already set a finalizer, but it's better to explicitly Close since the former is run from the GC. There is no guarantee how often the GC runs, so in the meantime leaked handles can use up fds. This in turn leads to fd exhaustion.
I've changed HandleIterator slighty and added a Take
method which should make this trick more obvious: 2b8def6
Add HandleIterator.Handle instead of taking **Handle in Next(). This allows adding a Take() function which makes it clearer how ownership is handled in code using HandleIterator.
Add a helper that iterates all BTF objects in the kernel and returns the first one for which a user supplied callback returns true.
Allow tracing functions in kernel modules via fentry, fexit, fmod_ret and tp_btf programs. The behaviour follows libbpf and is transparent to the user: if we can't find a target in vmlinux we attempt to find it in any loaded kernel module. Refactor TestProgramTypeLSM to test attaching via BTF. This removes LSM tests for BPF_F_SLEEPABLE, since they don't excercise library behaviour beyond passing ProgramSpec.Flags to the kernel. Updates cilium#705
program: support tracing of kernel modules
btf: add FindHandle
btf: allow passing *Type to Spec.TypeByName