From: Eduard Zingerman <eddyz87@gmail.com>
To: Fuyu Zhao <zhaofuyu@vivo.com>,
bpf@vger.kernel.org, andrii.nakryiko@gmail.com,
alan.maguire@oracle.com
Cc: andrii@kernel.org, ast@kernel.org, daniel@iogearbox.net,
memxor@gmail.com, martin.lau@linux.dev, song@kernel.org,
yonghong.song@linux.dev, jolsa@kernel.org, emil@etsalapatis.com,
ihor.solodrai@linux.dev, shuah@kernel.org, yatsenko@meta.com,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [RFC PATCH bpf-next v3 1/2] libbpf: support selective kernel module BTF loading via bpf_object_open_opts
Date: Thu, 20 Aug 2026 16:58:21 -0700 [thread overview]
Message-ID: <5efea1c6d5feb9bfc4ffe3eedde9ab98c2613776.camel@gmail.com> (raw)
In-Reply-To: <20260819090426.267-2-zhaofuyu@vivo.com>
On Wed, 2026-08-19 at 17:04 +0800, Fuyu Zhao wrote:
Overall the logic seem to be fine for me, please find a few comments below.
...
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 514e4e9daa82..37934ca49dd7 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -779,6 +779,13 @@ struct bpf_object {
> char *token_path;
> int token_fd;
>
> + /* kernel module BTFs to load, as specified via bpf_object_open_opts */
> + struct {
> + char **names;
> + size_t nr_names;
> + struct hashmap *names_map;
We already have a strset type, please use it instead of a direct
hashmap reference. This would remove the need for `names` field
and simplify the bpf_object__init_kmod_btfs() function.
Also, do you expect `nr_names` to be high?
If not, wouldn't plain array search be simpler/faster here?
> + } *kmod_btfs;
Why indirection?
Also, I agree with the bot here, prior fields use "module" in the name,
so something like "btf_module_names" or similar is a better fit.
Same for publicly visible 'opts' name.
...
> +static int bpf_object__init_kmod_btfs(struct bpf_object *obj,
> + const struct bpf_object_open_opts *opts)
> +{
> + const char **kmod_btf_names;
> + size_t i, kmod_btf_names_cnt;
> + int err;
> +
> + kmod_btf_names = OPTS_GET(opts, kmod_btf_names, NULL);
> + if (!kmod_btf_names)
> + return 0;
> +
> + kmod_btf_names_cnt = OPTS_GET(opts, kmod_btf_names_cnt, 0);
> + if (!kmod_btf_names_cnt) {
> + pr_warn("kmod_btf_names_cnt must be set when kmod_btf_names is provided\n");
> + return -EINVAL;
I kinda agree with the bot here, why disallow an empty filter here?
> + }
> +
> + obj->kmod_btfs = calloc(1, sizeof(*obj->kmod_btfs));
> + if (!obj->kmod_btfs)
> + return -ENOMEM;
> +
> + obj->kmod_btfs->names = calloc(kmod_btf_names_cnt, sizeof(char *));
> + if (!obj->kmod_btfs->names) {
> + err = -ENOMEM;
> + goto err_out;
> + }
> +
> + obj->kmod_btfs->names_map = hashmap__new(mod_name_hash_fn,
> + mod_name_equal_fn, NULL);
> + if (IS_ERR(obj->kmod_btfs->names_map)) {
> + err = PTR_ERR(obj->kmod_btfs->names_map);
> + obj->kmod_btfs->names_map = NULL;
> + goto err_out;
> + }
> +
> + for (i = 0; i < kmod_btf_names_cnt; i++) {
> + size_t idx = obj->kmod_btfs->nr_names;
> +
> + if (!kmod_btf_names[i] || !kmod_btf_names[i][0]) {
> + pr_warn("invalid kernel module BTF name at index %zu\n", i);
> + err = -EINVAL;
> + goto err_out;
> + }
> +
> + obj->kmod_btfs->names[idx] = strdup(kmod_btf_names[i]);
> + if (!obj->kmod_btfs->names[idx]) {
> + err = -ENOMEM;
> + goto err_out;
> + }
> +
> + err = hashmap__add(obj->kmod_btfs->names_map,
> + obj->kmod_btfs->names[idx], 0);
> + if (err) {
> + zfree(&obj->kmod_btfs->names[idx]);
> + if (err == -EEXIST) {
> + pr_warn("duplicate kmod BTF name '%s' ignored\n",
> + kmod_btf_names[i]);
Nit: I'd downgrade this to debug level, if at all.
> + continue;
> + }
> + goto err_out;
> + }
> + obj->kmod_btfs->nr_names++;
> + }
> + return 0;
> +
> +err_out:
> + bpf_object__free_kmod_btfs(obj);
> + return err;
> +}
...
> @@ -8515,6 +8645,7 @@ static struct bpf_object *bpf_object_open(const char *path, const void *obj_buf,
> err = err ? : bpf_object__init_maps(obj, opts);
> err = err ? : bpf_object_init_progs(obj, opts);
> err = err ? : bpf_object__collect_relos(obj);
> + err = err ? : bpf_object__init_kmod_btfs(obj, opts);
Nit: all other options are collected before bpf_object__elf_init()
call just above.
> if (err)
> goto out;
>
...
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index b965ad571540..2f8ff6d2d3df 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -224,10 +224,24 @@ struct bpf_object_open_opts {
> * point (/sys/fs/bpf), in case this default behavior is undesirable.
> */
> const char *bpf_token_path;
> + /*
> + * Optional list of kernel module names whose BTFs should be loaded.
> + * kmod_btf_names_cnt specifies the number of entries in
> + * kmod_btf_names.
> + *
> + * If kmod_btf_names is NULL, all module BTFs are loaded, preserving
> + * the default behavior. Otherwise, only the specified module BTFs
> + * are loaded.
> + *
> + * kmod_btf_names_cnt must be non-zero when kmod_btf_names is
> + * non-NULL; otherwise -EINVAL is returned.
Nit: please add a complete list of behaviors this affects, e.g. CO-RE,
fentry/fexit resolution, etc.
> + */
> + const char **kmod_btf_names;
> + size_t kmod_btf_names_cnt;
>
> size_t :0;
> };
> -#define bpf_object_open_opts__last_field bpf_token_path
> +#define bpf_object_open_opts__last_field kmod_btf_names_cnt
>
> /**
> * @brief **bpf_object__open()** creates a bpf_object by opening
next prev parent reply other threads:[~2026-08-20 23:58 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-19 9:04 [RFC PATCH bpf-next v3 0/2] libbpf: Improve BPF load performance by selectively loading kmod BTFs Fuyu Zhao
2026-08-19 9:04 ` [RFC PATCH bpf-next v3 1/2] libbpf: support selective kernel module BTF loading via bpf_object_open_opts Fuyu Zhao
2026-08-19 9:12 ` sashiko-bot
2026-08-20 3:19 ` Fuyu Zhao
2026-08-19 9:35 ` bot+bpf-ci
2026-08-20 3:27 ` Fuyu Zhao
2026-08-20 23:58 ` Eduard Zingerman [this message]
2026-08-21 3:51 ` Fuyu Zhao
2026-08-19 9:04 ` [RFC PATCH bpf-next v3 2/2] selftests/bpf: add tests for selective kmod BTF loading Fuyu Zhao
2026-08-19 9:12 ` sashiko-bot
2026-08-20 3:20 ` Fuyu Zhao
2026-08-19 9:50 ` bot+bpf-ci
2026-08-20 3:28 ` Fuyu Zhao
2026-08-21 0:10 ` Eduard Zingerman
2026-08-21 3:51 ` Fuyu Zhao
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=5efea1c6d5feb9bfc4ffe3eedde9ab98c2613776.camel@gmail.com \
--to=eddyz87@gmail.com \
--cc=alan.maguire@oracle.com \
--cc=andrii.nakryiko@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=emil@etsalapatis.com \
--cc=ihor.solodrai@linux.dev \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=yatsenko@meta.com \
--cc=yonghong.song@linux.dev \
--cc=zhaofuyu@vivo.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.