mbox series

[bpf-next,v2,0/8] libbpf, selftests/bpf: Support cross-endian usage

Message ID cover.1724313164.git.tony.ambardar@gmail.com
Headers show
Series libbpf, selftests/bpf: Support cross-endian usage | expand

Message

Tony Ambardar Aug. 22, 2024, 9:24 a.m. UTC
From: Tony Ambardar <tony.ambardar@gmail.com>

Hello all,

This patch series targets a long-standing BPF usability issue - the lack
of general cross-compilation support - by enabling cross-endian usage of
libbpf and bpftool, as well as supporting cross-endian build targets for
selftests/bpf.

Benefits include improved BPF development and testing for embedded systems
based on e.g. big-endian MIPS, more build options e.g for s390x systems,
and better accessibility to the very latest test tools e.g. 'test_progs'.

Initial development and testing used mips64, since this arch makes
switching the build byte-order trivial and is thus very handy for A/B
testing. However, it lacks some key features (bpf2bpf call, kfuncs, etc)
making for poor selftests/bpf coverage.

Final testing takes the kernel and selftests/bpf cross-built from x86_64
to s390x, and runs the result under QEMU/s390x. That same configuration
could also be used on kernel-patches/bpf CI for regression testing endian
support or perhaps load-sharing s390x builds across x86_64 systems.

This thread includes some background regarding testing on QEMU/s390x and
the generally favourable results:
    https://lore.kernel.org/bpf/ZsEcsaa3juxxQBUf@kodidev-ubuntu/

Feedback and suggestions are welcome!

Best regards,
Tony


Changelog:
---------
v1 -> v2:
 - fixed a light skeleton bug causing test_progs 'map_ptr' failure
 - simplified some BTF.ext related endianness logic
 - remove an 'inline' usage related to CI checkpatch failure
 - improve some formatting noted by checkpatch warnings
 - unexpected 'test_progs' failures drop 3 -> 2 (x86_64 to s390x cross)



Tony Ambardar (8):
  libbpf: Improve log message formatting
  libbpf: Fix header comment typos for BTF.ext
  libbpf: Fix output .symtab byte-order during linking
  libbpf: Support BTF.ext loading and output in either endianness
  libbpf: Support opening bpf objects of either endianness
  libbpf: Support linking bpf objects of either endianness
  libbpf: Support creating light skeleton of either endianness
  selftests/bpf: Support cross-endian building

 tools/lib/bpf/bpf_gen_internal.h     |   1 +
 tools/lib/bpf/btf.c                  | 168 ++++++++++++++++++++++--
 tools/lib/bpf/btf.h                  |   3 +
 tools/lib/bpf/btf_dump.c             |   2 +-
 tools/lib/bpf/btf_relocate.c         |   2 +-
 tools/lib/bpf/gen_loader.c           | 187 ++++++++++++++++++++-------
 tools/lib/bpf/libbpf.c               |  26 +++-
 tools/lib/bpf/libbpf.map             |   2 +
 tools/lib/bpf/libbpf_internal.h      |  17 ++-
 tools/lib/bpf/linker.c               | 108 +++++++++++++---
 tools/lib/bpf/relo_core.c            |   2 +-
 tools/lib/bpf/skel_internal.h        |   3 +-
 tools/testing/selftests/bpf/Makefile |   7 +-
 13 files changed, 444 insertions(+), 84 deletions(-)

Comments

Andrii Nakryiko Aug. 22, 2024, 11:36 p.m. UTC | #1
On Thu, Aug 22, 2024 at 2:24 AM Tony Ambardar <tony.ambardar@gmail.com> wrote:
>
> From: Tony Ambardar <tony.ambardar@gmail.com>
>
> Fix missing newlines and extraneous terminal spaces in messages.
>
> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> ---
>  tools/lib/bpf/btf.c          | 7 ++++---
>  tools/lib/bpf/btf_dump.c     | 2 +-
>  tools/lib/bpf/btf_relocate.c | 2 +-
>  tools/lib/bpf/libbpf.c       | 4 ++--
>  tools/lib/bpf/relo_core.c    | 2 +-
>  5 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 32c00db3b91b..cf4f7bd7ff5c 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -2940,7 +2940,8 @@ static int btf_ext_setup_info(struct btf_ext *btf_ext,
>
>         /* If no records, return failure now so .BTF.ext won't be used. */
>         if (!info_left) {
> -               pr_debug("%s section in .BTF.ext has no records", ext_sec->desc);
> +               pr_debug("%s section in .BTF.ext has no records\n",
> +                        ext_sec->desc);

if it fits in 100 characters, it stays in a single line, please don't touch that

>                 return -EINVAL;
>         }
>
> @@ -3028,7 +3029,7 @@ static int btf_ext_parse_hdr(__u8 *data, __u32 data_size)
>
>         if (data_size < offsetofend(struct btf_ext_header, hdr_len) ||
>             data_size < hdr->hdr_len) {
> -               pr_debug("BTF.ext header not found");
> +               pr_debug("BTF.ext header not found\n");
>                 return -EINVAL;
>         }
>
> @@ -3290,7 +3291,7 @@ int btf__dedup(struct btf *btf, const struct btf_dedup_opts *opts)
>
>         d = btf_dedup_new(btf, opts);
>         if (IS_ERR(d)) {
> -               pr_debug("btf_dedup_new failed: %ld", PTR_ERR(d));
> +               pr_debug("btf_dedup_new failed: %ld\n", PTR_ERR(d));
>                 return libbpf_err(-EINVAL);
>         }
>
> diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> index 894860111ddb..25e7c44d9f95 100644
> --- a/tools/lib/bpf/btf_dump.c
> +++ b/tools/lib/bpf/btf_dump.c
> @@ -1304,7 +1304,7 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
>                          * chain, restore stack, emit warning, and try to
>                          * proceed nevertheless
>                          */
> -                       pr_warn("not enough memory for decl stack:%d", err);
> +                       pr_warn("not enough memory for decl stack:%d\n", err);

nit: add space after ':' while you are at it?

>                         d->decl_stack_cnt = stack_start;
>                         return;
>                 }

