Re: [PATCH bpf-next v2] libbpf: add libbpf_load_vmlinux_btf/libbpf_load_module_btf APIs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2021/7/30 5:35 AM, Andrii Nakryiko wrote:
> On Wed, Jul 28, 2021 at 9:55 AM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote:
>>
>> Add two new APIs: libbpf_load_vmlinux_btf/libbpf_load_module_btf.
>> libbpf_load_vmlinux_btf is just an alias to the existing API named
>> libbpf_find_kernel_btf, rename it to be more precisely.
>> libbpf_load_module_btf can be used to load module BTF, add it for
>> completeness. These two APIs are useful for implementing tracing
>> tools and introspection tools.
>> This is part of the efforts towards libbpf 1.0. [1]
>>
>> [1] https://github.com/libbpf/libbpf/issues/280
>>
>> Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx>
>> ---
>>  tools/lib/bpf/btf.c      | 15 ++++++++++++++-
>>  tools/lib/bpf/btf.h      |  2 ++
>>  tools/lib/bpf/libbpf.c   |  4 ++--
>>  tools/lib/bpf/libbpf.map |  2 ++
>>  4 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
>> index b46760b93bb4..5f801739a1a2 100644
>> --- a/tools/lib/bpf/btf.c
>> +++ b/tools/lib/bpf/btf.c
>> @@ -4021,7 +4021,7 @@ static void btf_dedup_merge_hypot_map(struct btf_dedup *d)
>>                  */
>>                 if (d->hypot_adjust_canon)
>>                         continue;
>> -
>> +
>>                 if (t_kind == BTF_KIND_FWD && c_kind != BTF_KIND_FWD)
>>                         d->map[t_id] = c_id;
>>
>> @@ -4395,6 +4395,11 @@ static int btf_dedup_remap_types(struct btf_dedup *d)
>>   * data out of it to use for target BTF.
>>   */
>>  struct btf *libbpf_find_kernel_btf(void)
>> +{
>> +       return libbpf_load_vmlinux_btf();
>> +}
>> +
>> +struct btf *libbpf_load_vmlinux_btf(void)
>>  {
>>         struct {
>>                 const char *path_fmt;
>> @@ -4440,6 +4445,14 @@ struct btf *libbpf_find_kernel_btf(void)
>>         return libbpf_err_ptr(-ESRCH);
>>  }
>>
>> +struct btf *libbpf_load_module_btf(const char *module_name, struct btf *vmlinux_btf)
> 
> In the light of Quentin's btf__load_from_kernel_by_id(), I'm now
> wondering if it's better to keep the naming consistent as
> btf__load_vmlinux_btf() and btf__load_module_btf()? And we should put
> them after btf__parse() family of constructors, as just another set of
> (special, but still) constructors. WDYT?
> 
> Sorry for a bit of back and forth...
> 
> Otherwise everything looks good, thanks.
> 

Thanks for the review. Will update as you suggested.
Sometimes naming is the hardest part of programming, I am not good at that. :)

(BTW, Quentin's tweet led me to here)

>> +{
>> +       char path[80];
>> +
>> +       snprintf(path, sizeof(path), "/sys/kernel/btf/%s", module_name);
>> +       return btf__parse_split(path, vmlinux_btf);
>> +}
>> +
>>  int btf_type_visit_type_ids(struct btf_type *t, type_id_visit_fn visit, void *ctx)
>>  {
>>         int i, n, err;
>> diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
>> index 374e9f15de2e..1abf94e3bd9e 100644
>> --- a/tools/lib/bpf/btf.h
>> +++ b/tools/lib/bpf/btf.h
>> @@ -90,6 +90,8 @@ LIBBPF_API __u32 btf_ext__func_info_rec_size(const struct btf_ext *btf_ext);
>>  LIBBPF_API __u32 btf_ext__line_info_rec_size(const struct btf_ext *btf_ext);
>>
>>  LIBBPF_API struct btf *libbpf_find_kernel_btf(void);
>> +LIBBPF_API struct btf *libbpf_load_vmlinux_btf(void);
>> +LIBBPF_API struct btf *libbpf_load_module_btf(const char *module_name, struct btf *vmlinux_btf);
> 
> as mentioned above, let's move these right after btf__parse() family,
> so that all BTF constructor APIs are listed first.
> 

Sure, will do.

>>
>>  LIBBPF_API int btf__find_str(struct btf *btf, const char *s);
>>  LIBBPF_API int btf__add_str(struct btf *btf, const char *s);
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index a1ca6fb0c6d8..321d8f4889af 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -2680,7 +2680,7 @@ static int bpf_object__load_vmlinux_btf(struct bpf_object *obj, bool force)
>>         if (!force && !obj_needs_vmlinux_btf(obj))
>>                 return 0;
>>
>> -       obj->btf_vmlinux = libbpf_find_kernel_btf();
>> +       obj->btf_vmlinux = libbpf_load_vmlinux_btf();
>>         err = libbpf_get_error(obj->btf_vmlinux);
>>         if (err) {
>>                 pr_warn("Error loading vmlinux BTF: %d\n", err);
>> @@ -8297,7 +8297,7 @@ int libbpf_find_vmlinux_btf_id(const char *name,
>>         struct btf *btf;
>>         int err;
>>
>> -       btf = libbpf_find_kernel_btf();
>> +       btf = libbpf_load_vmlinux_btf();
>>         err = libbpf_get_error(btf);
>>         if (err) {
>>                 pr_warn("vmlinux BTF is not found\n");
>> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
>> index c240d488eb5e..2088bdbc0f50 100644
>> --- a/tools/lib/bpf/libbpf.map
>> +++ b/tools/lib/bpf/libbpf.map
>> @@ -377,4 +377,6 @@ LIBBPF_0.5.0 {
>>                 bpf_object__gen_loader;
>>                 btf_dump__dump_type_data;
>>                 libbpf_set_strict_mode;
>> +               libbpf_load_vmlinux_btf;
>> +               libbpf_load_module_btf;
>>  } LIBBPF_0.4.0;
>> --
>> 2.25.1
>>



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux