Skip to contentSkip to conversation

bpftool: Use batch lookups for map dumps

PATCH Superseded Tianyi Chen 3 participants 7 messages Nov 4

Subsystems (4)

v8 of 10 Superseded Latest is v10 →

lore
Share & tools
lore
#0 lore raw

v8 cover bpftool: Use batch lookups for map dumps

Use batch lookups for map dumps and continue with individual lookups from the last printed key after any batch error.

Changes in v8:
- Follow Alexei's simpler approach: use a 1 MiB buffer budget, remove buffer growth and error classes, and try batching for all map types.
- Count and fail bpftool's lookups in the selftest. Check plain and JSON output, including failures before and after the first batch.
- Drop Quentin's Acked-by on the rewritten patch.

Tested in an x86-64 VM: all 16 batch subtests pass; unpatched bpftool fails all 16. The combination with the recursive series also passes.

v7: https://lore.kernel.org/r/[email protected]

Tianyi Chen (2):
bpftool: Use batch lookups for map dumps
selftests/bpf: Check bpftool batch map dumps

 tools/bpf/bpftool/map.c                       |  97 ++++++--
 .../bpf/prog_tests/bpftool_map_batch.c        | 228 ++++++++++++++++++
 .../selftests/bpf/progs/bpftool_map_batch.c   |  42 ++++
 3 files changed, 352 insertions(+), 15 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/bpftool_map_batch.c
 create mode 100644 tools/testing/selftests/bpf/progs/bpftool_map_batch.c

base-commit: 8fb43ffc3327d12d57d3fbe6f4820b5a522de0d1

Signature · 1 line
2.56.0
#1 lore raw

PATCH v8 1/2 bpftool: Use batch lookups for map dumps

Dumping a map takes two BPF syscalls per element: one to get the next key and one to look it up. Use BPF_MAP_LOOKUP_BATCH instead, with keys and values bounded to 1 MiB and per-CPU value strides accounted for. The output is unchanged.

For a hash map with 100,000 4-byte keys and values, a plain dump takes 5 BPF syscalls instead of 200,005. With output to /dev/null, the median of 15 runs in an x86-64 KVM guest on Linux 7.3 dropped from 86 ms to 52 ms (JSON: 78 ms to 42 ms). Printing to a terminal dominates either way.

If a batch lookup fails, discard its output and continue with individual lookups after the last printed key. bpf_map_get_next_key() walks the map in the same order. This covers maps without batch support, and hash maps whose bucket lock cannot be taken, which individual lookups do not need.

---
 tools/bpf/bpftool/map.c | 97 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 82 insertions(+), 15 deletions(-)
tools/bpf/bpftool/map.c +82 -15
index 4b65edf74859b..c0bd9bf846492 100644
@@ -59,13 +59,16 @@ static int map_type_from_str(const char *type)
 	return -1;
 }
 
-static void *alloc_value(struct bpf_map_info *info)
+static size_t map_value_size(struct bpf_map_info *info)
 {
 	if (map_is_per_cpu(info->type))
-		return malloc(round_up(info->value_size, 8) *
-			      get_possible_cpus());
-	else
-		return malloc(info->value_size);
+		return (size_t)round_up(info->value_size, 8) * get_possible_cpus();
+	return info->value_size;
+}
+
+static void *alloc_value(struct bpf_map_info *info)
+{
+	return malloc(map_value_size(info));
 }
 
 static int do_dump_btf(const struct btf_dumper *d,
@@ -741,15 +744,10 @@ static int do_show(int argc, char **argv)
 	return errno == ENOENT ? 0 : -1;
 }
 
-static int dump_map_elem(int fd, void *key, void *value,
-			 struct bpf_map_info *map_info, struct btf *btf,
-			 json_writer_t *btf_wtr, const int *cpu_ids, int cpu_cnt)
+static void print_map_elem(void *key, void *value,
+			   struct bpf_map_info *map_info, struct btf *btf,
+			   json_writer_t *btf_wtr, const int *cpu_ids, int cpu_cnt)
 {
-	if (bpf_map_lookup_elem(fd, key, value)) {
-		print_entry_error(map_info, key, errno);
-		return -1;
-	}
-
 	if (json_output) {
 		print_entry_json(map_info, key, value, btf, cpu_ids, cpu_cnt);
 	} else if (btf) {
@@ -763,10 +761,74 @@ static int dump_map_elem(int fd, void *key, void *value,
 	} else {
 		print_entry_plain(map_info, key, value, cpu_ids, cpu_cnt);
 	}
+}
 
+static int dump_map_elem(int fd, void *key, void *value,
+			 struct bpf_map_info *map_info, struct btf *btf,
+			 json_writer_t *btf_wtr, const int *cpu_ids, int cpu_cnt)
+{
+	if (bpf_map_lookup_elem(fd, key, value)) {
+		print_entry_error(map_info, key, errno);
+		return -1;
+	}
+
+	print_map_elem(key, value, map_info, btf, btf_wtr, cpu_ids, cpu_cnt);
 	return 0;
 }
 
+#define MAP_DUMP_BATCH_BYTES (1U << 20)
+
+/*
+ * Print the elements returned by batched lookups, leaving the last one in
+ * @key. Return true at the end of the map. After any error, the caller
+ * continues with bpf_map_get_next_key(), which walks the map in the same order.
+ */
+static bool dump_map_batch(int fd, void *key, void *value,
+			   struct bpf_map_info *info, struct btf *btf,
+			   json_writer_t *wtr, const int *cpu_ids, int cpu_cnt,
+			   unsigned int *num_elems)
+{
+	size_t value_size = map_value_size(info);
+	void *keys, *values, *batch, *in_batch = NULL;
+	__u32 capacity, count, i;
+	bool done = false;
+	int err;
+
+	capacity = min(MAP_DUMP_BATCH_BYTES / (info->key_size + value_size),
+		       (size_t)info->max_entries);
+	if (!capacity)
+		return false;
+
+	/* Hash table batch cursors are u32 bucket indices; other maps use keys. */
+	batch = malloc(max(info->key_size, 4U));
+	keys = malloc((size_t)capacity * info->key_size);
+	values = malloc((size_t)capacity * value_size);
+	if (!batch || !keys || !values)
+		goto out;
+
+	do {
+		count = capacity;
+		err = bpf_map_lookup_batch(fd, in_batch, batch, keys, values,
+					   &count, NULL);
+		if (err && err != -ENOENT)
+			goto out;
+		for (i = 0; i < count; i++) {
+			/* Preserve alignment for odd-sized BTF keys and values. */
+			memcpy(key, keys + (size_t)i * info->key_size, info->key_size);
+			memcpy(value, values + (size_t)i * value_size, value_size);
+			print_map_elem(key, value, info, btf, wtr, cpu_ids, cpu_cnt);
+		}
+		*num_elems += count;
+		in_batch = batch;
+	} while (!err);
+	done = true;
+out:
+	free(batch);
+	free(keys);
+	free(values);
+	return done;
+}
+
 static int maps_have_btf(int *fds, int nb_fds)
 {
 	struct bpf_map_info info = {};
@@ -836,7 +898,8 @@ map_dump(int fd, struct bpf_map_info *info, json_writer_t *wtr,
 	struct btf *btf = NULL;
 	int *cpu_ids = NULL;
 	int cpu_cnt = 0;
-	int err;
+	bool done = false;
+	int err = 0;
 
 	key = malloc(info->key_size);
 	value = alloc_value(info);
@@ -895,7 +958,11 @@ map_dump(int fd, struct bpf_map_info *info, json_writer_t *wtr,
 		p_info("Warning: cannot read values from %s map with value_size != 8",
 		       map_type_str);
 	}
-	while (true) {
+	done = dump_map_batch(fd, key, value, info, btf, wtr, cpu_ids, cpu_cnt,
+			      &num_elems);
+	if (num_elems)
+		prev_key = key;
+	while (!done) {
 		err = bpf_map_get_next_key(fd, prev_key, key);
 		if (err) {
 			if (errno == ENOENT) {
Signature · 1 line
2.56.0
#2 lore raw

PATCH v8 2/2 selftests/bpf: Check bpftool batch map dumps

In reply to Tianyi Chen

Batched dumps must print the same output as individual lookups. Use an fmod_ret program on security_bpf() to count the map lookups made by bpftool children of the test, and to fail batch lookups with -EBUSY as a hash bucket lock failure would.

For each map type with batched lookups, compare plain and JSON dumps with a dump whose batch lookups all fail, and check that no individual lookups were needed. Cover BTF and per-CPU output, an empty map, 1-byte keys and unaligned values. With 4 KiB values, check dumps that need several batches, and that failing the first or second batch lookup continues with individual lookups without restarting the dump.

Assisted-by LLM
---
 .../bpf/prog_tests/bpftool_map_batch.c        | 228 ++++++++++++++++++
 .../selftests/bpf/progs/bpftool_map_batch.c   |  42 ++++
 2 files changed, 270 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/bpftool_map_batch.c
 create mode 100644 tools/testing/selftests/bpf/progs/bpftool_map_batch.c
tools/testing/selftests/bpf/prog_tests/bpftool_map_batch.c new file +228 -0
new file mode 100644
index 0000000000000..5cf764bb5e362
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <arpa/inet.h>
+#include <test_progs.h>
+#include <bpf/btf.h>
+#include "bpftool_helpers.h"
+#include "bpftool_map_batch.skel.h"
+
+/* Fits the plain and JSON dumps of the multi-batch cases below. */
+#define OUTPUT_SIZE (8U << 20)
+
+struct dump_case {
+	const char *name;
+	enum bpf_map_type type;
+	__u32 key_size;
+	__u32 value_size;
+	__u32 entries;
+	bool btf;
+	/* Values large enough that the 1 MiB budget needs several batches. */
+	bool multi_batch;
+};
+
+static bool is_percpu(enum bpf_map_type type)
+{
+	return type == BPF_MAP_TYPE_PERCPU_HASH ||
+	       type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
+	       type == BPF_MAP_TYPE_PERCPU_ARRAY;
+}
+
+static int run_dump(struct bpftool_map_batch *skel, __u32 id, bool json,
+		    int fail_batch, char *output)
+{
+	char command[64];
+	int err;
+
+	snprintf(command, sizeof(command), "%smap dump id %u",
+		 json ? "-j " : "", id);
+	skel->bss->fail_batch = fail_batch;
+	skel->bss->batch_calls = 0;
+	skel->bss->elem_calls = 0;
+	output[0] = '\0';
+	err = get_bpftool_command_output(command, output, OUTPUT_SIZE);
+	if (!ASSERT_LT(strlen(output), OUTPUT_SIZE - 1, "output_fits"))
+		return -1;
+	return err;
+}
+
+static void check_same(const char *output, const char *expected, const char *name)
+{
+	size_t i;
+
+	for (i = 0; output[i] && output[i] == expected[i]; i++)
+		;
+	if (!ASSERT_EQ(output[i], expected[i], name))
+		printf("%s: first difference at offset %zu\n", name, i);
+}
+
+/* Failing every batch lookup gives the dump of individual lookups. */
+static void check_dump(struct bpftool_map_batch *skel, const struct dump_case *test,
+		       __u32 id, bool json, char *expected, char *output)
+{
+	int fail;
+
+	if (!ASSERT_OK(run_dump(skel, id, json, -1, expected), "individual_dump") ||
+	    !ASSERT_EQ(skel->bss->elem_calls, test->entries, "individual_lookups"))
+		return;
+
+	if (!ASSERT_OK(run_dump(skel, id, json, 0, output), "batch_dump"))
+		return;
+	ASSERT_GE(skel->bss->batch_calls, test->multi_batch ? 2 : 1, "batch_calls");
+	ASSERT_EQ(skel->bss->elem_calls, 0, "batch_only");
+	check_same(output, expected, "batch_output");
+
+	if (!test->multi_batch)
+		return;
+	/* Fail before and after a batch has been printed. */
+	for (fail = 1; fail <= 2; fail++) {
+		if (!ASSERT_OK(run_dump(skel, id, json, fail, output), "fallback_dump"))
+			return;
+		ASSERT_EQ(skel->bss->batch_calls, fail, "fallback_batch_calls");
+		if (fail == 1) {
+			ASSERT_EQ(skel->bss->elem_calls, test->entries, "fallback_lookups");
+		} else {
+			ASSERT_GT(skel->bss->elem_calls, 0, "resumed_lookups");
+			ASSERT_LT(skel->bss->elem_calls, test->entries, "not_restarted");
+		}
+		check_same(output, expected, "fallback_output");
+	}
+}
+
+static void fill_bytes(void *buf, __u32 size, __u32 seed)
+{
+	__u8 *bytes = buf;
+	__u32 i;
+
+	for (i = 0; i < size; i++)
+		bytes[i] = seed * 37 + i;
+}
+
+static void run_case(struct bpftool_map_batch *skel, const struct dump_case *test,
+		     char *expected, char *output)
+{
+	LIBBPF_OPTS(bpf_map_create_opts, opts);
+	int fd = -1, inner_fd = -1, cpus = 1, cpu;
+	struct bpf_map_info info = {};
+	__u32 len = sizeof(info);
+	__u32 i, max_entries, stride;
+	struct btf *btf = NULL;
+	__u8 key[8] = {};
+	void *values = NULL;
+
+	if (is_percpu(test->type)) {
+		cpus = libbpf_num_possible_cpus();
+		if (!ASSERT_GT(cpus, 0, "possible_cpus"))
+			return;
+	}
+	stride = is_percpu(test->type) ? (test->value_size + 7) & ~7U : test->value_size;
+	values = calloc(cpus, stride);
+	if (!ASSERT_OK_PTR(values, "values"))
+		return;
+
+	if (test->btf) {
+		btf = btf__new_empty();
+		if (!ASSERT_OK_PTR(btf, "btf_new") ||
+		    !ASSERT_EQ(btf__add_int(btf, "unsigned int", 4, 0), 1, "btf_int") ||
+		    !ASSERT_OK(btf__load_into_kernel(btf), "btf_load"))
+			goto out;
+		opts.btf_fd = btf__fd(btf);
+		opts.btf_key_type_id = 1;
+		opts.btf_value_type_id = 1;
+	}
+	if (test->type == BPF_MAP_TYPE_ARRAY_OF_MAPS ||
+	    test->type == BPF_MAP_TYPE_HASH_OF_MAPS) {
+		inner_fd = bpf_map_create(BPF_MAP_TYPE_ARRAY, "batch_inner", 4, 4, 1, NULL);
+		if (!ASSERT_OK_FD(inner_fd, "inner_map"))
+			goto out;
+		opts.inner_map_fd = inner_fd;
+	}
+	if (test->type == BPF_MAP_TYPE_LPM_TRIE || test->type == BPF_MAP_TYPE_RHASH)
+		opts.map_flags = BPF_F_NO_PREALLOC;
+	max_entries = test->entries ?: 1;
+	/* Leave room for per-CPU LRU free lists instead of testing eviction. */
+	if (test->type == BPF_MAP_TYPE_LRU_HASH ||
+	    test->type == BPF_MAP_TYPE_LRU_PERCPU_HASH)
+		max_entries *= 4;
+
+	fd = bpf_map_create(test->type, "batch_dump", test->key_size,
+			    test->value_size, max_entries, &opts);
+	if (!ASSERT_OK_FD(fd, "map_create"))
+		goto out;
+	for (i = 0; i < test->entries; i++) {
+		if (test->type == BPF_MAP_TYPE_LPM_TRIE) {
+			__u32 prefix[] = { 32, htonl(i) };
+
+			memcpy(key, prefix, sizeof(prefix));
+		} else if (test->key_size == 1) {
+			key[0] = (__u8)i;
+		} else {
+			/* Array map indices use native byte order. */
+			memcpy(key, &i, sizeof(i));
+		}
+		if (inner_fd >= 0)
+			memcpy(values, &inner_fd, sizeof(inner_fd));
+		else
+			for (cpu = 0; cpu < cpus; cpu++)
+				fill_bytes(values + cpu * stride, test->value_size,
+					   i + cpu * 101);
+		if (!ASSERT_OK(bpf_map_update_elem(fd, key, values, BPF_ANY), "map_update"))
+			goto out;
+	}
+	if (!ASSERT_OK(bpf_map_get_info_by_fd(fd, &info, &len), "map_info"))
+		goto out;
+
+	check_dump(skel, test, info.id, false, expected, output);
+	check_dump(skel, test, info.id, true, expected, output);
+out:
+	if (fd >= 0)
+		close(fd);
+	if (inner_fd >= 0)
+		close(inner_fd);
+	btf__free(btf);
+	free(values);
+}
+
+void test_bpftool_map_batch(void)
+{
+	static const struct dump_case cases[] = {
+		{ "hash", BPF_MAP_TYPE_HASH, 4, 4, 100 },
+		{ "empty", BPF_MAP_TYPE_HASH, 4, 4, 0 },
+		/* The hash cursor is wider than the key; values are unaligned. */
+		{ "odd_sizes", BPF_MAP_TYPE_HASH, 1, 3, 100 },
+		{ "btf", BPF_MAP_TYPE_HASH, 4, 4, 100, true },
+		{ "lru_hash", BPF_MAP_TYPE_LRU_HASH, 4, 4, 100 },
+		{ "percpu_hash", BPF_MAP_TYPE_PERCPU_HASH, 4, 3, 100 },
+		{ "lru_percpu_hash", BPF_MAP_TYPE_LRU_PERCPU_HASH, 4, 4, 100 },
+		{ "btf_percpu_hash", BPF_MAP_TYPE_PERCPU_HASH, 4, 4, 100, true },
+		{ "rhash", BPF_MAP_TYPE_RHASH, 4, 4, 100 },
+		{ "array", BPF_MAP_TYPE_ARRAY, 4, 4, 100 },
+		{ "percpu_array", BPF_MAP_TYPE_PERCPU_ARRAY, 4, 3, 100 },
+		{ "lpm_trie", BPF_MAP_TYPE_LPM_TRIE, 8, 4, 100 },
+		{ "array_of_maps", BPF_MAP_TYPE_ARRAY_OF_MAPS, 4, 4, 100 },
+		{ "hash_of_maps", BPF_MAP_TYPE_HASH_OF_MAPS, 4, 4, 100 },
+		/* Bucket and key cursors, up to 255 entries per batch. */
+		{ "multi_batch_hash", BPF_MAP_TYPE_HASH, 4, 4096, 256, false, true },
+		{ "multi_batch_array", BPF_MAP_TYPE_ARRAY, 4, 4096, 256, false, true },
+	};
+	struct bpftool_map_batch *skel;
+	char *expected = NULL, *output = NULL;
+	unsigned int i;
+
+	skel = bpftool_map_batch__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
+		return;
+	skel->bss->test_tgid = getpid();
+	if (!ASSERT_OK(bpftool_map_batch__attach(skel), "skel_attach"))
+		goto out;
+	expected = malloc(OUTPUT_SIZE);
+	output = malloc(OUTPUT_SIZE);
+	if (!ASSERT_OK_PTR(expected, "expected") || !ASSERT_OK_PTR(output, "output"))
+		goto out;
+
+	for (i = 0; i < ARRAY_SIZE(cases); i++)
+		if (test__start_subtest(cases[i].name))
+			run_case(skel, &cases[i], expected, output);
+out:
+	free(expected);
+	free(output);
+	bpftool_map_batch__destroy(skel);
+}
tools/testing/selftests/bpf/progs/bpftool_map_batch.c new file +42 -0

42 lines not on this page · raw message

Signature · 1 line
2.56.0
#3 lore raw

Re: [PATCH bpf-next v8 2/2] selftests/bpf: Check bpftool batch map dumps

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] selftests/bpf: async rhashtable resize race on RHASH dump in run_case() --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/bpf: async rhashtable resize race on RHASH dump in run_case() --

commit 6de2831c6843be67feeb4ee441d5b150a4946b8a
Author: Tianyi Chen <[email protected]>

selftests/bpf: Check bpftool batch map dumps

This commit adds a BPF selftest that verifies bpftool batched map dumps produce the same plain and JSON output as individual element lookups across supported map types, including fallback handling when batch lookups return -EBUSY.

Quoted 6 lines
tools/testing/selftests/bpf/prog_tests/bpftool_map_batch.c new file +0 -0
new file mode 100644
index 0000000000000..5cf764bb5e362
@@ -0,0 +1,228 @@

[ ... ]

Quoted 5 lines
tools/testing/selftests/bpf/prog_tests/bpftool_map_batch.c new file +5 -0
+static void run_case(struct bpftool_map_batch *skel, const struct dump_case *test,
+		     char *expected, char *output)
+{
+	LIBBPF_OPTS(bpf_map_create_opts, opts);
+	int fd = -1, inner_fd = -1, cpus = 1, cpu;

[ ... ]

tools/testing/selftests/bpf/prog_tests/bpftool_map_batch.c line 147

Quoted 10 lines
tools/testing/selftests/bpf/prog_tests/bpftool_map_batch.c new file +10 -0
+	if (test->type == BPF_MAP_TYPE_LPM_TRIE || test->type == BPF_MAP_TYPE_RHASH)
+		opts.map_flags = BPF_F_NO_PREALLOC;
+	max_entries = test->entries ?: 1;
+	/* Leave room for per-CPU LRU free lists instead of testing eviction. */
+	if (test->type == BPF_MAP_TYPE_LRU_HASH ||
+	    test->type == BPF_MAP_TYPE_LRU_PERCPU_HASH)
+		max_entries *= 4;
+
+	fd = bpf_map_create(test->type, "batch_dump", test->key_size,
+			    test->value_size, max_entries, &opts);

[Severity: Medium]
Can the rhash subtest in test_bpftool_map_batch() race with asynchronous rhashtable resizing during check_dump()?

In run_case(), opts.map_extra is left at 0 when creating a BPF_MAP_TYPE_RHASH map with 100 entries. In the kernel, rhtab_map_alloc() passes params.nelem_hint = (u32)attr->map_extra to rhashtable_init(), so the rhashtable starts with the default 64 buckets (HASH_DEFAULT_SIZE).

During the 100-element bpf_map_update_elem() loop in run_case(), inserting the 49th element exceeds the 75% load threshold (rht_grow_above_75()) in __rhashtable_insert_fast() and schedules rht_deferred_worker() on system_wq to resize the table from 64 to 128 buckets. Inserting the 97th element then exceeds the 75% threshold of the 128-bucket table and queues a second asynchronous resize to 256 buckets just three updates before check_dump() runs:

run_case()
bpf_map_update_elem()

    rhtab_map_update_elem()
      rhashtable_lookup_get_insert_key()
        __rhashtable_insert_fast()
          if (rht_grow_above_75(ht, tbl))
            schedule_work(&ht->run_work); -> rht_deferred_worker()

If rht_deferred_worker() runs concurrently with or between the bpftool dumps in check_dump(), can an in-flight rehash cause bpf_map_get_next_key() to skip or revisit elements (failing the individual_lookups assertion), or finish rehashing with a new hash seed and bucket count between the individual dump and the batch dump so that their iteration order differs (failing check_same() on batch_output)?

Would setting opts.map_extra = max_entries for BPF_MAP_TYPE_RHASH avoid scheduling background resizes by pre-sizing the rhashtable to 256 buckets at creation time?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#4 lore raw

Re: [PATCH bpf-next v8 2/2] selftests/bpf: Check bpftool batch map dumps

Quoted 3 lines

Would setting opts.map_extra = max_entries for BPF_MAP_TYPE_RHASH avoid scheduling background resizes by pre-sizing the rhashtable to 256 buckets at creation time?

Yes. A smaller in-process comparison after 100 inserts, with no further userspace updates, produced 25 count mismatches in 1000 runs; with map_extra=100 it produced none. The complete old bpftool subtest passed 50 repetitions, so this is not a claim that it fails reliably.

v9 sets map_extra=max_entries for RHASH. The updated subtest passed 100 repetitions.

v9: https://lore.kernel.org/r/[email protected]

Thanks,
Tianyi

#5 lore raw

Re: [PATCH bpf-next v8 2/2] selftests/bpf: Check bpftool batch map dumps

In reply to Tianyi Chen
TL;DR: Commit message and comment say the selftest's injected -EBUSY mimics a hash bucket lock failure, but rqspinlock apparently returns -EDEADLK or -ETIMEDOUT; test still works, so only wording…
Quoted 1 line

selftests/bpf: Check bpftool batch map dumps

TL;DR: Commit message and comment say the selftest's injected -EBUSY mimics a hash bucket lock failure, but rqspinlock apparently returns -EDEADLK or -ETIMEDOUT; test still works, so only wording (or the errno) needs fixing.

Quoted 17 lines

Batched dumps must print the same output as individual lookups. Use an fmod_ret program on security_bpf() to count the map lookups made by bpftool children of the test, and to fail batch lookups with -EBUSY as a hash bucket lock failure would.

For each map type with batched lookups, compare plain and JSON dumps with a dump whose batch lookups all fail, and check that no individual lookups were needed. Cover BTF and per-CPU output, an empty map, 1-byte keys and unaligned values. With 4 KiB values, check dumps that need several batches, and that failing the first or second batch lookup continues with individual lookups without restarting the dump.

Assisted-by LLM
tools/testing/selftests/bpf/progs/bpftool_map_batch.c new file +0 -0
new file mode 100644

[ ... ]

tools/testing/selftests/bpf/progs/bpftool_map_batch.c line 40

Quoted 6 lines
tools/testing/selftests/bpf/progs/bpftool_map_batch.c new file +6 -0
+	} else if (cmd == BPF_MAP_LOOKUP_BATCH) {
+		batch_calls++;
+		/* Same error as a hash bucket lock failure. */
+		if (fail_batch < 0 || fail_batch == batch_calls)
+			return -EBUSY;
+	}

The commit message says batch lookups fail "with -EBUSY as a hash bucket lock failure would", and this comment says the same. Is that accurate?

Since commit 4fa8d68aa53e ("bpf: Convert hashtab.c to rqspinlock"), htab_lock_bucket() returns whatever raw_res_spin_lock_irqsave() returns:

    ret = raw_res_spin_lock_irqsave(&b->raw_lock, flags);
    if (ret)
        return ret;

and rqspinlock only fails with -EDEADLK or -ETIMEDOUT. __htab_map_lookup_and_delete_batch() passes that value back to user space via "goto after_loop", so no BPF_MAP_LOOKUP_BATCH path returns -EBUSY for a lock failure.

The test still works, because dump_map_batch() in bpftool treats any error other than -ENOENT the same way. Could the comment and the changelog say "an error other than -ENOENT" instead, or could this return -EDEADLK?

---
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/41893799307

#6 lore raw

Re: [PATCH bpf-next v8 2/2] selftests/bpf: Check bpftool batch map dumps

Quoted 2 lines

The test still works, because dump_map_batch() in bpftool treats any error other than -ENOENT the same way.

Correct. v9 removes the bucket-lock errno claim from the commit message and clarifies the comment. The injected -EBUSY still exercises non-ENOENT fallback; the test behavior is unchanged.

v9: https://lore.kernel.org/r/[email protected]

Thanks,
Tianyi