Re: [RFC PATCH bpf-next v2 1/2] libbpf: support selective kernel module BTF loading via .kmod_btfs section

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Missing increment of `nr_loaded` prevents early loop termination.
- [Low] Multi-line comment formatting violation.
--

--- Patch [1]: [RFC PATCH bpf-next v2 1/2] libbpf: support selective kernel module BTF loading via .kmod_btfs section ---
commit 3f4fb04bbfb95d7f7ea0012b5651135983af6e62
Author: Fuyu Zhao <[email protected]>

libbpf: support selective kernel module BTF loading via .kmod_btfs section

This patch adds support for a ".kmod_btfs" ELF section to allow BPF programs
to specify which kernel modules need BTF loading. It introduces functions to
collect the module names and filter the BTF loading process to avoid loading
unneeded modules.

> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> index 9d160b5b9c0ee..171ea055cd323 100644
> --- a/tools/lib/bpf/bpf_helpers.h
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -188,6 +188,20 @@ enum libbpf_tristate {
>  	TRI_MODULE = 2,
>  };
>  
> +/* Helper typedef for declaring kernel module names that need BTF loading.
> + *

[Severity: Low]
This isn't a bug, but the BPF subsystem coding style expects multi-line
comments to have the opening /* on its own line. Should this be reformatted
to match the preferred kernel style?

> + * Usage: define an array in the ".kmod_btfs" ELF section to specify
> + * which modules need BTF loading:
> + *

[ ... ]

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 514e4e9daa825..9fe8a653e8b6b 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[ ... ]

> @@ -5891,6 +5987,10 @@ static int load_module_btfs(struct bpf_object *obj)
>  			break;
>  		}
>  		obj->btf_module_cnt++;
> +
> +		if (obj->kmod_btfs &&
> +		    obj->kmod_btfs->nr_names == obj->kmod_btfs->nr_loaded)
> +			break;

[Severity: Medium]
Is obj->kmod_btfs->nr_loaded missing an increment?

Looking at the loop in load_module_btfs(), when a module BTF is successfully
loaded and obj->btf_module_cnt is incremented, obj->kmod_btfs->nr_loaded
doesn't appear to be updated anywhere.

Will this prevent the intended optimization of breaking out of the loop early,
causing it to always scan all remaining kernel BTF IDs unnecessarily?

>  	}
>  
>  	if (err) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1