On Mon, Feb 8, 2021 at 10:44 PM Dmitrii Banshchikov <me@xxxxxxxxxxxxx> wrote:
>
> test_global_func9 - check valid pointer's scenarios
> test_global_func10 - check that a smaller type cannot be passed as a
> larger one
> test_global_func11 - check that CTX pointer cannot be passed
> test_global_func12 - check access to a null pointer
> test_global_func13 - check access to an arbitrary pointer value
> test_global_func14 - check that an opaque pointer cannot be passed
> test_global_func15 - check that a variable has an unknown value after
> it was passed to a global function by pointer
> test_global_func16 - check access to uninitialized stack memory
>
> test_global_func_args - check read and write operations through a pointer
>
> Signed-off-by: Dmitrii Banshchikov <me@xxxxxxxxxxxxx>
> ---
> v1 -> v2:
> - Test pointer to a global variable, array, enum, int
> - Test reading / writing values by pointers in global functions
>
Some minor needs, but overall it looks great!
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> .../bpf/prog_tests/global_func_args.c | 56 ++++++++
> .../bpf/prog_tests/test_global_funcs.c | 8 ++
> .../selftests/bpf/progs/test_global_func10.c | 29 ++++
> .../selftests/bpf/progs/test_global_func11.c | 19 +++
> .../selftests/bpf/progs/test_global_func12.c | 21 +++
> .../selftests/bpf/progs/test_global_func13.c | 24 ++++
> .../selftests/bpf/progs/test_global_func14.c | 21 +++
> .../selftests/bpf/progs/test_global_func15.c | 22 +++
> .../selftests/bpf/progs/test_global_func16.c | 22 +++
> .../selftests/bpf/progs/test_global_func9.c | 132 ++++++++++++++++++
> .../bpf/progs/test_global_func_args.c | 79 +++++++++++
> 11 files changed, 433 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/global_func_args.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_func10.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_func11.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_func12.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_func13.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_func14.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_func15.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_func16.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_func9.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_func_args.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/global_func_args.c b/tools/testing/selftests/bpf/prog_tests/global_func_args.c
> new file mode 100644
> index 000000000000..643355e3358f
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/global_func_args.c
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "test_progs.h"
> +#include "network_helpers.h"
> +
> +static void test_global_func_args0(struct bpf_object *obj, __u32 duration)
I'd just add a single `static int duration;` at the top of the file
and forget about it.
> +{
> + int err, i, map_fd, actual_value;
> +
> + map_fd = bpf_find_map(__func__, obj, "values");
> + if (CHECK_FAIL(map_fd < 0))
no CHECK_FAIL, please use CHECK or ASSERT_XXX variations. CHECK_FAIL
leaves no trace when debugging, making life unnecessarily hard.
> + return;
> +
> + struct {
> + const char *descr;
> + int expected_value;
> + } tests[] = {
> + {"passing NULL pointer", 0},
> + {"returning value", 1},
> + {"reading local variable", 100 },
> + {"writing local variable", 101 },
> + {"reading global variable", 42 },
> + {"writing global variable", 43 },
> + {"writing to pointer-to-pointer", 1 },
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(tests); ++i) {
> + const int expected_value = tests[i].expected_value;
> +
> + err = bpf_map_lookup_elem(map_fd, &i, &actual_value);
> +
> + CHECK(err || actual_value != expected_value, tests[i].descr,
> + "err %d result %d expected %d\n", err, actual_value, expected_value);
> + }
> +}
> +
[...]
> diff --git a/tools/testing/selftests/bpf/progs/test_global_func_args.c b/tools/testing/selftests/bpf/progs/test_global_func_args.c
> new file mode 100644
> index 000000000000..c8e47e120bf6
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_global_func_args.c
> @@ -0,0 +1,79 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/bpf.h>
> +
> +#include <bpf/bpf_helpers.h>
> +
> +struct S {
> + int v;
> +};
> +
> +static struct S global_variable;
this can get optimized away. Just drop `static` to make it global, or
otherwise you'd need `static volatile`
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_ARRAY);
> + __uint(max_entries, 7);
> + __type(key, __u32);
> + __type(value, int);
> +} values SEC(".maps");
> +
