FYI: CI regression on big-endian arch (s390) after recent pahole changes

FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Eduard Zingerman]

Hi Arnaldo, Alan,

After recent pahole changes [1] BPF CI fails for s390 [2].
Song Liu identified that there is a mismatch between endianness of BTF
in .BTF and .BTF.base sections.

I think that the correct fix should be on libbpf side,
where btf__distill_base() should inherit endianness from source BTF.
If there are any plans for new pahole release,
could you please postpone it until current issue is resolved?
(I should have a fix for this thing by tomorrow).

Best regards,
Eduard

[1] c7b1f6a29ba1 ("btf_encoder: Add "distilled_base" BTF feature to split BTF generation")
[2] https://github.com/kernel-patches/bpf/actions/runs/10622763027/job/29447973415

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Tony Ambardar]

On Thu, Aug 29, 2024 at 05:05:20PM -0700, Eduard Zingerman wrote:
> Hi Arnaldo, Alan,
> 
> After recent pahole changes [1] BPF CI fails for s390 [2].
> Song Liu identified that there is a mismatch between endianness of BTF
> in .BTF and .BTF.base sections.
> 
> I think that the correct fix should be on libbpf side,
> where btf__distill_base() should inherit endianness from source BTF.
> If there are any plans for new pahole release,
> could you please postpone it until current issue is resolved?
> (I should have a fix for this thing by tomorrow).

Hi Eduard,

Thanks for looking at this. I ran into the CI failure while using s390x
to test a series adding libbpf bi-endian support. Since I'm deep into
endianness issues right now, I thought to try the fix you suggested just
to make some progress but noticed the CI failure has disappeared.[0]

Did something get fixed already? I can't seem to find the change.

Many thanks,
Tony

[0] https://github.com/kernel-patches/bpf/pull/7520
> 
> Best regards,
> Eduard
> 
> [1] c7b1f6a29ba1 ("btf_encoder: Add "distilled_base" BTF feature to split BTF generation")
> [2] https://github.com/kernel-patches/bpf/actions/runs/10622763027/job/29447973415
> 

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Eduard Zingerman]

On Thu, 2024-08-29 at 18:27 -0700, Tony Ambardar wrote:


> Thanks for looking at this. I ran into the CI failure while using s390x
> to test a series adding libbpf bi-endian support. Since I'm deep into
> endianness issues right now, I thought to try the fix you suggested just
> to make some progress but noticed the CI failure has disappeared.[0]

Hi Tony,

There is no fix yet, sorry :)
I think that something like below should do the trick:

--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -5394,6 +5394,7 @@ int btf__distill_base(const struct btf *src_btf, struct btf **new_base_btf,
        new_base = btf__new_empty();
        if (!new_base)
                return libbpf_err(-ENOMEM);
+       btf__set_endianness(new_base, btf__endianness(src_btf));
        dist.id_map = calloc(n, sizeof(*dist.id_map));
        if (!dist.id_map) {
                err = -ENOMEM;

as far as I understand btf__raw_data() should do all conversions after this.
But I have not tested it yet and would be AFK for a few hours.

> Did something get fixed already? I can't seem to find the change.

pahole version w/o support for distilled base was pinned on CI:
https://github.com/kernel-patches/vmtest/pull/285/commits/d3eff26fc978ca8fb3bce3f93421f7425aef0f55


Thanks,
Eduard

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Song Liu]

Hi Eduard, 

Thanks for sending the report!

> On Aug 29, 2024, at 5:05 PM, Eduard Zingerman <eddyz87@gmail.com> wrote:
> 
> Hi Arnaldo, Alan,
> 
> After recent pahole changes [1] BPF CI fails for s390 [2].
> Song Liu identified that there is a mismatch between endianness of BTF
> in .BTF and .BTF.base sections.

Clarification: 

With the regression, _both_ .BTF and .BTF.base sections (or at 
least part of these sections) are in little endian for s390:

$ objdump -s -j .BTF bpf_testmod.ko | head
bpf_testmod.ko:     file format elf64-big

Contents of section .BTF:
 0000 9feb0100 18000000 00000000 301a0000  ............0...
 0010 301a0000 80110000 00000000 0000000a  0...............
 0020 24000000 00000000 00000003 00000000  $...............
 0030 28000000 06000000 2b000000 00000000  (.......+.......
 0040 0000000a 29000000 ea010000 03000004  ....)...........
 0050 18000000 04020000 1e010000 00000000  ................

$ objdump -s -j .BTF.base bpf_testmod.ko | head
bpf_testmod.ko:     file format elf64-big

Contents of section .BTF.base:
 0000 9feb0100 18000000 00000000 fc010000  ................
 0010 fc010000 ea010000 01000000 00000001  ................
 0020 08000000 40000000 13000000 00000001  ....@...........
 0030 01000000 08000000 18000000 00000001  ................
 0040 04000000 20000000 25000000 00000001  .... ...%.......
 0050 01000000 08000001 31000000 00000001  ........1.......



Before the regression, the "9feb" part was "eb9f" for s390:

$ objdump -s -j .BTF bpf_testmod.ko | head
bpf_testmod.ko:     file format elf64-big

Contents of section .BTF:
 0000 eb9f0100 00000018 00000000 00001560  ...............`
 0010 00001560 00000fc2 00000000 0a000000  ...`............


Thanks,
Song

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Tony Ambardar]

On Thu, Aug 29, 2024 at 06:40:59PM -0700, Eduard Zingerman wrote:
> On Thu, 2024-08-29 at 18:27 -0700, Tony Ambardar wrote:
> 
> 
> > Thanks for looking at this. I ran into the CI failure while using s390x
> > to test a series adding libbpf bi-endian support. Since I'm deep into
> > endianness issues right now, I thought to try the fix you suggested just
> > to make some progress but noticed the CI failure has disappeared.[0]
> 
> Hi Tony,
> 
> There is no fix yet, sorry :)
> I think that something like below should do the trick:
> 
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -5394,6 +5394,7 @@ int btf__distill_base(const struct btf *src_btf, struct btf **new_base_btf,
>         new_base = btf__new_empty();
>         if (!new_base)
>                 return libbpf_err(-ENOMEM);
> +       btf__set_endianness(new_base, btf__endianness(src_btf));
>         dist.id_map = calloc(n, sizeof(*dist.id_map));
>         if (!dist.id_map) {
>                 err = -ENOMEM;
> 
> as far as I understand btf__raw_data() should do all conversions after this.
> But I have not tested it yet and would be AFK for a few hours.
> 

Hi Eduard,

Yes, btf__raw_data() will work as expected.

I updated my local pahole and managed to reproduce the problem after
cross-compiling to s390x. Looking at lib/bpf/btf.c, I see one more spot
that needs to preserve source endianness compared to patch above, and
local testing under QEMU now works for me:

    root@(none):/usr/libexec/kselftests-bpf# insmod bpf_testmod.ko
    bpf_testmod: loading out-of-tree module taints kernel.
    bpf_testmod: module verification failed: signature and/or required key
    missing - tainting kernel

    root@(none):/usr/libexec/kselftests-bpf# ./test_progs -a map_ptr
    #166     map_ptr:OK
    Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
 
and:

    $ llvm-readelf -x .BTF
    bpf_testmod.ko | head -5
    Hex dump of section '.BTF':
    0x00000000 eb9f0100 00000018 00000000 00001a30 ...............0
    0x00000010 00001a30 00001180 00000000 0a000000 ...0............
    0x00000020 00000022 00000000 03000000 00000000 ..."............
    0x00000030 00000028 00000006 0000002b 00000000 ...(.......+....
    
    $ llvm-readelf -x .BTF.base
    bpf_testmod.ko | head -5
    Hex dump of section '.BTF.base':
    0x00000000 eb9f0100 00000018 00000000 000001fc ................
    0x00000010 000001fc 000001ea 00000001 01000000 ................
    0x00000020 00000008 00000040 00000013 01000000 .......@........
    0x00000030 00000001 00000008 00000018 01000000 ................

Please try with the patch below, or I can just send a proper one to the
list with some added "Co-developed-by:" if easier?

--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -996,6 +996,7 @@ static struct btf *btf_new_empty(struct btf *base_btf)
                btf->base_btf = base_btf;
                btf->start_id = btf__type_cnt(base_btf);
                btf->start_str_off = base_btf->hdr->str_len;
+               btf->swapped_endian = base_btf->swapped_endian;
        }

        /* +1 for empty string at offset 0 */
@@ -5554,6 +5555,7 @@ int btf__distill_base(const struct btf *src_btf,
struct btf **new_base_btf,
        new_base = btf__new_empty();
        if (!new_base)
                return libbpf_err(-ENOMEM);
+       btf__set_endianness(new_base, btf__endianness(src_btf));
        dist.id_map = calloc(n, sizeof(*dist.id_map));
        if (!dist.id_map) {
                err = -ENOMEM;


> > Did something get fixed already? I can't seem to find the change.
> 
> pahole version w/o support for distilled base was pinned on CI:
> https://github.com/kernel-patches/vmtest/pull/285/commits/d3eff26fc978ca8fb3bce3f93421f7425aef0f55
> 

Ah, got it! That makes more sense now. Thanks for the extra details.

Take care,
Tony

> 
> Thanks,
> Eduard

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Eduard Zingerman]

On Thu, 2024-08-29 at 23:57 -0700, Tony Ambardar wrote:

[...]

> Please try with the patch below, or I can just send a proper one to the
> list with some added "Co-developed-by:" if easier?

Hi Tony,

Thank you for working on this. Sure, please send a proper patch, don't
bother with co-developed-by :) I'll send you a selftest shortly.

Thanks,
Eduard

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Eduard Zingerman]

On Fri, 2024-08-30 at 02:49 +0000, Song Liu wrote:

[...]

> Clarification: 
> 
> With the regression, _both_ .BTF and .BTF.base sections (or at 
> least part of these sections) are in little endian for s390:

Hi Song,

Understood, thank you for clarification and sorry for confusion.
This makes sense because btf__distill_base() generates
two new BTF structures and both need to inherit endianness.

Thanks,
Eduard

[...]

[PATCH bpf-next v1] libbpf: ensure new BTF objects inherit input endianness

[Tony Ambardar]

The pahole master branch recently added support for "distilled BTF" based
on libbpf v1.5, but may add .BTF and .BTF.base sections with the wrong byte
order (e.g. on s390x BPF CI), which then lead to kernel Oops when loaded.

Fix by updating libbpf's btf__distill_base() and btf_new_empty() to retain
the byte order of any source BTF objects when creating new ones.

Reported-by: Song Liu <song@kernel.org>
Reported-by: Eduard Zingerman <eddyz87@gmail.com>
Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://lore.kernel.org/bpf/6358db36c5f68b07873a0a5be2d062b1af5ea5f8.camel@gmail.com/
Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
---
 tools/lib/bpf/btf.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 064cfe126c09..7726b7c6d40a 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -996,6 +996,7 @@ static struct btf *btf_new_empty(struct btf *base_btf)
 		btf->base_btf = base_btf;
 		btf->start_id = btf__type_cnt(base_btf);
 		btf->start_str_off = base_btf->hdr->str_len;
+		btf->swapped_endian = base_btf->swapped_endian;
 	}
 
 	/* +1 for empty string at offset 0 */
@@ -5554,6 +5555,10 @@ int btf__distill_base(const struct btf *src_btf, struct btf **new_base_btf,
 	new_base = btf__new_empty();
 	if (!new_base)
 		return libbpf_err(-ENOMEM);
+	err = btf__set_endianness(new_base, btf__endianness(src_btf));
+	if (err < 0)
+		goto done;
+
 	dist.id_map = calloc(n, sizeof(*dist.id_map));
 	if (!dist.id_map) {
 		err = -ENOMEM;
-- 
2.34.1

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Alan Maguire]

On 30/08/2024 10:21, Eduard Zingerman wrote:
> On Fri, 2024-08-30 at 02:49 +0000, Song Liu wrote:
> 
> [...]
> 
>> Clarification: 
>>
>> With the regression, _both_ .BTF and .BTF.base sections (or at 
>> least part of these sections) are in little endian for s390:
> 
> Hi Song,
> 
> Understood, thank you for clarification and sorry for confusion.
> This makes sense because btf__distill_base() generates
> two new BTF structures and both need to inherit endianness.
> 
> Thanks,
> Eduard
> 
> [...]
> 

thanks all for the quick root-cause analysis and proposed fixes!
Explicitly checking these cases in the btf_endian selftest is probably
worthwhile; I've put together tests that do that for non-native
endianness but just noticed you mentioned you're working on tests
Eduard. Is that what you had in mind?

Arnaldo: apologies but I think we'll either need to back out the
distilled stuff for 1.28 or have a new libbpf resync that captures the
fixes for endian issues once they land. Let me know what works best for
you. Thanks!

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Eduard Zingerman]

On Fri, 2024-08-30 at 11:05 +0100, Alan Maguire wrote:


> thanks all for the quick root-cause analysis and proposed fixes!
> Explicitly checking these cases in the btf_endian selftest is probably
> worthwhile; I've put together tests that do that for non-native
> endianness but just noticed you mentioned you're working on tests
> Eduard. Is that what you had in mind?

Hi Alan,

Yes, but I need like 10-15 minutes more.
So we can go with your tests :)

Thanks,
Eduard

Re: [PATCH bpf-next v1] libbpf: ensure new BTF objects inherit input endianness

[Eduard Zingerman]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

On Fri, 2024-08-30 at 02:51 -0700, Tony Ambardar wrote:
> The pahole master branch recently added support for "distilled BTF" based
> on libbpf v1.5, but may add .BTF and .BTF.base sections with the wrong byte
> order (e.g. on s390x BPF CI), which then lead to kernel Oops when loaded.
> 
> Fix by updating libbpf's btf__distill_base() and btf_new_empty() to retain
> the byte order of any source BTF objects when creating new ones.
> 
> Reported-by: Song Liu <song@kernel.org>
> Reported-by: Eduard Zingerman <eddyz87@gmail.com>
> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> Link: https://lore.kernel.org/bpf/6358db36c5f68b07873a0a5be2d062b1af5ea5f8.camel@gmail.com/
> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

But we also need a test for this. Like the one attached.
Or Alan can share his test, which is much shorter but skips round trip to bytes and back.

[...]

