All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Yonghong Song <yhs@fb.com>
Cc: Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>,
	Networking <netdev@vger.kernel.org>,
	Alexei Starovoitov <ast@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <kernel-team@fb.com>, Hao Luo <haoluo@google.com>
Subject: Re: [PATCH v2 bpf-next 6/7] libbpf: support kernel module ksym externs
Date: Mon, 11 Jan 2021 13:37:14 -0800	[thread overview]
Message-ID: <CAEf4BzZGm9=XGWrj_1Q8ZpxZVhcogZVqb=5yCop2mNgdoTT0zA@mail.gmail.com> (raw)
In-Reply-To: <dc1a06fe-f957-deb8-772c-b4c65042c3b3@fb.com>

On Sun, Jan 10, 2021 at 8:15 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 1/8/21 2:09 PM, Andrii Nakryiko wrote:
> > Add support for searching for ksym externs not just in vmlinux BTF, but across
> > all module BTFs, similarly to how it's done for CO-RE relocations. Kernels
> > that expose module BTFs through sysfs are assumed to support new ldimm64
> > instruction extension with BTF FD provided in insn[1].imm field, so no extra
> > feature detection is performed.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> >   tools/lib/bpf/libbpf.c | 47 +++++++++++++++++++++++++++---------------
> >   1 file changed, 30 insertions(+), 17 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 6ae748f6ea11..57559a71e4de 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -395,7 +395,8 @@ struct extern_desc {
> >                       unsigned long long addr;
> >
> >                       /* target btf_id of the corresponding kernel var. */
> > -                     int vmlinux_btf_id;
> > +                     int kernel_btf_obj_fd;
> > +                     int kernel_btf_id;
> >
> >                       /* local btf_id of the ksym extern's type. */
> >                       __u32 type_id;
> > @@ -6162,7 +6163,8 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
> >                       } else /* EXT_KSYM */ {
> >                               if (ext->ksym.type_id) { /* typed ksyms */
> >                                       insn[0].src_reg = BPF_PSEUDO_BTF_ID;
> > -                                     insn[0].imm = ext->ksym.vmlinux_btf_id;
> > +                                     insn[0].imm = ext->ksym.kernel_btf_id;
> > +                                     insn[1].imm = ext->ksym.kernel_btf_obj_fd;
> >                               } else { /* typeless ksyms */
> >                                       insn[0].imm = (__u32)ext->ksym.addr;
> >                                       insn[1].imm = ext->ksym.addr >> 32;
> > @@ -7319,7 +7321,8 @@ static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
> >   static int bpf_object__resolve_ksyms_btf_id(struct bpf_object *obj)
> >   {
> >       struct extern_desc *ext;
> > -     int i, id;
> > +     struct btf *btf;
> > +     int i, j, id, btf_fd, err;
> >
> >       for (i = 0; i < obj->nr_extern; i++) {
> >               const struct btf_type *targ_var, *targ_type;
> > @@ -7331,8 +7334,22 @@ static int bpf_object__resolve_ksyms_btf_id(struct bpf_object *obj)
> >               if (ext->type != EXT_KSYM || !ext->ksym.type_id)
> >                       continue;
> >
> > -             id = btf__find_by_name_kind(obj->btf_vmlinux, ext->name,
> > -                                         BTF_KIND_VAR);
> > +             btf = obj->btf_vmlinux;
> > +             btf_fd = 0;
> > +             id = btf__find_by_name_kind(btf, ext->name, BTF_KIND_VAR);
> > +             if (id == -ENOENT) {
> > +                     err = load_module_btfs(obj);
> > +                     if (err)
> > +                             return err;
> > +
> > +                     for (j = 0; j < obj->btf_module_cnt; j++) {
> > +                             btf = obj->btf_modules[j].btf;
> > +                             btf_fd = obj->btf_modules[j].fd;
>
> Do we have possibility btf_fd == 0 here?

Extremely unlikely. But if we are really worried about 0 fd, we should
handle that in a centralized fashion in libbpf. I.e., for any
operation that can return FD, check if that FD is 0, and if yes, dup()
it. And then make everything call that helper. So in the context of
this patch I'm just ignoring such possibility.

>
> > +                             id = btf__find_by_name_kind(btf, ext->name, BTF_KIND_VAR);
> > +                             if (id != -ENOENT)
> > +                                     break;
> > +                     }
> > +             }
> >               if (id <= 0) {
> >                       pr_warn("extern (ksym) '%s': failed to find BTF ID in vmlinux BTF.\n",
> >                               ext->name);
> > @@ -7343,24 +7360,19 @@ static int bpf_object__resolve_ksyms_btf_id(struct bpf_object *obj)
> >               local_type_id = ext->ksym.type_id;
> >
> >               /* find target type_id */
> > -             targ_var = btf__type_by_id(obj->btf_vmlinux, id);
> > -             targ_var_name = btf__name_by_offset(obj->btf_vmlinux,
> > -                                                 targ_var->name_off);
> > -             targ_type = skip_mods_and_typedefs(obj->btf_vmlinux,
> > -                                                targ_var->type,
> > -                                                &targ_type_id);
> > +             targ_var = btf__type_by_id(btf, id);
> > +             targ_var_name = btf__name_by_offset(btf, targ_var->name_off);
> > +             targ_type = skip_mods_and_typedefs(btf, targ_var->type, &targ_type_id);
> >
> >               ret = bpf_core_types_are_compat(obj->btf, local_type_id,
> > -                                             obj->btf_vmlinux, targ_type_id);
> > +                                             btf, targ_type_id);
> >               if (ret <= 0) {
> >                       const struct btf_type *local_type;
> >                       const char *targ_name, *local_name;
> >
> >                       local_type = btf__type_by_id(obj->btf, local_type_id);
> > -                     local_name = btf__name_by_offset(obj->btf,
> > -                                                      local_type->name_off);
> > -                     targ_name = btf__name_by_offset(obj->btf_vmlinux,
> > -                                                     targ_type->name_off);
> > +                     local_name = btf__name_by_offset(obj->btf, local_type->name_off);
> > +                     targ_name = btf__name_by_offset(btf, targ_type->name_off);
> >
> >                       pr_warn("extern (ksym) '%s': incompatible types, expected [%d] %s %s, but kernel has [%d] %s %s\n",
> >                               ext->name, local_type_id,
> > @@ -7370,7 +7382,8 @@ static int bpf_object__resolve_ksyms_btf_id(struct bpf_object *obj)
> >               }
> >
> >               ext->is_set = true;
> > -             ext->ksym.vmlinux_btf_id = id;
> > +             ext->ksym.kernel_btf_obj_fd = btf_fd;
> > +             ext->ksym.kernel_btf_id = id;
> >               pr_debug("extern (ksym) '%s': resolved to [%d] %s %s\n",
> >                        ext->name, id, btf_kind_str(targ_var), targ_var_name);
> >       }
> >

  reply	other threads:[~2021-01-11 21:38 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-08 22:09 [PATCH v2 bpf-next 0/7] Support kernel module ksym variables Andrii Nakryiko
2021-01-08 22:09 ` [PATCH v2 bpf-next 1/7] bpf: add bpf_patch_call_args prototype to include/linux/bpf.h Andrii Nakryiko
2021-01-11  4:02   ` Yonghong Song
2021-01-08 22:09 ` [PATCH v2 bpf-next 2/7] bpf: avoid warning when re-casting __bpf_call_base into __bpf_call_base_args Andrii Nakryiko
2021-01-11  4:03   ` Yonghong Song
2021-01-08 22:09 ` [PATCH v2 bpf-next 3/7] bpf: declare __bpf_free_used_maps() unconditionally Andrii Nakryiko
2021-01-11  4:03   ` Yonghong Song
2021-01-08 22:09 ` [PATCH v2 bpf-next 4/7] selftests/bpf: sync RCU before unloading bpf_testmod Andrii Nakryiko
2021-01-11  4:05   ` Yonghong Song
2021-01-11 18:59     ` Hao Luo
2021-01-08 22:09 ` [PATCH v2 bpf-next 5/7] bpf: support BPF ksym variables in kernel modules Andrii Nakryiko
2021-01-11  4:13   ` Yonghong Song
2021-01-11 21:29     ` Andrii Nakryiko
2021-01-12  1:25       ` Yonghong Song
2021-01-11 18:59   ` Hao Luo
2021-01-11 21:31     ` Andrii Nakryiko
2021-01-08 22:09 ` [PATCH v2 bpf-next 6/7] libbpf: support kernel module ksym externs Andrii Nakryiko
2021-01-11  4:15   ` Yonghong Song
2021-01-11 21:37     ` Andrii Nakryiko [this message]
2021-01-12  1:34       ` Yonghong Song
2021-01-12  6:45         ` Andrii Nakryiko
2021-01-11 19:00   ` Hao Luo
2021-01-11 21:39     ` Andrii Nakryiko
2021-01-08 22:09 ` [PATCH v2 bpf-next 7/7] selftests/bpf: test " Andrii Nakryiko
2021-01-11  4:18   ` Yonghong Song
2021-01-11 21:40     ` Andrii Nakryiko
2021-01-11 19:00   ` Hao Luo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEf4BzZGm9=XGWrj_1Q8ZpxZVhcogZVqb=5yCop2mNgdoTT0zA@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.