[...]
Tony Ambardar Aug. 26, 2024, 10:51 a.m. UTC | #2
On Thu, Aug 22, 2024 at 04:36:13PM -0700, Andrii Nakryiko wrote:
> On Thu, Aug 22, 2024 at 2:24 AM Tony Ambardar <tony.ambardar@gmail.com> wrote:
> >
> > From: Tony Ambardar <tony.ambardar@gmail.com>
> >
> > Fix missing newlines and extraneous terminal spaces in messages.
> >
> > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> > ---
> >  tools/lib/bpf/btf.c          | 7 ++++---
> >  tools/lib/bpf/btf_dump.c     | 2 +-
> >  tools/lib/bpf/btf_relocate.c | 2 +-
> >  tools/lib/bpf/libbpf.c       | 4 ++--
> >  tools/lib/bpf/relo_core.c    | 2 +-
> >  5 files changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 32c00db3b91b..cf4f7bd7ff5c 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -2940,7 +2940,8 @@ static int btf_ext_setup_info(struct btf_ext *btf_ext,
> >
> >         /* If no records, return failure now so .BTF.ext won't be used. */
> >         if (!info_left) {
> > -               pr_debug("%s section in .BTF.ext has no records", ext_sec->desc);
> > +               pr_debug("%s section in .BTF.ext has no records\n",
> > +                        ext_sec->desc);
> 
> if it fits in 100 characters, it stays in a single line, please don't touch that

OK, will do. Most of these are due to CI and local checkpatch, plus kernel
docs on style/submission guidelines. It can be hard to decide which
warnings/errors can be ignored and which apply, so it would be helpful at
least if the CI flavour of checkpatch aligned with guidance above. Not sure
what that involves, but sure that others besides me would benefit.

> 
> >                 return -EINVAL;
> >         }
> >
> > @@ -3028,7 +3029,7 @@ static int btf_ext_parse_hdr(__u8 *data, __u32 data_size)
> >
> >         if (data_size < offsetofend(struct btf_ext_header, hdr_len) ||
> >             data_size < hdr->hdr_len) {
> > -               pr_debug("BTF.ext header not found");
> > +               pr_debug("BTF.ext header not found\n");
> >                 return -EINVAL;
> >         }
> >
> > @@ -3290,7 +3291,7 @@ int btf__dedup(struct btf *btf, const struct btf_dedup_opts *opts)
> >
> >         d = btf_dedup_new(btf, opts);
> >         if (IS_ERR(d)) {
> > -               pr_debug("btf_dedup_new failed: %ld", PTR_ERR(d));
> > +               pr_debug("btf_dedup_new failed: %ld\n", PTR_ERR(d));
> >                 return libbpf_err(-EINVAL);
> >         }
> >
> > diff --git a/tools/lib/bpf/btf_dump.c b/tools/lib/bpf/btf_dump.c
> > index 894860111ddb..25e7c44d9f95 100644
> > --- a/tools/lib/bpf/btf_dump.c
> > +++ b/tools/lib/bpf/btf_dump.c
> > @@ -1304,7 +1304,7 @@ static void btf_dump_emit_type_decl(struct btf_dump *d, __u32 id,
> >                          * chain, restore stack, emit warning, and try to
> >                          * proceed nevertheless
> >                          */
> > -                       pr_warn("not enough memory for decl stack:%d", err);
> > +                       pr_warn("not enough memory for decl stack:%d\n", err);
> 
> nit: add space after ':' while you are at it?

Right, done. Thanks!

> 
> >                         d->decl_stack_cnt = stack_start;
> >                         return;
> >                 }
> 
> [...]