> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index b749c01742ee0..a466aabf35e1c 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
[ ... ]
> @@ -5851,6 +5854,99 @@ 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)
> +{
Andrii Nakryiko raised a concern in v3 about the naming convention used
here. Internal helper functions should not use double underscore naming, as
that convention is reserved for extern method-like APIs. Would
bpf_object_free_module_names and bpf_object_init_module_names be more
appropriate, following the single underscore pattern for internal helpers?
Reference:
https://lore.kernel.org/bpf/caef4bzbbdjesw88eek2xrvaxq-hwuj7qtw4bynl3klh0f0u...@mail.gmail.com/
> + size_t i;
> +
> + if (!obj->btf_module_names)
> + return;
> +
> + 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;
> +}
> +
> +static int bpf_object_init_btf_module_names(struct bpf_object *obj,
> + const struct bpf_object_open_opts
> *opts)
> +{
> + const char **names;
> + size_t i, j, cnt;
> + int err;
> +
> + names = OPTS_GET(opts, btf_module_names, NULL);
> + if (!names)
> + return 0;
> +
> + 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,
> + 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);
> + err = -EINVAL;
> + goto err_out;
> + }
> +
> + 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]);
> + err = -EINVAL;
> + goto err_out;
> + }
> + }
Andrii Nakryiko suggested in v3 that the module name list should be sorted
and duplicates detected in a single pass rather than using the O(n^2)
nested loop approach. The suggestion was to error out with -EINVAL if
duplicates are found after sorting. Would this optimization be worth
implementing?
Reference:
https://lore.kernel.org/bpf/caef4bzbbdjesw88eek2xrvaxq-hwuj7qtw4bynl3klh0f0u...@mail.gmail.com/
> +
> + 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);
> + return err;
> +}
[ ... ]
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index b965ad5715403..b8c1d98d69d85 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.
> + *
> + * 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.
> + *
> + * The list must not contain duplicate entries; otherwise -EINVAL is
> + * returned.
> + *
> + * This affects:
> + * - BPF CO-RE relocations against types defined in modules;
> + * - BTF-based resolution of function attach targets for
> + * fentry/fexit/fmod_ret/freplace/LSM programs;
> + * - struct_ops kernel type resolution;
> + * - extern (ksym) resolution for kernel symbols defined in modules.
> + */
Looking at the 'This affects:' list and the -EINVAL contract, are there two
things missing?
First, load_module_btfs() has four callers in libbpf.c, but the list above
covers only three: bpf_core_find_cands (CO-RE relocations), find_kernel_btf_id
(fentry/fexit/etc), and find_ksym_btf_id (extern resolution). The fourth
caller is find_attach_module(), which is reached from tracing_multi_mod_fd()
and collect_func_ids_by_glob() for 'module:glob' attach patterns:
mod = find_attach_module(obj, pattern);
if (!mod) {
err = -EINVAL;
goto cleanup;
}
So a user who sets btf_module_names and then attaches a tracing-multi
program with 'mymod:foo*' gets -EINVAL if mymod is not in the list, but the
documented behavior above doesn't hint that this option affects multi-attach
module resolution.
Second, the -EINVAL contract mentions only duplicates, but
bpf_object_init_btf_module_names() also rejects NULL and empty-string
entries:
if (!names[i] || !names[i][0]) {
pr_warn("invalid kernel module BTF name at index %zu\n", i);
err = -EINVAL;
Would it be worth clarifying both of these in the documentation?
> + 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
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32974547426