[-- Attachment #2: 0001-selftests-bpf-checl-if-distilled-base-inherits-sourc.patch --]
[-- Type: text/x-patch, Size: 3792 bytes --]

From 30fd46e4c84ba90627b039b1e721449ca8b656ad Mon Sep 17 00:00:00 2001
From: Eduard Zingerman <eddyz87@gmail.com>
Date: Fri, 30 Aug 2024 04:08:44 -0700
Subject: [PATCH bpf-next] selftests/bpf: checl if distilled base inherits
 source endianness

Create a BTF with endianness different from host, make a distilled
base/split BTF pair from it, dump as raw bytes, import again and
verify that endianness is preserved.

Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 .../selftests/bpf/prog_tests/btf_distill.c    | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_distill.c b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
index bfbe795823a2..810b2e434562 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_distill.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
@@ -535,6 +535,77 @@ static void test_distilled_base_vmlinux(void)
 	btf__free(vmlinux_btf);
 }
 
+static bool is_host_big_endian(void)
+{
+	return htons(0x1234) == 0x1234;
+}
+
+/* Split and new base BTFs should inherit endianness from source BTF. */
+static void test_distilled_endianness(void)
+{
+	struct btf *base = NULL, *split = NULL, *new_base = NULL, *new_split = NULL;
+	struct btf *new_base1 = NULL, *new_split1 = NULL;
+	enum btf_endianness inverse_endianness;
+	const void *raw_data;
+	__u32 size;
+
+	printf("is_host_big_endian? %d\n", is_host_big_endian());
+	inverse_endianness = is_host_big_endian() ? BTF_LITTLE_ENDIAN : BTF_BIG_ENDIAN;
+	base = btf__new_empty();
+	btf__set_endianness(base, inverse_endianness);
+	if (!ASSERT_OK_PTR(base, "empty_main_btf"))
+		return;
+	btf__add_int(base, "int", 4, BTF_INT_SIGNED);   /* [1] int */
+	VALIDATE_RAW_BTF(
+		base,
+		"[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED");
+	split = btf__new_empty_split(base);
+	if (!ASSERT_OK_PTR(split, "empty_split_btf"))
+		goto cleanup;
+	btf__add_ptr(split, 1);
+	VALIDATE_RAW_BTF(
+		split,
+		"[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED",
+		"[2] PTR '(anon)' type_id=1");
+	if (!ASSERT_EQ(0, btf__distill_base(split, &new_base, &new_split),
+		       "distilled_base") ||
+	    !ASSERT_OK_PTR(new_base, "distilled_base") ||
+	    !ASSERT_OK_PTR(new_split, "distilled_split") ||
+	    !ASSERT_EQ(2, btf__type_cnt(new_base), "distilled_base_type_cnt"))
+		goto cleanup;
+	VALIDATE_RAW_BTF(
+		new_split,
+		"[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED",
+		"[2] PTR '(anon)' type_id=1");
+
+	raw_data = btf__raw_data(new_base, &size);
+	if (!ASSERT_OK_PTR(raw_data, "btf__raw_data #1"))
+		goto cleanup;
+	new_base1 = btf__new(raw_data, size);
+	if (!ASSERT_OK_PTR(new_base1, "new_base1 = btf__new()"))
+		goto cleanup;
+	raw_data = btf__raw_data(new_split, &size);
+	if (!ASSERT_OK_PTR(raw_data, "btf__raw_data #2"))
+		goto cleanup;
+	new_split1 = btf__new_split(raw_data, size, new_base1);
+	if (!ASSERT_OK_PTR(new_split1, "new_split1 = btf__new()"))
+		goto cleanup;
+
+	ASSERT_EQ(btf__endianness(new_base1), inverse_endianness, "new_base1 endianness");
+	ASSERT_EQ(btf__endianness(new_split1), inverse_endianness, "new_split1 endianness");
+	VALIDATE_RAW_BTF(
+		new_split1,
+		"[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED",
+		"[2] PTR '(anon)' type_id=1");
+cleanup:
+	btf__free(new_split1);
+	btf__free(new_base1);
+	btf__free(new_split);
+	btf__free(new_base);
+	btf__free(split);
+	btf__free(base);
+}
+
 void test_btf_distill(void)
 {
 	if (test__start_subtest("distilled_base"))
@@ -549,4 +620,6 @@ void test_btf_distill(void)
 		test_distilled_base_multi_err2();
 	if (test__start_subtest("distilled_base_vmlinux"))
 		test_distilled_base_vmlinux();
+	if (test__start_subtest("distilled_endianness"))
+		test_distilled_endianness();
 }
-- 
2.46.0

Re: [PATCH bpf-next v1] libbpf: ensure new BTF objects inherit input endianness

[Alan Maguire]

On 30/08/2024 12:15, Eduard Zingerman wrote:
> On Fri, 2024-08-30 at 02:51 -0700, Tony Ambardar wrote:
>> The pahole master branch recently added support for "distilled BTF" based
>> on libbpf v1.5, but may add .BTF and .BTF.base sections with the wrong byte
>> order (e.g. on s390x BPF CI), which then lead to kernel Oops when loaded.
>>
>> Fix by updating libbpf's btf__distill_base() and btf_new_empty() to retain
>> the byte order of any source BTF objects when creating new ones.
>>
>> Reported-by: Song Liu <song@kernel.org>
>> Reported-by: Eduard Zingerman <eddyz87@gmail.com>
>> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
>> Link: https://lore.kernel.org/bpf/6358db36c5f68b07873a0a5be2d062b1af5ea5f8.camel@gmail.com/
>> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
>> ---
> 
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
>

Tested-by: Alan Maguire <alan.maguire@oracle.com>

Thanks for the fix!

> But we also need a test for this. Like the one attached.
> Or Alan can share his test, which is much shorter but skips round trip to bytes and back.
>

Eduard's test is better than mine; mine was a simple addition to
btf_endian() tests that checked split/distilled BTF matched endianness
of the originating BTF for non-native endianness. Having actual
non-native endianness _use_ as in Eduard's test is much preferred I think.

> [...]

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Arnaldo Carvalho de Melo]

On Fri, Aug 30, 2024 at 11:05:30AM +0100, Alan Maguire wrote:
> On 30/08/2024 10:21, Eduard Zingerman wrote:
> > On Fri, 2024-08-30 at 02:49 +0000, Song Liu wrote:
> >> With the regression, _both_ .BTF and .BTF.base sections (or at 
> >> least part of these sections) are in little endian for s390:

> > Hi Song,

> > Understood, thank you for clarification and sorry for confusion.
> > This makes sense because btf__distill_base() generates
> > two new BTF structures and both need to inherit endianness.
 
> thanks all for the quick root-cause analysis and proposed fixes!
> Explicitly checking these cases in the btf_endian selftest is probably
> worthwhile; I've put together tests that do that for non-native
> endianness but just noticed you mentioned you're working on tests
> Eduard. Is that what you had in mind?
 
> Arnaldo: apologies but I think we'll either need to back out the
> distilled stuff for 1.28 or have a new libbpf resync that captures the
> fixes for endian issues once they land. Let me know what works best for
> you. Thanks!

It was useful, we got it tested more widely and caught this one.

Andrii, what do you think? Can we get a 1.5.1 with this soon so that we
do a resying in pahole and then release 1.28?

- Arnaldo

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Andrii Nakryiko]

