Re: [PATCH dwarves] btf: Generate btf for functions in the .BTF_ids section

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Tue, Apr 27, 2021 at 5:34 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Tue, Apr 27, 2021 at 01:38:20PM +0200, Jiri Olsa escreveu:
> > On Mon, Apr 26, 2021 at 04:26:11PM -0700, Andrii Nakryiko wrote:
> > > On Fri, Apr 23, 2021 at 2:37 PM Martin KaFai Lau <kafai@fb.com> wrote:
> > > > BTF is currently generated for functions that are in ftrace list
> > > > or extern.
> > > >
> > > > A recent use case also needs BTF generated for functions included in
> > > > allowlist.  In particular, the kernel
> > > > commit e78aea8b2170 ("bpf: tcp: Put some tcp cong functions in allowlist for bpf-tcp-cc")
> > > > allows bpf program to directly call a few tcp cc kernel functions.  Those
> > > > functions are specified under an ELF section .BTF_ids.  The symbols
> > > > in this ELF section is like __BTF_ID__func__<kernel_func>__[digit]+.
> > > > For example, __BTF_ID__func__cubictcp_init__1.  Those kernel
> > > > functions are currently allowed only if CONFIG_DYNAMIC_FTRACE is
> > > > set to ensure they are in the ftrace list but this kconfig dependency
> > > > is unnecessary.
> > > >
> > > > pahole can generate BTF for those kernel functions if it knows they
> > > > are in the allowlist.  This patch is to capture those symbols
> > > > in the .BTF_ids section and generate BTF for them.
>
> > > I wonder if we just record all functions how bad that would be. Jiri,
> > > do you remember from the time you were experimenting with static
> > > functions how much more functions we'd be recording if we didn't do
> > > ftrace filtering?
>
> > hum, I can't find that.. but should be just matter of removing
> > that is_ftrace_func check
>
> > if we decided to do that, maybe we could add some bit indicating
> > that the function is traceable? it would save us check with
> > available_filter_functions file
>
> You mean encoding it in BTF, in 'struct btf_type'? Seems important to
> have it, there are free bits there:
>
> /* Max # of type identifier */
> #define BTF_MAX_TYPE    0x000fffff
> /* Max offset into the string section */
> #define BTF_MAX_NAME_OFFSET     0x00ffffff
> /* Max # of struct/union/enum members or func args */
> #define BTF_MAX_VLEN    0xffff
>
> struct btf_type {
>         __u32 name_off;
>         /* "info" bits arrangement
>          * bits  0-15: vlen (e.g. # of struct's members)
>          * bits 16-23: unused
>          * bits 24-27: kind (e.g. int, ptr, array...etc)
>          * bits 28-30: unused
>          * bit     31: kind_flag, currently used by
>          *             struct, union and fwd
>          */
>         __u32 info;
>         /* "size" is used by INT, ENUM, STRUCT, UNION and DATASEC.
>          * "size" tells the size of the type it is describing.
>          *
>          * "type" is used by PTR, TYPEDEF, VOLATILE, CONST, RESTRICT,
>          * FUNC, FUNC_PROTO and VAR.
>          * "type" is a type_id referring to another type.
>          */
>         union {
>                 __u32 size;
>                 __u32 type;
>         };
> };
>
> And tools that expect to trace a function can get that information from
> the BTF info instead of getting some failure when trying to trace those
> functions, right?

I don't think it belongs in BTF, though. Plus there are additional
limitations enforced by BPF verifier that will prevent some functions
to be attached. So just because the function is in BTF doesn't mean
it's 100% attachable.

>
> - Arnaldo