From: Fuyu Zhao <zhaofuyu@vivo.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: bpf@vger.kernel.org, eddyz87@gmail.com, alan.maguire@oracle.com
Subject: Re: [PATCH bpf-next v6 1/2] libbpf: support selective kernel module BTF loading via bpf_object_open_opts
Date: Thu, 3 Sep 2026 20:52:39 +0800 [thread overview]
Message-ID: <8506da55-e99b-4f23-93bb-e11229760917@vivo.com> (raw)
In-Reply-To: <CAEf4BzaHTTEob-vmhmcRt1pmED+eBBLWnehEDUk=t7O7GUyyFg@mail.gmail.com>
On 9/3/2026 8:12 AM, Andrii Nakryiko wrote:
> On Mon, Aug 31, 2026 at 6:38 AM Fuyu Zhao <zhaofuyu@vivo.com> wrote:
>>
>> Add btf_module_names and nr_btf_module_names fields to
>> bpf_object_open_opts to support selective kernel module BTF loading.
>>
>> With btf_module_names:
>> - when provided, only the specified kernel module BTFs are loaded;
>> - when an empty list is provided, no module BTFs are loaded;
>> - when NULL, all module BTFs are loaded as before.
>>
>> This avoids unnecessary module BTF loading and reduces BPF object
>> loading time when only a subset of kernel module BTFs is needed.
>>
>> Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
>
> nit: use andrii@kernel.org as my email, thanks
>
Will do.
>> Signed-off-by: Fuyu Zhao <zhaofuyu@vivo.com>
>> ---
>> tools/lib/bpf/libbpf.c | 117 +++++++++++++++++++++++++++++++++++++++++
>> tools/lib/bpf/libbpf.h | 24 ++++++++-
>> 2 files changed, 140 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 514e4e9daa82..e19d0afd2592 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -779,6 +779,9 @@ struct bpf_object {
>> char *token_path;
>> int token_fd;
>>
>> + char **btf_module_names;
>> + size_t nr_btf_module_names;
>
> there is entire module related set of fields higher up, put these new
> fields close to them
>
Sure, I'll move them next to the existing module-related fields.
> but also naming-wise:
> - use blah_cnt instead of nr_blah naming
> - we should call this "btf_module_allowlist" and
> btf_module_allowlist_cnt, a bit verbose but explains what it is and
> leaves doors open for denylist, if we ever need it. Please use these
> names internally in bpf_object and in bpf_object_open_opts
>
Makes sense. I'll rename them to btf_module_allowlist and
btf_module_allowlist_cnt.
> overall, the code as is super verbose and doesn't feel like libbpf
> code, please try to preserve naming/styling/succinct approach as much
> as possible
>
Understood. I'll simplify the implementation and keep it closer to the
existing libbpf style.
>> +
>> char path[];
>> };
>>
>> @@ -5803,6 +5806,103 @@ int bpf_core_add_cands(struct bpf_core_cand *local_cand,
>> return 0;
>> }
>>
>> +static void bpf_object_free_btf_module_names(struct bpf_object *obj)
>> +{
>> + size_t i;
>> +
>> + if (!obj->btf_module_names)
>> + return;
>> +
>
> no need for this check, count should be zero and everything works as
> is without extra guard
>
Right, I'll drop it.
>> + for (i = 0; i < obj->nr_btf_module_names; i++)
>> + zfree(&obj->btf_module_names[i]);
>> + zfree(&obj->btf_module_names);
>> + obj->nr_btf_module_names = 0;
>
> just inline this logic in bpf_object__close()
>
Sure.
>> +}
>> +
>> +static int bpf_object_init_btf_module_names(struct bpf_object *obj,
>> + const struct bpf_object_open_opts *opts)
>
> if you look at bpf_object_open(), all the opts processing is done
> inline , let's do the same here. we can do validation and duplicate
> detection before we even get to bpf_object__new. and alloc and strdup
> loop after obj is created
>
Agreed.
> pw-bot: cr
>
>> +{
>> + const char **names;
>> + size_t i, j, cnt;
>> + int err;
>> +
>> + names = OPTS_GET(opts, btf_module_names, NULL);
>> + if (!names)
>> + return 0;
>
> don't exit early, we need to validate that if allowlist is not
> specified, corresponding count is zero and vice versa
>
Understood.
>> +
>> + cnt = OPTS_GET(opts, nr_btf_module_names, 0);
>> +
>> + /*
>> + * Allocate one entry for an empty list to distinguish it from the
>> + * default behavior.
>> + */
>> + obj->btf_module_names = calloc(cnt ?: 1,
>
> this count trick is kind of ugly... should we just set count to <0
> instead? we'll still be able to do for loop without any extra
> handling, but -1 vs 0 for count will be a sign of having no or empty
> allowlist
>
Yes, setting count to -1 can better address this issue.
>> + sizeof(*obj->btf_module_names));
>> + if (!obj->btf_module_names)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < cnt; i++) {
>> + if (!names[i] || !names[i][0]) {
>> + pr_warn("invalid kernel module BTF name at index %zu\n", i);
>
> I don't think this will be some common mistake, let's drop all this
> verbosity with pt_warn and so on, just error out
>
Sure, I'll remove the warning and return the error directly.
>> + err = -EINVAL;
>> + goto err_out;
>> + }
>> +
>> + /*
>> + * The list is expected to be small, so a simple nested scan is
>> + * sufficient for duplicate detection.
>> + */
>
> unnecessary comment, drop it
>
Got it.
>> + for (j = 0; j < i; j++) {
>> + if (strcmp(obj->btf_module_names[j], names[i]) == 0) {
>> + pr_warn("duplicate kernel module BTF name '%s'\n",
>> + names[i]);
>
> ditto, just error out, no pr_warn()
>
Will do.
>> + err = -EINVAL;
>> + goto err_out;
>> + }
>> + }
>> +
>> + obj->btf_module_names[i] = strdup(names[i]);
>> + if (!obj->btf_module_names[i]) {
>> + err = -ENOMEM;
>> + goto err_out;
>> + }
>> +
>> + obj->nr_btf_module_names++;
>> + }
>> + return 0;
>> +
>> +err_out:
>> + bpf_object_free_btf_module_names(obj);
>
> I don't think we need to clean up here, bpf_object__close() will be
> closed and will take care of this cleanup, keep the logic simple
>
Agreed.
>> + return err;
>> +}
>> +
>> +static bool is_module_btf_needed(const struct bpf_object *obj, const char *name)
>
> let's call this is_btf_mod_allowed()
>
Sure, I'll rename it to is_btf_mod_allowed().
>> +{
>> + size_t i;
>> +
>> + if (!obj->btf_module_names)
>> + return true;
>> +
>> + for (i = 0; i < obj->nr_btf_module_names; i++) {
>> + if (strcmp(obj->btf_module_names[i], name) == 0)
>> + return true;
>> + }
>> +
>> + pr_debug("skipping module BTF '%s', not in btf_module_names\n", name);
>
> drop this pr_debug(), do you think it will be very useful? this whole
> feature feels super niche, whoever is going to use it should know what
> they are doing, so I find it unlikely that they might be surprised by
> ignoring kernel module they explicitly excluded
>
Makes sense. I'll remove it.
>> + return false;
>> +}
>> +
>> +static bool no_module_btfs_needed(const struct bpf_object *obj)
>> +{
>> + return obj->btf_module_names && !obj->nr_btf_module_names;
>> +}
>> +
>> +static bool all_needed_module_btfs_loaded(const struct bpf_object *obj)
>> +{
>> + return obj->btf_module_names &&
>> + obj->nr_btf_module_names == obj->btf_module_cnt;
>> +}
>> +
>
> these two helpers are just verbosity, let's get rid of them and do
> corresponding checks inline
>
Understood. I'll remove both helpers and inline the checks.
>> static int load_module_btfs(struct bpf_object *obj)
>> {
>> struct bpf_btf_info info;
>> @@ -5825,6 +5925,9 @@ static int load_module_btfs(struct bpf_object *obj)
>> if (!kernel_supports(obj, FEAT_MODULE_BTF))
>> return 0;
>>
>> + if (no_module_btfs_needed(obj))
>> + return 0;
>
> under new convention, it will be `if (btf_mod_allowlist_cnt == 0) return 0;`
>
Right. I'll handle it with the new convention.
>> +
>> while (true) {
>> err = bpf_btf_get_next_id(id, &id);
>> if (err && errno == ENOENT)
>> @@ -5867,6 +5970,11 @@ static int load_module_btfs(struct bpf_object *obj)
>> continue;
>> }
>>
>> + if (!is_module_btf_needed(obj, name)) {
>> + close(fd);
>> + continue;
>> + }
>> +
>> btf = btf_get_from_fd(fd, obj->btf_vmlinux);
>> err = libbpf_get_error(btf);
>> if (err) {
>> @@ -5891,6 +5999,9 @@ static int load_module_btfs(struct bpf_object *obj)
>> break;
>> }
>> obj->btf_module_cnt++;
>> +
>> + if (all_needed_module_btfs_loaded(obj))
>> + break;
>> }
>>
>> if (err) {
>> @@ -8508,6 +8619,10 @@ static struct bpf_object *bpf_object_open(const char *path, const void *obj_buf,
>> }
>> }
>>
>> + err = bpf_object_init_btf_module_names(obj, opts);
>> + if (err)
>> + goto out;
>> +
>> err = bpf_object__elf_init(obj);
>> err = err ? : bpf_object__elf_collect(obj);
>> err = err ? : bpf_object__collect_externs(obj);
>> @@ -9629,6 +9744,8 @@ void bpf_object__close(struct bpf_object *obj)
>> close(obj->jumptable_maps[i].fd);
>> zfree(&obj->jumptable_maps);
>>
>> + bpf_object_free_btf_module_names(obj);
>> +
>
> just inline that simple clean up logic here without helper function
>
Will do. I'll inline this.
>> free(obj);
>> }
>>
>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>> index b965ad571540..1cabf7e46554 100644
>> --- a/tools/lib/bpf/libbpf.h
>> +++ b/tools/lib/bpf/libbpf.h
>> @@ -224,10 +224,32 @@ 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.
>> + * nr_btf_module_names specifies the number of entries in
>> + * btf_module_names.
>
> not really, it's not "what should be loaded" but what is allowed to be
> loaded. quite a difference, because with "should be loaded" we should
> be failing if we don't find all those modules, but I don't think that
> was the intent, was it?
>
Yes, exactly as you said.
>> + *
>> + * With btf_module_names:
>> + * - when provided, only the BTFs of the specified modules are loaded;
>> + * - when an empty list is provided, no module BTFs are loaded;
>> + * - when NULL, all module BTFs are loaded as before.
>> + *
>
> also mention that count should be zero for empty or NULL list
>
Sure, I'll clarify the count requirement in the documentation.
>> + * The list must contain valid, non-empty module names and must not
>> + * contain duplicate entries; otherwise -EINVAL is returned.
>> + *
>> + * This affects:
>
> just say that this allowlist affects any place where kernel module BTF
> might be needed by limiting which kernel module BTFs libbpf will
> consult. no need to enumerate all the different places where we use
> BTF, it will get out of sync too easily.
>
Makes sense. I'll simplify the documentation to describe the general
allowlist behavior.
>> + * - BPF CO-RE relocations against types defined in modules;
>> + * - BTF-based resolution of attach targets;
>> + * - module-qualified tracing multi-attach targets;
>> + * - struct_ops kernel type resolution;
>> + * - extern (ksym) resolution for kernel symbols defined in modules.
>> + */
>> + const char **btf_module_names;
>> + size_t nr_btf_module_names;
>>
>> size_t :0;
>> };
>> -#define bpf_object_open_opts__last_field bpf_token_path
>> +#define bpf_object_open_opts__last_field nr_btf_module_names
>>
>> /**
>> * @brief **bpf_object__open()** creates a bpf_object by opening
>> --
>> 2.34.1
>>
next prev parent reply other threads:[~2026-09-03 12:52 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-31 13:38 [PATCH bpf-next v6 0/2] libbpf: Improve BPF load performance by selectively loading module BTFs Fuyu Zhao
2026-08-31 13:38 ` [PATCH bpf-next v6 1/2] libbpf: support selective kernel module BTF loading via bpf_object_open_opts Fuyu Zhao
2026-08-31 14:46 ` bot+bpf-ci
2026-09-01 3:55 ` Fuyu Zhao
2026-08-31 14:46 ` Alan Maguire
2026-09-01 3:15 ` Fuyu Zhao
2026-09-01 7:33 ` Alan Maguire
2026-09-03 0:12 ` Andrii Nakryiko
2026-09-03 12:52 ` Fuyu Zhao [this message]
2026-08-31 13:38 ` [PATCH bpf-next v6 2/2] selftests/bpf: add tests for selective module BTF loading Fuyu Zhao
2026-08-31 14:46 ` bot+bpf-ci
2026-09-01 3:56 ` Fuyu Zhao
2026-09-03 0:12 ` Andrii Nakryiko
2026-09-03 12:52 ` 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=8506da55-e99b-4f23-93bb-e11229760917@vivo.com \
--to=zhaofuyu@vivo.com \
--cc=alan.maguire@oracle.com \
--cc=andrii.nakryiko@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=eddyz87@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox