On 18/05/2024 03:38, Eduard Zingerman wrote: > On Fri, 2024-05-17 at 11:22 +0100, Alan Maguire wrote: > > (Also, please note that CI fails for this series). > > [...] > >> Also explored Eduard's suggestion of doing an implicit fallback >> to checking for .BTF.base section in btf__parse() when it is >> called to get base BTF. However while it is doable, it turned >> out to be difficult operationally. Since fallback is implicit >> we do not know the source of the BTF - was it from .BTF or >> .BTF.base? In bpftool, we want to try first standalone BTF, >> then split, then split with distilled base. Having a way >> to explicitly request .BTF.base via btf__parse_opts() fits >> that model better. > > I don't think this is the case. Here is what I mean: > https://github.com/eddyz87/bpf/tree/distilled-base-alternative-parse-elf > > The branch above is a modification for btf_parse_elf() and a few > reverts on top of this patch-set. > > I modified btf_parse_elf() to follow the logic below: > > | base_btf | .BTF.base | Effect | > | specified? | present? | | > |------------+-----------+---------------------------------------------| > | no | no | load btf from .BTF | > |------------+-----------+---------------------------------------------| > | yes | no | load btf from .BTF using base_btf as base | > | | | | > |------------+-----------+---------------------------------------------| > | no | yes | load btf from .BTF using .BTF.base as base | > | | | | > |------------+-----------+---------------------------------------------| > | yes | yes | load btf from .BTF using .BTF.base as base, | > | | | relocate btf against base_btf | > > When organized like that, there is no need to modify libbpf clients to > work with split BTF. > > The `bpftool btf dump file ./btf_testmod.ko` would print non-relocated BTF. > The `bpftool btf -B ../../../vmlinux dump file ./btf_testmod.ko` would > print relocated BTF, no need for separate -R flag. > Imo, loading split BTF w/o relocation when .BTF.base is present > is interesting only for debug purposes and could be handled separately > as all building blocks are present in the library. > This is a neat approach, and as you say it eliminates the need to modify bpftool to handle distilled base BTF and relocation. The only wrinkle is resolve_btfids; we call resolve_btfids for modules with a "-B vmlinux" argument, so in that case we'd be calling btf_parse_elf() with both a split and base BTF. According to the approach outlined above, we'd relocate split BTF - originally relative to .BTF.base - to be relative to vmlinux BTF, but in the case of resolve_btfids we don't want that relocation. We want the BTF ids to reflect the distilled base BTF ids since they will be relocated later on module load along with the split BTF references themselves. We can handle this by having a -R flag to skip relocation; it would simply ensure we first try calling btf__parse(), falling back to btf__parse_split(); we need the fallback logic as it is possible the pahole version didn't add .BTF.base sections. This logic would only be activated for out-of-tree module builds so seems acceptable to me. If that makes sense, with your permission I can rework the series to include your BTF parsing patch. Thanks! Alan > [...] >
