On Fri, Apr 28, 2023 at 3:28 PM JP Kobryn <inwardvessel@xxxxxxxxx> wrote:
>
> This patch adds test coverage for resizing datasec maps. There are two
> tests which run a bpf program that sums the elements in the datasec array.
> After the datasec array is resized, each elements is assigned a value of 1
> so that the sum will be equal to the length of the array. Assertions are
> done to verify this. The first test attempts to resize to an aligned
> length while the second attempts to resize to a mis-aligned length where
> rounding up is expected to occur. The third test attempts to resize maps
> that do not meet the necessary criteria and assertions are done to confirm
> error codes are returned.
>
> Signed-off-by: JP Kobryn <inwardvessel@xxxxxxxxx>
> ---
> .../bpf/prog_tests/global_map_resize.c | 187 ++++++++++++++++++
> .../bpf/progs/test_global_map_resize.c | 33 ++++
> 2 files changed, 220 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/global_map_resize.c
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_map_resize.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/global_map_resize.c b/tools/testing/selftests/bpf/prog_tests/global_map_resize.c
> new file mode 100644
> index 000000000000..f38df37664a7
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/global_map_resize.c
> @@ -0,0 +1,187 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +
> +#include <errno.h>
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +
> +#include "test_global_map_resize.skel.h"
> +#include "test_progs.h"
> +
> +static void run_program(void)
> +{
> + (void)syscall(__NR_getpid);
> +}
> +
> +static int setup(struct test_global_map_resize **skel)
> +{
> + if (!skel)
> + return -1;
> +
> + *skel = test_global_map_resize__open();
> + if (!ASSERT_OK_PTR(skel, "test_global_map_resize__open"))
> + return -1;
> +
> + (*skel)->rodata->pid = getpid();
> +
this simple logic is also so common in lots of tests, that it doesn't
make sense to have dedicated setup() routine for that (just more
jumping around the code to know what's going on). Please inline in
respective callers.
> + return 0;
> +}
> +
> +static void teardown(struct test_global_map_resize **skel)
> +{
> + if (skel && *skel)
> + test_global_map_resize__destroy(*skel);
skeleton's destroy handles NULL, so this whole teardown() is not
necessary, just call destroy() directly in respective functions
> +}
> +
> +static int resize_test(struct test_global_map_resize *skel,
> + __u32 element_sz, __u32 desired_sz)
> +{
> + int ret = 0;
> + struct bpf_map *map;
> + __u32 initial_sz, actual_sz;
> + size_t nr_elements;
> + int *initial_val;
> + size_t initial_val_sz;
> +
> + map = skel->maps.data_my_array;
> +
> + initial_sz = bpf_map__value_size(map);
> + ASSERT_EQ(initial_sz, element_sz, "initial size");
> +
> + /* round up desired size to align with element size */
> + desired_sz = roundup(desired_sz, element_sz);
> + ret = bpf_map__set_value_size(map, desired_sz);
> + if (!ASSERT_OK(ret, "bpf_map__set_value_size"))
> + return ret;
> +
> + /* refresh map pointer to avoid invalidation issues */
> + map = skel->maps.data_my_array;
> +
> + actual_sz = bpf_map__value_size(map);
> + ASSERT_EQ(actual_sz, desired_sz, "resize");
> +
> + /* set the expected number of elements based on the resized array */
> + nr_elements = roundup(actual_sz, element_sz) / element_sz;
> + skel->rodata->n = nr_elements;
> +
> + /* create array for initial map value */
> + initial_val_sz = element_sz * nr_elements;
> + initial_val = malloc(initial_val_sz);
> + if (!ASSERT_OK_PTR(initial_val, "malloc initial_val")) {
> + ret = -ENOMEM;
> +
> + goto cleanup;
> + }
> +
> + /* fill array with ones */
> + for (int i = 0; i < nr_elements; ++i)
> + initial_val[i] = 1;
> +
> + /* set initial value */
> + ASSERT_EQ(initial_val_sz, actual_sz, "initial value size");
> +
> + ret = bpf_map__set_initial_value(map, initial_val, initial_val_sz);
> + if (!ASSERT_OK(ret, "bpf_map__set_initial_val"))
> + goto cleanup;
it would be good to demonstrate that it's still possible to use BPF
skeleton to initialize everything, you'll just need to reassign
pointer:
skel->data_my_array = bpf_map__initial_value(skel->maps.data_my_array, &new_sz);
skel->data_my_array.my_var[i] = 123;
can you please add this case as well?
> +
> + ret = test_global_map_resize__load(skel);
> + if (!ASSERT_OK(ret, "test_global_map_resize__load"))
> + goto cleanup;
> +
> + ret = test_global_map_resize__attach(skel);
> + if (!ASSERT_OK(ret, "test_global_map_resize__attach"))
> + goto cleanup;
> +
> + /* run the bpf program which will sum the contents of the array */
> + run_program();
> +
> + if (!ASSERT_EQ(skel->bss->sum, nr_elements, "sum"))
> + goto cleanup;
> +
> +cleanup:
> + if (initial_val)
> + free(initial_val);
> +
> + return ret;
> +}
> +
> +static void global_map_resize_aligned_subtest(void)
> +{
> + struct test_global_map_resize *skel;
> + const __u32 element_sz = (__u32)sizeof(int);
> + const __u32 desired_sz = (__u32)sysconf(_SC_PAGE_SIZE) * 2;
> +
> + /* preliminary check that desired_sz aligns with element_sz */
> + if (!ASSERT_EQ(desired_sz % element_sz, 0, "alignment"))
> + return;
> +
> + if (setup(&skel))
> + goto teardown;
> +
> + if (resize_test(skel, element_sz, desired_sz))
> + goto teardown;
> +
> +teardown:
> + teardown(&skel);
> +}
> +
> +static void global_map_resize_roundup_subtest(void)
> +{
> + struct test_global_map_resize *skel;
> + const __u32 element_sz = (__u32)sizeof(int);
> + /* set desired size a fraction of element size beyond an aligned size */
> + const __u32 desired_sz = (__u32)sysconf(_SC_PAGE_SIZE) * 2 + element_sz / 2;
> +
> + /* preliminary check that desired_sz does NOT align with element_sz */
> + if (!ASSERT_NEQ(desired_sz % element_sz, 0, "alignment"))
> + return;
> +
> + if (setup(&skel))
> + goto teardown;
> +
> + if (resize_test(skel, element_sz, desired_sz))
> + goto teardown;
> +
> +teardown:
> + teardown(&skel);
> +}
> +
> +static void global_map_resize_invalid_subtest(void)
> +{
> + int err;
> + struct test_global_map_resize *skel;
> + struct bpf_map *map;
> + const __u32 desired_sz = 8192;
> +
> + if (setup(&skel))
> + goto teardown;
> +
> + /* attempt to resize a global datasec map which is an array
> + * BUT is with a var in same datasec
> + */
> + map = skel->maps.data_my_array_and_var;
> + err = bpf_map__set_value_size(map, desired_sz);
> + if (!ASSERT_EQ(err, -EINVAL, "bpf_map__set_value_size"))
> + goto teardown;
> +
> + /* attempt to resize a global datasec map which is NOT an array */
> + map = skel->maps.data_my_non_array;
> + err = bpf_map__set_value_size(map, desired_sz);
> + if (!ASSERT_EQ(err, -EINVAL, "bpf_map__set_value_size"))
> + goto teardown;
> +
> +teardown:
> + teardown(&skel);
> +}
> +
> +void test_global_map_resize(void)
> +{
> + if (test__start_subtest("global_map_resize_aligned"))
> + global_map_resize_aligned_subtest();
> +
> + if (test__start_subtest("global_map_resize_roundup"))
> + global_map_resize_roundup_subtest();
> +
> + if (test__start_subtest("global_map_resize_invalid"))
> + global_map_resize_invalid_subtest();
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_global_map_resize.c b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
> new file mode 100644
> index 000000000000..cffbba1b6020
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_global_map_resize.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +const volatile pid_t pid;
> +const volatile size_t n;
> +
> +int my_array[1] SEC(".data.my_array");
> +
> +int my_array_with_neighbor[1] SEC(".data.my_array_and_var");
> +int my_var_with_neighbor SEC(".data.my_array_and_var");
> +
> +int my_non_array SEC(".data.my_non_array");
> +
> +int sum = 0;
> +
> +SEC("tp/syscalls/sys_enter_getpid")
> +int array_sum(void *ctx)
> +{
> + if (pid != (bpf_get_current_pid_tgid() >> 32))
> + return 0;
> +
> + sum = 0;
> +
> + for (size_t i = 0; i < n; ++i)
> + sum += my_array[i];
> +
> + return 0;
> +}
> --
> 2.40.0
>
