Re: [PATCH bpf-next v1 05/14] resolve_btfids: Index BTF ID symbols by address
Ihor Solodrai <[email protected]>
| Newsgroups | org.kernel.vger.linux-kbuild,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On 6/3/26 4:45 PM, Andrii Nakryiko wrote: > On Mon, Jun 1, 2026 at 3:19 PM Ihor Solodrai <[email protected]> wrote: >> >> Keep an address-sorted index of parsed .BTF_ids symbols so code that >> the original BTF_ID symbol name can be recovered from an entry >> address. >> >> Use the index in find_kfunc_flags() to scan BTF_SET8_KFUNCS entries >> directly and match each entry back to the requested kfunc. >> >> Signed-off-by: Ihor Solodrai <[email protected]> >> --- >> tools/bpf/resolve_btfids/main.c | 103 +++++++++++++++++++++++++------- >> 1 file changed, 80 insertions(+), 23 deletions(-) >> >> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c >> index f8a91fa7584f..43512af13148 100644 >> --- a/tools/bpf/resolve_btfids/main.c >> +++ b/tools/bpf/resolve_btfids/main.c >> @@ -119,6 +119,11 @@ struct btf_id { >> Elf64_Addr addr[ADDR_CNT]; >> }; >> >> +struct addr_sym { >> + Elf64_Addr addr; >> + const char *name; >> +}; >> + >> struct object { >> const char *path; >> const char *btf_path; >> @@ -150,6 +155,10 @@ struct object { >> int nr_structs; >> int nr_unions; >> int nr_typedefs; >> + >> + struct addr_sym *addr_syms; >> + int nr_addr_syms; >> + int max_addr_syms; > > nit: max seems misnamed, it's "capacity", so I'd choose > "addr_syms_cnt" and "addr_syms_cap" naming (I believe libbpf does that > relatively consistently) > >> }; >> >> #define KF_IMPLICIT_ARGS (1 << 16) > > [...] > >> for (next = rb_first(&obj->sets); next; next = rb_next(next)) { >> set_id = rb_entry(next, struct btf_id, rb_node); >> if (set_id->kind != BTF_ID_KIND_SET8 || set_id->addr_cnt != 1) >> continue; >> >> - set_lower_addr = set_id->addr[0]; >> - set_upper_addr = set_lower_addr + set_id->cnt * sizeof(u64); >> + set_addr = set_id->addr[0]; >> + idx = (set_addr - obj->efile.idlist_addr) / sizeof(u32) + 1; > > where is this +1 coming from? we have some reserved zero entry in > .BTF_ids section? I'd understand if this was symbols table, where we > do have zero entry, but I'm not quite following here... We do a +1 in find_kfunc_flags() three times for slightly different reasons: // Here we extract the *set* flags from the header idx = (set_addr - obj->efile.idlist_addr) / sizeof(u32) + 1; set_flags = elf_data_ptr[idx]; [...] // here we skip the btf_id_set header Elf64_Addr addr = set_addr + sizeof(u64) * (i + 1); [...] // and here we extract the flags from a pair idx = (addr - obj->efile.idlist_addr) / sizeof(u32) + 1; return elf_data_ptr[idx]; I think the way to make it less confusing is to cast the data pointer to struct btf_id_set8 before inspecting it, and read the fields. I'll do that in v2. > > > >> + set_flags = elf_data_ptr[idx]; >> + if (!(set_flags & BTF_SET8_KFUNCS)) >> + continue; >> >> - for (u32 i = 0; i < kfunc_id->addr_cnt; i++) { >> - addr = kfunc_id->addr[i]; >> - /* >> - * Lower bound is exclusive to skip the 8-byte header of the set. >> - * Upper bound is inclusive to capture the last entry at offset 8*cnt. >> - */ >> - if (set_lower_addr < addr && addr <= set_upper_addr) { >> - pr_debug("found kfunc %s in BTF_ID_FLAGS %s\n", >> - kfunc_id->name, set_id->name); >> - idx = addr - obj->efile.idlist_addr; >> - idx = idx / sizeof(u32) + 1; >> - flags = elf_data_ptr[idx]; >> - >> - return flags; >> - } >> + for (u32 i = 0; i < set_id->cnt; i++) { >> + Elf64_Addr addr = set_addr + sizeof(u64) * (i + 1); >> + const char *name = find_name_by_addr(obj, addr); >> + >> + if (!name || strcmp(name, kfunc_id->name) != 0) >> + continue; >> + >> + pr_debug("found kfunc %s in BTF_ID_FLAGS %s\n", >> + kfunc_id->name, set_id->name); >> + >> + idx = (addr - obj->efile.idlist_addr) / sizeof(u32) + 1; >> + return elf_data_ptr[idx]; >> } >> } >> >> @@ -1575,6 +1631,7 @@ int main(int argc, const char **argv) >> btf_id__free_all(&obj.typedefs); >> btf_id__free_all(&obj.funcs); >> btf_id__free_all(&obj.sets); >> + free(obj.addr_syms); >> if (obj.efile.elf) { >> elf_end(obj.efile.elf); >> close(obj.efile.fd); >> -- >> 2.54.0 >>