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

fentry/fexit (aka kfunc/kretfunc) BPF trampoline support #1833

Closed
brendangregg opened this issue May 6, 2021 · 9 comments
Closed

fentry/fexit (aka kfunc/kretfunc) BPF trampoline support #1833

brendangregg opened this issue May 6, 2021 · 9 comments
Labels
enhancement New feature or request, changes on existing features

Comments

@brendangregg
Copy link
Contributor

This ticket is to discuss adding fentry support to bpftrace. It's faster, provides typed arguments on entry and return (using BTF). From https://github.com/libbpf/libbpf-bootstrap/blob/master/README.md (thanks @anakryiko, @danobi, @jvijtiuk):

Important differences, compared to kprobes, are improved performance and usability. In this example, better usability is shown with the ability to directly dereference pointer arguments, like in normal C, instead of using various read helpers. The big distinction between fexit and kretprobe programs is that fexit one has access to both input arguments and returned result, while kretprobe can only access the result.

libbpf-tools have been switching over. It does require a Linux 5.5 kernel, and support isn't on all architectures yet (e.g., ARM is missing it).

Now, it would be straightforward to add two new probe types, "fentry" and "fexit". But perhaps it would be better to enhance the kprobe and kretprobe types so that they uses fentry if available; and if it is not available, reject advanced features (uncasted derferences, and args on kretprobe). Reasons to call it kprobe:

  • shouldn't break existing tools and documentation (some of which is paper :-)
  • makes more sense when bpftrace appears on other OSes

Back in the DTrace days, our kernel tracing was called "fbt" for function boundry tracing, and by the time it was ported to other OSes (as bpftrace will be) it became increasingly obvious that it wasn't an intuitive name. I think the same will be true with fentry. It's a gcc thing. It just doesn't feel as obvious as kprobe. I guess we could call it "kfentry" and "kfexit"...but seems to be getting ugly. And it still feels like we're forcing people to learn a Linux kernel implementation thing (kprobes vs fentry) when they just want to trace a kernel function.

Now, other OSes may have such a bizzare kernel tracer that it doesn't resemble kprobes at all, and they just want to call it their own probe type. And that's ok. But I just want us to think about making things generic, where possible, to aid bpftrace ports.

Thoughts?

@brendangregg brendangregg added the enhancement New feature or request, changes on existing features label May 6, 2021
@fbs
Copy link
Contributor

fbs commented May 6, 2021

Isn't this what kfunc does?


refguide:

These are kernel function probes implemented via eBPF trampolines which allows
kernel code to call into BPF programs with practically zero overhead.

Examples:

# bpftrace -e 'kfunc:x86_pmu_stop { printf("pmu %s stop\n", str(args->event->pmu->name)); }'
# bpftrace -e 'kretfunc:fget { printf("fd %d name %s\n", args->fd, str(retval->f_path.dentry->d_name.name));  }'

You can get list of available functions via list option:

# bpftrace -l
...
kfunc:ksys_ioperm
kfunc:ksys_unshare
kfunc:ksys_setsid
kfunc:ksys_sync_helper
kfunc:ksys_fadvise64_64
kfunc:ksys_readahead
kfunc:ksys_mmap_pgoff
...

@brendangregg
Copy link
Contributor Author

Ah right, there it is, thanks! I was searching for "fentry" and couldn't find it added, but did see the kfunc in bcc. These seem to becoming known as fentry/fexit over in libbpf-tools, so one of us should add the term "fentry" to the docs so others find it.

So I like how it has a "k" in it: that's better than "fentry". That leaves the problem of having two interfaces that basically do the same thing. Imagine telling a user "on newer kernels change all your kprobes to kfunc and it'll be faster", "oh? why doesn't bpftrace just do it then". Hmmm. And do we change all the docs to be kfunc, or change kprobes to emit kfunc if available?

@viktormalik
Copy link
Contributor

That leaves the problem of having two interfaces that basically do the same thing. Imagine telling a user "on newer kernels change all your kprobes to kfunc and it'll be faster", "oh? why doesn't bpftrace just do it then". Hmmm. And do we change all the docs to be kfunc, or change kprobes to emit kfunc if available?

AFAIK we don't. One of the issues why we can't simply replace all kprobes by kfunc is that there are cases when kprobes are better, especially when attaching a single program to many probes. For instance, on my localhost:

# time src/bpftrace -e 'kfunc:vfs_* { @[probe] = count(); } interval:s:1 { exit(); }'
Attaching 67 probes...
[...]
real	0m14.035s
user	0m0.716s
sys	0m1.619s
# time src/bpftrace -e 'kprobe:vfs_* { @[probe] = count(); } interval:s:1 { exit(); }'
Attaching 68 probes...
[...]
real	0m3.979s
user	0m0.444s
sys	0m1.604s

The difference is caused by the attachement time, which is much slower for fentry probes. The issue is being addressed in kernel (see here), but it'll probably take time to get in. Until then, I don't think that we can simply suggest general kprobe -> kfunc replacement. We could still do that for many of the existing bpftrace tools, though.

@brendangregg
Copy link
Contributor Author

Ok thanks. That is pretty slow for 67 probes, but yes, it's getting fixed.

I still don't think the answer is to change the existing bpftrace tools to kfunc. If the future might be they work as-is and are faster (and kfunc behind the scenes) then that's the least API churn for end users.

@viktormalik
Copy link
Contributor

I see. Yeah, this could make more sense in the future since at this moment, we could use kfunc behind the scenes only for probes attached to a single function and I don't think that would bring any significant improvement.

Still, the more important benefits of kfuncs are their ability to access function arguments by name and the fact that kretfuncs have access to arguments (not only to the retval).

@danobi
Copy link
Member

danobi commented May 6, 2021

Also note that kprobes and kfuncs (fentry/fexit) have different semantics in the kernel. kprobes can be attached to arbitrary offsets while kfunc cannot. And (I think) kprobes get access to struct pt_regs while kfunc does not.

@fbs
Copy link
Contributor

fbs commented May 6, 2021

Having two names/apis makes sense to me. kprobe and kfunc both trace kernel functions but thats the only overlap they have.

In kprobes you only have arg1..argN as option, all types you need you have to pull in yourself.
In kfuncs you get btf based type info in the args struct and it fixes the whole type chain for you

By putting them both under the same name it would only be confusing, you don't know which one you can use based on the probe name. And there is no mapping from argX to args->X that can work reliably as as there is no guarantee that the kernel signature doesn't change from int fn(int x, int y) to int fn(int x, char * foo, int y)

Mixing user types (#include) with BTF has also caused a lot of issues.

kretfuncs also have args access, while kprobes don't have it

kfuncs don't work on an offset afaik.

@brendangregg
Copy link
Contributor Author

Thanks for the comments. I need to come back and think about this, but BPF on Microsoft Windows certainly gives us more to think about: would we want them to have a kprobes or kprobes & kfunc etc. I get that they are different and the details get messy, but that's exactly what I'm worried about -- exposing these Linux-isms when bpftrace is going beyond Linux. Things to consider. :)

marselester added a commit to marselester/libbpf-tools that referenced this issue Nov 4, 2021
It seems fentry is not supported yet, so I tried using kprobe
instead, see bpftrace/bpftrace#1833.

So far I get the following error: Unrecognized arg#0 type PTR.
@viktormalik
Copy link
Contributor

I believe that we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request, changes on existing features
Projects
None yet
Development

No branches or pull requests

4 participants