On Fri, Aug 30, 2024 at 6:19 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Fri, Aug 30, 2024 at 11:05:30AM +0100, Alan Maguire wrote:
> > On 30/08/2024 10:21, Eduard Zingerman wrote:
> > > On Fri, 2024-08-30 at 02:49 +0000, Song Liu wrote:
> > >> With the regression, _both_ .BTF and .BTF.base sections (or at
> > >> least part of these sections) are in little endian for s390:
>
> > > Hi Song,
>
> > > Understood, thank you for clarification and sorry for confusion.
> > > This makes sense because btf__distill_base() generates
> > > two new BTF structures and both need to inherit endianness.
>
> > thanks all for the quick root-cause analysis and proposed fixes!
> > Explicitly checking these cases in the btf_endian selftest is probably
> > worthwhile; I've put together tests that do that for non-native
> > endianness but just noticed you mentioned you're working on tests
> > Eduard. Is that what you had in mind?
>
> > Arnaldo: apologies but I think we'll either need to back out the
> > distilled stuff for 1.28 or have a new libbpf resync that captures the
> > fixes for endian issues once they land. Let me know what works best for
> > you. Thanks!
>
> It was useful, we got it tested more widely and caught this one.
>
> Andrii, what do you think? Can we get a 1.5.1 with this soon so that we
> do a resying in pahole and then release 1.28?

Did you mean 1.4.6? We haven't released v1.5 just yet.

But yes, I'm going to cut a new set of bugfix releases to libbpf
anyways, there is one more skeleton-related fix I have to backport.

So I'll try to review, land, and backport the fix ASAP.


>
> - Arnaldo

Re: [PATCH bpf-next v1] libbpf: ensure new BTF objects inherit input endianness

[Andrii Nakryiko]

On Fri, Aug 30, 2024 at 4:26 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 30/08/2024 12:15, Eduard Zingerman wrote:
> > On Fri, 2024-08-30 at 02:51 -0700, Tony Ambardar wrote:
> >> The pahole master branch recently added support for "distilled BTF" based
> >> on libbpf v1.5, but may add .BTF and .BTF.base sections with the wrong byte
> >> order (e.g. on s390x BPF CI), which then lead to kernel Oops when loaded.
> >>
> >> Fix by updating libbpf's btf__distill_base() and btf_new_empty() to retain
> >> the byte order of any source BTF objects when creating new ones.
> >>
> >> Reported-by: Song Liu <song@kernel.org>
> >> Reported-by: Eduard Zingerman <eddyz87@gmail.com>
> >> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> >> Link: https://lore.kernel.org/bpf/6358db36c5f68b07873a0a5be2d062b1af5ea5f8.camel@gmail.com/
> >> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> >> ---
> >
> > Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> >
>
> Tested-by: Alan Maguire <alan.maguire@oracle.com>
>
> Thanks for the fix!
>
> > But we also need a test for this. Like the one attached.
> > Or Alan can share his test, which is much shorter but skips round trip to bytes and back.
> >
>
> Eduard's test is better than mine; mine was a simple addition to
> btf_endian() tests that checked split/distilled BTF matched endianness
> of the originating BTF for non-native endianness. Having actual
> non-native endianness _use_ as in Eduard's test is much preferred I think.

Eduard, please send it as a proper patch?

>
> > [...]

Re: [PATCH bpf-next v1] libbpf: ensure new BTF objects inherit input endianness

[Andrii Nakryiko]

On Fri, Aug 30, 2024 at 2:52 AM Tony Ambardar <tony.ambardar@gmail.com> wrote:
>
> The pahole master branch recently added support for "distilled BTF" based
> on libbpf v1.5, but may add .BTF and .BTF.base sections with the wrong byte

there is no libbpf v1.5 release, are we talking about using unreleased
master branch?

