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 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.