Re: [RFC PATCH bpf-next v3 1/2] libbpf: support selective kernel module BTF loading via bpf_object_open_opts

Eduard Zingerman <[email protected]>
Newsgroups org.kernel.vger.linux-kselftest,org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.