On Tue, May 21, 2024 at 3:04 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Mon, May 20, 2024 at 04:47:18PM -0700, Andrii Nakryiko wrote:
> > Libbpf is automatically (and transparently to user) detecting
> > multi-uprobe support in the kernel, and, if supported, uses
> > multi-uprobes to improve USDT attachment speed.
> >
> > USDTs can be attached system-wide or for the specific process by PID. In
> > the latter case, we rely on correct kernel logic of not triggering USDT
> > for unrelated processes.
> >
> > As such, on older kernels that do support multi-uprobes, but still have
> > broken PID filtering logic, we need to fall back to singular uprobes.
> >
> > Unfortunately, whether user is using PID filtering or not is known at
> > the attachment time, which happens after relevant BPF programs were
> > loaded into the kernel. Also unfortunately, we need to make a call
> > whether to use multi-uprobes or singular uprobe for SEC("usdt") programs
> > during BPF object load time, at which point we have no information about
> > possible PID filtering.
> >
> > The distinction between single and multi-uprobes is small, but important
> > for the kernel. Multi-uprobes get BPF_TRACE_UPROBE_MULTI attach type,
> > and kernel internally substitiute different implementation of some of
> > BPF helpers (e.g., bpf_get_attach_cookie()) depending on whether uprobe
> > is multi or singular. So, multi-uprobes and singular uprobes cannot be
> > intermixed.
> >
> > All the above implies that we have to make an early and conservative
> > call about the use of multi-uprobes. And so this patch modifies libbpf's
> > existing feature detector for multi-uprobe support to also check correct
> > PID filtering. If PID filtering is not yet fixed, we fall back to
> > singular uprobes for USDTs.
> >
> > This extension to feature detection is simple thanks to kernel's -EINVAL
> > addition for pid < 0.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > ---
> > tools/lib/bpf/features.c | 31 ++++++++++++++++++++++++++++++-
> > 1 file changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
> > index a336786a22a3..cff8640ca66f 100644
> > --- a/tools/lib/bpf/features.c
> > +++ b/tools/lib/bpf/features.c
> > @@ -392,11 +392,40 @@ static int probe_uprobe_multi_link(int token_fd)
> > link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, &link_opts);
> > err = -errno; /* close() can clobber errno */
> >
> > + if (link_fd >= 0 || err != -EBADF) {
> > + close(link_fd);
> > + close(prog_fd);
> > + return 0;
> > + }
> > +
> > + /* Initial multi-uprobe support in kernel didn't handle PID filtering
> > + * correctly (it was doing thread filtering, not process filtering).
> > + * So now we'll detect if PID filtering logic was fixed, and, if not,
> > + * we'll pretend multi-uprobes are not supported, if not.
> > + * Multi-uprobes are used in USDT attachment logic, and we need to be
> > + * conservative here, because multi-uprobe selection happens early at
> > + * load time, while the use of PID filtering is known late at
> > + * attachment time, at which point it's too late to undo multi-uprobe
> > + * selection.
> > + *
> > + * Creating uprobe with pid == -1 for (invalid) '/' binary will fail
> > + * early with -EINVAL on kernels with fixed PID filtering logic;
> > + * otherwise -ESRCH would be returned if passed correct binary path
> > + * (but we'll just get -BADF, of course).
> > + */
> > + link_opts.uprobe_multi.pid = -1, /* invalid PID */
>
> ^ s/,/;/
>
> so this affects just USDT load/attach, you right?
good eye, fixing :)
and yes, for libbpf this affects only USDTs. If user uses multi-uprobe
directly through bpf_program__attach_uprobe_multi(), they will need to
do similar feature detection, if they care about PID filtering.
>
> Acked-by: Jiri Olsa <jolsa@xxxxxxxxxx>
>
> thanks,
> jirka
>
>
> > + link_opts.uprobe_multi.path = "/"; /* invalid path */
> > + link_opts.uprobe_multi.offsets = &offset;
> > + link_opts.uprobe_multi.cnt = 1;
> > +
> > + link_fd = bpf_link_create(prog_fd, -1, BPF_TRACE_UPROBE_MULTI, &link_opts);
> > + err = -errno; /* close() can clobber errno */
> > +
> > if (link_fd >= 0)
> > close(link_fd);
> > close(prog_fd);
> >
> > - return link_fd < 0 && err == -EBADF;
> > + return link_fd < 0 && err == -EINVAL;
> > }
> >
> > static int probe_kern_bpf_cookie(int token_fd)
> > --
> > 2.43.0
> >
> >
