[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5fc877ec37f7d_1123e208e8@john-XPS-13-9370.notmuch>
Date: Wed, 02 Dec 2020 21:30:20 -0800
From: John Fastabend <john.fastabend@...il.com>
To: Andrii Nakryiko <andrii@...nel.org>, bpf@...r.kernel.org,
netdev@...r.kernel.org, ast@...com, daniel@...earbox.net
Cc: andrii@...nel.org, kernel-team@...com
Subject: RE: [PATCH v5 bpf-next 05/14] libbpf: add kernel module BTF support
for CO-RE relocations
Andrii Nakryiko wrote:
> Teach libbpf to search for candidate types for CO-RE relocations across kernel
> modules BTFs, in addition to vmlinux BTF. If at least one candidate type is
> found in vmlinux BTF, kernel module BTFs are not iterated. If vmlinux BTF has
> no matching candidates, then find all kernel module BTFs and search for all
> matching candidates across all of them.
>
> Kernel's support for module BTFs are inferred from the support for BTF name
> pointer in BPF UAPI.
>
> Signed-off-by: Andrii Nakryiko <andrii@...nel.org>
> ---
small typo and missing close maybe? Otherwise,
Acked-by: John Fastabend <john.fastabend@...il.com>
> +static int load_module_btfs(struct bpf_object *obj)
> +{
> + struct bpf_btf_info info;
> + struct module_btf *mod_btf;
> + struct btf *btf;
> + char name[64];
> + __u32 id = 0, len;
> + int err, fd;
> +
> + if (obj->btf_modules_loaded)
> + return 0;
> +
> + /* don't do this again, even if we find no module BTFs */
I wonder a bit if we might load modules at some point and want
to then run this, but I wouldn't worry about it. Its likely
easy enough to just reload or delay load until module is in place.
> + obj->btf_modules_loaded = true;
> +
> + /* kernel too old to support module BTFs */
> + if (!kernel_supports(FEAT_MODULE_BTF))
> + return 0;
> +
> + while (true) {
> + err = bpf_btf_get_next_id(id, &id);
> + if (err && errno == ENOENT)
> + return 0;
> + if (err) {
> + err = -errno;
> + pr_warn("failed to iterate BTF objects: %d\n", err);
> + return err;
> + }
> +
> + fd = bpf_btf_get_fd_by_id(id);
> + if (fd < 0) {
> + if (errno == ENOENT)
> + continue; /* expected race: BTF was unloaded */
> + err = -errno;
> + pr_warn("failed to get BTF object #%d FD: %d\n", id, err);
> + return err;
> + }
> +
> + len = sizeof(info);
> + memset(&info, 0, sizeof(info));
> + info.name = ptr_to_u64(name);
> + info.name_len = sizeof(name);
> +
> + err = bpf_obj_get_info_by_fd(fd, &info, &len);
> + if (err) {
do we also need to close(fd) here? If not then we don't need the close in xsk.c
I guess.
> + err = -errno;
> + pr_warn("failed to get BTF object #%d info: %d\n", id, err);
> + return err;
> + }
> +
> + /* ignore non-module BTFs */
> + if (!info.kernel_btf || strcmp(name, "vmlinux") == 0) {
> + close(fd);
> + continue;
> + }
> +
> + btf = btf_get_from_fd(fd, obj->btf_vmlinux);
> + close(fd);
> + if (IS_ERR(btf)) {
> + pr_warn("failed to load module [%s]'s BTF object #%d: %ld\n",
> + name, id, PTR_ERR(btf));
> + return PTR_ERR(btf);
> + }
> +
> + err = btf_ensure_mem((void **)&obj->btf_modules, &obj->btf_module_cap,
> + sizeof(*obj->btf_modules), obj->btf_module_cnt + 1);
> + if (err)
> + return err;
> +
> + mod_btf = &obj->btf_modules[obj->btf_module_cnt++];
> +
> + mod_btf->btf = btf;
> + mod_btf->id = id;
> + mod_btf->name = strdup(name);
> + if (!mod_btf->name)
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> static struct core_cand_list *
> bpf_core_find_cands(struct bpf_object *obj, const struct btf *local_btf, __u32 local_type_id)
> {
> struct core_cand local_cand = {};
> struct core_cand_list *cands;
> + const struct btf *main_btf;
> size_t local_essent_len;
> - int err;
> + int err, i;
>
> local_cand.btf = local_btf;
> local_cand.t = btf__type_by_id(local_btf, local_type_id);
> @@ -4697,15 +4824,38 @@ bpf_core_find_cands(struct bpf_object *obj, const struct btf *local_btf, __u32 l
> return ERR_PTR(-ENOMEM);
>
> /* Attempt to find target candidates in vmlinux BTF first */
> - err = bpf_core_add_cands(&local_cand, local_essent_len,
> - obj->btf_vmlinux_override ?: obj->btf_vmlinux,
> - "vmlinux", 1, cands);
> - if (err) {
> - bpf_core_free_cands(cands);
> - return ERR_PTR(err);
> + main_btf = obj->btf_vmlinux_override ?: obj->btf_vmlinux;
> + err = bpf_core_add_cands(&local_cand, local_essent_len, main_btf, "vmlinux", 1, cands);
> + if (err)
> + goto err_out;
> +
> + /* if vmlinux BTF has any candidate, don't got for module BTFs */
small typo: don't got for -> don't go for
> + if (cands->len)
> + return cands;
> +
> + /* if vmlinux BTF was overridden, don't attempt to load module BTFs */
> + if (obj->btf_vmlinux_override)
> + return cands;
> +
> + /* now look through module BTFs, trying to still find candidates */
> + err = load_module_btfs(obj);
> + if (err)
> + goto err_out;
> +
> + for (i = 0; i < obj->btf_module_cnt; i++) {
> + err = bpf_core_add_cands(&local_cand, local_essent_len,
> + obj->btf_modules[i].btf,
> + obj->btf_modules[i].name,
> + btf__get_nr_types(obj->btf_vmlinux) + 1,
> + cands);
> + if (err)
> + goto err_out;
> }
>
> return cands;
> +err_out:
> + bpf_core_free_cands(cands);
> + return ERR_PTR(err);
> }
>
Powered by blists - more mailing lists