[RFC PATCH] bpf: test: parallel execution of basic tests

[RFC PATCH] bpf: test: parallel execution of basic tests

[Dmitry Antipov]

Since an amount of basic tests is large enough, implement
simple workqueue-based parallel execution, with an option
to fallback to similar-to-legacy behavior.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 lib/test_bpf.c | 160 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 104 insertions(+), 56 deletions(-)

diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index af6f3340c034..270899aed1e7 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -13,6 +13,7 @@
 #include <linux/bpf.h>
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
+#include <linux/workqueue.h>
 #include <linux/if_vlan.h>
 #include <linux/prandom.h>
 #include <linux/highmem.h>
@@ -24,6 +25,7 @@
 #define MAX_DATA	128
 #define MAX_INSNS	512
 #define MAX_K		0xffffFFFF
+#define MAX_OUTPUT	256
 
 /* Few constants used to init test 'skb' */
 #define SKB_TYPE	3
@@ -83,6 +85,10 @@ struct bpf_test {
 	__u8 frag_data[MAX_DATA];
 	int stack_depth; /* for eBPF only, since tests don't call verifier */
 	int nr_testruns; /* Custom run count, defaults to MAX_TESTRUNS if 0 */
+	struct work_struct work;
+	char output[MAX_OUTPUT];
+	int nbytes;
+	int index;
 };
 
 /* Large test cases need separate allocation and fill handler. */
@@ -3174,6 +3180,7 @@ static int bpf_fill_staggered_jsle32_reg(struct bpf_test *self)
 	return __bpf_fill_staggered_jumps(self, &jmp, -1, -1);
 }
 