> order (e.g. on s390x BPF CI), which then lead to kernel Oops when loaded.
>
> Fix by updating libbpf's btf__distill_base() and btf_new_empty() to retain
> the byte order of any source BTF objects when creating new ones.
>
> Reported-by: Song Liu <song@kernel.org>
> Reported-by: Eduard Zingerman <eddyz87@gmail.com>
> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> Link: https://lore.kernel.org/bpf/6358db36c5f68b07873a0a5be2d062b1af5ea5f8.camel@gmail.com/
> Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> ---
>  tools/lib/bpf/btf.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 064cfe126c09..7726b7c6d40a 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -996,6 +996,7 @@ static struct btf *btf_new_empty(struct btf *base_btf)
>                 btf->base_btf = base_btf;
>                 btf->start_id = btf__type_cnt(base_btf);
>                 btf->start_str_off = base_btf->hdr->str_len;
> +               btf->swapped_endian = base_btf->swapped_endian;
>         }
>
>         /* +1 for empty string at offset 0 */
> @@ -5554,6 +5555,10 @@ int btf__distill_base(const struct btf *src_btf, struct btf **new_base_btf,
>         new_base = btf__new_empty();
>         if (!new_base)
>                 return libbpf_err(-ENOMEM);
> +       err = btf__set_endianness(new_base, btf__endianness(src_btf));
> +       if (err < 0)
> +               goto done;

This error check is really unnecessary and paranoid, because the only
way btf__set_endianness() can fail is if the provided endianness enum
is corrupted (some invalid int cast to enum). But in this case we are
getting it from libbpf itself, which will always be correct. So I
think I'll drop the error check while applying.

> +
>         dist.id_map = calloc(n, sizeof(*dist.id_map));
>         if (!dist.id_map) {
>                 err = -ENOMEM;
> --
> 2.34.1
>

Re: [PATCH bpf-next v1] libbpf: ensure new BTF objects inherit input endianness

[Andrii Nakryiko]

On Fri, Aug 30, 2024 at 9:00 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Aug 30, 2024 at 2:52 AM Tony Ambardar <tony.ambardar@gmail.com> wrote:
> >
> > The pahole master branch recently added support for "distilled BTF" based
> > on libbpf v1.5, but may add .BTF and .BTF.base sections with the wrong byte
>
> there is no libbpf v1.5 release, are we talking about using unreleased
> master branch?
>
> > order (e.g. on s390x BPF CI), which then lead to kernel Oops when loaded.
> >
> > Fix by updating libbpf's btf__distill_base() and btf_new_empty() to retain
> > the byte order of any source BTF objects when creating new ones.
> >
> > Reported-by: Song Liu <song@kernel.org>
> > Reported-by: Eduard Zingerman <eddyz87@gmail.com>
> > Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> > Link: https://lore.kernel.org/bpf/6358db36c5f68b07873a0a5be2d062b1af5ea5f8.camel@gmail.com/
> > Signed-off-by: Tony Ambardar <tony.ambardar@gmail.com>
> > ---
> >  tools/lib/bpf/btf.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >

Also added

Fixes: ba451366bf44 ("libbpf: Implement basic split BTF support")
Fixes: 58e185a0dc35 ("libbpf: Add btf__distill_base() creating split
BTF with distilled base BTF")

> > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> > index 064cfe126c09..7726b7c6d40a 100644
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -996,6 +996,7 @@ static struct btf *btf_new_empty(struct btf *base_btf)
> >                 btf->base_btf = base_btf;
> >                 btf->start_id = btf__type_cnt(base_btf);
> >                 btf->start_str_off = base_btf->hdr->str_len;
> > +               btf->swapped_endian = base_btf->swapped_endian;
> >         }
> >
> >         /* +1 for empty string at offset 0 */
> > @@ -5554,6 +5555,10 @@ int btf__distill_base(const struct btf *src_btf, struct btf **new_base_btf,
> >         new_base = btf__new_empty();
> >         if (!new_base)
> >                 return libbpf_err(-ENOMEM);
> > +       err = btf__set_endianness(new_base, btf__endianness(src_btf));
> > +       if (err < 0)
> > +               goto done;
>
> This error check is really unnecessary and paranoid, because the only
> way btf__set_endianness() can fail is if the provided endianness enum
> is corrupted (some invalid int cast to enum). But in this case we are
> getting it from libbpf itself, which will always be correct. So I
> think I'll drop the error check while applying.
>
> > +
> >         dist.id_map = calloc(n, sizeof(*dist.id_map));
> >         if (!dist.id_map) {
> >                 err = -ENOMEM;
> > --
> > 2.34.1
> >

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Arnaldo Carvalho de Melo]

On Fri, Aug 30, 2024 at 08:56:08AM -0700, Andrii Nakryiko wrote:
> On Fri, Aug 30, 2024 at 6:19 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > On Fri, Aug 30, 2024 at 11:05:30AM +0100, Alan Maguire wrote:
> > > Arnaldo: apologies but I think we'll either need to back out the
> > > distilled stuff for 1.28 or have a new libbpf resync that captures the
> > > fixes for endian issues once they land. Let me know what works best for
> > > you. Thanks!
> >
> > It was useful, we got it tested more widely and caught this one.
> >
> > Andrii, what do you think? Can we get a 1.5.1 with this soon so that we
> > do a resying in pahole and then release 1.28?
> 
> Did you mean 1.4.6? We haven't released v1.5 just yet.
> 
> But yes, I'm going to cut a new set of bugfix releases to libbpf
> anyways, there is one more skeleton-related fix I have to backport.
> 
> So I'll try to review, land, and backport the fix ASAP.

Well, Alan sent patches updating libbpf to 1.5.0, so I misunderstood, I
think he meant what is to become 1.5.0, so even better, I think its just
a matter of updating the submodule sha:

⬢[acme@toolbox pahole]$ git show b6def578aa4a631f870568e13bfd647312718e7f
commit b6def578aa4a631f870568e13bfd647312718e7f
Author: Alan Maguire <alan.maguire@oracle.com>
Date:   Mon Jul 29 12:13:16 2024 +0100

    pahole: Sync with libbpf-1.5
    
    This will pull in BTF support for distilled base BTF.
    
    Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
    Cc: Alexei Starovoitov <ast@kernel.org>
    Cc: Andrii Nakryiko <andrii@kernel.org>
    Cc: Eduard Zingerman <eddyz87@gmail.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: bpf@vger.kernel.org
    Cc: dwarves@vger.kernel.org
    Link: https://lore.kernel.org/r/20240729111317.140816-2-alan.maguire@oracle.com
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/lib/bpf b/lib/bpf
index 6597330c45d18538..686f600bca59e107 160000
--- a/lib/bpf
+++ b/lib/bpf
@@ -1 +1 @@
-Subproject commit 6597330c45d185381900037f0130712cd326ae59
+Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
⬢[acme@toolbox pahole]$

Right?

- Arnaldo

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Andrii Nakryiko]

On Fri, Aug 30, 2024 at 1:49 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Fri, Aug 30, 2024 at 08:56:08AM -0700, Andrii Nakryiko wrote:
> > On Fri, Aug 30, 2024 at 6:19 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > On Fri, Aug 30, 2024 at 11:05:30AM +0100, Alan Maguire wrote:
> > > > Arnaldo: apologies but I think we'll either need to back out the
> > > > distilled stuff for 1.28 or have a new libbpf resync that captures the
> > > > fixes for endian issues once they land. Let me know what works best for
> > > > you. Thanks!
> > >
> > > It was useful, we got it tested more widely and caught this one.
> > >
> > > Andrii, what do you think? Can we get a 1.5.1 with this soon so that we
> > > do a resying in pahole and then release 1.28?
> >
> > Did you mean 1.4.6? We haven't released v1.5 just yet.
> >
> > But yes, I'm going to cut a new set of bugfix releases to libbpf
> > anyways, there is one more skeleton-related fix I have to backport.
> >
> > So I'll try to review, land, and backport the fix ASAP.
>
> Well, Alan sent patches updating libbpf to 1.5.0, so I misunderstood, I
> think he meant what is to become 1.5.0, so even better, I think its just
> a matter of updating the submodule sha:
>
> ⬢[acme@toolbox pahole]$ git show b6def578aa4a631f870568e13bfd647312718e7f
> commit b6def578aa4a631f870568e13bfd647312718e7f
> Author: Alan Maguire <alan.maguire@oracle.com>
> Date:   Mon Jul 29 12:13:16 2024 +0100
>
>     pahole: Sync with libbpf-1.5
>
>     This will pull in BTF support for distilled base BTF.
>
>     Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>     Cc: Alexei Starovoitov <ast@kernel.org>
>     Cc: Andrii Nakryiko <andrii@kernel.org>
>     Cc: Eduard Zingerman <eddyz87@gmail.com>
>     Cc: Jiri Olsa <jolsa@kernel.org>
>     Cc: bpf@vger.kernel.org
>     Cc: dwarves@vger.kernel.org
>     Link: https://lore.kernel.org/r/20240729111317.140816-2-alan.maguire@oracle.com
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> diff --git a/lib/bpf b/lib/bpf
> index 6597330c45d18538..686f600bca59e107 160000
> --- a/lib/bpf
> +++ b/lib/bpf
> @@ -1 +1 @@
> -Subproject commit 6597330c45d185381900037f0130712cd326ae59
> +Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
> ⬢[acme@toolbox pahole]$
>
> Right?

Yes, and I'm doing another Github sync today.

Separate question, I think pahole supports the shared library version
of libbpf, as an option, is that right? How do you guys handle missing
APIs for distilled BTF in such a case?

>
> - Arnaldo

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Alan Maguire]

On 30/08/2024 21:49, Arnaldo Carvalho de Melo wrote:
> On Fri, Aug 30, 2024 at 08:56:08AM -0700, Andrii Nakryiko wrote:
>> On Fri, Aug 30, 2024 at 6:19 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>> On Fri, Aug 30, 2024 at 11:05:30AM +0100, Alan Maguire wrote:
>>>> Arnaldo: apologies but I think we'll either need to back out the
>>>> distilled stuff for 1.28 or have a new libbpf resync that captures the
>>>> fixes for endian issues once they land. Let me know what works best for
>>>> you. Thanks!
>>>
>>> It was useful, we got it tested more widely and caught this one.
>>>
>>> Andrii, what do you think? Can we get a 1.5.1 with this soon so that we
>>> do a resying in pahole and then release 1.28?
>>
>> Did you mean 1.4.6? We haven't released v1.5 just yet.
>>
>> But yes, I'm going to cut a new set of bugfix releases to libbpf
>> anyways, there is one more skeleton-related fix I have to backport.
>>
>> So I'll try to review, land, and backport the fix ASAP.
> 
> Well, Alan sent patches updating libbpf to 1.5.0, so I misunderstood, I
> think he meant what is to become 1.5.0, so even better, I think its just
> a matter of updating the submodule sha:

yep, sorry I should have been clearer here; at the start of a new libbpf
cycle the version is updated, but the sha I specified in my changes
wasn't an official libbpf 1.5 release; the goal was to pull in the
changes we needed for BTF relocation so we'd be in a position to test
pahole + libbpf together and shake out issues prior to syncing to an
official libbpf release.

> 
> ⬢[acme@toolbox pahole]$ git show b6def578aa4a631f870568e13bfd647312718e7f
> commit b6def578aa4a631f870568e13bfd647312718e7f
> Author: Alan Maguire <alan.maguire@oracle.com>
> Date:   Mon Jul 29 12:13:16 2024 +0100
> 
>     pahole: Sync with libbpf-1.5
>     
>     This will pull in BTF support for distilled base BTF.
>     
>     Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>     Cc: Alexei Starovoitov <ast@kernel.org>
>     Cc: Andrii Nakryiko <andrii@kernel.org>
>     Cc: Eduard Zingerman <eddyz87@gmail.com>
>     Cc: Jiri Olsa <jolsa@kernel.org>
>     Cc: bpf@vger.kernel.org
>     Cc: dwarves@vger.kernel.org
>     Link: https://lore.kernel.org/r/20240729111317.140816-2-alan.maguire@oracle.com
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> diff --git a/lib/bpf b/lib/bpf
> index 6597330c45d18538..686f600bca59e107 160000
> --- a/lib/bpf
> +++ b/lib/bpf
> @@ -1 +1 @@
> -Subproject commit 6597330c45d185381900037f0130712cd326ae59
> +Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
> ⬢[acme@toolbox pahole]$
> 
> Right?

yep; tho the above sha doesn't have the endianness fixes yet. Those have
landed in bpf-next, but are not in libbpf github yet. So we'd want to
pull those in too for the pahole release I think. Thanks!

Alan

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Alan Maguire]

