Re: [PATCH v2 bpf-next 6/7] libbpf: support kernel module ksym externs

On Mon, Jan 11, 2021 at 11:00 AM Hao Luo [off-list ref] wrote:

Acked-by: Hao Luo <redacted>, with a couple of nits.

On Fri, Jan 8, 2021 at 2:09 PM Andrii Nakryiko [off-list ref] wrote:

quoted

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

[...]

quoted

@@ -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);

Is "if (id <= 0)" better? Just in case, more error code is introduced in future.

There is id <= 0 right below after special-casing -ENOENT, so all
works as you want, no?

quoted

+               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;
+                               id = btf__find_by_name_kind(btf, ext->name, BTF_KIND_VAR);
+                               if (id != -ENOENT)
+                                       break;
+                       }
+               }
                if (id <= 0) {

Nit: the warning message isn't accurate any more, right? We also
searched kernel modules' BTF.

Right, how about just "failed to find BTF ID in kernel BTF"? Where
"kernel BTF" is "vmlinux BTF or any of kernel modules' BTFs"?

quoted

                        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)

[...]

quoted

--
2.24.1