On Fri, Jun 16, 2023 at 10:17 AM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote: > > BTF kind layouts provide information to parse BTF kinds. > By separating parsing BTF from using all the information > it provides, we allow BTF to encode new features even if > they cannot be used. This is helpful in particular for > cases where newer tools for BTF generation run on an > older kernel; BTF kinds may be present that the kernel > cannot yet use, but at least it can parse the BTF > provided. Meanwhile userspace tools with newer libbpf > may be able to use the newer information. > > The intent is to support encoding of kind layouts > optionally so that tools like pahole can add this > information. So for each kind we record > > - kind-related flags > - length of singular element following struct btf_type > - length of each of the btf_vlen() elements following > > In addition we make space in the BTF header for > CRC32s computed over the BTF along with a CRC for > the base BTF; this allows split BTF to identify > a mismatch explicitly. > > The ideas here were discussed at [1], [2]; hence > > Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx> > > [1] https://lore.kernel.org/bpf/CAEf4BzYjWHRdNNw4B=eOXOs_ONrDwrgX4bn=Nuc1g8JPFC34MA@xxxxxxxxxxxxxx/ > [2] https://lore.kernel.org/bpf/20230531201936.1992188-1-alan.maguire@xxxxxxxxxx/ > --- > include/uapi/linux/btf.h | 24 ++++++++++++++++++++++++ > tools/include/uapi/linux/btf.h | 24 ++++++++++++++++++++++++ > 2 files changed, 48 insertions(+) > > diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h > index ec1798b6d3ff..cea9125ed953 100644 > --- a/include/uapi/linux/btf.h > +++ b/include/uapi/linux/btf.h > @@ -8,6 +8,22 @@ > #define BTF_MAGIC 0xeB9F > #define BTF_VERSION 1 > > +/* is this information required? If so it cannot be sanitized safely. */ > +#define BTF_KIND_LAYOUT_OPTIONAL (1 << 0) hm.. I thought we agreed to not have OPTIONAL flag last time, no? From kernel's perspective nothing is optional. From libbpf perspective everything should be optional, unless we get type_id reference to something that we don't recognize. So why the flag and extra code to handle it? We can always add it later, if necessary. > + > +/* kind layout section consists of a struct btf_kind_layout for each known > + * kind at BTF encoding time. > + */ > +struct btf_kind_layout { > + __u16 flags; /* see BTF_KIND_LAYOUT_* values above */ > + __u8 info_sz; /* size of singular element after btf_type */ > + __u8 elem_sz; /* size of each of btf_vlen(t) elements */ > +}; > + > +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */ > +#define BTF_FLAG_CRC_SET (1 << 0) > +#define BTF_FLAG_BASE_CRC_SET (1 << 1) > + > struct btf_header { > __u16 magic; > __u8 version; > @@ -19,8 +35,16 @@ struct btf_header { > __u32 type_len; /* length of type section */ > __u32 str_off; /* offset of string section */ > __u32 str_len; /* length of string section */ > + __u32 kind_layout_off;/* offset of kind layout section */ > + __u32 kind_layout_len;/* length of kind layout section */ > + > + __u32 crc; /* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */ > + __u32 base_crc; /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */ > }; > > +/* required minimum BTF header length */ > +#define BTF_HEADER_MIN_LEN (sizeof(struct btf_header) - 16) offsetof(struct btf_header, kind_layout_off) ? but actually why this needs to be a part of UAPI? > + > /* Max # of type identifier */ > #define BTF_MAX_TYPE 0x000fffff > /* Max offset into the string section */ > diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h > index ec1798b6d3ff..cea9125ed953 100644 > --- a/tools/include/uapi/linux/btf.h > +++ b/tools/include/uapi/linux/btf.h > @@ -8,6 +8,22 @@ > #define BTF_MAGIC 0xeB9F > #define BTF_VERSION 1 > > +/* is this information required? If so it cannot be sanitized safely. */ > +#define BTF_KIND_LAYOUT_OPTIONAL (1 << 0) > + > +/* kind layout section consists of a struct btf_kind_layout for each known > + * kind at BTF encoding time. > + */ > +struct btf_kind_layout { > + __u16 flags; /* see BTF_KIND_LAYOUT_* values above */ > + __u8 info_sz; /* size of singular element after btf_type */ > + __u8 elem_sz; /* size of each of btf_vlen(t) elements */ > +}; > + > +/* for CRCs for BTF, base BTF to be considered usable, flags must be set. */ > +#define BTF_FLAG_CRC_SET (1 << 0) > +#define BTF_FLAG_BASE_CRC_SET (1 << 1) > + > struct btf_header { > __u16 magic; > __u8 version; > @@ -19,8 +35,16 @@ struct btf_header { > __u32 type_len; /* length of type section */ > __u32 str_off; /* offset of string section */ > __u32 str_len; /* length of string section */ > + __u32 kind_layout_off;/* offset of kind layout section */ > + __u32 kind_layout_len;/* length of kind layout section */ > + > + __u32 crc; /* crc of BTF; used if flags set BTF_FLAG_CRC_VALID */ why are we making crc optional? shouldn't we just say that crc is always filled out? > + __u32 base_crc; /* crc of base BTF; used if flags set BTF_FLAG_BASE_CRC_VALID */ here it would be nice if we could just rely on zero meaning not set, but I suspect not everyone will be happy about this, as technically crc 0 is a valid crc :( > }; > > +/* required minimum BTF header length */ > +#define BTF_HEADER_MIN_LEN (sizeof(struct btf_header) - 16) > + > /* Max # of type identifier */ > #define BTF_MAX_TYPE 0x000fffff > /* Max offset into the string section */ > -- > 2.39.3 >