On 30/08/2024 23:20, Andrii Nakryiko wrote:
> On Fri, Aug 30, 2024 at 1:49 PM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
>>
>> On Fri, Aug 30, 2024 at 08:56:08AM -0700, Andrii Nakryiko wrote:
>>> On Fri, Aug 30, 2024 at 6:19 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>>> On Fri, Aug 30, 2024 at 11:05:30AM +0100, Alan Maguire wrote:
>>>>> Arnaldo: apologies but I think we'll either need to back out the
>>>>> distilled stuff for 1.28 or have a new libbpf resync that captures the
>>>>> fixes for endian issues once they land. Let me know what works best for
>>>>> you. Thanks!
>>>>
>>>> It was useful, we got it tested more widely and caught this one.
>>>>
>>>> Andrii, what do you think? Can we get a 1.5.1 with this soon so that we
>>>> do a resying in pahole and then release 1.28?
>>>
>>> Did you mean 1.4.6? We haven't released v1.5 just yet.
>>>
>>> But yes, I'm going to cut a new set of bugfix releases to libbpf
>>> anyways, there is one more skeleton-related fix I have to backport.
>>>
>>> So I'll try to review, land, and backport the fix ASAP.
>>
>> Well, Alan sent patches updating libbpf to 1.5.0, so I misunderstood, I
>> think he meant what is to become 1.5.0, so even better, I think its just
>> a matter of updating the submodule sha:
>>
>> ⬢[acme@toolbox pahole]$ git show b6def578aa4a631f870568e13bfd647312718e7f
>> commit b6def578aa4a631f870568e13bfd647312718e7f
>> Author: Alan Maguire <alan.maguire@oracle.com>
>> Date:   Mon Jul 29 12:13:16 2024 +0100
>>
>>     pahole: Sync with libbpf-1.5
>>
>>     This will pull in BTF support for distilled base BTF.
>>
>>     Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>>     Cc: Alexei Starovoitov <ast@kernel.org>
>>     Cc: Andrii Nakryiko <andrii@kernel.org>
>>     Cc: Eduard Zingerman <eddyz87@gmail.com>
>>     Cc: Jiri Olsa <jolsa@kernel.org>
>>     Cc: bpf@vger.kernel.org
>>     Cc: dwarves@vger.kernel.org
>>     Link: https://lore.kernel.org/r/20240729111317.140816-2-alan.maguire@oracle.com
>>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>>
>> diff --git a/lib/bpf b/lib/bpf
>> index 6597330c45d18538..686f600bca59e107 160000
>> --- a/lib/bpf
>> +++ b/lib/bpf
>> @@ -1 +1 @@
>> -Subproject commit 6597330c45d185381900037f0130712cd326ae59
>> +Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
>> ⬢[acme@toolbox pahole]$
>>
>> Right?
> 
> Yes, and I'm doing another Github sync today.
> 
> Separate question, I think pahole supports the shared library version
> of libbpf, as an option, is that right? How do you guys handle missing
> APIs for distilled BTF in such a case?
>

Good question - at present the distill-related code is conditionally
compiled if LIBBPF_MAJOR_VERSION >=1 and LIBBF_MINOR_VERSION >= 5; so if
an older shared library libbpf+headers is used, the btf_feature is
simply ignored as if we didn't know about it. See [1] for the relevant
code in btf_encoder.c. This problem doesn't arise if we're using the
synced libbpf.

There might be a better way to handle this, but I think that's enough to
ensure we avoid compilation failures at least.

[1]
https://github.com/acmel/dwarves/blob/fd14dc67cb6aaead553074afb4a1ddad10209892/btf_encoder.c#L1766

>>
>> - Arnaldo

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Andrii Nakryiko]

On Fri, Aug 30, 2024 at 3:34 PM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 30/08/2024 23:20, Andrii Nakryiko wrote:
> > On Fri, Aug 30, 2024 at 1:49 PM Arnaldo Carvalho de Melo
> > <acme@kernel.org> wrote:
> >>
> >> On Fri, Aug 30, 2024 at 08:56:08AM -0700, Andrii Nakryiko wrote:
> >>> On Fri, Aug 30, 2024 at 6:19 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >>>> On Fri, Aug 30, 2024 at 11:05:30AM +0100, Alan Maguire wrote:
> >>>>> Arnaldo: apologies but I think we'll either need to back out the
> >>>>> distilled stuff for 1.28 or have a new libbpf resync that captures the
> >>>>> fixes for endian issues once they land. Let me know what works best for
> >>>>> you. Thanks!
> >>>>
> >>>> It was useful, we got it tested more widely and caught this one.
> >>>>
> >>>> Andrii, what do you think? Can we get a 1.5.1 with this soon so that we
> >>>> do a resying in pahole and then release 1.28?
> >>>
> >>> Did you mean 1.4.6? We haven't released v1.5 just yet.
> >>>
> >>> But yes, I'm going to cut a new set of bugfix releases to libbpf
> >>> anyways, there is one more skeleton-related fix I have to backport.
> >>>
> >>> So I'll try to review, land, and backport the fix ASAP.
> >>
> >> Well, Alan sent patches updating libbpf to 1.5.0, so I misunderstood, I
> >> think he meant what is to become 1.5.0, so even better, I think its just
> >> a matter of updating the submodule sha:
> >>
> >> ⬢[acme@toolbox pahole]$ git show b6def578aa4a631f870568e13bfd647312718e7f
> >> commit b6def578aa4a631f870568e13bfd647312718e7f
> >> Author: Alan Maguire <alan.maguire@oracle.com>
> >> Date:   Mon Jul 29 12:13:16 2024 +0100
> >>
> >>     pahole: Sync with libbpf-1.5
> >>
> >>     This will pull in BTF support for distilled base BTF.
> >>
> >>     Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> >>     Cc: Alexei Starovoitov <ast@kernel.org>
> >>     Cc: Andrii Nakryiko <andrii@kernel.org>
> >>     Cc: Eduard Zingerman <eddyz87@gmail.com>
> >>     Cc: Jiri Olsa <jolsa@kernel.org>
> >>     Cc: bpf@vger.kernel.org
> >>     Cc: dwarves@vger.kernel.org
> >>     Link: https://lore.kernel.org/r/20240729111317.140816-2-alan.maguire@oracle.com
> >>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >>
> >> diff --git a/lib/bpf b/lib/bpf
> >> index 6597330c45d18538..686f600bca59e107 160000
> >> --- a/lib/bpf
> >> +++ b/lib/bpf
> >> @@ -1 +1 @@
> >> -Subproject commit 6597330c45d185381900037f0130712cd326ae59
> >> +Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
> >> ⬢[acme@toolbox pahole]$
> >>
> >> Right?
> >
> > Yes, and I'm doing another Github sync today.
> >
> > Separate question, I think pahole supports the shared library version
> > of libbpf, as an option, is that right? How do you guys handle missing
> > APIs for distilled BTF in such a case?
> >
>
> Good question - at present the distill-related code is conditionally
> compiled if LIBBPF_MAJOR_VERSION >=1 and LIBBF_MINOR_VERSION >= 5; so if
> an older shared library libbpf+headers is used, the btf_feature is
> simply ignored as if we didn't know about it. See [1] for the relevant
> code in btf_encoder.c. This problem doesn't arise if we're using the
> synced libbpf.

