Re: [PATCH v2 bpf-next 1/9] btf: add kind layout encoding, crcs to UAPI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux