diff mbox series

[bpf-next,v2,3/3] selftests/bpf: add test cases for htab update

Message ID 20220827100134.1621137-3-houtao@huaweicloud.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series [bpf-next,v2,1/3] bpf: Disable preemption when increasing per-cpu map_locked | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter warning Series does not have a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 5 maintainers not CCed: shuah@kernel.org linux-kselftest@vger.kernel.org song@kernel.org martin.lau@linux.dev mykolal@fb.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 81 exceeds 80 columns WARNING: line length of 90 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-6 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 fail Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-9 fail Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-1 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-4 success Logs for llvm-toolchain
bpf/vmtest-bpf-next-VM_Test-5 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for x86_64 with llvm-16

Commit Message

Hou Tao Aug. 27, 2022, 10:01 a.m. UTC 
From: Hou Tao <houtao1@huawei.com>

One test demonstrates the reentrancy of hash map update fails, and
another one shows concureently updates of the same hash map bucket
succeed.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 .../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

Comments

KP Singh Aug. 28, 2022, 10:39 p.m. UTC | #1 
On Sat, Aug 27, 2022 at 11:43 AM Hou Tao <houtao@huaweicloud.com> wrote:
>
> From: Hou Tao <houtao1@huawei.com>
>
> 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@huawei.com>
> ---
>  .../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
>
Hou Tao Aug. 29, 2022, 1:12 a.m. UTC | #2 
Hi,

On 8/29/2022 6:39 AM, KP Singh wrote:
> On Sat, Aug 27, 2022 at 11:43 AM Hou Tao <houtao@huaweicloud.com> wrote:
>> From: Hou Tao <houtao1@huawei.com>
>>
>> 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?
Will elaborate the description.
>
> 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.
Yes. There is a patch 0 for v2, but I forge to send it out.
>
>> succeed.
>>
>> Signed-off-by: Hou Tao <houtao1@huawei.com>
>> ---
>>  .../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
>>
> .
diff mbox series

Patch

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;
+	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) {
+		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;
+}