netdev - Re: [RFC PATCH v2 0/2] libbpf: Add support for dynamic tracepoints

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CALOAHbAXyLy6gcRLwtF9rz8v5hcpx5ua4En25sst3pgUu0K=xA@mail.gmail.com>
Date: Wed, 15 Jan 2025 11:13:01 +0800
From: Yafang Shao <laoar.shao@...il.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: andrii@...nel.org, eddyz87@...il.com, ast@...nel.org, daniel@...earbox.net, 
	martin.lau@...ux.dev, song@...nel.org, yonghong.song@...ux.dev, 
	john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me, 
	haoluo@...gle.com, jolsa@...nel.org, edumazet@...gle.com, dxu@...uu.xyz, 
	bpf@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [RFC PATCH v2 0/2] libbpf: Add support for dynamic tracepoints

On Wed, Jan 15, 2025 at 6:32 AM Andrii Nakryiko
<andrii.nakryiko@...il.com> wrote:
>
> On Sat, Jan 11, 2025 at 10:45 PM Yafang Shao <laoar.shao@...il.com> wrote:
> >
> > The primary goal of this change is to enable tracing of inlined kernel
> > functions with BPF programs.
> >
> > Dynamic tracepoints can be created using tools like perf-probe, debugfs, or
> > similar utilities. For example:
> >
> >   $ perf probe -a 'tcp_listendrop sk'
> >   $ ls /sys/kernel/debug/tracing/events/probe/tcp_listendrop/
> >   enable  filter  format  hist  id  trigger
> >
> > Here, tcp_listendrop() is an example of an inlined kernel function.
> >
> > While these dynamic tracepoints are functional, they cannot be easily
> > attached to BPF programs. For instance, attempting to use them with
> > bpftrace results in the following error:
> >
> >   $ bpftrace -l 'tracepoint:probe:*'
> >   tracepoint:probe:tcp_listendrop
> >
> >   $ bpftrace -e 'tracepoint:probe:tcp_listendrop {print(comm)}'
> >   Attaching 1 probe...
> >   ioctl(PERF_EVENT_IOC_SET_BPF): Invalid argument
> >   ERROR: Error attaching probe: tracepoint:probe:tcp_listendrop
> >
> > The issue lies in how these dynamic tracepoints are implemented: despite
> > being exposed as tracepoints, they remain kprobe events internally. As a
> > result, loading them as a tracepoint program fails. Instead, they must be
> > loaded as kprobe programs.
> >
> > This change introduces support for such use cases in libbpf by adding a
> > new section: SEC("kprobe/SUBSYSTEM/PROBE")
> >
> > - Future work
> >   Extend support for dynamic tracepoints in bpftrace.
>
> Seems like the primary motivation is bpftrace support, so let's start
> there.

I believe we should extend support for this feature in BCC as well.
Since this is a common and widely applicable feature, wouldn't it make
sense to include it directly in libbpf?

> I'm hesitant to include this in libbpf right now. The whole
> SEC("kprobe") that calls "attach_tracepoint()" underneath makes me
> uncomfortable.

I reused the bpf_program__attach_tracepoint() function, but if we
examine its implementation closely, we can see that it actually
creates a DECLARE_LIBBPF_OPTS(bpf_perf_event_opts, pe_opts); rather
than a bpf_tracepoint_opts.

If naming is a significant concern, it wouldn’t be difficult to
reimplement the same logic with a more appropriate name, such as
attach_kprobe_based_tracepoint().

>
> You can still attach to such dynamic probe today with pure libbpf
> (e.g., if bpftrace needs to do this, for example) by creating
> perf_event FD from tracefs' id, and then using
> bpf_program__attach_perf_event_opts() to attach to it. It will be on
> the user to use either tracepoint or kprobe BPF program for such
> attachment.

You're right—we can manually attach it in the tools, but if we can
make it auto-attachable in libbpf, why not take advantage of that and
simplify the process?


>
> Yes, this doesn't work "declaratively" with a nice SEC("...") syntax,
> but at least it's doable programmatically, and that's what matters for
> bpftrace.


--
Regards
Yafang