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

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

 



On Fri, Aug 30, 2024 at 12:30 AM Tony Ambardar <tony.ambardar@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>  tools/lib/bpf/btf.c             | 192 +++++++++++++++++++++++++++++---
>  tools/lib/bpf/btf.h             |   3 +
>  tools/lib/bpf/libbpf.map        |   2 +
>  tools/lib/bpf/libbpf_internal.h |  33 ++++++
>  4 files changed, 214 insertions(+), 16 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index f5081de86ee0..064cfe126c09 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -3022,25 +3022,102 @@ static int btf_ext_setup_core_relos(struct btf_ext *btf_ext)
>         return btf_ext_setup_info(btf_ext, &param);
>  }
>
> -static int btf_ext_parse_hdr(__u8 *data, __u32 data_size)
> +/* Swap byte-order of BTF.ext header with any endianness */
> +static void btf_ext_bswap_hdr(struct btf_ext *btf_ext, __u32 hdr_len)
>  {
> -       const struct btf_ext_header *hdr = (struct btf_ext_header *)data;
> +       struct btf_ext_header *h = btf_ext->hdr;
>
> -       if (data_size < offsetofend(struct btf_ext_header, hdr_len) ||
> -           data_size < hdr->hdr_len) {
> -               pr_debug("BTF.ext header not found\n");
> +       h->magic = bswap_16(h->magic);
> +       h->hdr_len = bswap_32(h->hdr_len);
> +       h->func_info_off = bswap_32(h->func_info_off);
> +       h->func_info_len = bswap_32(h->func_info_len);
> +       h->line_info_off = bswap_32(h->line_info_off);
> +       h->line_info_len = bswap_32(h->line_info_len);
> +
> +       if (hdr_len < offsetofend(struct btf_ext_header, core_relo_len))
> +               return;
> +
> +       h->core_relo_off = bswap_32(h->core_relo_off);
> +       h->core_relo_len = bswap_32(h->core_relo_len);
> +}
> +
> +/* Swap byte-order of a generic info subsection */
> +static void info_subsec_bswap(const struct btf_ext_header *hdr, bool native,
> +                             __u32 off, __u32 len, anon_info_bswap_fn_t bswap)

ok, so I'm not a fan of this bswap callback, tbh. Also, we don't
really enforce that each kind of record has exact size we expect
(i.e., bpf_line_info_min and bpf_func_info_min shouldn't be "min" for
byte-swapped case, it should be exact).

How about this slight modification: split byte swapping of
sections/subsection metadata, so we adjust record size, sec_name_off
and num_info separately from adjusting each record.

Once this swapping is done we:

a) validate record size for each section is expected (according to its
type, of course)
b) we can then use for_each_btf_ext_sec() and for_each_btf_ext_rec()
macro (which assume proper in-memory metadata byte order) and then
hard-code swapping of each record fields

No callbacks.

This has also a benefit of not needing this annoying "bool native"
flag when producing raw bytes. We just ensure proper order of
operation:

a) swap records
b) swap metadata (so just mirrored order from initialization)

WDYT?

pw-bot: cr

> +{
> +       __u32 left, i, *rs, rec_size, num_info;
> +       struct btf_ext_info_sec *si;
> +       void *p;
> +
> +       if (len == 0)
> +               return;
> +
> +       rs = (void *)hdr + hdr->hdr_len + off;  /* record size */
> +       si = (void *)rs + sizeof(__u32);        /* sec info #1 */
> +       rec_size = native ? *rs : bswap_32(*rs);
> +       *rs = bswap_32(*rs);
> +       left = len - sizeof(__u32);
> +       while (left > 0) {
> +               num_info = native ? si->num_info : bswap_32(si->num_info);
> +               si->sec_name_off = bswap_32(si->sec_name_off);
> +               si->num_info = bswap_32(si->num_info);
> +               left -= offsetof(struct btf_ext_info_sec, data);
> +               p = si->data;
> +               for (i = 0; i < num_info; i++)  /* list of records */
> +                       p += bswap(p);
> +               si = p;
> +               left -=  rec_size * num_info;

nit: extra space here

> +       }
> +}
> +

[...]





[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux