[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAEf4BzY2nDzT4FjfARiPJdu=G-0uwhxrUHpNrdAEB9NRxu4RqQ@mail.gmail.com>
Date: Tue, 5 Jan 2021 21:26:26 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Sean Young <sean@...s.org>
Cc: Yonghong Song <yhs@...com>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>,
John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>,
Nathan Chancellor <natechancellor@...il.com>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Quentin Monnet <quentin@...valent.com>,
Toke Høiland-Jørgensen <toke@...hat.com>,
linux-doc@...r.kernel.org, Networking <netdev@...r.kernel.org>,
bpf <bpf@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
clang-built-linux <clang-built-linux@...glegroups.com>
Subject: Re: [PATCH v3 4/4] bpf: add tests for ints larger than 128 bits
On Tue, Jan 5, 2021 at 6:45 AM Sean Young <sean@...s.org> wrote:
>
> clang supports arbitrary length ints using the _ExtInt extension. This
> can be useful to hold very large values, e.g. 256 bit or 512 bit types.
>
> Larger types (e.g. 1024 bits) are possible but I am unaware of a use
> case for these.
>
> This requires the _ExtInt extension enabled in clang, which is under
> review.
>
> Link: https://clang.llvm.org/docs/LanguageExtensions.html#extended-integer-types
> Link: https://reviews.llvm.org/D93103
>
> Signed-off-by: Sean Young <sean@...s.org>
> ---
> tools/testing/selftests/bpf/Makefile | 3 +-
> tools/testing/selftests/bpf/prog_tests/btf.c | 3 +-
> .../selftests/bpf/progs/test_btf_extint.c | 50 ++
> tools/testing/selftests/bpf/test_extint.py | 535 ++++++++++++++++++
> 4 files changed, 589 insertions(+), 2 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/progs/test_btf_extint.c
> create mode 100755 tools/testing/selftests/bpf/test_extint.py
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 8c33e999319a..436ad1aed3d9 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -70,7 +70,8 @@ TEST_PROGS := test_kmod.sh \
> test_bpftool_build.sh \
> test_bpftool.sh \
> test_bpftool_metadata.sh \
> - test_xsk.sh
> + test_xsk.sh \
> + test_extint.py
>
> TEST_PROGS_EXTENDED := with_addr.sh \
> with_tunnels.sh \
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index 8ae97e2a4b9d..96a93502cf27 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -4073,6 +4073,7 @@ struct btf_file_test {
> static struct btf_file_test file_tests[] = {
> { .file = "test_btf_haskv.o", },
> { .file = "test_btf_newkv.o", },
> + { .file = "test_btf_extint.o", },
> { .file = "test_btf_nokv.o", .btf_kv_notfound = true, },
> };
>
> @@ -4414,7 +4415,7 @@ static struct btf_raw_test pprint_test_template[] = {
> * will have both int and enum types.
> */
> .raw_types = {
> - /* unsighed char */ /* [1] */
> + /* unsigned char */ /* [1] */
unintentional whitespaces change?
> BTF_TYPE_INT_ENC(NAME_TBD, 0, 0, 8, 1),
> /* unsigned short */ /* [2] */
> BTF_TYPE_INT_ENC(NAME_TBD, 0, 0, 16, 2),
> diff --git a/tools/testing/selftests/bpf/progs/test_btf_extint.c b/tools/testing/selftests/bpf/progs/test_btf_extint.c
> new file mode 100644
> index 000000000000..b0fa9f130dda
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_btf_extint.c
> @@ -0,0 +1,50 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_legacy.h"
> +
> +struct extint {
> + _ExtInt(256) v256;
> + _ExtInt(512) v512;
> +};
> +
> +struct bpf_map_def SEC("maps") btf_map = {
> + .type = BPF_MAP_TYPE_ARRAY,
> + .key_size = sizeof(int),
> + .value_size = sizeof(struct extint),
> + .max_entries = 1,
> +};
> +
> +BPF_ANNOTATE_KV_PAIR(btf_map, int, struct extint);
this is deprecated, don't add new tests using it. Please use BTF-based
map definition instead (see any other selftests).
> +
> +__attribute__((noinline))
> +int test_long_fname_2(void)
> +{
> + struct extint *bi;
> + int key = 0;
> +
> + bi = bpf_map_lookup_elem(&btf_map, &key);
> + if (!bi)
> + return 0;
> +
> + bi->v256 <<= 64;
> + bi->v256 += (_ExtInt(256))0xcafedead;
> + bi->v512 <<= 128;
> + bi->v512 += (_ExtInt(512))0xff00ff00ff00ffull;
> +
> + return 0;
> +}
> +
> +__attribute__((noinline))
> +int test_long_fname_1(void)
> +{
> + return test_long_fname_2();
> +}
> +
> +SEC("dummy_tracepoint")
> +int _dummy_tracepoint(void *arg)
> +{
> + return test_long_fname_1();
> +}
why the chain of test_long_fname functions? Please minimize the test
to only test the essential logic - _ExtInt handling.
> +
> +char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/test_extint.py b/tools/testing/selftests/bpf/test_extint.py
> new file mode 100755
> index 000000000000..86af815a0cf6
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_extint.py
this looks like a total overkill (with a lot of unrelated code) for a
pretty simple test you need to perform. you can run bpftool and get
its output from test_progs with popen().
> @@ -0,0 +1,535 @@
> +#!/usr/bin/python3
[...]
Powered by blists - more mailing lists