| Message ID | cover.1714133551.git.vmalik@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | libbpf: support "module:function" syntax for tracing programs | expand |
On Fri, Apr 26, 2024 at 5:17 AM Viktor Malik <vmalik@redhat.com> wrote: > > In some situations, it is useful to explicitly specify a kernel module > to search for a tracing program target (e.g. when a function of the same > name exists in multiple modules or in vmlinux). > > This change enables that by allowing the "module:function" syntax for > the find_kernel_btf_id function. Thanks to this, the syntax can be used > both from a SEC macro (i.e. `SEC(fentry/module:function)`) and via the > bpf_program__set_attach_target API call. > how about function[module] syntax. This follows how modules are reported in kallsyms and a bunch of other kernel-generated files. I've been using this syntax in retsnoop for a while, and it feels very natural. It's also distinctive enough to be recognizable and parseable without any possible confusions. Can you please also check if we can/should support this for kprobes as well? > Viktor Malik (2): > libbpf: support "module:function" syntax for tracing programs > selftests/bpf: add tests for the "module:function" syntax > > tools/lib/bpf/libbpf.c | 33 ++++++++++++++----- > .../selftests/bpf/prog_tests/module_attach.c | 6 ++++ > .../selftests/bpf/progs/test_module_attach.c | 23 +++++++++++++ > 3 files changed, 53 insertions(+), 9 deletions(-) > > -- > 2.44.0 >
On 4/26/24 18:54, Andrii Nakryiko wrote: > On Fri, Apr 26, 2024 at 5:17 AM Viktor Malik <vmalik@redhat.com> wrote: >> >> In some situations, it is useful to explicitly specify a kernel module >> to search for a tracing program target (e.g. when a function of the same >> name exists in multiple modules or in vmlinux). >> >> This change enables that by allowing the "module:function" syntax for >> the find_kernel_btf_id function. Thanks to this, the syntax can be used >> both from a SEC macro (i.e. `SEC(fentry/module:function)`) and via the >> bpf_program__set_attach_target API call. >> > > how about function[module] syntax. This follows how modules are > reported in kallsyms and a bunch of other kernel-generated files. I've > been using this syntax in retsnoop for a while, and it feels very > natural. It's also distinctive enough to be recognizable and parseable > without any possible confusions. > > Can you please also check if we can/should support this for kprobes as well? For kprobes, it's a bit more complicated. The legacy kprobe attachment (via tracefs) supports the "module:function" syntax [1] (which is the reason I chose that syntax in the first place). On the other hand, kprobe attachment via the perf_event_open syscall eventually calls kallsyms_lookup_name for the passed function name which doesn't support specifying the module at all. So, to properly support this for kprobes, we'd have to extend kallsyms_lookup_name to handle the "function[module]" or "module:function" syntax (here, the former makes more sense). Since kallsyms_lookup_name is used in many places, I would prefer adding the kprobe support separately. In any case, the "function[module]" syntax feels more natural for non-legacy kprobes, so I'm ok with using it. libbpf will just have to do the transformation to "module:function" for legacy kprobe attachment. Let me know if that makes sense and I'll post v2. Thanks! Viktor [1] https://www.kernel.org/doc/Documentation/trace/kprobetrace.rst > >> Viktor Malik (2): >> libbpf: support "module:function" syntax for tracing programs >> selftests/bpf: add tests for the "module:function" syntax >> >> tools/lib/bpf/libbpf.c | 33 ++++++++++++++----- >> .../selftests/bpf/prog_tests/module_attach.c | 6 ++++ >> .../selftests/bpf/progs/test_module_attach.c | 23 +++++++++++++ >> 3 files changed, 53 insertions(+), 9 deletions(-) >> >> -- >> 2.44.0 >> >
On Mon, Apr 29, 2024 at 5:32 AM Viktor Malik <vmalik@redhat.com> wrote: > > On 4/26/24 18:54, Andrii Nakryiko wrote: > > On Fri, Apr 26, 2024 at 5:17 AM Viktor Malik <vmalik@redhat.com> wrote: > >> > >> In some situations, it is useful to explicitly specify a kernel module > >> to search for a tracing program target (e.g. when a function of the same > >> name exists in multiple modules or in vmlinux). > >> > >> This change enables that by allowing the "module:function" syntax for > >> the find_kernel_btf_id function. Thanks to this, the syntax can be used > >> both from a SEC macro (i.e. `SEC(fentry/module:function)`) and via the > >> bpf_program__set_attach_target API call. > >> > > > > how about function[module] syntax. This follows how modules are > > reported in kallsyms and a bunch of other kernel-generated files. I've > > been using this syntax in retsnoop for a while, and it feels very > > natural. It's also distinctive enough to be recognizable and parseable > > without any possible confusions. > > > > Can you please also check if we can/should support this for kprobes as well? > > For kprobes, it's a bit more complicated. The legacy kprobe attachment > (via tracefs) supports the "module:function" syntax [1] (which is the > reason I chose that syntax in the first place). On the other hand, > kprobe attachment via the perf_event_open syscall eventually calls > kallsyms_lookup_name for the passed function name which doesn't support > specifying the module at all. > > So, to properly support this for kprobes, we'd have to extend > kallsyms_lookup_name to handle the "function[module]" or > "module:function" syntax (here, the former makes more sense). > > Since kallsyms_lookup_name is used in many places, I would prefer adding > the kprobe support separately. In any case, the "function[module]" > syntax feels more natural for non-legacy kprobes, so I'm ok with using > it. libbpf will just have to do the transformation to "module:function" > for legacy kprobe attachment. > > Let me know if that makes sense and I'll post v2. Thinking some more, I think we should stick to "<module>:<function>" for a) simplicity (it's slightly easier to parse) and b) consistency with uprobe, where we have "<path>:<function>", where path is pointing to ELF binary. Let me take a look at patches again. But yes, I think it would be useful to add support to kprobes/multi-kprobes as well for completeness, but it of course would be a separate work thread. > > Thanks! > Viktor > > [1] https://www.kernel.org/doc/Documentation/trace/kprobetrace.rst > > > > >> Viktor Malik (2): > >> libbpf: support "module:function" syntax for tracing programs > >> selftests/bpf: add tests for the "module:function" syntax > >> > >> tools/lib/bpf/libbpf.c | 33 ++++++++++++++----- > >> .../selftests/bpf/prog_tests/module_attach.c | 6 ++++ > >> .../selftests/bpf/progs/test_module_attach.c | 23 +++++++++++++ > >> 3 files changed, 53 insertions(+), 9 deletions(-) > >> > >> -- > >> 2.44.0 > >> > > >