+static struct workqueue_struct *bpf_wq;
 
 static struct bpf_test tests[] = {
 	{
@@ -14974,6 +14981,18 @@ static void release_test_data(const struct bpf_test *test, void *data)
 		kfree_skb(data);
 }
 
+static void __printf(2, 3)
+test_log(struct bpf_test *test, const char *fmt, ...)
+{
+	va_list args;
+
+	va_start(args, fmt);
+	test->nbytes += vscnprintf(test->output + test->nbytes,
+				   sizeof(test->output) - test->nbytes,
+				   fmt, args);
+	va_end(args);
+}
+
 static int filter_length(int which)
 {
 	struct sock_filter *fp;
@@ -15000,7 +15019,8 @@ static void *filter_pointer(int which)
 
 static struct bpf_prog *generate_filter(int which, int *err)
 {
-	__u8 test_type = tests[which].aux & TEST_TYPE_MASK;
+	struct bpf_test *test = &tests[which];
+	__u8 test_type = test->aux & TEST_TYPE_MASK;
 	unsigned int flen = filter_length(which);
 	void *fptr = filter_pointer(which);
 	struct sock_fprog_kern fprog;
@@ -15012,14 +15032,14 @@ static struct bpf_prog *generate_filter(int which, int *err)
 		fprog.len = flen;
 
 		*err = bpf_prog_create(&fp, &fprog);
-		if (tests[which].aux & FLAG_EXPECTED_FAIL) {
-			if (*err == tests[which].expected_errcode) {
-				pr_cont("PASS\n");
+		if (test->aux & FLAG_EXPECTED_FAIL) {
+			if (*err == test->expected_errcode) {
+				test_log(test, "PASS\n");
 				/* Verifier rejected filter as expected. */
 				*err = 0;
 				return NULL;
 			} else {
-				pr_cont("UNEXPECTED_PASS\n");
+				test_log(test, "UNEXPECTED_PASS\n");
 				/* Verifier didn't reject the test that's
 				 * bad enough, just return!
 				 */
@@ -15028,8 +15048,8 @@ static struct bpf_prog *generate_filter(int which, int *err)
 			}
 		}
 		if (*err) {
-			pr_cont("FAIL to prog_create err=%d len=%d\n",
-				*err, fprog.len);
+			test_log(test, "FAIL to prog_create err=%d len=%d\n",
+				 *err, fprog.len);
 			return NULL;
 		}
 		break;
@@ -15037,7 +15057,7 @@ static struct bpf_prog *generate_filter(int which, int *err)
 	case INTERNAL:
 		fp = bpf_prog_alloc(bpf_prog_size(flen), 0);
 		if (fp == NULL) {
-			pr_cont("UNEXPECTED_FAIL no memory left\n");
+			test_log(test, "UNEXPECTED_FAIL no memory left\n");
 			*err = -ENOMEM;
 			return NULL;
 		}
@@ -15046,8 +15066,8 @@ static struct bpf_prog *generate_filter(int which, int *err)
 		/* Type doesn't really matter here as long as it's not unspec. */
 		fp->type = BPF_PROG_TYPE_SOCKET_FILTER;
 		memcpy(fp->insnsi, fptr, fp->len * sizeof(struct bpf_insn));
-		fp->aux->stack_depth = tests[which].stack_depth;
-		fp->aux->verifier_zext = !!(tests[which].aux &
+		fp->aux->stack_depth = test->stack_depth;
+		fp->aux->verifier_zext = !!(test->aux &
 					    FLAG_VERIFIER_ZEXT);
 
 		/* We cannot error here as we don't need type compatibility
@@ -15055,7 +15075,7 @@ static struct bpf_prog *generate_filter(int which, int *err)
 		 */
 		fp = bpf_prog_select_runtime(fp, err);
 		if (*err) {
-			pr_cont("FAIL to select_runtime err=%d\n", *err);
+			test_log(test, "FAIL to select_runtime err=%d\n", *err);
 			return NULL;
 		}
 		break;
@@ -15125,7 +15145,7 @@ static int run_one(const struct bpf_prog *fp, struct bpf_test *test)
 
 		data = generate_test_data(test, i);
 		if (!data && !(test->aux & FLAG_NO_DATA)) {
-			pr_cont("data generation failed ");
+			test_log(test, "data generation failed ");
 			err_cnt++;
 			break;
 		}
@@ -15133,12 +15153,12 @@ static int run_one(const struct bpf_prog *fp, struct bpf_test *test)
 		release_test_data(test, data);
 
 		if (ret == test->test[i].result) {
-			pr_cont("%lld ", duration);
+			test_log(test, "%lld ", duration);
 		} else {
 			s32 res = test->test[i].result;
 
-			pr_cont("ret %d != %d (%#x != %#x)",
-				ret, res, ret, res);
+			test_log(test, "ret %d != %d (%#x != %#x)",
+				 ret, res, ret, res);
 			err_cnt++;
 		}
 	}
@@ -15155,6 +15175,9 @@ module_param(test_id, int, 0);
 static int test_range[2] = { 0, INT_MAX };
 module_param_array(test_range, int, NULL, 0);
 
+static bool legacy;
+module_param(legacy, bool, 0);
+
 static bool exclude_test(int test_id)
 {
 	return test_id < test_range[0] || test_id > test_range[1];
@@ -15345,65 +15368,80 @@ static __init int test_skb_segment(void)
 	return err_cnt ? -EINVAL : 0;
 }
 
-static __init int test_bpf(void)
+static atomic_t err_cnt = ATOMIC_INIT(0), pass_cnt = ATOMIC_INIT(0);
+static atomic_t jit_cnt = ATOMIC_INIT(0), run_cnt = ATOMIC_INIT(0);
+
+static void test_bpf_work_handler(struct work_struct *work)
 {
-	int i, err_cnt = 0, pass_cnt = 0;
-	int jit_cnt = 0, run_cnt = 0;
+	struct bpf_test *test = container_of(work, struct bpf_test, work);
+	struct bpf_prog *fp;
+	int err;
 
-	for (i = 0; i < ARRAY_SIZE(tests); i++) {
-		struct bpf_prog *fp;
-		int err;
+	test_log(test, "#%d %s ", test->index, test->descr);
 
-		cond_resched();
-		if (exclude_test(i))
-			continue;
+	fp = generate_filter(test->index, &err);
 
-		pr_info("#%d %s ", i, tests[i].descr);
+	if (test->fill_helper) {
+		kfree(test->u.ptr.insns);
+		test->u.ptr.insns = NULL;
+	}
 
-		if (tests[i].fill_helper &&
-		    tests[i].fill_helper(&tests[i]) < 0) {
-			pr_cont("FAIL to prog_fill\n");
-			continue;
+	if (fp == NULL) {
+		if (err == 0) {
+			atomic_inc(&pass_cnt);
+			return;
 		}
+		atomic_inc(&err_cnt);
+		return;
+	}
 
-		fp = generate_filter(i, &err);
+	test_log(test, "jited:%u ", fp->jited);
 
-		if (tests[i].fill_helper) {
-			kfree(tests[i].u.ptr.insns);
-			tests[i].u.ptr.insns = NULL;
-		}
+	atomic_inc(&run_cnt);
+	if (fp->jited)
+		atomic_inc(&jit_cnt);
 
-		if (fp == NULL) {
-			if (err == 0) {
-				pass_cnt++;
-				continue;
-			}
-			err_cnt++;
-			continue;
-		}
+	err = run_one(fp, test);
+	release_filter(fp, test->index);
 
-		pr_cont("jited:%u ", fp->jited);
+	if (err) {
+		test_log(test, "FAIL (%d times)\n", err);
+		atomic_inc(&err_cnt);
+	} else {
+		test_log(test, "PASS\n");
+		atomic_inc(&pass_cnt);
+	}
 
-		run_cnt++;
-		if (fp->jited)
-			jit_cnt++;
+	pr_info("%s", test->output);
+}
 
-		err = run_one(fp, &tests[i]);
-		release_filter(fp, i);
+static __init int test_bpf(void)
+{
+	int i;
 
-		if (err) {
-			pr_cont("FAIL (%d times)\n", err);
-			err_cnt++;
-		} else {
-			pr_cont("PASS\n");
-			pass_cnt++;
+	for (i = 0; i < ARRAY_SIZE(tests); i++) {
+		if (exclude_test(i))
+			continue;
+
+		if (tests[i].fill_helper &&
+		    tests[i].fill_helper(&tests[i]) < 0) {
+			pr_err("#%d %s: FAIL to setup helper\n",
+			       i, tests[i].descr);
+			continue;
 		}
+
+		INIT_WORK(&tests[i].work, test_bpf_work_handler);
+		tests[i].index = i;
+		queue_work(bpf_wq, &tests[i].work);
 	}
 
+	flush_workqueue(bpf_wq);
+
 	pr_info("Summary: %d PASSED, %d FAILED, [%d/%d JIT'ed]\n",
-		pass_cnt, err_cnt, jit_cnt, run_cnt);
+		atomic_read(&pass_cnt), atomic_read(&err_cnt),
+		atomic_read(&jit_cnt), atomic_read(&run_cnt));
 
-	return err_cnt ? -EINVAL : 0;
+	return atomic_read(&err_cnt) ? -EINVAL : 0;
 }
 
 struct tail_call_test {
@@ -15901,6 +15939,14 @@ static int __init test_bpf_init(void)
 	if (ret < 0)
 		return ret;
 
+	if (legacy)
+		bpf_wq = alloc_ordered_workqueue("test_bpf_wq", 0);
+	else
+		bpf_wq = alloc_workqueue("test_bpf_wq", WQ_UNBOUND, num_online_cpus());
+
+	if (!bpf_wq)
+		return -ENOMEM;
+
 	if (!strlen(test_suite) || !strcmp(test_suite, "test_bpf")) {
 		ret = test_bpf();
 		if (ret)
@@ -15925,6 +15971,8 @@ static int __init test_bpf_init(void)
 
 static void __exit test_bpf_exit(void)
 {
+	if (bpf_wq)
+		destroy_workqueue(bpf_wq);
 }
 
 module_init(test_bpf_init);
-- 
2.55.0

Re: [RFC PATCH] bpf: test: parallel execution of basic tests

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] bpf: test: missing workqueue cleanup on error paths in test_bpf_init()
- [Medium] bpf: data race on global net_device dev in populate_skb()
- [Low] bpf: test: silent log drop for expected failures and generation errors
- [Low] bpf: memory leak of bpf_prog in generate_filter() error paths
--

commit eef3e387313bc16bbeb43208bc729ec2b250d2a8
Author: Dmitry Antipov <dmantipov@yandex.ru>

bpf: test: parallel execution of basic tests

This commit implements a workqueue-based parallel execution for the basic
BPF tests, adding a legacy fallback option.

> diff --git a/lib/test_bpf.c b/lib/test_bpf.c
> --- a/lib/test_bpf.c
> +++ b/lib/test_bpf.c
> @@ -15012,14 +15032,14 @@ static struct bpf_prog *generate_filter(int which, int *err)
>  		fprog.len = flen;
>  
>  		*err = bpf_prog_create(&fp, &fprog);
> -		if (tests[which].aux & FLAG_EXPECTED_FAIL) {
> -			if (*err == tests[which].expected_errcode) {
> -				pr_cont("PASS\n");
> +		if (test->aux & FLAG_EXPECTED_FAIL) {
> +			if (*err == test->expected_errcode) {
> +				test_log(test, "PASS\n");
>  				/* Verifier rejected filter as expected. */
>  				*err = 0;
>  				return NULL;
>  			} else {
> -				pr_cont("UNEXPECTED_PASS\n");
> +				test_log(test, "UNEXPECTED_PASS\n");
>  				/* Verifier didn't reject the test that's
>  				 * bad enough, just return!
>  				 */
>  				*err = -EINVAL;
>  				return NULL;
>  			}

[Severity: Low]
Is there a memory leak of the fp pointer on an unexpected pass?

If bpf_prog_create successfully allocates fp (returning 0) but the test
was marked with FLAG_EXPECTED_FAIL, this branch sets *err to -EINVAL and
returns NULL without cleaning up the allocated fp.

[ ... ]

>  		/* We cannot error here as we don't need type compatibility
> @@ -15055,7 +15075,7 @@ static struct bpf_prog *generate_filter(int which, int *err)
>  		 */
>  		fp = bpf_prog_select_runtime(fp, err);
>  		if (*err) {
> -			pr_cont("FAIL to select_runtime err=%d\n", *err);
> +			test_log(test, "FAIL to select_runtime err=%d\n", *err);
>  			return NULL;
>  		}
>  		break;

[Severity: Low]
Similarly, if bpf_prog_select_runtime fails, it returns the allocated
fp. Could returning NULL here without freeing it result in a memory leak?

[ ... ]

>  static void test_bpf_work_handler(struct work_struct *work)
>  {
[ ... ]
> +	if (fp == NULL) {
> +		if (err == 0) {
> +			atomic_inc(&pass_cnt);
> +			return;
> +		}
> +		atomic_inc(&err_cnt);
> +		return;
> +	}

[Severity: Low]
Is it intended for this early return to bypass printing the test output
buffered in test->output?

For tests marked with FLAG_EXPECTED_FAIL that successfully trigger a
verifier rejection, or for unexpected generation errors, returning
early here skips the final pr_info call, which causes the test log
output to be completely lost.

[ ... ]

> +	err = run_one(fp, test);

[Severity: Medium]
Can parallel workers executing run_one() cause a data race on the global
net_device dev in populate_skb()?

Workers execute this path concurrently, and generate_test_data() calls
populate_skb(), which unconditionally writes to the shared static struct
net_device dev without synchronization:

lib/test_bpf.c:populate_skb() {
    ...
    dev_net_set(&dev, &init_net);
    skb->dev = &dev;
    skb->dev->ifindex = SKB_DEV_IFINDEX;
    skb->dev->type = SKB_DEV_TYPE;
    ...
}

[ ... ]

>  static int __init test_bpf_init(void)
>  {
[ ... ]
>  	if (!strlen(test_suite) || !strcmp(test_suite, "test_bpf")) {
>  		ret = test_bpf();
>  		if (ret)
>  			return ret;
>  	}
>  
>  	if (!strlen(test_suite) || !strcmp(test_suite, "test_tail_calls")) {
>  		ret = prepare_tail_call_tests(&progs);
>  		if (ret)
>  			return ret;
>  		ret = test_tail_calls(progs);
>  		destroy_tail_call_tests(progs);
>  		if (ret)
>  			return ret;
>  	}

[Severity: Medium]
Would these error paths leave the newly allocated bpf_wq workqueue leaked
during module initialization?

If test_bpf(), prepare_tail_call_tests(), or test_tail_calls() return an
error code, the direct return paths leave the globally allocated bpf_wq
un-freed. Should these error paths be calling destroy_workqueue(bpf_wq)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006192840.155680-1-dmantipov@yandex.ru?part=1

Re: [RFC PATCH] bpf: test: parallel execution of basic tests

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 4705 bytes --]

> bpf: test: parallel execution of basic tests

TL;DR:
- test_bpf fill_helper() runs up front, so ~125MB may be live at once; failed
  allocations could silently skip tests
- Buffered output may hide which test oopsed or hung
- legacy parameter lacks documentation

> diff --git a/lib/test_bpf.c b/lib/test_bpf.c
> index af6f3340c034..270899aed1e7 100644
> --- a/lib/test_bpf.c
> +++ b/lib/test_bpf.c

[ ... ]

> @@ -15155,6 +15175,9 @@ module_param(test_id, int, 0);
>  static int test_range[2] = { 0, INT_MAX };
>  module_param_array(test_range, int, NULL, 0);
>  
> +static bool legacy;
> +module_param(legacy, bool, 0);
> +

Is a description of the new legacy parameter needed somewhere?

A subsystem pattern flags this as potentially concerning: module_param(legacy,
bool, 0) can also be set as test_bpf.legacy= on the kernel command line, but
the patch adds no MODULE_PARM_DESC() and does not touch
Documentation/admin-guide/kernel-parameters.txt.  Nothing tells users that
parallel execution is now the default or how to restore sequential execution.

On the other hand, TEST_BPF is 'depends on m && NET', so the parameter is
normally passed through insmod or modprobe, and the existing test_bpf
parameters (test_name, test_id, test_range, test_suite) are not listed in
kernel-parameters.txt either.

Would a MODULE_PARM_DESC(), or a note in Documentation/networking/filter.rst,
be the better place to document it?

[ ... ]

> @@ -15345,65 +15368,80 @@ static __init int test_skb_segment(void)

[ ... ]

> +	test_log(test, "#%d %s ", test->index, test->descr);

[ ... ]

> +	pr_info("%s", test->output);
> +}

Can the test that triggers an oops or hang still be identified?

The "#%d %s " header and the "jited:%u " text now go into test->output, and
nothing reaches the log until this pr_info("%s", test->output) at the end of
test_bpf_work_handler().  Before the patch the header was printed with
pr_info() before generate_filter() and run_one() were called.

If bpf_prog_select_runtime() or __run_one() oopses, hangs or triggers an RCU
stall, the log no longer says which test was running.  The backtrace only
shows a kworker in test_bpf_work_handler(), and with several tests running in
parallel the last test that did print says nothing about the one that failed.

Could the test number and description be printed before the program is JITed
and run?

[ ... ]

> +static __init int test_bpf(void)
> +{
> +	int i;
>  
[ ... ]

> +	for (i = 0; i < ARRAY_SIZE(tests); i++) {
> +		if (exclude_test(i))
> +			continue;
> +
> +		if (tests[i].fill_helper &&
> +		    tests[i].fill_helper(&tests[i]) < 0) {
> +			pr_err("#%d %s: FAIL to setup helper\n",
> +			       i, tests[i].descr);
> +			continue;
>  		}
> +
> +		INIT_WORK(&tests[i].work, test_bpf_work_handler);
> +		tests[i].index = i;
> +		queue_work(bpf_wq, &tests[i].work);
>  	}
>  
> +	flush_workqueue(bpf_wq);

Can the fill_helper() calls in this loop leave most of the test programs
allocated at the same time?

The loop calls every fill_helper() up front and only queues the work, and
the insns buffer is freed later by test_bpf_work_handler() after
generate_filter().  Nothing throttles the loop, and flush_workqueue() is the
only wait.  The loop just builds instruction arrays, while each worker has to
copy and JIT a program of up to ~200K instructions and then run it, so nearly
all of the buffers are likely live at once.  Before the patch each buffer was
freed right after generate_filter(), so only one was live at a time.

__bpf_fill_pattern() allocates len = 3 + count * emit_max instructions, with
count = 14112 for the imm variants and 22176 for the reg variants.  Roughly:

  ~790KB for each ALU imm test and each JMP/JMP32 imm test
  ~1.6MB for each ALU reg test
  ~1.24MB for each JMP/JMP32 reg test

That is about 125MB of high-order kmalloc allocations live at once, compared
to a peak of about 2MB before.  This also happens with legacy=1, because the
producer loop is the same and the ordered workqueue only serializes the
execution.  Is that consistent with the commit message's "fallback to
similar-to-legacy behavior"?

On small or 32-bit boards these high-order allocations can fail.  A failed
fill_helper() only prints "FAIL to setup helper" and continues, without
touching err_cnt, so tests are silently skipped and test_bpf() can still
report 0 FAILED and return 0.

Should fill_helper() be called from test_bpf_work_handler() instead, or
should the loop be throttled?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/37521119328