On Sat, Aug 27, 2022 at 11:43 AM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote:
>
> From: Hou Tao <houtao1@xxxxxxxxxx>
>
> One test demonstrates the reentrancy of hash map update fails, and
> another one shows concureently updates of the same hash map bucket
"concurrent updates of the same hashmap"
This is just a description of what the test does but not why?
What's the expected behaviour? Was it broken?
I think your whole series will benefit from a cover letter to explain
the stuff you are fixing.
> succeed.
>
> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
> ---
> .../selftests/bpf/prog_tests/htab_update.c | 126 ++++++++++++++++++
> .../testing/selftests/bpf/progs/htab_update.c | 29 ++++
> 2 files changed, 155 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/htab_update.c
> create mode 100644 tools/testing/selftests/bpf/progs/htab_update.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/htab_update.c b/tools/testing/selftests/bpf/prog_tests/htab_update.c
> new file mode 100644
> index 000000000000..e2a4034daa79
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/htab_update.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2022. Huawei Technologies Co., Ltd */
> +#define _GNU_SOURCE
> +#include <sched.h>
> +#include <stdbool.h>
> +#include <test_progs.h>
> +#include "htab_update.skel.h"
> +
> +struct htab_update_ctx {
> + int fd;
> + int loop;
> + bool stop;
> +};
> +
> +static void test_reenter_update(void)
> +{
> + struct htab_update *skel;
> + unsigned int key, value;
> + int err;
> +
> + skel = htab_update__open();
> + if (!ASSERT_OK_PTR(skel, "htab_update__open"))
> + return;
> +
> + /* lookup_elem_raw() may be inlined and find_kernel_btf_id() will return -ESRCH */
> + bpf_program__set_autoload(skel->progs.lookup_elem_raw, true);
> + err = htab_update__load(skel);
> + if (!ASSERT_TRUE(!err || err == -ESRCH, "htab_update__load") || err)
> + goto out;
> +
> + skel->bss->pid = getpid();
> + err = htab_update__attach(skel);
> + if (!ASSERT_OK(err, "htab_update__attach"))
> + goto out;
> +
> + /* Will trigger the reentrancy of bpf_map_update_elem() */
> + key = 0;
> + value = 0;
nit: just move these initializations to the top.
> + err = bpf_map_update_elem(bpf_map__fd(skel->maps.htab), &key, &value, 0);
> + if (!ASSERT_OK(err, "add element"))
> + goto out;
> +
> + ASSERT_EQ(skel->bss->update_err, -EBUSY, "no reentrancy");
> +out:
> + htab_update__destroy(skel);
> +}
> +
> +static void *htab_update_thread(void *arg)
> +{
> + struct htab_update_ctx *ctx = arg;
> + cpu_set_t cpus;
> + int i;
> +
> + /* Pin on CPU 0 */
> + CPU_ZERO(&cpus);
> + CPU_SET(0, &cpus);
> + pthread_setaffinity_np(pthread_self(), sizeof(cpus), &cpus);
> +
> + i = 0;
> + while (i++ < ctx->loop && !ctx->stop) {
nit: for loop?
> + unsigned int key = 0, value = 0;
> + int err;
> +
> + err = bpf_map_update_elem(ctx->fd, &key, &value, 0);
> + if (err) {
> + ctx->stop = true;
> + return (void *)(long)err;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static void test_concurrent_update(void)
> +{
> + struct htab_update_ctx ctx;
> + struct htab_update *skel;
> + unsigned int i, nr;
> + pthread_t *tids;
> + int err;
> +
> + skel = htab_update__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "htab_update__open_and_load"))
> + return;
> +
> + ctx.fd = bpf_map__fd(skel->maps.htab);
> + ctx.loop = 1000;
> + ctx.stop = false;
> +
> + nr = 4;
> + tids = calloc(nr, sizeof(*tids));
> + if (!ASSERT_NEQ(tids, NULL, "no mem"))
> + goto out;
> +
> + for (i = 0; i < nr; i++) {
> + err = pthread_create(&tids[i], NULL, htab_update_thread, &ctx);
> + if (!ASSERT_OK(err, "pthread_create")) {
> + unsigned int j;
> +
> + ctx.stop = true;
> + for (j = 0; j < i; j++)
> + pthread_join(tids[j], NULL);
> + goto out;
> + }
> + }
> +
> + for (i = 0; i < nr; i++) {
> + void *thread_err = NULL;
> +
> + pthread_join(tids[i], &thread_err);
> + ASSERT_EQ(thread_err, NULL, "update error");
> + }
> +
> +out:
> + if (tids)
> + free(tids);
> + htab_update__destroy(skel);
> +}
> +
> +void test_htab_update(void)
> +{
> + if (test__start_subtest("reenter_update"))
> + test_reenter_update();
> + if (test__start_subtest("concurrent_update"))
> + test_concurrent_update();
> +}
> diff --git a/tools/testing/selftests/bpf/progs/htab_update.c b/tools/testing/selftests/bpf/progs/htab_update.c
> new file mode 100644
> index 000000000000..7481bb30b29b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/htab_update.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2022. Huawei Technologies Co., Ltd */
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_HASH);
> + __uint(max_entries, 1);
> + __uint(key_size, sizeof(__u32));
> + __uint(value_size, sizeof(__u32));
> +} htab SEC(".maps");
> +
> +int pid = 0;
> +int update_err = 0;
> +
> +SEC("?fentry/lookup_elem_raw")
> +int lookup_elem_raw(void *ctx)
> +{
> + __u32 key = 0, value = 1;
> +
> + if ((bpf_get_current_pid_tgid() >> 32) != pid)
> + return 0;
> +
> + update_err = bpf_map_update_elem(&htab, &key, &value, 0);
> + return 0;
> +}
> --
> 2.29.2
>
