Re: [PATCH v5 bpf-next 8/9] libbpf,bpf: share BTF relocate-related code with kernel

[Alan Maguire <alan.maguire@oracle.com>]

On 01/06/2024 02:59, Andrii Nakryiko wrote:
> On Fri, May 31, 2024 at 12:04 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>>
>> On Tue, May 28, 2024 at 5:25 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>>>
>>> Share relocation implementation with the kernel.  As part of this,
>>> we also need the type/string visitation functions so add them to a
>>> btf_common.c file that also gets shared with the kernel. Relocation
>>> code in kernel and userspace is identical save for the impementation
>>> of the reparenting of split BTF to the relocated base BTF and
>>> retrieval of BTF header from "struct btf"; these small functions
>>> need separate user-space and kernel implementations.
>>>
>>> One other wrinkle on the kernel side is we have to map .BTF.ids in
>>> modules as they were generated with the type ids used at BTF encoding
>>> time. btf_relocate() optionally returns an array mapping from old BTF
>>> ids to relocated ids, so we use that to fix up these references where
>>> needed for kfuncs.
>>>
>>> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>>> ---
>>>  include/linux/btf.h          |  45 ++++++++++
>>>  kernel/bpf/Makefile          |  10 ++-
>>>  kernel/bpf/btf.c             | 168 +++++++++++++++++++++++++----------
>>>  tools/lib/bpf/Build          |   2 +-
>>>  tools/lib/bpf/btf.c          | 130 ---------------------------
>>>  tools/lib/bpf/btf_iter.c     | 143 +++++++++++++++++++++++++++++
>>>  tools/lib/bpf/btf_relocate.c |  23 +++++
>>>  7 files changed, 344 insertions(+), 177 deletions(-)
>>>  create mode 100644 tools/lib/bpf/btf_iter.c
>>>
>>
>> [...]
>>
>>> +static inline struct btf_decl_tag *btf_decl_tag(const struct btf_type *t)
>>> +{
>>> +       return (struct btf_decl_tag *)(t + 1);
>>> +}
>>> +
>>>  static inline int btf_id_cmp_func(const void *a, const void *b)
>>>  {
>>>         const int *pa = a, *pb = b;
>>> @@ -515,9 +528,17 @@ static inline const struct bpf_struct_ops_desc *bpf_struct_ops_find(struct btf *
>>>  }
>>>  #endif
>>>
>>> +typedef int (*type_id_visit_fn)(__u32 *type_id, void *ctx);
>>> +typedef int (*str_off_visit_fn)(__u32 *str_off, void *ctx);
>>> +
>>
>> let me take a quick stab at implementing type/str field iterator in
>> libbpf. If I don't get stuck anywhere, maybe you can just rebase on
>> that and avoid the callback hell and the need for this
>> callback-vs-iter churn in the kernel code as well
>>
> 
> Sent it out as one RFC patch (which unfortunately makes it harder to
> see iterator logic, sorry; but I ran out of time to split it
> properly), see [0].
> It is especially nice when per-field logic is very simple (like
> bpftool's gen.c logic, where we just remap ID). Please take a look and
> let me know what you think.
> 
>   [0] https://patchwork.kernel.org/project/netdevbpf/patch/20240601014505.3443241-1-andrii@kernel.org/
>

thanks for this! Looking now...



>>>  #ifdef CONFIG_BPF_SYSCALL
>>>  const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id);
>>> +void btf_set_base_btf(struct btf *btf, const struct btf *base_btf);
>>> +int btf_relocate(struct btf *btf, const struct btf *base_btf, __u32 **map_ids);
>>> +int btf_type_visit_type_ids(struct btf_type *t, type_id_visit_fn visit, void *ctx);
>>> +int btf_type_visit_str_offs(struct btf_type *t, str_off_visit_fn visit, void *ctx);
>>>  const char *btf_name_by_offset(const struct btf *btf, u32 offset);
>>> +const char *btf_str_by_offset(const struct btf *btf, u32 offset);
>>>  struct btf *btf_parse_vmlinux(void);
>>>  struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog);
>>>  u32 *btf_kfunc_id_set_contains(const struct btf *btf, u32 kfunc_btf_id,
>>
>> [...]