[bpf-next] libbpf: add getters for BTF.ext func and line info

Commit Message

Mykyta Yatsenko March 26, 2025, 6:07 p.m. UTC

From: Mykyta Yatsenko <[e-mail]>

Introducing new libbpf API getters for BTF.ext func and line info,
namely:
  bpf_program__func_info
  bpf_program__func_info_cnt
  bpf_program__func_info_rec_size
  bpf_program__line_info
  bpf_program__line_info_cnt
  bpf_program__line_info_rec_size

This change enables scenarios, when user needs to load bpf_program
directly using `bpf_prog_load`, instead of higher-level
`bpf_object__load`. Line and func info are required for checking BTF
info in verifier; verification may fail without these fields if, for
example, program calls `bpf_obj_new`.

Signed-off-by: Mykyta Yatsenko <[e-mail]>
---
 tools/lib/bpf/libbpf.c   | 30 ++++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.h   |  8 ++++++++
 tools/lib/bpf/libbpf.map |  6 ++++++
 3 files changed, 44 insertions(+)

Comments

Eduard Zingerman March 27, 2025, 9:06 p.m. UTC | #1

On Wed, 2025-03-26 at 18:07 +0000, Mykyta Yatsenko wrote:

[...]

> @@ -940,6 +940,14 @@ LIBBPF_API int bpf_program__set_log_level(struct bpf_program *prog, __u32 log_le
>  LIBBPF_API const char *bpf_program__log_buf(const struct bpf_program *prog, size_t *log_size);
>  LIBBPF_API int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log_size);
>  
> +LIBBPF_API void *bpf_program__func_info(struct bpf_program *prog);

I don't like `void *` being returned here, but alternatives are ugly.
Maybe add a comment listing possible return types?

> +LIBBPF_API __u32 bpf_program__func_info_cnt(struct bpf_program *prog);
> +LIBBPF_API __u32 bpf_program__func_info_rec_size(struct bpf_program *prog);
> +
> +LIBBPF_API void *bpf_program__line_info(struct bpf_program *prog);
> +LIBBPF_API __u32 bpf_program__line_info_cnt(struct bpf_program *prog);
> +LIBBPF_API __u32 bpf_program__line_info_rec_size(struct bpf_program *prog);
> +

[...]

Andrii Nakryiko March 28, 2025, 5:14 p.m. UTC | #2

On Wed, Mar 26, 2025 at 11:07 AM Mykyta Yatsenko
<[e-mail]> wrote:
>
> From: Mykyta Yatsenko <[e-mail]>
>
> Introducing new libbpf API getters for BTF.ext func and line info,
> namely:
>   bpf_program__func_info
>   bpf_program__func_info_cnt
>   bpf_program__func_info_rec_size
>   bpf_program__line_info
>   bpf_program__line_info_cnt
>   bpf_program__line_info_rec_size
>
> This change enables scenarios, when user needs to load bpf_program
> directly using `bpf_prog_load`, instead of higher-level
> `bpf_object__load`. Line and func info are required for checking BTF
> info in verifier; verification may fail without these fields if, for
> example, program calls `bpf_obj_new`.
>

Really, bpf_obj_new() needs func_info/line_info? Can you point where
in the verifier we check this, curious why we do that.

> Signed-off-by: Mykyta Yatsenko <[e-mail]>
> ---
>  tools/lib/bpf/libbpf.c   | 30 ++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |  8 ++++++++
>  tools/lib/bpf/libbpf.map |  6 ++++++
>  3 files changed, 44 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6b85060f07b3..bc15526ed84c 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -9455,6 +9455,36 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
>         return 0;
>  }
>
> +void *bpf_program__func_info(struct bpf_program *prog)

const struct bpf_program, here and everywhere else

> +{
> +       return prog->func_info;
> +}
> +
> +__u32 bpf_program__func_info_cnt(struct bpf_program *prog)
> +{
> +       return prog->func_info_cnt;
> +}
> +
> +__u32 bpf_program__func_info_rec_size(struct bpf_program *prog)
> +{
> +       return prog->func_info_rec_size;
> +}
> +
> +void *bpf_program__line_info(struct bpf_program *prog)

