[PATCH bpf v3] selftests/bpf: Check returned keys in map batch validators

[PATCH bpf v3] selftests/bpf: Check returned keys in map batch validators

[Tianyi Chen]

From: Tianyi Chen <hi@tychen.cc>

The hash, array and LPM trie batch validators mark visited entries by
output position, making the subsequent completeness checks always
succeed. Duplicate keys with matching values can therefore hide
missing entries.

Check each returned key against the set populated by the test and
index the visited array by key. This detects missing entries while
preserving unordered results and existing per-CPU value validation.

For LPM trie keys, check the /32 prefix and complete IPv4 address
before using the host octet as the index. Compare the address directly
in host byte order instead of parsing its textual representation.

Fixes: 30ff3c59137d ("selftests/bpf: Add batch ops testing for htab and htab_percpu map")
Fixes: f0fac2cec286 ("selftests/bpf: Add batch ops testing to array bpf map")
Fixes: e9bd8cbd970b ("bpf: selftests: Add tests for batched ops in LPM trie maps")
Assisted-by: LLM
Signed-off-by: Tianyi Chen <hi@tychen.cc>
---
Changes in v3:
- Rebase onto current bpf/master; the v2 test logic is unchanged.

Validation: test_maps passed in an x86-64 KVM guest running the rebuilt
bpf/master kernel (Linux 7.3.0-rc2), with LLVM 20-built selftests.
The hash, array and LPM batch tests passed; test_maps reported 0 skipped
and returned 0.

The v2 CI PR expired after repeated "Patch is empty" reports without
conflicting hunks. This patch applies cleanly to the current tree and is
sent in a new thread with git format-patch and git send-email.

