Linux Kernel Selftest development
 help / color / mirror / Atom feed
From: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>
To: "Tianyi Chen" <hi@tychen.cc>, <bpf@vger.kernel.org>
Cc: <qmo@kernel.org>, <andrii@kernel.org>, <eddyz87@gmail.com>,
	<ihor.solodrai@linux.dev>, <linux-kselftest@vger.kernel.org>,
	<daniel@iogearbox.net>
Subject: Re: [PATCH bpf-next v7 1/2] bpftool: Use batch lookups for hash map dumps
Date: Wed, 04 Nov 2026 02:26:25 +0000	[thread overview]
Message-ID: <DMM2K9N35AGL.3KWJ07UTOLU4Q@gmail.com> (raw)
In-Reply-To: <a107a124d50c11a6d26363acce98e5e3b8377155.1793732386.git.hi@tychen.cc>

On Tue, Nov 03, 2026 at 11:01 AM Tianyi Chen <hi@tychen.cc> wrote:
> Bound the combined key/value buffers to 4 MiB. Start with up to 256
> entries and grow on ENOSPC using the same input cursor. At the buffer
> limit, continue individual lookups from the last emitted key without
> restarting output. Also fall back if the initial batch is unsupported.

This is overkill for 32 ms per 100k elements.
v4 measured 0.774 s vs 0.734 s without the redirect to /dev/null.

n_buckets >= max_entries in htab. A bucket with more than 256 elements
is not a real case. The growth is dead code. No subtest in patch 2
reaches it.

bpf_map_get_next_key() walks the map in the same order as the batch.
So continue from the last printed key after any error, not only after
ENOSPC. Then no need for the growth, can_restart, the list of errnos,
three return values, fprintf(stderr) and the end_dump label.

> +static bool map_dump_can_batch(const struct bpf_map_info *info)
> +{
> +	return info->type == BPF_MAP_TYPE_HASH && info->max_entries &&

No need for the type check either. The kernel has three batch lookups:

  kernel function                        maps                      cursor
  -------------------------------------  ------------------------  ------
  __htab_map_lookup_and_delete_batch()   hash, lru_hash,           u32
                                         percpu_hash,
                                         lru_percpu_hash,
                                         hash_of_maps
  generic_map_lookup_batch()             array, percpu_array,      key
                                         array_of_maps, lpm_trie
  __rhtab_map_lookup_and_delete_batch()  rhash                     key

All three walk the map in the order of bpf_map_get_next_key().
A map without map_lookup_batch returns ENOTSUPP and takes the old path.

> +		if (err && err != ENOENT)
> +			goto error;

The batch takes the bucket lock. htab_lock_bucket() can fail, and the
batch returns that error. bpf_map_lookup_elem() doesn't take the lock.
So a dump that works today fails here.

Something like this (not even compile tested):

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 4b65edf..a522d80 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -59,13 +59,18 @@ 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());
+		return (size_t)round_up(info->value_size, 8) *
+		       get_possible_cpus();
 	else
-		return malloc(info->value_size);
+		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 +746,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 +763,70 @@ 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 that batched lookups return. 'key' is the last of them.
+ * Return true at the end of the map. After any error the caller continues
+ * with bpf_map_get_next_key(). It walks the map in the same order.
+ */
+static bool dump_map_batch(int fd, void *key, struct bpf_map_info *info,
+			   struct btf *btf, json_writer_t *btf_wtr,
+			   const int *cpu_ids, int cpu_cnt,
+			   unsigned int *num_elems)
+{
+	void *keys, *values, *batch, *in_batch = NULL;
+	size_t value_size = map_value_size(info);
+	__u32 max_cnt, cnt, i;
+	bool done = false;
+	int err;
+
+	max_cnt = min(MAP_DUMP_BATCH_BYTES / (info->key_size + value_size),
+		      (size_t)info->max_entries);
+	/* The cursor is a u32 in htab maps and a key in all other maps */
+	batch = malloc(max(info->key_size, 4U));
+	keys = malloc(max_cnt * info->key_size);
+	values = malloc(max_cnt * value_size);
+	if (!max_cnt || !batch || !keys || !values)
+		goto out;
+
+	do {
+		cnt = max_cnt;
+		err = bpf_map_lookup_batch(fd, in_batch, batch, keys, values,
+					   &cnt, NULL);
+		if (err && err != -ENOENT)
+			goto out;
+		for (i = 0; i < cnt; i++) {
+			memcpy(key, keys + i * info->key_size, info->key_size);
+			print_map_elem(key, values + i * value_size, info, btf,
+				       btf_wtr, cpu_ids, cpu_cnt);
+		}
+		*num_elems += cnt;
+		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 +896,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;
+	int err = 0;
+	bool done;

 	key = malloc(info->key_size);
 	value = alloc_value(info);
@@ -895,7 +956,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, 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) {

pw-bot: cr

  reply	other threads:[~2026-11-04  2:26 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-03 19:01 [PATCH bpf-next v7 0/2] bpftool: Batch hash map dumps with bounded buffers Tianyi Chen
2026-11-03 19:01 ` [PATCH bpf-next v7 1/2] bpftool: Use batch lookups for hash map dumps Tianyi Chen
2026-11-04  2:26   ` Alexei Starovoitov [this message]
2026-11-03 19:01 ` [PATCH bpf-next v7 2/2] selftests/bpf: Check bpftool batch map dump contents Tianyi Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DMM2K9N35AGL.3KWJ07UTOLU4Q@gmail.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=hi@tychen.cc \
    --cc=ihor.solodrai@linux.dev \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=qmo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox