Skip to contentSkip to conversation

bpftool: Use batch lookups for map dumps

PATCH Superseded Tianyi Chen 3 participants 7 messages Nov 4 – Nov 10

Subsystems (4)
Kselftest BPF BPF selftests bpftool

v9 of 10 Superseded Latest is v10 →

lore
Share & tools
lore

Tianyi Chen

author
#0 lore raw

v9 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 v9 (selftests only):
- Pre-size RHASH maps to keep asynchronous growth out of the order-sensitive dump comparisons (Sashiko).
- Correct the explanation of the injected -EBUSY: it exercises fallback, but is not the current hash bucket lock failure errno (BPF CI).

Tested in an x86-64 VM: all 16 batch subtests, 100 repetitions of the RHASH subtest, and the combination with the recursive series pass.

v8: 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        | 231 ++++++++++++++++++
 .../selftests/bpf/progs/bpftool_map_batch.c   |  42 ++++
 3 files changed, 355 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
author
#1 lore raw

PATCH v9 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.

Link https://github.com/libbpf/bpftool/issues/63
Suggested-by Alexei Starovoitov <[email protected]>
Assisted-by LLM
Signed-off-by Tianyi Chen <[email protected]>
---
 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
AI review
#2 lore raw
TL;DR: For BPF_MAP_TYPE_RHASH, a concurrent delete may make batch lookup return -ENOENT early, so bpftool map dump could silently print a short dump as success, unlike the old get_next_key walk.
Quoted 1 line

bpftool: Use batch lookups for map dumps

TL;DR: For BPF_MAP_TYPE_RHASH, a concurrent delete may make batch lookup return -ENOENT early, so bpftool map dump could silently print a short dump as success, unlike the old get_next_key walk.

Quoted 3 lines
tools/bpf/bpftool/map.c +0 -0

[ ... ]

