> commit 29ab48522e4514042981557a125a6b9d94bd4e9c
> Author: Tianyi Chen <[email protected]>
> Date: Mon Sep 7 09:21:26 2026 +0800
>
> bpftool: Add recursive map dumping
>
> Dumping a map-of-maps currently shows inner map IDs without their
> contents. Add -r/--recursive to dump referenced inner maps as well,
> leaving the default output unchanged.
>
> Show the selected maps followed by discovered inner maps, with each
> map's header and entries. JSON uses an array of these map objects.
> Preserve BTF formatting in plain output, including when typed and
> untyped maps are encountered in the same traversal.
>
> Keep selected map FDs open and queue distinct inner map IDs. Open,
> dump and close each queued map in turn, requiring only one additional
> map FD regardless of the number of inner maps. Preserve per-entry
> lookup error reporting for maps that do not support element lookups.
>
> Report failure if an inner map cannot be opened and close any JSON
> containers before returning. Document deferred ID resolution and the
> non-atomic dump behavior, and add option completion.
This isn't a bug, but could the changelog spend a little less space
restating the hunks and a little more on why the queue-then-reopen
approach was picked over keeping every inner-map FD open? The first
paragraph states the problem and fix well, but the following paragraphs
walk through the diff hunk by hunk, and the final sentence enumerates the
documentation and bash-completion changes themselves. The one piece of
rationale a reviewer would want - why a single reused inner-map FD was
chosen over holding every discovered FD open - is stated as a fact
("requiring only one additional map FD") rather than motivated.
[ ... ]
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index 684a8fb72414..c400b15f16dd 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
[ ... ]
> @@ -919,9 +980,34 @@ static int do_dump(int argc, char **argv)
> if (nb_fds < 1)
> goto exit_free;
>
> + if (recursive_dump) {
> + ctx.seen = hashmap__new(hash_fn_for_key_as_id,
> + equal_fn_for_key_as_id, NULL);
> + if (IS_ERR(ctx.seen)) {
> + ctx.seen = NULL;
> + p_err("failed to create hashmap for recursive dump");
> + goto exit_close;
> + }
> + /* Record the selected maps before discovering any inner maps.
> */
> + for (i = 0; i < nb_fds; i++) {
> + len = sizeof(info);
> + if (bpf_map_get_info_by_fd(fds[i], &info, &len)) {
> + p_err("can't get map info: %s",
> strerror(errno));
> + err = -1;
> + goto exit_close;
> + }
> + err = hashmap__add(ctx.seen, info.id, 0);
> + if (err) {
> + p_err("failed to record map id %u: %s", info.id,
> + strerror(-err));
> + goto exit_close;
> + }
> + }
> + }
^^^^
A subsystem pattern flags this as potentially concerning: the pre-pass
assigns the raw negative errno returned by hashmap__add() to err and then
propagates it out of do_dump() as the command return value. Every other
error path in do_dump() and map.c sets err = -1, and the new
collect_inner_map() helper deliberately normalizes the identical
hashmap__add() failure to -1.
A -ENOMEM here would make 'bpftool map dump -r' exit with status 244
instead of the 255 used everywhere else in the file. The -EEXIST case is
unreachable (map_parse_fds() yields maps with distinct ids), so this is
OOM-only and both statuses are non-zero - but it is an inconsistency
introduced by this patch and trivially fixed by 'err = -1;' as in the
sibling helper.
> @@ -936,7 +1022,7 @@ static int do_dump(int argc, char **argv)
> }
> }
>
> - if (wtr && nb_fds > 1)
> + if (wtr && (nb_fds > 1 || recursive_dump))
> jsonw_start_array(wtr); /* root array */
> for (i = 0; i < nb_fds; i++) {
> if (bpf_map_get_info_by_fd(fds[i], &info, &len)) {
This isn't a bug, but in recursive mode the pre-pass and the dump loop
each call bpf_map_get_info_by_fd() on the same fds. Could the pre-pass
hand the ids it already collected to the dump loop instead of querying
every selected map twice? The pre-pass genuinely has to run before any
dumping so that a root map that is also someone's inner map is
de-duplicated, but the info it already fetched could be reused (e.g. by
recording the ids into an array alongside the hashmap) instead of being
re-queried.
---
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/34073252721
