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

On 8/13/2026 12:22 PM, [email protected] wrote:
>> 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.
>> + *
>> + * Usage: define an array in the ".kmod_btfs" ELF section to specify
>> + * which modules need BTF loading:
>> + *
>> + *   DEFINE_KMOD_BTFS(_needed_kmods)= { "module1", "module2", ... };
>> + *
>> + * This avoids unnecessary BTF loading and speeds up the BPF program
>> + * load process.
>> + */
>> +#define KMOD_NAME_LEN 64
>> +#define DEFINE_KMOD_BTFS(name) \
>> +	SEC(".kmod_btfs") char name[][KMOD_NAME_LEN]
> 
> KMOD_NAME_LEN establishes the on-disk record size for the .kmod_btfs section
> format, yet it's duplicated in two files with only a prose comment connecting
> them:
> 
>   tools/lib/bpf/bpf_helpers.h:201   #define KMOD_NAME_LEN 64
>   tools/lib/bpf/libbpf.c:707-708    /* Should match the definition in ... */
>                                     #define KMOD_NAME_LEN 64
> 
> If the two ever diverge, bpf_object__collect_kmod_btf_names() will silently
> mis-slice the section. The d_size % KMOD_NAME_LEN check only catches
> non-multiples, not a mismatch between the two values. Could this use a shared
> header or a static assert to ensure they stay in sync?
> 
> Also, the new macro has no #ifndef guard. This header is included by nearly
> every BPF program alongside vmlinux.h and user headers. Other unprefixed
> names this header exports (NULL at lines 78-80, KERNEL_VERSION at 82-84) all
> have #ifndef wrappers. Should KMOD_NAME_LEN follow that pattern?
> 

Thanks, I agree this is worth addressing. I'm not sure there is a
suitable shared header for this definition, and a static assertion
cannot directly ensure that the two definitions stay in sync. I'll keep
looking for a better solution. Suggestions are welcome.

>> 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
> 
> [ ... ]
> 
>> @@ -5803,6 +5888,11 @@ int bpf_core_add_cands(struct bpf_core_cand *local_cand,
>>  	return 0;
>>  }
>>
>> +static bool is_kmod_btf_needed(struct bpf_object *obj, const char *name)
>> +{
>> +	return hashmap_find(obj->kmod_btfs->hashmap, (long)name, NULL);
>> +}
> 
> This calls the raw hashmap_find() with a manual (long) cast, rather than
> the hashmap__find() wrapper that hashmap.h provides:
> 
>   hashmap.h:159
>   #define hashmap__find(map, key, value) \
>       hashmap_find((map), (long)(key), hashmap_cast_ptr(value))
> 
> The header comments at lines 50 and 54 say the intent is to use the wrapper
> ("behind auxiliary macros") so that the _Static_assert on the value pointer
> runs. Bypassing the macro also bypasses that check.
> 
> This is the only direct caller of the unversioned function in the whole
> library. The other function added by this same patch does use the wrapper
> (bpf_object__collect_kmod_btf_names at libbpf.c:964 uses hashmap__find), so
> the two halves of the change are inconsistent. Should this use the wrapper
> too?
> 
> [ ... ]
> 

Agreed. I'll use hashmap__find() here for consistency.

>> @@ -9030,6 +9130,12 @@ static void bpf_object_cleanup_btf(struct bpf_object *obj)
>>  	/* clean up vmlinux BTF */
>>  	btf__free(obj->btf_vmlinux);
>>  	obj->btf_vmlinux = NULL;
>> +
>> +	if (obj->kmod_btfs) {
>> +		hashmap__free(obj->kmod_btfs->hashmap);
>> +		zfree(&obj->kmod_btfs->data);
>> +		zfree(&obj->kmod_btfs);
>> +	}
>>  }
> 
> The .kmod_btfs filter list is ELF open-time data, but it's freed from
> bpf_object_cleanup_btf(), which runs while the bpf_object is still alive and
> load_module_btfs() can still be called. The filter therefore stops applying
> part-way through the object's lifetime. Concrete path:
> 
>   bpf_object_load()                       libbpf.c:9182
>     ... -> bpf_object_post_load_cleanup() libbpf.c:9229
>              -> bpf_object_cleanup_btf()  libbpf.c:9147
>                   obj->btf_modules_loaded = false;   (libbpf.c:9128)
>                   zfree(&obj->kmod_btfs);            (libbpf.c:9137)
> 
>   bpf_program__attach_tracing_multi()     libbpf.c:12791
>     -> collect_func_ids_by_glob()         libbpf.c:12752
>          -> find_attach_module()          libbpf.c:7877
>               -> load_module_btfs()       libbpf.c:5896
> 
> At this second entry into load_module_btfs(), btf_modules_loaded is false so
> the full scan re-runs, but obj->kmod_btfs is NULL. The filter at
> libbpf.c:5960 is skipped and every module BTF in the system is loaded,
> exactly what the section is supposed to prevent. collect_func_ids_by_glob()
> also calls bpf_object_cleanup_btf(obj) unconditionally at its cleanup: label
> (libbpf.c:12786), so the list is destroyed again on every attach.
> 
> The other open-time members with the same lifetime (obj->jumptables_data,
> obj->arena_data, obj->kconfig) are freed in bpf_object__close()
> (libbpf.c:9720-9733), not in the BTF cleanup helper. Should obj->kmod_btfs
> be freed there instead?
> 

Thanks, this makes sense. I'll look into this more carefully and check
the appropriate lifetime for kmod_btfs.

> 
> ---
> 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/31664172915