Re: [PATCH dwarves v3] pahole: Inject kfunc decl tags into BTF

[Jiri Olsa <olsajiri@gmail.com>]

On Sun, Feb 04, 2024 at 11:08:45AM -0700, Daniel Xu wrote:
> On Wed, Jan 31, 2024 at 11:17:23AM +0100, Jiri Olsa wrote:
> > On Sun, Jan 28, 2024 at 06:30:19PM -0700, Daniel Xu wrote:
> > > This commit teaches pahole to parse symbols in .BTF_ids section in
> > > vmlinux and discover exported kfuncs. Pahole then takes the list of
> > > kfuncs and injects a BTF_KIND_DECL_TAG for each kfunc.
> > > 
> > > Example of encoding:
> > > 
> > >         $ bpftool btf dump file .tmp_vmlinux.btf | rg "DECL_TAG 'bpf_kfunc'" | wc -l
> > >         121
> > > 
> > >         $ bpftool btf dump file .tmp_vmlinux.btf | rg 56337
> > >         [56337] FUNC 'bpf_ct_change_timeout' type_id=56336 linkage=static
> > >         [127861] DECL_TAG 'bpf_kfunc' type_id=56337 component_idx=-1
> > > 
> > > This enables downstream users and tools to dynamically discover which
> > > kfuncs are available on a system by parsing vmlinux or module BTF, both
> > > available in /sys/kernel/btf.
> > > 
> > > Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> > > 
> > > ---
> > > Changes from v2:
> > > * More reliably detect kfunc membership in set8 by tracking set addr ranges
> > > * Rename some variables/functions to be more clear about kfunc vs func
> > > 
> > > Changes from v1:
> > > * Fix resource leaks
> > > * Fix callee -> caller typo
> > > * Rename btf_decl_tag from kfunc -> bpf_kfunc
> > > * Only grab btf_id_set funcs tagged kfunc
> > > * Presort btf func list
> > > 
> > >  btf_encoder.c | 347 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 347 insertions(+)
> > > 
> > > diff --git a/btf_encoder.c b/btf_encoder.c
> > > index fd04008..4f742b1 100644
> > > --- a/btf_encoder.c
> > > +++ b/btf_encoder.c
> > > @@ -34,6 +34,11 @@
> > >  #include <pthread.h>
> > >  
> > >  #define BTF_ENCODER_MAX_PROTO	512
> > > +#define BTF_IDS_SECTION		".BTF_ids"
> > > +#define BTF_ID_FUNC_PFX		"__BTF_ID__func__"
> > > +#define BTF_ID_SET8_PFX		"__BTF_ID__set8__"
> > > +#define BTF_SET8_KFUNCS		(1 << 0)
> > > +#define BTF_KFUNC_TYPE_TAG	"bpf_kfunc"
> > >  
> > >  /* state used to do later encoding of saved functions */
> > >  struct btf_encoder_state {
> > > @@ -79,6 +84,7 @@ struct btf_encoder {
> > >  			  gen_floats,
> > >  			  is_rel;
> > >  	uint32_t	  array_index_id;
> > > +	struct gobuffer   btf_funcs;
> > 
> > why does this need to be stored in encoder?
> 
> I suppose it doesn't. It's used in multiple functions so I figured it'd
> be less verbose than passing it around. Also since it's fairly generic.
> 
> I can move it to explicit arg passing if you want.

I spent some time trying to figure out why it'd need to be in the
btf_encoder object, so if it's not needed there I'd move it out

thanks,
jirka