diff mbox series

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

Message ID 20220829023709.1958204-4-houtao@huaweicloud.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series fixes for concurrent htab updates | 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 success Series has 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-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-6 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-9 fail Logs for test_progs 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-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
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-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

Commit Message

Hou Tao Aug. 29, 2022, 2:37 a.m. UTC 
From: Hou Tao <houtao1@huawei.com>

One test demonstrates the reentrancy of hash map update on the same
bucket should fail, and another one shows concureently updates of
the same hash map bucket should succeed and not fail due to
the reentrancy checking for bucket lock.

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

Daniel Borkmann Aug. 29, 2022, 3:51 p.m. UTC | #1 
On 8/29/22 4:37 AM, Hou Tao wrote:
> From: Hou Tao <houtao1@huawei.com>
> 
> One test demonstrates the reentrancy of hash map update on the same
> bucket should fail, and another one shows concureently updates of
> the same hash map bucket should succeed and not fail due to
> the reentrancy checking for bucket lock.
> 
> 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..2bc85f4814f4
> --- /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;
> +
> +	/* Pinned 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;
> +}

The test fails the CI on s390x, link:

https://github.com/kernel-patches/bpf/runs/8062792183?check_suite_focus=true

   All error logs:
   test_reenter_update:PASS:htab_update__open 0 nsec
   test_reenter_update:PASS:htab_update__load 0 nsec
   libbpf: prog 'lookup_elem_raw': failed to attach: ERROR: strerror_r(-524)=22
   libbpf: prog 'lookup_elem_raw': failed to auto-attach: -524
   test_reenter_update:FAIL:htab_update__attach unexpected error: -524 (errno 524)
   #83/1    htab_update/reenter_update:FAIL
   #83      htab_update:FAIL
   Summary: 189/973 PASSED, 27 SKIPPED, 1 FAILED

You'd have to add this to the s390 denylist, see also tools/testing/selftests/bpf/DENYLIST.s390x .
Hou Tao Aug. 30, 2022, 1:47 a.m. UTC | #2 
Hi,

On 8/29/2022 11:51 PM, Daniel Borkmann wrote:
> On 8/29/22 4:37 AM, Hou Tao wrote:
>> From: Hou Tao <houtao1@huawei.com>
>>
>> One test demonstrates the reentrancy of hash map update on the same
>> bucket should fail, and another one shows concureently updates of
>> the same hash map bucket should succeed and not fail due to
>> the reentrancy checking for bucket lock.
SNIP
> The test fails the CI on s390x, link:
>
> https://github.com/kernel-patches/bpf/runs/8062792183?check_suite_focus=true
>
>   All error logs:
>   test_reenter_update:PASS:htab_update__open 0 nsec
>   test_reenter_update:PASS:htab_update__load 0 nsec
>   libbpf: prog 'lookup_elem_raw': failed to attach: ERROR: strerror_r(-524)=22
>   libbpf: prog 'lookup_elem_raw': failed to auto-attach: -524
>   test_reenter_update:FAIL:htab_update__attach unexpected error: -524 (errno 524)
>   #83/1    htab_update/reenter_update:FAIL
>   #83      htab_update:FAIL
>   Summary: 189/973 PASSED, 27 SKIPPED, 1 FAILED
>
> You'd have to add this to the s390 denylist, see also
> tools/testing/selftests/bpf/DENYLIST.s390x .
> .
Should I post it in a single patch, or send v4 to include it ?
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..2bc85f4814f4
--- /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;
+
+	/* Pinned 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;
+}