> Exercise recursive map dumping for array-of-maps and hash-of-maps,
> including shared inner maps, empty outer and inner maps, BTF-formatted
> values and multiple selected roots.
>
> Check complete JSON documents against the existing nonrecursive entry
> representations, and check plain headers and typed values. Verify both
> short and long options, root ordering, deduplication and unchanged
> default output.
>
> Cover ordinary and nested perf event arrays to ensure unsupported
> element lookups retain their error markers without failing the dump.
> Dump 64 distinct inner maps under RLIMIT_NOFILE=32 in both JSON and
> plain modes to catch holding every discovered inner-map FD open.
>
> Open 32 extra descriptors before forking, close inherited descriptors
> in the child before imposing the limit, and verify the parent's file
> descriptors and outer map remain usable.
This isn't a bug, but could the changelog be trimmed to focus on the
coverage added and the reason RLIMIT_NOFILE=32 / 64 inner maps is the
interesting case?
The last paragraph in particular restates the code almost line for line
(opening 32 descriptors, closing inherited descriptors in the child,
lowering the limit, re-checking the parent), which a reader can get from
the diff. The load-bearing part of this patch is the one non-obvious idea,
i.e. that RLIMIT_NOFILE=32 with 64 inner maps is what makes the FD-per-
inner-map regression detectable.
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump.c
> b/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump.c
> new file mode 100644
> index 000000000000..1fb877fc68c8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump.c
[ ... ]
> +static void test_outer(enum bpf_map_type type, int entries, bool empty_inner,
> + bool typed)
> +{
> + LIBBPF_OPTS(bpf_map_create_opts, opts);
> + LIBBPF_OPTS(bpf_map_create_opts, inner_opts);
> + struct btf *btf = NULL;
> + char outer[OUTPUT_SIZE], inner[OUTPUT_SIZE], output[OUTPUT_SIZE];
> + char expected[OUTPUT_SIZE * 3], reference[64];
> + const char *type_name = libbpf_bpf_map_type_str(type);
> + int inner_fd = -1, outer_fd = -1;
> + __u32 root_id, inner_id, key, value = 0x01010101;
[ ... ]
> + outer_fd = bpf_map_create(type, "dump_outer", sizeof(key),
> sizeof(__u32),
> + 3, &opts);
> + if (!ASSERT_OK_FD(outer_fd, "create_outer"))
> + goto out;
> + /* The unused third array slot also exercises failed lookups. */
> + for (key = 0; key < entries; key++)
> + if (!ASSERT_OK(bpf_map_update_elem(outer_fd, &key, &inner_fd,
> BPF_ANY),
> + "populate_outer"))
> + goto out;
This isn't a bug, but would it be clearer to say the outer map is over-
sized so the array_of_maps cases also walk unfilled slots, rather than
naming 'the unused third array slot'?
The comment describes one specific configuration rather than what the loop
does across the seven call sites. test_outer is invoked with
BPF_MAP_TYPE_HASH_OF_MAPS in four of the seven subtests, where a
hash_of_maps enumerates only the keys that were inserted, so there is no
third slot and no failed lookup at all; and for entries == 0 or 1 (four
more subtests) the number of unfilled array slots is three or two, not
one.
[ ... ]
> +static void test_many_inner_maps(bool json)
> +{
> + enum {
> + DUMP_OK,
> + DUMP_ERR_RLIMIT,
> + DUMP_ERR_COMMAND,
> + DUMP_ERR_COUNTS,
> + DUMP_ERR_IDS,
> + DUMP_ERR_JSON,
> + DUMP_ERR_FDS,
> + };
> + LIBBPF_OPTS(bpf_map_create_opts, opts);
> + const struct rlimit limit = { .rlim_cur = 32, .rlim_max = 32 };
> + char command[MAX_BPFTOOL_CMD_LEN], token[64];
> + __u32 ids[64], root_id, key;
> + int inner_fd = -1, outer_fd = -1, status;
> + int inherited_fds[32], nr_inherited = 0, i;
> + char *output = NULL;
> + pid_t pid;
[ ... ]
> + root_id = map_id(outer_fd);
> + output = calloc(1, 65536);
> + if (!root_id || !ASSERT_OK_PTR(output, "allocate_output"))
> + goto out;
[ ... ]
> + snprintf(command, sizeof(command), "%s -r map dump id %u",
> + json ? "-j" : "", root_id);
> + if (get_bpftool_command_output(command, output, 65535))
> + _exit(DUMP_ERR_COMMAND);
This isn't a bug, but would a macro alongside OUTPUT_SIZE (and passing
size - 1 to the helper, as the other call sites do) keep these two sizes
from drifting apart?
The output buffer size is spelled out as two related magic numbers in two
places (calloc(1, 65536) and get_bpftool_command_output(..., 65535)),
which have to be kept in the N / N-1 relationship by hand. Every other
buffer in the file goes through the OUTPUT_SIZE macro and passes
sizeof(buf) - 1, e.g. dump_map()'s get_bpftool_command_output(command,
output, OUTPUT_SIZE - 1) and check_plain()'s sizeof(output) - 1, so this
is the only place where the size is not derived from a single definition.
---
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/34560347455
