On Tue, Jun 6, 2023 at 9:50 AM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > Agreed, I don't think we can ever make BTF dedup work reliably with > KINDs it doesn't understand. I wouldn't even try. I'd also say that > kernel should keep being strict about this (even if we add > "is-it-optional" field, kernel can't trust it). Libbpf and other > libraries will have to keep sanitizing BTF anyways. Good point. "it-is-optional" flag should be for user space only. > > If we go this simple route I'm fine with hard coded crc and base_crc > > fields. They probably should go to btf_header though. > > Yep, on btf_header fields. But I'd not hardcode "crc" name. If we are > doing them as strings (which I think we should instead of dooming them > to 32-bit integer crc32 value only), then can we just say generically > that it's either "id" or "checksum"? > > But I guess crc32 would be fine in practice as well. So not something > I strongly feel about. I still fail to see how generic string "id" helps. We have to standardize on a way to checksum BTF-s. Say, we pick crc32. pahole/clang would have to use the same algorithm. Then kernel during BTF_LOAD should check that crc32 matches to make sure btf data didn't get corrupted between its creation and loading into the kernel. Just like btrfs uses crc32 to make sure data doesn't get corrupted by disk. libbpf doing sanitization would need to tweak crc32 too. So it's going to be hard coded semantics at every level. id and especially string id would be cumbersome for all these layers to deal with. > > We don't need "struct btf_metadata" as well. > > It's making things sound beyond what it actually is. > > btf_header can point to an array of struct btf_kind_description. > > As simple as it can get. > > Agreed. Still, it's a third section, and we should at least have a > count of those btf_kind_layout items somewhere. of course. In btf_header we have __u32 type_off; /* offset of type section */ __u32 type_len; /* length of type section */ I meant we add: __u32 kind_layouts_off; __u32 kind_layouts_len;