Is it possible to compile against newer libbpf headers, but run with
older shared library?

BTW, I've just synced the latest libbpf sources to Github ([0]), feel
free to pull the latest submodule reference.

  [0] https://github.com/libbpf/libbpf/pull/848

>
> There might be a better way to handle this, but I think that's enough to
> ensure we avoid compilation failures at least.
>
> [1]
> https://github.com/acmel/dwarves/blob/fd14dc67cb6aaead553074afb4a1ddad10209892/btf_encoder.c#L1766
>
> >>
> >> - Arnaldo

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Alan Maguire]

On 31/08/2024 00:30, Andrii Nakryiko wrote:
> On Fri, Aug 30, 2024 at 3:34 PM Alan Maguire <alan.maguire@oracle.com> wrote:
>>
>> On 30/08/2024 23:20, Andrii Nakryiko wrote:
>>> On Fri, Aug 30, 2024 at 1:49 PM Arnaldo Carvalho de Melo
>>> <acme@kernel.org> wrote:
>>>>
>>>> On Fri, Aug 30, 2024 at 08:56:08AM -0700, Andrii Nakryiko wrote:
>>>>> On Fri, Aug 30, 2024 at 6:19 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>>>>> On Fri, Aug 30, 2024 at 11:05:30AM +0100, Alan Maguire wrote:
>>>>>>> Arnaldo: apologies but I think we'll either need to back out the
>>>>>>> distilled stuff for 1.28 or have a new libbpf resync that captures the
>>>>>>> fixes for endian issues once they land. Let me know what works best for
>>>>>>> you. Thanks!
>>>>>>
>>>>>> It was useful, we got it tested more widely and caught this one.
>>>>>>
>>>>>> Andrii, what do you think? Can we get a 1.5.1 with this soon so that we
>>>>>> do a resying in pahole and then release 1.28?
>>>>>
>>>>> Did you mean 1.4.6? We haven't released v1.5 just yet.
>>>>>
>>>>> But yes, I'm going to cut a new set of bugfix releases to libbpf
>>>>> anyways, there is one more skeleton-related fix I have to backport.
>>>>>
>>>>> So I'll try to review, land, and backport the fix ASAP.
>>>>
>>>> Well, Alan sent patches updating libbpf to 1.5.0, so I misunderstood, I
>>>> think he meant what is to become 1.5.0, so even better, I think its just
>>>> a matter of updating the submodule sha:
>>>>
>>>> ⬢[acme@toolbox pahole]$ git show b6def578aa4a631f870568e13bfd647312718e7f
>>>> commit b6def578aa4a631f870568e13bfd647312718e7f
>>>> Author: Alan Maguire <alan.maguire@oracle.com>
>>>> Date:   Mon Jul 29 12:13:16 2024 +0100
>>>>
>>>>     pahole: Sync with libbpf-1.5
>>>>
>>>>     This will pull in BTF support for distilled base BTF.
>>>>
>>>>     Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>>>>     Cc: Alexei Starovoitov <ast@kernel.org>
>>>>     Cc: Andrii Nakryiko <andrii@kernel.org>
>>>>     Cc: Eduard Zingerman <eddyz87@gmail.com>
>>>>     Cc: Jiri Olsa <jolsa@kernel.org>
>>>>     Cc: bpf@vger.kernel.org
>>>>     Cc: dwarves@vger.kernel.org
>>>>     Link: https://lore.kernel.org/r/20240729111317.140816-2-alan.maguire@oracle.com
>>>>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>>>>
>>>> diff --git a/lib/bpf b/lib/bpf
>>>> index 6597330c45d18538..686f600bca59e107 160000
>>>> --- a/lib/bpf
>>>> +++ b/lib/bpf
>>>> @@ -1 +1 @@
>>>> -Subproject commit 6597330c45d185381900037f0130712cd326ae59
>>>> +Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
>>>> ⬢[acme@toolbox pahole]$
>>>>
>>>> Right?
>>>
>>> Yes, and I'm doing another Github sync today.
>>>
>>> Separate question, I think pahole supports the shared library version
>>> of libbpf, as an option, is that right? How do you guys handle missing
>>> APIs for distilled BTF in such a case?
>>>
>>
>> Good question - at present the distill-related code is conditionally
>> compiled if LIBBPF_MAJOR_VERSION >=1 and LIBBF_MINOR_VERSION >= 5; so if
>> an older shared library libbpf+headers is used, the btf_feature is
>> simply ignored as if we didn't know about it. See [1] for the relevant
>> code in btf_encoder.c. This problem doesn't arise if we're using the
>> synced libbpf.
> 
> Is it possible to compile against newer libbpf headers, but run with
> older shared library?
>

It would be possible alright; the most important case is package build
time versus package install time. IIRC rpmbuild will auto-detect the
version dependency (as long as libbpf is packaged too I think). We
probably don't want an explicit libbpf "Requires:" dependency in the
dwarves spec file since that wouldn't be needed for the static libbpf
library case.

> BTW, I've just synced the latest libbpf sources to Github ([0]), feel
> free to pull the latest submodule reference.
> 
>   [0] https://github.com/libbpf/libbpf/pull/848
> 

Great, thanks! I'll send a patch to update the sha from the dwarves side.

Alan

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Arnaldo Carvalho de Melo]

On Fri, Aug 30, 2024 at 11:34:40PM +0100, Alan Maguire wrote:
> On 30/08/2024 23:20, Andrii Nakryiko wrote:
> > On Fri, Aug 30, 2024 at 1:49 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >> On Fri, Aug 30, 2024 at 08:56:08AM -0700, Andrii Nakryiko wrote:
> >> +++ b/lib/bpf
> >> @@ -1 +1 @@
> >> -Subproject commit 6597330c45d185381900037f0130712cd326ae59
> >> +Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
> >> ⬢[acme@toolbox pahole]$

> >> Right?

> > Yes, and I'm doing another Github sync today.

So, I just commited this locally:

⬢[acme@toolbox pahole]$ git show
commit 5fd558301891d1c0456fcae79798a789b499c1f9 (HEAD -> master)
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Mon Sep 2 11:05:06 2024 -0300

    libbpf: Sync with master, i.e. what will become 1.5.0
    
    To pick this distilled BPF fix:
    
      fe28fae57a9463fbf ("libbpf: Ensure new BTF objects inherit input endianness")
    
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/lib/bpf b/lib/bpf
index 686f600bca59e107..caa17bdcbfc58e68 160000
--- a/lib/bpf
+++ b/lib/bpf
@@ -1 +1 @@
-Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
+Subproject commit caa17bdcbfc58e68eaf4d017c058e6577606bf56
⬢[acme@toolbox pahole]$