should be `const void *`, if we went with `void *`, but see below about types

> +{
> +       return prog->line_info;
> +}
> +
> +__u32 bpf_program__line_info_cnt(struct bpf_program *prog)
> +{
> +       return prog->line_info_cnt;
> +}
> +
> +__u32 bpf_program__line_info_rec_size(struct bpf_program *prog)
> +{
> +       return prog->line_info_rec_size;
> +}
> +

As Eduard mentioned, I don't think `void *` is a good interface. We
have bpf_line_info_min and bpf_func_info_min structs in
libbpf_internal.h. We have never changed those types, so at this point
I feel comfortable enough to expose them as API types. Let's drop the
_min suffix, and move definitions to btf.h?

The only question is whether to document that each record could be
bigger in size than sizeof(struct bpf_func_info) (and similarly for
bpf_line_info), and thus user should always care about
func_info_rec_size? Or, to keep it ergonomic and simple, and basically
always return sizeof(struct bpf_func_info) data (and if it so happens
that we'll in the future have different record sizes, then we'll
create a local trimmed representation for user; it's a pain, but we
won't be really stuck from API compatibility standpoint).

I'd go with simple and ergonomic, given we haven't ever extended these
records, and it's unlikely we will. Those types work well and provide
enough information as is. So let's not even add _rec_size() APIs (at
least for now; we can always revisit this later)

pw-bot: cr

