[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAEf4BzYVjLo4J=ZbNqYqVJL4Ntkc+vbJa1X1NEs4uvLBxWa3fQ@mail.gmail.com>
Date: Thu, 16 Jan 2025 15:05:22 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Yafang Shao <laoar.shao@...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 Tue, Jan 14, 2025 at 7:13 PM Yafang Shao <laoar.shao@...il.com> wrote:
>
> 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().
"kprobe based tracepoint" is exactly the thing that makes me unwilling
to include it right now. Let's put this on hold for a little bit and
see how bpftrace users use it first, we can always add something to
libbpf later.
>
> >
> > 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
Powered by blists - more mailing lists