On Mon, Apr 29, 2024 at 5:32 AM Viktor Malik <vmalik@xxxxxxxxxx> wrote: > > On 4/26/24 18:54, Andrii Nakryiko wrote: > > On Fri, Apr 26, 2024 at 5:17 AM Viktor Malik <vmalik@xxxxxxxxxx> 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 > >> > > >
