Re: [PATCH bpf-next v6 4/8] libbpf: Support BTF.ext loading and output in either endianness

On Mon, Sep 16, 2024 at 1:38 AM Tony Ambardar tony.ambardar@gmail.com wrote:

Support for handling BTF data of either endianness was added in [1], but did not include BTF.ext data for lack of use cases. Later, support for static linking [2] provided a use case, but this feature and later ones were restricted to native-endian usage.

Add support for BTF.ext handling in either endianness. Convert BTF.ext data to native endianness when read into memory for further processing, and support raw data access that restores the original byte-order for output. Add internal header functions for byte-swapping func, line, and core info records.

Add new API functions btf_ext__endianness() and btf_ext__set_endianness() for query and setting byte-order, as already exist for BTF data.

[1] 3289959b97ca ("libbpf: Support BTF loading and raw data output in both endianness") [2] 8fd27bf69b86 ("libbpf: Add BPF static linker BTF and BTF.ext support")

Signed-off-by: Tony Ambardar tony.ambardar@gmail.com

tools/lib/bpf/btf.c | 280 +++++++++++++++++++++++++------- tools/lib/bpf/btf.h | 3 + tools/lib/bpf/libbpf.map | 2 + tools/lib/bpf/libbpf_internal.h | 30 ++++ 4 files changed, 258 insertions(+), 57 deletions(-)

[...]

•   /* The record size needs to meet the minimum standard */
  

•   record_size = *(__u32 *)info;
  

•   /* The record size needs to meet either the minimum standard or, when
  

•    * handling non-native endianness data, the exact standard so as
  

•    * to allow safe byte-swapping.
  

•    */
  

