On Tue, May 21, 2024 at 9:19 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Tue, 2024-05-21 at 10:15 +0100, Alan Maguire wrote: > > [...] > > > 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. > > You are correct, I missed this detail, resolve_btfids needs distilled > base instead of vmlinux for out of tree modules. > > > 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. > > Makes sense to me, but I'm curious whether you and Andrii consider > this a good interface, compared to _opts version. > Hey, sorry for delays, just getting back to reviewing patches, I will go through this revision today. I'm probably leaning towards not doing automatic relocations in btf__parse(), tbh. Distilled BTF is a rather special kernel-specific feature, if we need to teach resolve_btfids and bpftool to do something extra for that case (i.e., call another API for relocation, if necessary), then it's fine, doesn't seems like a problem. Much worse is having to do some workarounds to prevent an API from doing some undesired extra steps (like in resolve_btfids not wanting a relocation). Orthogonality FTW, IMO. > Thanks, > Eduard