Re: [PATCH v6 bpf-next 8/9] libbpf,bpf: share BTF relocate-related code with kernel

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2024-06-17 at 14:31 +0100, Alan Maguire wrote:

[...]

Hi Alan,

> great catch! I think we need
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index da70914264fa..ef793731d40f 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -1676,14 +1676,8 @@ static void btf_free_kfunc_set_tab(struct btf *btf)
> 
>         if (!tab)
>                 return;
> -       /* For module BTF, we directly assign the sets being registered, so
> -        * there is nothing to free except kfunc_set_tab.
> -        */
> -       if (btf_is_module(btf))
> -               goto free_tab;
>         for (hook = 0; hook < ARRAY_SIZE(tab->sets); hook++)
>                 kfree(tab->sets[hook]);
> -free_tab:
>         kfree(tab);
>         btf->kfunc_set_tab = NULL;
>  }

Agree

[...]

> > > @@ -8451,6 +8522,13 @@ int register_btf_id_dtor_kfuncs(const struct btf_id_dtor_kfunc *dtors, u32 add_c
> > >  	btf->dtor_kfunc_tab = tab;
> > >  
> > >  	memcpy(tab->dtors + tab->cnt, dtors, add_cnt * sizeof(tab->dtors[0]));
> > > +
> > > +	/* remap BTF ids based on BTF relocation (if any) */
> > > +	for (i = tab_cnt; i < tab_cnt + add_cnt; i++) {
> > > +		tab->dtors[i].btf_id = btf_relocate_id(btf, tab->dtors[i].btf_id);
> > > +		tab->dtors[i].kfunc_btf_id = btf_relocate_id(btf, tab->dtors[i].kfunc_btf_id);
> > 
> > The register_btf_id_dtor_kfuncs() is exported and thus could to be
> > called from the modules, that's why you update it, right?
> > Do we want to add such call to bpf_testmod? Currently, with kernel
> > config used for selftests, I see only identity mappings.
> > 
> 
> Yep, we don't currently have coverage for dtors in bpf_testmod. I'll
> look at adding that. Thanks!

Great, thank you!

Eduard