>  #define SEC_DEF(sec_pfx, ptype, atype, flags, ...) {                       \
>         .sec = (char *)sec_pfx,                                             \
>         .prog_type = BPF_PROG_TYPE_##ptype,                                 \
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index e0605403f977..29a5fd7f51f0 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -940,6 +940,14 @@ LIBBPF_API int bpf_program__set_log_level(struct bpf_program *prog, __u32 log_le
>  LIBBPF_API const char *bpf_program__log_buf(const struct bpf_program *prog, size_t *log_size);
>  LIBBPF_API int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log_size);
>
> +LIBBPF_API void *bpf_program__func_info(struct bpf_program *prog);
> +LIBBPF_API __u32 bpf_program__func_info_cnt(struct bpf_program *prog);
> +LIBBPF_API __u32 bpf_program__func_info_rec_size(struct bpf_program *prog);
> +
> +LIBBPF_API void *bpf_program__line_info(struct bpf_program *prog);
> +LIBBPF_API __u32 bpf_program__line_info_cnt(struct bpf_program *prog);
> +LIBBPF_API __u32 bpf_program__line_info_rec_size(struct bpf_program *prog);
> +
>  /**
>   * @brief **bpf_program__set_attach_target()** sets BTF-based attach target
>   * for supported BPF program types:
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index d8b71f22f197..a5d83189c084 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -437,6 +437,12 @@ LIBBPF_1.6.0 {
>                 bpf_linker__add_fd;
>                 bpf_linker__new_fd;
>                 bpf_object__prepare;
> +               bpf_program__func_info;
> +               bpf_program__func_info_cnt;
> +               bpf_program__func_info_rec_size;
> +               bpf_program__line_info;
> +               bpf_program__line_info_cnt;
> +               bpf_program__line_info_rec_size;
>                 btf__add_decl_attr;
>                 btf__add_type_attr;
>  } LIBBPF_1.5.0;
> --
> 2.48.1
>

Eduard Zingerman March 28, 2025, 5:29 p.m. UTC | #3

On Fri, 2025-03-28 at 10:14 -0700, Andrii Nakryiko wrote:

[...]

> As Eduard mentioned, I don't think `void *` is a good interface. We
> have bpf_line_info_min and bpf_func_info_min structs in
> libbpf_internal.h. We have never changed those types, so at this point
> I feel comfortable enough to expose them as API types. Let's drop the
> _min suffix, and move definitions to btf.h?
> 
> The only question is whether to document that each record could be
> bigger in size than sizeof(struct bpf_func_info) (and similarly for
> bpf_line_info), and thus user should always care about
> func_info_rec_size? Or, to keep it ergonomic and simple, and basically
> always return sizeof(struct bpf_func_info) data (and if it so happens
> that we'll in the future have different record sizes, then we'll
> create a local trimmed representation for user; it's a pain, but we
> won't be really stuck from API compatibility standpoint).

There is also an option to do:

    const struct bpf_line_info *bpf_program__line_info(const struct bpf_program *prog, u32 idx)

and hide the rec_size.
But yes, simply assuming the array of `struct bpf_line_info` looks reasonable.

[...]

Andrii Nakryiko March 28, 2025, 5:33 p.m. UTC | #4

On Fri, Mar 28, 2025 at 10:29 AM Eduard Zingerman <[e-mail]> wrote:
>
> On Fri, 2025-03-28 at 10:14 -0700, Andrii Nakryiko wrote:
>
> [...]
>
> > As Eduard mentioned, I don't think `void *` is a good interface. We
> > have bpf_line_info_min and bpf_func_info_min structs in
> > libbpf_internal.h. We have never changed those types, so at this point
> > I feel comfortable enough to expose them as API types. Let's drop the
> > _min suffix, and move definitions to btf.h?
> >
> > The only question is whether to document that each record could be
> > bigger in size than sizeof(struct bpf_func_info) (and similarly for
> > bpf_line_info), and thus user should always care about
> > func_info_rec_size? Or, to keep it ergonomic and simple, and basically
> > always return sizeof(struct bpf_func_info) data (and if it so happens
> > that we'll in the future have different record sizes, then we'll
> > create a local trimmed representation for user; it's a pain, but we
> > won't be really stuck from API compatibility standpoint).
>
> There is also an option to do:
>
>     const struct bpf_line_info *bpf_program__line_info(const struct bpf_program *prog, u32 idx)
>
> and hide the rec_size.
> But yes, simply assuming the array of `struct bpf_line_info` looks reasonable.

The main (and could be the only one; though I can see bpftool/veristat
or some other tools using this to do a better program dump as well,
but alas) use case is for manual bpf_prog_load() of a program prepared
by libbpf as part of bpf_object__prepare(), so making users
reconstruct an array one item at a time seems counterproductive, IMO.

So I'd say it comes down to whether we expose "line_info struct can
grow, theoretically" parts in this API or not. But I can see how we
can accommodate this with adding new APIs (and keeping old ones
working), if line_info or func_info ever grows, so it feels
justifiable to do it in a simple and nice way today.

>
> [...]
>

Mykyta Yatsenko March 28, 2025, 7:16 p.m. UTC | #5

On 28/03/2025 17:14, Andrii Nakryiko wrote:
> On Wed, Mar 26, 2025 at 11:07 AM Mykyta Yatsenko
> <[e-mail]> wrote:
>> From: Mykyta Yatsenko <[e-mail]>
>>
>> Introducing new libbpf API getters for BTF.ext func and line info,
>> namely:
>>    bpf_program__func_info
>>    bpf_program__func_info_cnt
>>    bpf_program__func_info_rec_size
>>    bpf_program__line_info
>>    bpf_program__line_info_cnt
>>    bpf_program__line_info_rec_size
>>
>> This change enables scenarios, when user needs to load bpf_program
>> directly using `bpf_prog_load`, instead of higher-level
>> `bpf_object__load`. Line and func info are required for checking BTF
>> info in verifier; verification may fail without these fields if, for
>> example, program calls `bpf_obj_new`.
>>
> Really, bpf_obj_new() needs func_info/line_info? Can you point where
> in the verifier we check this, curious why we do that.
Indirectly, yes:
in verifier.c function check_btf_info_early sets
`env->prog->aux->btf = btf;`
only if line_info_cnt or func_info_cnt are non zero.
and then there is a check that errors out:
`verbose(env, "bpf_obj_new/bpf_percpu_obj_new requires prog BTF\n");`
perhaps this can be improved as well, by setting aux->btf even if no 
func info and line info
>
>> Signed-off-by: Mykyta Yatsenko <[e-mail]>
>> ---
>>   tools/lib/bpf/libbpf.c   | 30 ++++++++++++++++++++++++++++++
>>   tools/lib/bpf/libbpf.h   |  8 ++++++++
>>   tools/lib/bpf/libbpf.map |  6 ++++++
>>   3 files changed, 44 insertions(+)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 6b85060f07b3..bc15526ed84c 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -9455,6 +9455,36 @@ int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
>>          return 0;
>>   }
>>
>> +void *bpf_program__func_info(struct bpf_program *prog)
> const struct bpf_program, here and everywhere else
>
>> +{
>> +       return prog->func_info;
>> +}
>> +
>> +__u32 bpf_program__func_info_cnt(struct bpf_program *prog)
>> +{
>> +       return prog->func_info_cnt;
>> +}
>> +
>> +__u32 bpf_program__func_info_rec_size(struct bpf_program *prog)
>> +{
>> +       return prog->func_info_rec_size;
>> +}
>> +
>> +void *bpf_program__line_info(struct bpf_program *prog)
> should be `const void *`, if we went with `void *`, but see below about types
>
>> +{
>> +       return prog->line_info;
>> +}
>> +
>> +__u32 bpf_program__line_info_cnt(struct bpf_program *prog)
>> +{
>> +       return prog->line_info_cnt;
>> +}
>> +
>> +__u32 bpf_program__line_info_rec_size(struct bpf_program *prog)
>> +{
>> +       return prog->line_info_rec_size;
>> +}
>> +
> As Eduard mentioned, I don't think `void *` is a good interface. We
> have bpf_line_info_min and bpf_func_info_min structs in
> libbpf_internal.h. We have never changed those types, so at this point
> I feel comfortable enough to expose them as API types. Let's drop the
> _min suffix, and move definitions to btf.h?
>
> The only question is whether to document that each record could be
> bigger in size than sizeof(struct bpf_func_info) (and similarly for
> bpf_line_info), and thus user should always care about
> func_info_rec_size? Or, to keep it ergonomic and simple, and basically
> always return sizeof(struct bpf_func_info) data (and if it so happens
> that we'll in the future have different record sizes, then we'll
> create a local trimmed representation for user; it's a pain, but we
> won't be really stuck from API compatibility standpoint).
>
> I'd go with simple and ergonomic, given we haven't ever extended these
> records, and it's unlikely we will. Those types work well and provide
> enough information as is. So let's not even add _rec_size() APIs (at
> least for now; we can always revisit this later)
>
> pw-bot: cr
Thanks for reviewing, I'll send v2.
>>   #define SEC_DEF(sec_pfx, ptype, atype, flags, ...) {                       \
>>          .sec = (char *)sec_pfx,                                             \
>>          .prog_type = BPF_PROG_TYPE_##ptype,                                 \
>> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
>> index e0605403f977..29a5fd7f51f0 100644
>> --- a/tools/lib/bpf/libbpf.h
>> +++ b/tools/lib/bpf/libbpf.h
>> @@ -940,6 +940,14 @@ LIBBPF_API int bpf_program__set_log_level(struct bpf_program *prog, __u32 log_le
>>   LIBBPF_API const char *bpf_program__log_buf(const struct bpf_program *prog, size_t *log_size);
>>   LIBBPF_API int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log_size);
>>
>> +LIBBPF_API void *bpf_program__func_info(struct bpf_program *prog);
>> +LIBBPF_API __u32 bpf_program__func_info_cnt(struct bpf_program *prog);
>> +LIBBPF_API __u32 bpf_program__func_info_rec_size(struct bpf_program *prog);
>> +
>> +LIBBPF_API void *bpf_program__line_info(struct bpf_program *prog);
>> +LIBBPF_API __u32 bpf_program__line_info_cnt(struct bpf_program *prog);
>> +LIBBPF_API __u32 bpf_program__line_info_rec_size(struct bpf_program *prog);
>> +
>>   /**
>>    * @brief **bpf_program__set_attach_target()** sets BTF-based attach target
>>    * for supported BPF program types:
>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
>> index d8b71f22f197..a5d83189c084 100644
>> --- a/tools/lib/bpf/libbpf.map
>> +++ b/tools/lib/bpf/libbpf.map
>> @@ -437,6 +437,12 @@ LIBBPF_1.6.0 {
>>                  bpf_linker__add_fd;
>>                  bpf_linker__new_fd;
>>                  bpf_object__prepare;
>> +               bpf_program__func_info;
>> +               bpf_program__func_info_cnt;
>> +               bpf_program__func_info_rec_size;
>> +               bpf_program__line_info;
>> +               bpf_program__line_info_cnt;
>> +               bpf_program__line_info_rec_size;
>>                  btf__add_decl_attr;
>>                  btf__add_type_attr;
>>   } LIBBPF_1.5.0;
>> --
>> 2.48.1
>>

Andrii Nakryiko March 28, 2025, 8:52 p.m. UTC | #6

On Fri, Mar 28, 2025 at 12:16 PM Mykyta Yatsenko
<[e-mail]> wrote:
>
> On 28/03/2025 17:14, Andrii Nakryiko wrote:
> > On Wed, Mar 26, 2025 at 11:07 AM Mykyta Yatsenko
> > <[e-mail]> wrote:
> >> From: Mykyta Yatsenko <[e-mail]>
> >>
> >> Introducing new libbpf API getters for BTF.ext func and line info,
> >> namely:
> >>    bpf_program__func_info
> >>    bpf_program__func_info_cnt
> >>    bpf_program__func_info_rec_size
> >>    bpf_program__line_info
> >>    bpf_program__line_info_cnt
> >>    bpf_program__line_info_rec_size
> >>
> >> This change enables scenarios, when user needs to load bpf_program
> >> directly using `bpf_prog_load`, instead of higher-level
> >> `bpf_object__load`. Line and func info are required for checking BTF
> >> info in verifier; verification may fail without these fields if, for
> >> example, program calls `bpf_obj_new`.
> >>
> > Really, bpf_obj_new() needs func_info/line_info? Can you point where
> > in the verifier we check this, curious why we do that.
> Indirectly, yes:
> in verifier.c function check_btf_info_early sets
> `env->prog->aux->btf = btf;`
> only if line_info_cnt or func_info_cnt are non zero.
> and then there is a check that errors out:
> `verbose(env, "bpf_obj_new/bpf_percpu_obj_new requires prog BTF\n");`
> perhaps this can be improved as well, by setting aux->btf even if no
> func info and line info

lol, doesn't seem intentional (just in the early days prog BTF was
only referenced and used with func_info/line_info, which isn't true
anymore), we can probably swap the order and load and remember prog
BTF regardless of func_info/line_info. Feel free to send a patch.

> >
> >> Signed-off-by: Mykyta Yatsenko <[e-mail]>
> >> ---
> >>   tools/lib/bpf/libbpf.c   | 30 ++++++++++++++++++++++++++++++
> >>   tools/lib/bpf/libbpf.h   |  8 ++++++++
> >>   tools/lib/bpf/libbpf.map |  6 ++++++
> >>   3 files changed, 44 insertions(+)
> >>

[...]

> >> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> >> index d8b71f22f197..a5d83189c084 100644
> >> --- a/tools/lib/bpf/libbpf.map
> >> +++ b/tools/lib/bpf/libbpf.map
> >> @@ -437,6 +437,12 @@ LIBBPF_1.6.0 {
> >>                  bpf_linker__add_fd;
> >>                  bpf_linker__new_fd;
> >>                  bpf_object__prepare;
> >> +               bpf_program__func_info;
> >> +               bpf_program__func_info_cnt;
> >> +               bpf_program__func_info_rec_size;
> >> +               bpf_program__line_info;
> >> +               bpf_program__line_info_cnt;
> >> +               bpf_program__line_info_rec_size;

nit: hm... please check tabs vs spaces, formatting looks off

> >>                  btf__add_decl_attr;
> >>                  btf__add_type_attr;
> >>   } LIBBPF_1.5.0;
> >> --
> >> 2.48.1
> >>
>

Mykyta Yatsenko March 28, 2025, 8:54 p.m. UTC | #7

On 28/03/2025 20:52, Andrii Nakryiko wrote:
> On Fri, Mar 28, 2025 at 12:16 PM Mykyta Yatsenko
> <[e-mail]> wrote:
>> On 28/03/2025 17:14, Andrii Nakryiko wrote:
>>> On Wed, Mar 26, 2025 at 11:07 AM Mykyta Yatsenko
>>> <[e-mail]> wrote:
>>>> From: Mykyta Yatsenko <[e-mail]>
>>>>
>>>> Introducing new libbpf API getters for BTF.ext func and line info,
>>>> namely:
>>>>     bpf_program__func_info
>>>>     bpf_program__func_info_cnt
>>>>     bpf_program__func_info_rec_size
>>>>     bpf_program__line_info
>>>>     bpf_program__line_info_cnt
>>>>     bpf_program__line_info_rec_size
>>>>
>>>> This change enables scenarios, when user needs to load bpf_program
>>>> directly using `bpf_prog_load`, instead of higher-level
>>>> `bpf_object__load`. Line and func info are required for checking BTF
>>>> info in verifier; verification may fail without these fields if, for
>>>> example, program calls `bpf_obj_new`.
>>>>
>>> Really, bpf_obj_new() needs func_info/line_info? Can you point where
>>> in the verifier we check this, curious why we do that.
>> Indirectly, yes:
>> in verifier.c function check_btf_info_early sets
>> `env->prog->aux->btf = btf;`
>> only if line_info_cnt or func_info_cnt are non zero.
>> and then there is a check that errors out:
>> `verbose(env, "bpf_obj_new/bpf_percpu_obj_new requires prog BTF\n");`
>> perhaps this can be improved as well, by setting aux->btf even if no
>> func info and line info
> lol, doesn't seem intentional (just in the early days prog BTF was
> only referenced and used with func_info/line_info, which isn't true
> anymore), we can probably swap the order and load and remember prog
> BTF regardless of func_info/line_info. Feel free to send a patch.
Sure, I'll do it.
>>>> Signed-off-by: Mykyta Yatsenko <[e-mail]>
>>>> ---
>>>>    tools/lib/bpf/libbpf.c   | 30 ++++++++++++++++++++++++++++++
>>>>    tools/lib/bpf/libbpf.h   |  8 ++++++++
>>>>    tools/lib/bpf/libbpf.map |  6 ++++++
>>>>    3 files changed, 44 insertions(+)
>>>>
> [...]
>
>>>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
>>>> index d8b71f22f197..a5d83189c084 100644
>>>> --- a/tools/lib/bpf/libbpf.map
>>>> +++ b/tools/lib/bpf/libbpf.map
>>>> @@ -437,6 +437,12 @@ LIBBPF_1.6.0 {
>>>>                   bpf_linker__add_fd;
>>>>                   bpf_linker__new_fd;
>>>>                   bpf_object__prepare;
>>>> +               bpf_program__func_info;
>>>> +               bpf_program__func_info_cnt;
>>>> +               bpf_program__func_info_rec_size;
>>>> +               bpf_program__line_info;
>>>> +               bpf_program__line_info_cnt;
>>>> +               bpf_program__line_info_rec_size;
> nit: hm... please check tabs vs spaces, formatting looks off
>
>>>>                   btf__add_decl_attr;
>>>>                   btf__add_type_attr;
>>>>    } LIBBPF_1.5.0;
>>>> --
>>>> 2.48.1
>>>>

Alexei Starovoitov March 28, 2025, 10:24 p.m. UTC | #8

On Fri, Mar 28, 2025 at 1:54 PM Mykyta Yatsenko
<[e-mail]> wrote:
>
> On 28/03/2025 20:52, Andrii Nakryiko wrote:
> > On Fri, Mar 28, 2025 at 12:16 PM Mykyta Yatsenko
> > <[e-mail]> wrote:
> >> On 28/03/2025 17:14, Andrii Nakryiko wrote:
> >>> On Wed, Mar 26, 2025 at 11:07 AM Mykyta Yatsenko
> >>> <[e-mail]> wrote:
> >>>> From: Mykyta Yatsenko <[e-mail]>
> >>>>
> >>>> Introducing new libbpf API getters for BTF.ext func and line info,
> >>>> namely:
> >>>>     bpf_program__func_info
> >>>>     bpf_program__func_info_cnt
> >>>>     bpf_program__func_info_rec_size
> >>>>     bpf_program__line_info
> >>>>     bpf_program__line_info_cnt
> >>>>     bpf_program__line_info_rec_size
> >>>>
> >>>> This change enables scenarios, when user needs to load bpf_program
> >>>> directly using `bpf_prog_load`, instead of higher-level
> >>>> `bpf_object__load`. Line and func info are required for checking BTF
> >>>> info in verifier; verification may fail without these fields if, for
> >>>> example, program calls `bpf_obj_new`.
> >>>>
> >>> Really, bpf_obj_new() needs func_info/line_info? Can you point where
> >>> in the verifier we check this, curious why we do that.
> >> Indirectly, yes:
> >> in verifier.c function check_btf_info_early sets
> >> `env->prog->aux->btf = btf;`
> >> only if line_info_cnt or func_info_cnt are non zero.
> >> and then there is a check that errors out:
> >> `verbose(env, "bpf_obj_new/bpf_percpu_obj_new requires prog BTF\n");`
> >> perhaps this can be improved as well, by setting aux->btf even if no
> >> func info and line info
> > lol, doesn't seem intentional (just in the early days prog BTF was
> > only referenced and used with func_info/line_info, which isn't true
> > anymore), we can probably swap the order and load and remember prog
> > BTF regardless of func_info/line_info. Feel free to send a patch.
> Sure, I'll do it.

No. It's intentional.

The prog must contain func and line info for good debuggability.

Patch

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6b85060f07b3..bc15526ed84c 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -9455,6 +9455,36 @@  int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log
 	return 0;
 }
 
+void *bpf_program__func_info(struct bpf_program *prog)
+{
+	return prog->func_info;
+}
+
+__u32 bpf_program__func_info_cnt(struct bpf_program *prog)
+{
+	return prog->func_info_cnt;
+}
+
+__u32 bpf_program__func_info_rec_size(struct bpf_program *prog)
+{
+	return prog->func_info_rec_size;
+}
+
+void *bpf_program__line_info(struct bpf_program *prog)
+{
+	return prog->line_info;
+}
+
+__u32 bpf_program__line_info_cnt(struct bpf_program *prog)
+{
+	return prog->line_info_cnt;
+}
+
+__u32 bpf_program__line_info_rec_size(struct bpf_program *prog)
+{
+	return prog->line_info_rec_size;
+}
+
 #define SEC_DEF(sec_pfx, ptype, atype, flags, ...) {			    \
 	.sec = (char *)sec_pfx,						    \
 	.prog_type = BPF_PROG_TYPE_##ptype,				    \
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index e0605403f977..29a5fd7f51f0 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -940,6 +940,14 @@  LIBBPF_API int bpf_program__set_log_level(struct bpf_program *prog, __u32 log_le
 LIBBPF_API const char *bpf_program__log_buf(const struct bpf_program *prog, size_t *log_size);
 LIBBPF_API int bpf_program__set_log_buf(struct bpf_program *prog, char *log_buf, size_t log_size);
 
+LIBBPF_API void *bpf_program__func_info(struct bpf_program *prog);
+LIBBPF_API __u32 bpf_program__func_info_cnt(struct bpf_program *prog);
+LIBBPF_API __u32 bpf_program__func_info_rec_size(struct bpf_program *prog);
+
+LIBBPF_API void *bpf_program__line_info(struct bpf_program *prog);
+LIBBPF_API __u32 bpf_program__line_info_cnt(struct bpf_program *prog);
+LIBBPF_API __u32 bpf_program__line_info_rec_size(struct bpf_program *prog);
+
 /**
  * @brief **bpf_program__set_attach_target()** sets BTF-based attach target
  * for supported BPF program types:
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index d8b71f22f197..a5d83189c084 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -437,6 +437,12 @@  LIBBPF_1.6.0 {
 		bpf_linker__add_fd;
 		bpf_linker__new_fd;
 		bpf_object__prepare;
+		bpf_program__func_info;
+		bpf_program__func_info_cnt;
+		bpf_program__func_info_rec_size;
+		bpf_program__line_info;
+		bpf_program__line_info_cnt;
+		bpf_program__line_info_rec_size;
 		btf__add_decl_attr;
 		btf__add_type_attr;
 } LIBBPF_1.5.0;