On Fri, Jun 30, 2023 at 1:36 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> Adding uprobe-multi link detection. It will be used later in
> bpf_program__attach_usdt function to check and use uprobe_multi
> link over standard uprobe links.
>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> tools/lib/bpf/libbpf.c | 35 +++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf_internal.h | 2 ++
> 2 files changed, 37 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 06092b9752f1..4f61f9dc1748 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4817,6 +4817,38 @@ static int probe_perf_link(void)
> return link_fd < 0 && err == -EBADF;
> }
>
> +static int probe_uprobe_multi_link(void)
> +{
> + LIBBPF_OPTS(bpf_prog_load_opts, load_opts,
> + .expected_attach_type = BPF_TRACE_UPROBE_MULTI,
> + );
> + LIBBPF_OPTS(bpf_link_create_opts, link_opts);
> + struct bpf_insn insns[] = {
> + BPF_MOV64_IMM(BPF_REG_0, 0),
> + BPF_EXIT_INSN(),
> + };
> + unsigned long offset = 0;
> + int prog_fd, link_fd;
> +
> + prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, NULL, "GPL",
> + insns, ARRAY_SIZE(insns), &load_opts);
> + if (prog_fd < 0)
> + return -errno;
> +
> + /* create single uprobe on offset 0 in current process */
> + link_opts.uprobe_multi.path = "/proc/self/exe";
> + 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);
> +
so I'd like us to avoid successfully attaching anything. This might
have unintended consequences (e.g., unintentionally breaking backing
huge pages into normal pages, just because we happen to successfully
attach briefly). So let's work on feature detection that fails to
create a link, but does it in a way that we know that the feature
itself is supported by the kernel
Some ideas we could do:
1. Pass invalid file (e.g., just root, "/" as path), but modify
kernel-side logic to return not -EINVAL, but -EBADF (and I think it
would be good to do this anyway). Then expect -EBADF as a signal that
the feature is supported.
2. Also, we can return -EPROTO instead of -EINVAL on invalid
combination of paramers or something like that
I'd start with -EBADF change.
In general, we should write kernel-side code in such a way that allows
simple and efficient feature-detection. We shouldn't repeat the
nightmare of memcg-based mem accounting :(
> + if (link_fd >= 0)
> + close(link_fd);
> + close(prog_fd);
> +
> + return link_fd >= 0;
> +}
> +
> static int probe_kern_bpf_cookie(void)
> {
> struct bpf_insn insns[] = {
> @@ -4913,6 +4945,9 @@ static struct kern_feature_desc {
> [FEAT_SYSCALL_WRAPPER] = {
> "Kernel using syscall wrapper", probe_kern_syscall_wrapper,
> },
> + [FEAT_UPROBE_MULTI_LINK] = {
> + "BPF uprobe multi link support", probe_uprobe_multi_link,
nit: BPF multi-uprobe link support
> + },
> };
>
> bool kernel_supports(const struct bpf_object *obj, enum kern_feature_id feat_id)
> diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
> index 7d75b92e531a..9c04b3fe1207 100644
> --- a/tools/lib/bpf/libbpf_internal.h
> +++ b/tools/lib/bpf/libbpf_internal.h
> @@ -354,6 +354,8 @@ enum kern_feature_id {
> FEAT_BTF_ENUM64,
> /* Kernel uses syscall wrapper (CONFIG_ARCH_HAS_SYSCALL_WRAPPER) */
> FEAT_SYSCALL_WRAPPER,
> + /* BPF uprobe_multi link support */
same, multi-uprobe link support
> + FEAT_UPROBE_MULTI_LINK,
> __FEAT_CNT,
> };
>
> --
> 2.41.0
>
