[PATCH bpf-next] selftests/bpf: check if distilled base inherits source endianness

[PATCH bpf-next] selftests/bpf: check if distilled base inherits source endianness

[Eduard Zingerman]

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] selftests/bpf: check if distilled base inherits source endianness

[Eduard Zingerman]

On Fri, 2024-08-30 at 10:34 -0700, Eduard Zingerman wrote:
> 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>
> ---

Note: this would fail on CI unless [1] is applied.

[1] https://lore.kernel.org/bpf/20240830095150.278881-1-tony.ambardar@gmail.com/

Re: [PATCH bpf-next] selftests/bpf: check if distilled base inherits source endianness

[Andrii Nakryiko]

On Fri, Aug 30, 2024 at 10:34 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> 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());

removed printf

> +       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"))

moved check before set_endianness

applied to bpf-next after Tony's endianness fix, thanks!

> +               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);

[...]

Re: [PATCH bpf-next] selftests/bpf: check if distilled base inherits source endianness

[Alan Maguire]

On 30/08/2024 18:34, Eduard Zingerman wrote:
> 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>

One small thing below, but

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

Thanks!

> ---
>  .../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();


nit: I think you could avoid the need for is_host_big_endian() by doing
this:

inverse_endianness = btf__endianness(base) == BTF_LITTLE_ENDIAN ?
		     BTF_BIG_ENDIAN : BTF_LITTLE_ENDIAN;


> +	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();
>  }

Re: [PATCH bpf-next] selftests/bpf: check if distilled base inherits source endianness

[Eduard Zingerman]

On Fri, 2024-08-30 at 10:51 -0700, Andrii Nakryiko wrote:

[...]

> > +/* 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());
> 
> removed printf

:facepalm:, sorry, my bad.

[...]

Re: [PATCH bpf-next] selftests/bpf: check if distilled base inherits source endianness

[Andrii Nakryiko]

On Fri, Aug 30, 2024 at 10:54 AM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> On 30/08/2024 18:34, Eduard Zingerman wrote:
> > 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>
>
> One small thing below, but
>
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> Tested-by: Alan Maguire <alan.maguire@oracle.com>

will add and force-push, thanks

>
> Thanks!
>
> > ---
> >  .../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();
>
>
> nit: I think you could avoid the need for is_host_big_endian() by doing
> this:
>
> inverse_endianness = btf__endianness(base) == BTF_LITTLE_ENDIAN ?
>                      BTF_BIG_ENDIAN : BTF_LITTLE_ENDIAN;

good point, I'll fix it up

>
>
> > +     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();
> >  }

Re: [PATCH bpf-next] selftests/bpf: check if distilled base inherits source endianness

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Fri, 30 Aug 2024 10:34:06 -0700 you wrote:
> 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(+)

Here is the summary with links:
  - [bpf-next] selftests/bpf: check if distilled base inherits source endianness
    https://git.kernel.org/bpf/bpf-next/c/217c4dfec4a6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html