v2: https://lore.kernel.org/r/178870889323.880246.16480713814270899611.bpf-batch-v2@tychen.cc

 .../bpf/map_tests/array_map_batch_ops.c       |  5 ++++-
 .../bpf/map_tests/htab_map_batch_ops.c        |  4 +++-
 .../bpf/map_tests/lpm_trie_map_batch_ops.c    | 19 ++++++++++---------
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c
index b595556315bc..76d3800a82af 100644
--- a/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c
+++ b/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c
@@ -45,6 +45,9 @@ static void map_batch_verify(int *visited, __u32 max_entries, int *keys,
 
 	memset(visited, 0, max_entries * sizeof(*visited));
 	for (i = 0; i < max_entries; i++) {
+		CHECK(keys[i] < 0 || keys[i] >= max_entries, "key checking",
+		      "error: i %d key %d out of range\n", i, keys[i]);
+
 		if (is_pcpu) {
 			cpu_offset = i * nr_cpus;
 			for (j = 0; j < nr_cpus; j++) {
@@ -59,7 +62,7 @@ static void map_batch_verify(int *visited, __u32 max_entries, int *keys,
 			      "error: i %d key %d value %lld\n", i, keys[i],
 			      values[i]);
 		}
-		visited[i] = 1;
+		visited[keys[i]] = 1;
 	}
 	for (i = 0; i < max_entries; i++) {
 		CHECK(visited[i] != 1, "visited checking",
diff --git a/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c
index 5da493b94ae2..430949f9691d 100644
--- a/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c
+++ b/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c
@@ -50,6 +50,8 @@ static void map_batch_verify(int *visited, __u32 max_entries,
 
 	memset(visited, 0, max_entries * sizeof(*visited));
 	for (i = 0; i < max_entries; i++) {
+		CHECK(keys[i] < 1 || keys[i] > max_entries, "key checking",
+		      "error: i %d key %d out of range\n", i, keys[i]);
 
 		if (is_pcpu) {
 			for (j = 0; j < bpf_num_possible_cpus(); j++) {
@@ -65,7 +67,7 @@ static void map_batch_verify(int *visited, __u32 max_entries,
 			      ((int *)values)[i]);
 		}
 
-		visited[i] = 1;
+		visited[keys[i] - 1] = 1;
 
 	}
 	for (i = 0; i < max_entries; i++) {
diff --git a/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c
index fe3e19f96244..3b51670b3cd4 100644
--- a/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c
+++ b/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c
@@ -44,18 +44,19 @@ static void map_batch_update(int map_fd, __u32 max_entries,
 static void map_batch_verify(int *visited, __u32 max_entries,
 			     struct test_lpm_key *keys, int *values)
 {
-	char buff[16] = { 0 };
-	int lower_byte = 0;
-	__u32 i;
+	__u32 i, ipv4, key;
 
 	memset(visited, 0, max_entries * sizeof(*visited));
 	for (i = 0; i < max_entries; i++) {
-		inet_ntop(AF_INET, &keys[i].ipv4, buff, 32);
-		CHECK(sscanf(buff, "192.168.1.%d", &lower_byte) == EOF,
-		      "sscanf()", "error: i %d\n", i);
-		CHECK(lower_byte != values[i], "key/value checking",
-		      "error: i %d key %s value %d\n", i, buff, values[i]);
-		visited[i] = 1;
+		ipv4 = ntohl(keys[i].ipv4.s_addr);
+		key = ipv4 & 0xff;
+		/* Expected keys are 192.168.1.1..max_entries with a /32 prefix. */
+		CHECK(keys[i].prefix != 32 || (ipv4 & 0xffffff00) != 0xc0a80100 ||
+		      key == 0 || key > max_entries, "key checking",
+		      "error: i %u prefix %u ipv4 %#x\n", i, keys[i].prefix, ipv4);
+		CHECK(key != values[i], "key/value checking",
+		      "error: i %u key %u value %d\n", i, key, values[i]);
+		visited[key - 1] = 1;
 	}
 	for (i = 0; i < max_entries; i++) {
 		CHECK(visited[i] != 1, "visited checking",

base-commit: 15071f2a1263e82150c77eeb1e94dbfc31950a8e
-- 
2.55.0

Re: [PATCH bpf v3] selftests/bpf: Check returned keys in map batch validators

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces new uses of the deprecated `CHECK()` macro for assertions instead of migrating to the required `ASSERT_*()` macros.
--

commit ea80e2126710dbf0e0f3a1f8861acf3e34b2207d
Author: Tianyi Chen <hi@tychen.cc>

selftests/bpf: Check returned keys in map batch validators

This patch updates the hash, array and LPM trie batch validators to check
returned keys against the populated set, and indexes the visited array by
key. It aims to detect missing entries while preserving unordered results
and per-CPU value validation.

> diff --git a/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c
> index b595556315bc3..76d3800a82af9 100644
> --- a/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c
> +++ b/tools/testing/selftests/bpf/map_tests/array_map_batch_ops.c
> @@ -45,6 +45,9 @@ static void map_batch_verify(int *visited, __u32 max_entries, int *keys,
>  
>  	memset(visited, 0, max_entries * sizeof(*visited));
>  	for (i = 0; i < max_entries; i++) {
> +		CHECK(keys[i] < 0 || keys[i] >= max_entries, "key checking",
> +		      "error: i %d key %d out of range\n", i, keys[i]);
> +

[Severity: Low]
Does this new assertion use a deprecated macro?

According to the BPF selftest subsystem guidelines, the CHECK() macro is
deprecated and should be avoided in new tests or patches. Should we use the
modern ASSERT_*() family of macros (like ASSERT_GE and ASSERT_LT) here
instead?

[ ... ]
> diff --git a/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c
> index 5da493b94ae25..430949f9691d7 100644
> --- a/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c
> +++ b/tools/testing/selftests/bpf/map_tests/htab_map_batch_ops.c
> @@ -50,6 +50,8 @@ static void map_batch_verify(int *visited, __u32 max_entries,
>  
>  	memset(visited, 0, max_entries * sizeof(*visited));
>  	for (i = 0; i < max_entries; i++) {
> +		CHECK(keys[i] < 1 || keys[i] > max_entries, "key checking",
> +		      "error: i %d key %d out of range\n", i, keys[i]);

[Severity: Low]
Could this assertion also be replaced with ASSERT_GE() and ASSERT_LE() to
follow current selftests standards?

[ ... ]
> diff --git a/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c b/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c
> index fe3e19f962446..3b51670b3cd4b 100644
> --- a/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c
> +++ b/tools/testing/selftests/bpf/map_tests/lpm_trie_map_batch_ops.c
> @@ -44,18 +44,19 @@ static void map_batch_update(int map_fd, __u32 max_entries,
>  static void map_batch_verify(int *visited, __u32 max_entries,
>  			     struct test_lpm_key *keys, int *values)
>  {
> -	char buff[16] = { 0 };
> -	int lower_byte = 0;
> -	__u32 i;
> +	__u32 i, ipv4, key;
>  
>  	memset(visited, 0, max_entries * sizeof(*visited));
>  	for (i = 0; i < max_entries; i++) {
> -		inet_ntop(AF_INET, &keys[i].ipv4, buff, 32);
> -		CHECK(sscanf(buff, "192.168.1.%d", &lower_byte) == EOF,
> -		      "sscanf()", "error: i %d\n", i);
> -		CHECK(lower_byte != values[i], "key/value checking",
> -		      "error: i %d key %s value %d\n", i, buff, values[i]);
> -		visited[i] = 1;
> +		ipv4 = ntohl(keys[i].ipv4.s_addr);
> +		key = ipv4 & 0xff;
> +		/* Expected keys are 192.168.1.1..max_entries with a /32 prefix. */
> +		CHECK(keys[i].prefix != 32 || (ipv4 & 0xffffff00) != 0xc0a80100 ||
> +		      key == 0 || key > max_entries, "key checking",
> +		      "error: i %u prefix %u ipv4 %#x\n", i, keys[i].prefix, ipv4);

[Severity: Low]
Even though the surrounding code previously used CHECK() in this function, the
BPF guidelines indicate that we should avoid introducing new uses of it. Can we
upgrade these new assertions to use the modern ASSERT_*() macros instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911025036.190487-1-diannaaav@gmail.com?part=1

Re: [PATCH bpf v3] selftests/bpf: Check returned keys in map batch validators

[Tianyi Chen]

These files run under test_maps, not test_progs. They include test_maps.h,
whose CHECK() reports the failure and calls exit(-1). That header does
not provide ASSERT_*(). The suggested ASSERT_GE/LT/LE helpers are defined
in test_progs.h and depend on that runner's failure reporting.

The distinction matters for these range checks: execution must stop
before indexing visited with an invalid returned key. ASSERT_*() returns
a boolean and does not itself stop the caller, so a mechanical replacement
would not preserve that behavior, even after resolving the runner/header
dependencies.

I am retaining CHECK() for this targeted test_maps correctness fix.
Migrating these tests to the test_progs assertion framework would be a
separate change requiring compatible failure reporting and control flow.
The current v3 passed test_maps with no skips on the rebuilt bpf/master
kernel (7.3.0-rc2).

Thanks,
Tianyi

Re: [PATCH bpf v3] selftests/bpf: Check returned keys in map batch validators

[Tianyi Chen]

Hi CI team,

Could you rerun the failed jobs for series 1162495 and compare the
unpatched baseline if the test failures persist?

This patch changes only array_map_batch_ops.c, htab_map_batch_ops.c and
lpm_trie_map_batch_ops.c under map_tests/. Those objects are linked into
test_maps, not test_progs-no_alu32. No kernel, libbpf, BPF program or
shared runner code is changed. All five affected batch tests passed in
the x86_64 GCC 15, x86_64 LLVM 21 and aarch64 GCC 15 test_maps jobs.

I checked all three failed jobs in the run:

1. x86_64 GCC 15, test_progs_no_alu32: tc_edt exceeded its 2% rate
   tolerance. ASSERT_LE compares doubles but prints long long values,
   explaining the misleading "actual 2 > expected 2" diagnostic.
   The same test passed in the x86_64 LLVM 21 no_alu32 job.
   https://github.com/kernel-patches/bpf/actions/runs/34557418186/job/103135036328

2. s390x GCC 15, test_progs_no_alu32: userspace SIGSEGV in
   ring_buffer__poll+0xc6, with an unnamed caller at +0x2f9760. The runner
   exited 139 and left its JSON empty; no kernel splats were reported.
   All ringbuf subtests passed in the normal s390x test_progs job.
   https://github.com/kernel-patches/bpf/actions/runs/34557418186/job/103134982838

   A baseline path worth checking is ringbuf_subtest's cleanup after an
   unsuccessful pthread_tryjoin_np: it can free the ring-buffer manager
   while the polling thread is still active. I have not established
   that as this crash's cause. Symbolizing the caller with the exact
   s390x executable and rerunning the baseline would help narrow it down.

3. x86_64 LLVM 21 / GCC BPF: failed before testing because ListArtifacts
   returned HTTP 403 from an intermediary.
   https://github.com/kernel-patches/bpf/actions/runs/34557418186/job/103136087069

I do not see a patch change that explains these failures, so I am
keeping v3 unchanged pending the rerun/baseline results.

Thanks,
Tianyi

Re: [PATCH bpf v3] selftests/bpf: Check returned keys in map batch validators

[Tianyi Chen]

Hi CI team,

The latest failure for series 1162495 is in VM startup, before veristat
runs. Could you refresh the vmtest executable and rerun the failed job?

The sole failing job is x86_64 GCC 15 / veristat-meta:
https://github.com/kernel-patches/bpf/actions/runs/34808652400/job/103868698882

Its /usr/bin/vmtest appears to contain an HTML document rather than the
expected executable. The log reports:

  /usr/bin/vmtest: line 1: !DOCTYPE: No such file or directory
  /usr/bin/vmtest: line 4: Hello: command not found

It then exits 2 with an unmatched-quote error before the guest starts.
There is no verifier regression result from this job.

The action requests vmtest v0.18.0. The run-vmtest installer in libbpf/ci
uses curl -L to write directly to /usr/bin/vmtest, followed by chmod,
without HTTP failure checking or executable-content validation. Adding
those checks would make this fail clearly at download/install time
instead of trying to execute an HTML response. The log does not identify
which HTTP response or intermediary supplied the page.

The earlier run of this same patch version completed veristat-meta and
reported that its output matched its baseline:
https://github.com/kernel-patches/bpf/actions/runs/34800861264/job/103846813446

The latest run's test_maps jobs also passed on x86_64 GCC, x86_64 LLVM
and aarch64 GCC. This patch changes only three map_tests validators;
it does not change vmtest, CI scripts or the verifier. I am keeping v3
unchanged for this infrastructure failure.

Thanks,
Tianyi