Ack?
 
> > Separate question, I think pahole supports the shared library version
> > of libbpf, as an option, is that right? How do you guys handle missing
> > APIs for distilled BTF in such a case?
 
> Good question - at present the distill-related code is conditionally
> compiled if LIBBPF_MAJOR_VERSION >=1 and LIBBF_MINOR_VERSION >= 5; so if
> an older shared library libbpf+headers is used, the btf_feature is
> simply ignored as if we didn't know about it. See [1] for the relevant
> code in btf_encoder.c. This problem doesn't arise if we're using the
> synced libbpf.
 
> There might be a better way to handle this, but I think that's enough to
> ensure we avoid compilation failures at least.

I guess this is good enough,

- Arnaldo
 
> [1]
> https://github.com/acmel/dwarves/blob/fd14dc67cb6aaead553074afb4a1ddad10209892/btf_encoder.c#L1766

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Alan Maguire]

On 02/09/2024 15:08, Arnaldo Carvalho de Melo wrote:
> On Fri, Aug 30, 2024 at 11:34:40PM +0100, Alan Maguire wrote:
>> On 30/08/2024 23:20, Andrii Nakryiko wrote:
>>> On Fri, Aug 30, 2024 at 1:49 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>>>> On Fri, Aug 30, 2024 at 08:56:08AM -0700, Andrii Nakryiko wrote:
>>>> +++ b/lib/bpf
>>>> @@ -1 +1 @@
>>>> -Subproject commit 6597330c45d185381900037f0130712cd326ae59
>>>> +Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
>>>> ⬢[acme@toolbox pahole]$
> 
>>>> Right?
> 
>>> Yes, and I'm doing another Github sync today.
> 
> So, I just commited this locally:
> 
> ⬢[acme@toolbox pahole]$ git show
> commit 5fd558301891d1c0456fcae79798a789b499c1f9 (HEAD -> master)
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Mon Sep 2 11:05:06 2024 -0300
> 
>     libbpf: Sync with master, i.e. what will become 1.5.0
>     
>     To pick this distilled BPF fix:
>     
>       fe28fae57a9463fbf ("libbpf: Ensure new BTF objects inherit input endianness")
>     
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> diff --git a/lib/bpf b/lib/bpf
> index 686f600bca59e107..caa17bdcbfc58e68 160000
> --- a/lib/bpf
> +++ b/lib/bpf
> @@ -1 +1 @@
> -Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
> +Subproject commit caa17bdcbfc58e68eaf4d017c058e6577606bf56
> ⬢[acme@toolbox pahole]$
> 
> Ack?
>

Acked-by: Alan Maguire <alan.maguire@oracle.com>

My patch for the same change crossed with your email [1], just ignore
it. Thanks!

Alan

[1]
https://lore.kernel.org/dwarves/20240902141043.177815-1-alan.maguire@oracle.com/T/#u

>>> Separate question, I think pahole supports the shared library version
>>> of libbpf, as an option, is that right? How do you guys handle missing
>>> APIs for distilled BTF in such a case?
>  
>> Good question - at present the distill-related code is conditionally
>> compiled if LIBBPF_MAJOR_VERSION >=1 and LIBBF_MINOR_VERSION >= 5; so if
>> an older shared library libbpf+headers is used, the btf_feature is
>> simply ignored as if we didn't know about it. See [1] for the relevant
>> code in btf_encoder.c. This problem doesn't arise if we're using the
>> synced libbpf.
>  
>> There might be a better way to handle this, but I think that's enough to
>> ensure we avoid compilation failures at least.
> 
> I guess this is good enough,
> 
> - Arnaldo
>  
>> [1]
>> https://github.com/acmel/dwarves/blob/fd14dc67cb6aaead553074afb4a1ddad10209892/btf_encoder.c#L1766

Re: FYI: CI regression on big-endian arch (s390) after recent pahole changes

[Arnaldo Carvalho de Melo]

On Mon, Sep 02, 2024 at 03:59:26PM +0100, Alan Maguire wrote:
> On 02/09/2024 15:08, Arnaldo Carvalho de Melo wrote:
> > On Fri, Aug 30, 2024 at 11:34:40PM +0100, Alan Maguire wrote:
> >> On 30/08/2024 23:20, Andrii Nakryiko wrote:
> >>> On Fri, Aug 30, 2024 at 1:49 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> >>>> On Fri, Aug 30, 2024 at 08:56:08AM -0700, Andrii Nakryiko wrote:
> >>>> +++ b/lib/bpf
> >>>> @@ -1 +1 @@
> >>>> -Subproject commit 6597330c45d185381900037f0130712cd326ae59
> >>>> +Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
> >>>> ⬢[acme@toolbox pahole]$
> > 
> >>>> Right?
> > 
> >>> Yes, and I'm doing another Github sync today.
> > 
> > So, I just commited this locally:
> > 
> > ⬢[acme@toolbox pahole]$ git show
> > commit 5fd558301891d1c0456fcae79798a789b499c1f9 (HEAD -> master)
> > Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Date:   Mon Sep 2 11:05:06 2024 -0300
> > 
> >     libbpf: Sync with master, i.e. what will become 1.5.0
> >     
> >     To pick this distilled BPF fix:
> >     
> >       fe28fae57a9463fbf ("libbpf: Ensure new BTF objects inherit input endianness")
> >     
> >     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > diff --git a/lib/bpf b/lib/bpf
> > index 686f600bca59e107..caa17bdcbfc58e68 160000
> > --- a/lib/bpf
> > +++ b/lib/bpf
> > @@ -1 +1 @@
> > -Subproject commit 686f600bca59e107af4040d0838ca2b02c14ff50
> > +Subproject commit caa17bdcbfc58e68eaf4d017c058e6577606bf56
> > ⬢[acme@toolbox pahole]$
> > 
> > Ack?
> >
> 
> Acked-by: Alan Maguire <alan.maguire@oracle.com>
> 
> My patch for the same change crossed with your email [1], just ignore
> it. Thanks!

I dropped mine and picked yours :-)

Thanks!

- Arnaldo
 
> Alan
> 
> [1]
> https://lore.kernel.org/dwarves/20240902141043.177815-1-alan.maguire@oracle.com/T/#u
> 
> >>> Separate question, I think pahole supports the shared library version
> >>> of libbpf, as an option, is that right? How do you guys handle missing
> >>> APIs for distilled BTF in such a case?
> >  
> >> Good question - at present the distill-related code is conditionally
> >> compiled if LIBBPF_MAJOR_VERSION >=1 and LIBBF_MINOR_VERSION >= 5; so if
> >> an older shared library libbpf+headers is used, the btf_feature is
> >> simply ignored as if we didn't know about it. See [1] for the relevant
> >> code in btf_encoder.c. This problem doesn't arise if we're using the
> >> synced libbpf.
> >  
> >> There might be a better way to handle this, but I think that's enough to
> >> ensure we avoid compilation failures at least.
> > 
> > I guess this is good enough,
> > 
> > - Arnaldo
> >  
> >> [1]
> >> https://github.com/acmel/dwarves/blob/fd14dc67cb6aaead553074afb4a1ddad10209892/btf_encoder.c#L1766