BPF List
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Tony Ambardar <tony.ambardar@gmail.com>, bpf@vger.kernel.org
Cc: Andrii Nakryiko <andrii@kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	 Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	Song Liu <song@kernel.org>,
	 Yonghong Song <yonghong.song@linux.dev>,
	John Fastabend <john.fastabend@gmail.com>,
	KP Singh <kpsingh@kernel.org>,
	Stanislav Fomichev <sdf@fomichev.me>, Hao Luo <haoluo@google.com>,
	Jiri Olsa <jolsa@kernel.org>
Subject: Re: [PATCH bpf-next v1] libbpf: ensure new BTF objects inherit input endianness
Date: Fri, 30 Aug 2024 04:15:47 -0700	[thread overview]
Message-ID: <7425efdc2c8f52a780e2b4817e15911f8dd491f2.camel@gmail.com> (raw)
In-Reply-To: <20240830095150.278881-1-tony.ambardar@gmail.com>

[-- 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


  reply	other threads:[~2024-08-30 11:15 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-08-30  0:05 FYI: CI regression on big-endian arch (s390) after recent pahole changes Eduard Zingerman
2024-08-30  1:27 ` Tony Ambardar
2024-08-30  1:40   ` Eduard Zingerman
2024-08-30  6:57     ` Tony Ambardar
2024-08-30  9:13       ` Eduard Zingerman
2024-08-30  9:51         ` [PATCH bpf-next v1] libbpf: ensure new BTF objects inherit input endianness Tony Ambardar
2024-08-30 11:15           ` Eduard Zingerman [this message]
2024-08-30 11:25             ` Alan Maguire
2024-08-30 15:58               ` Andrii Nakryiko
2024-08-30 16:00           ` Andrii Nakryiko
2024-08-30 17:51             ` Andrii Nakryiko
2024-08-30  2:49 ` FYI: CI regression on big-endian arch (s390) after recent pahole changes Song Liu
2024-08-30  9:21   ` Eduard Zingerman
2024-08-30 10:05     ` Alan Maguire
2024-08-30 10:07       ` Eduard Zingerman
2024-08-30 13:19       ` Arnaldo Carvalho de Melo
2024-08-30 15:56         ` Andrii Nakryiko
2024-08-30 20:49           ` Arnaldo Carvalho de Melo
2024-08-30 22:20             ` Andrii Nakryiko
2024-08-30 22:34               ` Alan Maguire
2024-08-30 23:30                 ` Andrii Nakryiko
2024-09-02 13:06                   ` Alan Maguire
2024-09-02 14:08                 ` Arnaldo Carvalho de Melo
2024-09-02 14:59                   ` Alan Maguire
2024-09-02 18:44                     ` Arnaldo Carvalho de Melo
2024-08-30 22:22             ` Alan Maguire

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7425efdc2c8f52a780e2b4817e15911f8dd491f2.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=tony.ambardar@gmail.com \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox