On Wed, Jun 7, 2023 at 9:14 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Wed, 2023-06-07 at 08:29 -0700, Yonghong Song wrote: > > > > On 6/7/23 4:55 AM, Eduard Zingerman wrote: > > > On Tue, 2023-06-06 at 13:30 +0200, Toke Høiland-Jørgensen wrote: > > > [...] > > > > > > > > As for bumping the version number, I don't think it's a good idea to > > > > deliberately break compatibility this way unless it's absolutely > > > > necessary. With "absolutely necessary" meaning "things will break in > > > > subtle ways in any case, so it's better to make the breakage obvious". > > > > But it libbpf is not checking the version field anyway, that becomes > > > > kind of a moot point, as bumping it doesn't really gain us anything, > > > > then... > > > > > > It seems to me that in terms of backward compatibility, the ability to > > > specify the size for each kind entry is more valuable than the > > > capability to add new BTF kinds: > > > - The former allows for extending kind records in > > > a backward-compatible manner, such as adding a function address to > > > BTF_KIND_FUNC. > > > > Eduard, the new proposal is to add new kind, e.g., BTF_KIND_KFUNC, which > > will have an 'address' field. BTF_KIND_KFUNC is for kernel functions. > > So we will not have size compatibility issue for BTF_KIND_FUNC. > > Well, actually this might be a way to avoid BTF_KIND_KFUNC :) > What I wanted to say is that any use of this feature leads to > incompatibility with current BTF parsers, as either size of existing > kinds would be changed or a new kind with unknown size would be added. > It seems to me that this warrants version bump (or some other way to > signal existing parsers that format is incompatible). It is probably too late to have existing KINDs changing their size. If this layout metadata was mandatory from the very beginning, then we could have relied on it for determining new extra fields for BTF_KIND_FUNC. As things stand right now, new BTF_KIND_KFUNC is both a signal of newer format (for kernel-side BTF; nothing changes for BPF object file BTFs, which is great side-effect making backwards compat pain smaller), and is a simpler and safer way to add extra information. > > > > > > - The latter is much more fragile. Types refer to each other, > > > compatibility is already lost once a new "unknown" tag is introduced > > > in a type chain. > > > > > > However, changing the size of existing BTF kinds is itself a > > > backward-incompatible change. Therefore, a version bump may be > > > warranted in this regard. > See above and previous emails. Not having to bump version means we can start emitting this layout info from Clang and pahole with no extra opt-in flags, and not worry about breaking existing tools and apps. This is great, so let's not ruin that property :)
