On Mon, Jan 27, 2025 at 8:22 AM Leon Hwang <leon.hwang@xxxxxxxxx> wrote:
>
> If the arch, like s390x, does not support percpu insn, this case won't
> test global percpu data by checking -EOPNOTSUPP when load prog.
>
> The following APIs have been tested for global percpu data:
> 1. bpf_map__set_initial_value()
> 2. bpf_map__initial_value()
> 3. generated percpu struct pointer that points to internal map's data
> 4. bpf_map__lookup_elem() for global percpu data map
>
> cd tools/testing/selftests/bpf; ./test_progs -t global_percpu_data
> 124 global_percpu_data_init:OK
> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Leon Hwang <leon.hwang@xxxxxxxxx>
> ---
> .../bpf/prog_tests/global_data_init.c | 89 ++++++++++++++++++-
> .../bpf/progs/test_global_percpu_data.c | 21 +++++
> 2 files changed, 109 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/progs/test_global_percpu_data.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/global_data_init.c b/tools/testing/selftests/bpf/prog_tests/global_data_init.c
> index 8466332d7406f..a5d0890444f67 100644
> --- a/tools/testing/selftests/bpf/prog_tests/global_data_init.c
> +++ b/tools/testing/selftests/bpf/prog_tests/global_data_init.c
> @@ -1,5 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0
> #include <test_progs.h>
> +#include "test_global_percpu_data.skel.h"
>
> void test_global_data_init(void)
> {
> @@ -8,7 +9,7 @@ void test_global_data_init(void)
> __u8 *buff = NULL, *newval = NULL;
> struct bpf_object *obj;
> struct bpf_map *map;
> - __u32 duration = 0;
> + __u32 duration = 0;
> size_t sz;
>
> obj = bpf_object__open_file(file, NULL);
> @@ -60,3 +61,89 @@ void test_global_data_init(void)
> free(newval);
> bpf_object__close(obj);
> }
> +
> +void test_global_percpu_data_init(void)
> +{
> + struct test_global_percpu_data *skel = NULL;
> + u64 *percpu_data = NULL;
there is that test_global_percpu_data__percpu type you are declaring
in the BPF skeleton, right? We should try using it here.
And for that array access, we should make sure that it's __aligned(8),
so indexing by CPU index works correctly.
Also, you define per-CPU variable as int, but here it is u64, what's
up with that?
> + struct bpf_map *map;
> + size_t init_data_sz;
> + char buff[128] = {};
> + int init_value = 2;
> + int key, value_sz;
> + int prog_fd, err;
> + int *init_data;
> + int num_cpus;
> +
nit: LIBBPF_OPTS below is variable declaration, so there shouldn't be
an empty line here (and maybe group those int variables a bit more
tightly?)
> + LIBBPF_OPTS(bpf_test_run_opts, topts,
> + .data_in = buff,
> + .data_size_in = sizeof(buff),
> + .repeat = 1,
> + );
> +
> + num_cpus = libbpf_num_possible_cpus();
> + if (!ASSERT_GT(num_cpus, 0, "libbpf_num_possible_cpus"))
> + return;
> +
> + percpu_data = calloc(num_cpus, sizeof(*percpu_data));
> + if (!ASSERT_FALSE(percpu_data == NULL, "calloc percpu_data"))
ASSERT_OK_PTR()
> + return;
> +
> + value_sz = sizeof(*percpu_data) * num_cpus;
> + memset(percpu_data, 0, value_sz);
you calloc()'ed it, it's already zero-initialized
> +
> + skel = test_global_percpu_data__open();
> + if (!ASSERT_OK_PTR(skel, "test_global_percpu_data__open"))
> + goto out;
> +
> + ASSERT_EQ(skel->percpu->percpu_data, -1, "skel->percpu->percpu_data");
> +
> + map = skel->maps.percpu;
> + err = bpf_map__set_initial_value(map, &init_value,
> + sizeof(init_value));
> + if (!ASSERT_OK(err, "bpf_map__set_initial_value"))
> + goto out;
> +
> + init_data = bpf_map__initial_value(map, &init_data_sz);
> + if (!ASSERT_OK_PTR(init_data, "bpf_map__initial_value"))
> + goto out;
> +
> + ASSERT_EQ(*init_data, init_value, "initial_value");
> + ASSERT_EQ(init_data_sz, sizeof(init_value), "initial_value size");
> +
> + if (!ASSERT_EQ((void *) init_data, (void *) skel->percpu, "skel->percpu"))
> + goto out;
> + ASSERT_EQ(skel->percpu->percpu_data, init_value, "skel->percpu->percpu_data");
> +
> + err = test_global_percpu_data__load(skel);
> + if (err == -EOPNOTSUPP)
> + goto out;
> + if (!ASSERT_OK(err, "test_global_percpu_data__load"))
> + goto out;
> +
> + ASSERT_EQ(bpf_map__type(map), BPF_MAP_TYPE_PERCPU_ARRAY,
> + "bpf_map__type");
> +
> + prog_fd = bpf_program__fd(skel->progs.update_percpu_data);
> + err = bpf_prog_test_run_opts(prog_fd, &topts);
at least one of BPF programs (don't remember which one, could be
raw_tp) supports specifying CPU index to run on, it would be nice to
loop over CPUs, triggering BPF program on each one and filling per-CPU
variable with current CPU index. Then we can check that all per-CPU
values have expected values.
> + ASSERT_OK(err, "update_percpu_data");
> + ASSERT_EQ(topts.retval, 0, "update_percpu_data retval");
> +
> + key = 0;
> + err = bpf_map__lookup_elem(map, &key, sizeof(key), percpu_data,
> + value_sz, 0);
> + if (!ASSERT_OK(err, "bpf_map__lookup_elem"))
> + goto out;
> +
> + if (!ASSERT_LT(skel->bss->curr_cpu, num_cpus, "curr_cpu"))
> + goto out;
> + ASSERT_EQ((int) percpu_data[skel->bss->curr_cpu], 1, "percpu_data");
> + if (num_cpus > 1)
> + ASSERT_EQ((int) percpu_data[(skel->bss->curr_cpu+1)%num_cpus],
> + init_value, "init_value");
> +
> +out:
> + test_global_percpu_data__destroy(skel);
> + if (percpu_data)
> + free(percpu_data);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_global_percpu_data.c b/tools/testing/selftests/bpf/progs/test_global_percpu_data.c
> new file mode 100644
> index 0000000000000..731c3214b0bb4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_global_percpu_data.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright Leon Hwang */
> +
> +#include <linux/bpf.h>
> +#include <linux/pkt_cls.h>
> +
> +#include <bpf/bpf_helpers.h>
> +
> +int percpu_data SEC(".percpu") = -1;
> +int curr_cpu;
> +
> +SEC("tc")
> +int update_percpu_data(struct __sk_buff *skb)
> +{
> + curr_cpu = bpf_get_smp_processor_id();
> + percpu_data = 1;
> +
> + return TC_ACT_OK;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.47.1
>