Quoted 5 lines
tools/bpf/bpftool/map.c +5 -0
+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)
+{

[ ... ]

Quoted 6 lines
tools/bpf/bpftool/map.c +6 -0
+	do {
+		count = capacity;
+		err = bpf_map_lookup_batch(fd, in_batch, batch, keys, values,
+					   &count, NULL);
+		if (err && err != -ENOENT)
+			goto out;

tools/bpf/bpftool/map.c static int dump_map_elem(int fd, void *key, void *value, · line 824

Quoted 10 lines
tools/bpf/bpftool/map.c +10 -0
+		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;

For BPF_MAP_TYPE_RHASH, can the kernel return -ENOENT before the real end of the map, so that this code silently produces a short dump?

In __rhtab_map_lookup_and_delete_batch(), each element is copied and then the next one is looked up from the copied key:

    memcpy(dst_key, elem->data, key_size);
    ...
    elem = rhashtable_next_key(&rhtab->ht, dst_key);
    ...
    if (elem && !IS_ERR(elem)) {
            memcpy(cursor, elem->data, key_size);
            has_next_cursor = true;
    }
    ...
    if (!has_next_cursor)
            ret = -ENOENT;

If a BPF program or another process deletes that element between the memcpy() and rhashtable_next_key(), __rhashtable_next_in_table() cannot find prev_key and returns ERR_PTR(-ENOENT). The call then returns the elements copied so far with -ENOENT, the same code used for a real end of the map.

dump_map_batch() prints those elements, leaves the loop, sets done = true, and map_dump() skips the bpf_map_get_next_key() loop and prints "Found N elements" and returns 0. Every element after that point is missing, with no error.

Before this patch, bpftool walked RHASH maps with bpf_map_get_next_key(), and rhtab_map_get_next_key() restarts from the first key when the previous key was deleted:

    if (PTR_ERR(elem) == -ENOENT) elem = rhashtable_next_key(&rhtab->ht, NULL);

So a concurrent delete could previously cause duplicates but not a short dump reported as success. The comment in __rhtab_map_lookup_and_delete_batch() says a lost cursor returns -EAGAIN so userspace can distinguish it from -ENOENT, but that only holds between calls, not within a single call.

Does this contradict the commit message statements "The output is unchanged" and the fallback "after any error"? The root cause may be in the kernel's rhtab batch code, but this patch makes bpftool depend on it. I did not see a later patch in the series that addresses this.

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

author
#3 lore raw

Hi,

Quoted 2 lines

For BPF_MAP_TYPE_RHASH, can the kernel return -ENOENT before the real end of the map, so that this code silently produces a short dump?

Yes, I reproduced it with a thread deleting and reinserting the first key. The dump returned success after printing one entry while the other 63 keys were still present.

I've sent a separate RHASH fix to return -EAGAIN for this case:
https://lore.kernel.org/r/[email protected]

With that fix, the existing bpftool fallback handles the race, so I've left the batch series unchanged.

Thanks,
Tianyi

maintainer Acked-by on v9 1/2
#4 lore raw
In reply to Tianyi Chen
Quoted 16 lines · Tianyi Chen

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.

Quoted 4 lines
Link https://github.com/libbpf/bpftool/issues/63
Suggested-by Alexei Starovoitov <[email protected]>
Assisted-by LLM
Signed-off-by Tianyi Chen <[email protected]>

Looks good to me, thanks!

Acked-by Quentin Monnet <[email protected]>
Quoted 78 lines
---
 tools/bpf/bpftool/map.c | 97 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 82 insertions(+), 15 deletions(-)
tools/bpf/bpftool/map.c +32 -13
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,

tools/bpf/bpftool/map.c static int dump_map_elem(int fd, void *key, void *value, · line 798

Quoted 10 lines
tools/bpf/bpftool/map.c +10 -0
+			   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);

Nit: If you respin, dump_map_batch() might be worth a comment to tell it assumes key_size + value_size to be non-zero. Right now we're good, we exit early from map_dump() (before calling dump_map_batch()) if we have no key.

author
#5 lore raw

PATCH v9 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.

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.

Pre-size RHASH maps so asynchronous growth does not race with the order-sensitive dump comparisons.

Assisted-by LLM
Signed-off-by Tianyi Chen <[email protected]>
---
 .../bpf/prog_tests/bpftool_map_batch.c        | 231 ++++++++++++++++++
 .../selftests/bpf/progs/bpftool_map_batch.c   |  42 ++++
 2 files changed, 273 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 +231 -0
new file mode 100644
index 0000000000000..4cca77f410a06
@@ -0,0 +1,231 @@
+// 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;
+	/* Avoid asynchronous growth during dump comparisons. */
+	if (test->type == BPF_MAP_TYPE_RHASH)
+		opts.map_extra = max_entries;
+
+	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
maintainer Acked-by on v9 2/2
#6 lore raw

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

Quoted 11 lines · 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.

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.

Quoted 5 lines

Pre-size RHASH maps so asynchronous growth does not race with the order-sensitive dump comparisons.

Assisted-by LLM
Signed-off-by Tianyi Chen <[email protected]>
Acked-by Quentin Monnet <[email protected]>

... with a few, non-blocking suggestions, inline below.

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

Quoted 12 lines
---
 .../bpf/prog_tests/bpftool_map_batch.c        | 231 ++++++++++++++++++
 .../selftests/bpf/progs/bpftool_map_batch.c   |  42 ++++
 2 files changed, 273 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 +0 -0
new file mode 100644
index 0000000000000..4cca77f410a06

Maybe bpftool_map_dump_batch.c, so it's easier to visually group with the existing bpftool_map_dump.c?

Quoted 1 line
tools/testing/selftests/bpf/prog_tests/bpftool_map_batch.c new file +0 -0
@@ -0,0 +1,231 @@

[...]

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

Quoted 11 lines
tools/testing/selftests/bpf/prog_tests/bpftool_map_batch.c new file +11 -0
+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 },

If I remember correctly, lru_hash/lru_percpu_hash are mostly the same code as hash/percpu_hash, so I'm not sure they're worth testing, unless you want completeness.

You could also avoid checking both plain and JSON for all types (pick one, and just do a few check for the other?), if you want to save some bpftool invocations; I don't expect differences in the code related to batch lookups between the plain and JSON output printing code.

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

Quoted 39 lines
tools/testing/selftests/bpf/prog_tests/bpftool_map_batch.c new file +34 -0
+		{ "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 +0 -0
new file mode 100644
index 0000000000000..0ca6f7973e880

bpftool_map_dump_batch.c?