On Wed, Jul 26, 2023 at 8:29 AM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote: > > On 26/07/2023 11:39, Jiri Olsa wrote: > > On Thu, Jul 20, 2023 at 09:14:42PM +0100, Alan Maguire wrote: > >> When a newer pahole is run on an older kernel, it often knows about BTF > >> kinds that the kernel does not support. This is a problem because the BTF > >> generated is then embedded in the kernel image and read, and if unknown > >> kinds are found, BTF handling fails and core BPF functionality is > >> unavailable. > >> > >> The scripts/pahole-flags.sh script enumerates the various pahole options > >> available associated with various versions of pahole, but the problem is > >> what matters in the case of an older kernel is the set of kinds the kernel > >> understands. Because recent features such as BTF_KIND_ENUM64 are added > >> by default (and only skipped if --skip_encoding_btf_* is set), BTF will > >> be created with these newer kinds that the older kernel cannot read. > >> This can be fixed by stable-backporting --skip options, but this is > >> cumbersome and would have to be done every time a new BTF kind is > >> introduced. > >> > >> Here instead we pre-process the DWARF information associated with the > >> target for BTF generation; if we find an enum with a BTF_KIND_MAX > >> value in the DWARF associated with the object, we use that to > >> determine the maximum BTF kind supported. Note that the enum > >> representation of BTF kinds starts for the 5.16 kernel; prior to this > >> The benefit of auto-detection is that no work is required for older > >> kernels when new kinds are added, and --skip_encoding options are > >> less needed. > >> > >> [1] https://github.com/oracle-samples/bpftune/issues/35 > >> > >> Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx> > >> --- > >> btf_encoder.c | 12 ++++++++++++ > >> dwarf_loader.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ > >> dwarves.h | 2 ++ > >> 3 files changed, 66 insertions(+) > >> > >> diff --git a/btf_encoder.c b/btf_encoder.c > >> index 65f6e71..98c7529 100644 > >> --- a/btf_encoder.c > >> +++ b/btf_encoder.c > >> @@ -1889,3 +1889,15 @@ struct btf *btf_encoder__btf(struct btf_encoder *encoder) > >> { > >> return encoder->btf; > >> } > >> + > >> +void dwarves__set_btf_kind_max(struct conf_load *conf_load, int btf_kind_max) > >> +{ > >> + if (btf_kind_max < 0 || btf_kind_max >= BTF_KIND_MAX) > >> + return; > >> + if (btf_kind_max < BTF_KIND_DECL_TAG) > >> + conf_load->skip_encoding_btf_decl_tag = true; > >> + if (btf_kind_max < BTF_KIND_TYPE_TAG) > >> + conf_load->skip_encoding_btf_type_tag = true; > >> + if (btf_kind_max < BTF_KIND_ENUM64) > >> + conf_load->skip_encoding_btf_enum64 = true; > >> +} > > > > hi, > > so there are some older kernels other than stable that would use this feature > > right? because stable already have proper setup for pahole options > > > > or it's just there to be complete and we'd eventually add new rules in here? > > wouldn't that be covered by the BTF kind layout stuff you work on? is there > > some overlap? > > > > Yeah, the idea is to minimize the complexity when adding new kinds. The > approach explored here does this because when adding a new kind we have > to either > > - make it a pahole opt-in parameter, which means more --btf_encode_* > parameters for dwarves; or > - make it an opt-out parameter, which means more stable backports to set > the opt-out. > > My original hope was BTF kind layout encoding would solve the problem, > but the problem is that new kinds are often entwined tightly in the > representation of structures, functions etc. When that happens, even > knowing the kind layout won't help an older kernel interpret what the > BTF actually means when it was generated by a newer pahole. Kind layout > still has value - it means BTF can always be dumped for example - but it > can't really help the kernel to reliabily _use_ kernel/module BTF > information. So the approach here is to try to streamline the process by > not requiring new options when a new kind is added; we can simply detect > if the kernel knows about the kind and skip it if not. Since this > detection is all internal to pahole, nothing needs to be exposed to the > user. > > > >> diff --git a/dwarf_loader.c b/dwarf_loader.c > >> index ccf3194..8984043 100644 > >> --- a/dwarf_loader.c > >> +++ b/dwarf_loader.c > >> @@ -3358,8 +3358,60 @@ static int __dwarf_cus__process_cus(struct dwarf_cus *dcus) > >> return 0; > >> } > >> > >> +/* Find enumeration value for BTF_KIND_MAX; replace conf_load->btf_kind_max with > >> + * this value if found since it indicates that the target object does not know > >> + * about kinds > its BTF_KIND_MAX value. This is valuable for kernel/module > >> + * BTF where a newer pahole/libbpf operate on an older kernel which cannot > >> + * parse some of the newer kinds pahole can generate. > >> + */ > >> +static void dwarf__find_btf_kind_max(struct dwarf_cus *dcus) > >> +{ > >> + struct conf_load *conf = dcus->conf; > >> + uint8_t pointer_size, offset_size; > >> + Dwarf_Off off = 0, noff; > >> + size_t cuhl; > >> + > >> + while (dwarf_nextcu(dcus->dw, off, &noff, &cuhl, NULL, &pointer_size, &offset_size) == 0) { > >> + Dwarf_Die die_mem; > >> + Dwarf_Die *cu_die = dwarf_offdie(dcus->dw, off + cuhl, &die_mem); > >> + Dwarf_Die child; > >> + > >> + if (cu_die == NULL) > >> + break; > >> + if (dwarf_child(cu_die, &child) == 0) { > >> + Dwarf_Die *die = &child; > >> + > >> + do { > >> + Dwarf_Die echild, *edie; > >> + > >> + if (dwarf_tag(die) != DW_TAG_enumeration_type || > >> + !dwarf_haschildren(die) || > >> + dwarf_child(die, &echild) != 0) > >> + continue; > >> + edie = &echild; > >> + do { > >> + const char *ename; > >> + int btf_kind_max; > >> + > >> + if (dwarf_tag(edie) != DW_TAG_enumerator) > >> + continue; > >> + ename = attr_string(edie, DW_AT_name, conf); > >> + if (!ename || strcmp(ename, "BTF_KIND_MAX") != 0) > >> + continue; > >> + btf_kind_max = attr_numeric(edie, DW_AT_const_value); > >> + dwarves__set_btf_kind_max(conf, btf_kind_max); > >> + return; > >> + } while (dwarf_siblingof(edie, edie) == 0); > >> + } while (dwarf_siblingof(die, die) == 0); > >> + } > >> + off = noff; > >> + } > >> +} > >> + > >> static int dwarf_cus__process_cus(struct dwarf_cus *dcus) > >> { > >> + dwarf__find_btf_kind_max(dcus); > > > > first I though this should be enabled by some (detect) option.. but that > > would probably beat the main purpose.. also I think we don't need kernel > > with BTF that it can't process > > > > Hmm, maybe it might be good to have it associated with --btf_gen_all in > case we ever want to disable it for debugging purposes? What do you > think? Thanks! Overall approach makes sense to me and an extra flag like --btf_gen_all to disable this smartness also makes sense.
