Re: [PATCH bpf-next 2/3] libbpf: teach bpf_link_create() to fallback to bpf_raw_tracepoint_open()

[Kui-Feng Lee <kuifeng@fb.com>]

On Wed, 2022-04-20 at 20:39 -0700, Andrii Nakryiko wrote:
> Teach bpf_link_create() to fallback to bpf_raw_tracepoint_open() on
> older kernels for programs that are attachable through
> BPF_RAW_TRACEPOINT_OPEN. This makes bpf_link_create() more unified
> and
> convenient interface for creating bpf_link-based attachments.
> 
> With this approach end users can just use bpf_link_create() for
> tp_btf/fentry/fexit/fmod_ret/lsm program attachments without needing
> to
> care about kernel support, as libbpf will handle this transparently.
> On
> the other hand, as newer features (like BPF cookie) are added to
> LINK_CREATE interface, they will be readily usable though the same
> bpf_link_create() API without any major refactoring from user's
> standpoint.
> 
> bpf_program__attach_btf_id() is now using bpf_link_create()
> internally
> as well and will take advantaged of this unified interface when BPF
> cookie is added for fentry/fexit.
> 
> Doing proactive feature detection of LINK_CREATE support for
> fentry/tp_btf/etc is quite involved. It requires parsing vmlinux BTF,
> determining some stable and guaranteed to be in all kernels versions
> target BTF type (either raw tracepoint or fentry target function),
> actually attaching this program and thus potentially affecting the
> performance of the host kernel briefly, etc. So instead we are taking
> much simpler "lazy" approach of falling back to
> bpf_raw_tracepoint_open() call only if initial LINK_CREATE command
> fails. For modern kernels this will mean zero added overhead, while
> older kernels will incur minimal overhead with a single fast-failing
> LINK_CREATE call.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Reviewed-by: Kui-Feng Lee <kuifeng@fb.com>

This is very straight forward.

> ---
>  tools/lib/bpf/bpf.c    | 34 ++++++++++++++++++++++++++++++++--
>  tools/lib/bpf/libbpf.c |  3 ++-
>  2 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index cf27251adb92..a9d292c106c2 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -817,7 +817,7 @@ int bpf_link_create(int prog_fd, int target_fd,
>  {
>         __u32 target_btf_id, iter_info_len;
>         union bpf_attr attr;
> -       int fd;
> +       int fd, err;
>  
>         if (!OPTS_VALID(opts, bpf_link_create_opts))
>                 return libbpf_err(-EINVAL);
> @@ -870,7 +870,37 @@ int bpf_link_create(int prog_fd, int target_fd,
>         }
>  proceed:
>         fd = sys_bpf_fd(BPF_LINK_CREATE, &attr, sizeof(attr));
> -       return libbpf_err_errno(fd);
> +       if (fd >= 0)
> +               return fd;
> +       /* we'll get EINVAL if LINK_CREATE doesn't support attaching
> fentry
> +        * and other similar programs
> +        */
> +       err = -errno;
> +       if (err != -EINVAL)
> +               return libbpf_err(err);
> +
> +       /* if user used features not supported by
> +        * BPF_RAW_TRACEPOINT_OPEN command, then just give up
> immediately
> +        */
> +       if (attr.link_create.target_fd ||
> attr.link_create.target_btf_id)
> +               return libbpf_err(err);
> +       if (!OPTS_ZEROED(opts, sz))
> +               return libbpf_err(err);
> +
> +       /* otherwise, for few select kinds of programs that can be
> +        * attached using BPF_RAW_TRACEPOINT_OPEN command, try that
> as
> +        * a fallback for older kernels
> +        */
> +       switch (attach_type) {
> +       case BPF_TRACE_RAW_TP:
> +       case BPF_LSM_MAC:
> +       case BPF_TRACE_FENTRY:
> +       case BPF_TRACE_FEXIT:
> +       case BPF_MODIFY_RETURN:
> +               return bpf_raw_tracepoint_open(NULL, prog_fd);
> +       default:
> +               return libbpf_err(err);
> +       }
>  }
>  
>  int bpf_link_detach(int link_fd)
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 94940497354b..ae317df1fc57 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -11262,7 +11262,8 @@ static struct bpf_link
> *bpf_program__attach_btf_id(const struct bpf_program *pro
>                 return libbpf_err_ptr(-ENOMEM);
>         link->detach = &bpf_link__detach_fd;
>  
> -       pfd = bpf_raw_tracepoint_open(NULL, prog_fd);
> +       /* libbpf is smart enough to redirect to
> BPF_RAW_TRACEPOINT_OPEN on old kernels */
> +       pfd = bpf_link_create(prog_fd, 0,
> bpf_program__expected_attach_type(prog), NULL);
>         if (pfd < 0) {
>                 pfd = -errno;
>                 free(link);