•   record_size = is_native ? *(__u32 *)info : bswap_32(*(__u32 *)info);
    if (record_size < ext_sec->min_rec_size ||
  

•       (!is_native && record_size != ext_sec->rec_size) ||
  

ok, so the way you define min_rec_size and rec_size is actually broken. bpf_func_info, bpf_line_info, and now also bpf_core_relo all come from kernel UAPI header, and it could happen that libbpf is compiled against newest definitions of them without actually "knowing"/supporting those newer and bigger struct definitions. So assuming that sizeof(struct bpf_line_info) is a correct expected record size is wrong.

As it is right now, min_rec_size is *the* rec_size, so I removed that part and updated this check to record_size != ext_sec->min_rec_size. If we ever extend .BTF.ext records, we'll need to add a bit more code to accomodate for that.

        record_size & 0x03) {
            pr_debug("%s section in .BTF.ext has invalid record size %u\n",
                     ext_sec->desc, record_size);

@@ -2956,7 +2968,7 @@ static int btf_ext_setup_info(struct btf_ext *btf_ext, return -EINVAL; }

•           num_records = sinfo->num_info;
  

•           num_records = is_native ? sinfo->num_info : bswap_32(sinfo->num_info);
            if (num_records == 0) {
                    pr_debug("%s section has incorrect num_records in .BTF.ext\n",
                         ext_sec->desc);
  

@@ -2984,64 +2996,160 @@ static int btf_ext_setup_info(struct btf_ext *btf_ext, return 0; }

-static int btf_ext_setup_func_info(struct btf_ext *btf_ext) +/* Parse all info secs in the BTF.ext info data */ +static int btf_ext_parse_info(struct btf_ext *btf_ext, bool is_native) {

•   struct btf_ext_sec_setup_param param = {
  

•   struct btf_ext_sec_info_param func_info = {
            .off = btf_ext->hdr->func_info_off,
            .len = btf_ext->hdr->func_info_len,
            .min_rec_size = sizeof(struct bpf_func_info_min),
  

•           .rec_size = sizeof(struct bpf_func_info),
            .ext_info = &btf_ext->func_info,
            .desc = "func_info"
    };
  

•   return btf_ext_setup_info(btf_ext, &param);
  

-}

-static int btf_ext_setup_line_info(struct btf_ext *btf_ext) -{

•   struct btf_ext_sec_setup_param param = {
  

•   struct btf_ext_sec_info_param line_info = {
            .off = btf_ext->hdr->line_info_off,
            .len = btf_ext->hdr->line_info_len,
            .min_rec_size = sizeof(struct bpf_line_info_min),
  

•           .rec_size = sizeof(struct bpf_line_info),
            .ext_info = &btf_ext->line_info,
            .desc = "line_info",
    };
  

•   return btf_ext_setup_info(btf_ext, &param);
  

-}

-static int btf_ext_setup_core_relos(struct btf_ext *btf_ext) -{

•   struct btf_ext_sec_setup_param param = {
  

•   struct btf_ext_sec_info_param core_relo = {
            .off = btf_ext->hdr->core_relo_off,
            .len = btf_ext->hdr->core_relo_len,
            .min_rec_size = sizeof(struct bpf_core_relo),
  

•           .rec_size = sizeof(struct bpf_core_relo),
            .ext_info = &btf_ext->core_relo_info,
            .desc = "core_relo",
    };
  

[...]

diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h index 4e349ad79ee6..47ee8f6ac489 100644 --- a/tools/lib/bpf/btf.h +++ b/tools/lib/bpf/btf.h @@ -167,6 +167,9 @@ LIBBPF_API const char *btf__str_by_offset(const struct btf *btf, __u32 offset); LIBBPF_API struct btf_ext *btf_ext__new(const __u8 *data, __u32 size); LIBBPF_API void btf_ext__free(struct btf_ext *btf_ext); LIBBPF_API const void *btf_ext__raw_data(const struct btf_ext *btf_ext, __u32 *size); +LIBBPF_API enum btf_endianness btf_ext__endianness(const struct btf_ext *btf_ext); +LIBBPF_API int btf_ext__set_endianness(struct btf_ext *btf_ext,

•                                  enum btf_endianness endian);
  
  

LIBBPF_API int btf__find_str(struct btf *btf, const char *s); LIBBPF_API int btf__add_str(struct btf *btf, const char *s); diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map index 0096e483f7eb..f40ccc2946e7 100644 --- a/tools/lib/bpf/libbpf.map +++ b/tools/lib/bpf/libbpf.map @@ -421,6 +421,8 @@ LIBBPF_1.5.0 { global: btf__distill_base; btf__relocate;

•           btf_ext__endianness;
  

•           btf_ext__set_endianness;
            bpf_map__autoattach;
            bpf_map__set_autoattach;
            bpf_object__token_fd;
  

diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h index 8cda511a1982..1307753b49b3 100644 --- a/tools/lib/bpf/libbpf_internal.h +++ b/tools/lib/bpf/libbpf_internal.h

missing #include <byteswap.h> in libbpf_internal.h, you added too late (in the next patch). I moved that include into this patch/commit to not break bisectability, please be careful about things like that

@@ -484,6 +484,8 @@ struct btf_ext { struct btf_ext_header *hdr; void *data; };

•   void *data_swapped;
  

•   bool swapped_endian;
    struct btf_ext_info func_info;
    struct btf_ext_info line_info;
    struct btf_ext_info core_relo_info;
  

@@ -511,6 +513,34 @@ struct bpf_line_info_min { __u32 line_col; };

+/* Functions to byte-swap info records */

+typedef void (*info_rec_bswap_fn)(void *);

+static inline void bpf_func_info_bswap(struct bpf_func_info *i) +{

•   i->insn_off = bswap_32(i->insn_off);
  

•   i->type_id = bswap_32(i->type_id);
  

+}

+static inline void bpf_line_info_bswap(struct bpf_line_info *i) +{

•   i->insn_off = bswap_32(i->insn_off);
  

•   i->file_name_off = bswap_32(i->file_name_off);
  

•   i->line_off = bswap_32(i->line_off);
  

•   i->line_col = bswap_32(i->line_col);
  

+}

+static inline void bpf_core_relo_bswap(struct bpf_core_relo *i) +{

•   _Static_assert(sizeof(i->kind) == sizeof(__u32),
  

•                  "enum bpf_core_relo_kind is not 32-bit\n");
  

Do we need the endline in _Static_assert() messages? And generally I think this is a bit too paranoid to check that that enum is backed by u32, I just dropped the assertion. This is part of kernel UAPI, it shouldn't change.

•   i->insn_off = bswap_32(i->insn_off);
  

•   i->type_id = bswap_32(i->type_id);
  

•   i->access_str_off = bswap_32(i->access_str_off);
  

•   i->kind = bswap_32(i->kind);
  

+}

enum btf_field_iter_kind { BTF_FIELD_ITER_IDS, BTF_FIELD_ITER_STRS, -- 2.34.1