[PATCH bpf-next v11 0/3] bpftool: Add recursive map dumping

[PATCH bpf-next v11 0/3] bpftool: Add recursive map dumping

[Tianyi Chen]

Dump each inner map in the entry that references it. Open it by ID, dump
it and close it immediately; a missing inner map is an error only in
that entry, and shared inner maps are printed in every referring entry.

Changes in v11:
- Drop the queue, deduplication and second pass, as Alexei suggested.
- Drop the ineffective map-info test. Keep the map-info clearing fix
  for bpftool running on kernels without the kernel-side fix.
- Rewrite the recursive tests and drop Quentin's Acked-by on the
  rewritten patch.

Tested in an x86-64 VM, including shared and missing inner maps, BTF,
multiple roots, and the combination with the batch series.

v10: https://lore.kernel.org/r/cover.1793732336.git.hi@tychen.cc

Tianyi Chen (3):
  bpftool: Reset map info before each query in show and dump loops
  bpftool: Add recursive map dumping
  selftests/bpf: Cover recursive bpftool map dumps

 .../bpf/bpftool/Documentation/bpftool-map.rst |   9 +-
 tools/bpf/bpftool/bash-completion/bpftool     |   3 +
 tools/bpf/bpftool/map.c                       |  67 +++-
 .../prog_tests/bpftool_map_dump_recursive.c   | 375 ++++++++++++++++++
 .../bpf/progs/bpftool_map_dump_recursive.c    |  17 +
 5 files changed, 465 insertions(+), 6 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/bpftool_map_dump_recursive.c
 create mode 100644 tools/testing/selftests/bpf/progs/bpftool_map_dump_recursive.c


base-commit: 8fb43ffc3327d12d57d3fbe6f4820b5a522de0d1
-- 
2.56.0

[PATCH bpf-next v11 1/3] bpftool: Reset map info before each query in show and dump loops

[Tianyi Chen]

On kernels that copy bpf_map_info from userspace without clearing output
fields, reusing the structure for a map without BTF retains the preceding
map's BTF IDs.

Clear the structure and restore its length before each query in map
listing, named-map listing, BTF detection and dumping. This prevents
stale BTF IDs and formatting a map with another map's key and value types,
including when bpftool runs on kernels without the kernel-side fix.

Fixes: 71bb428fe2c1 ("tools: bpf: add bpftool")
Fixes: 99f9863a0c45 ("bpftool: Match maps by name")
Reported-by: Quentin Monnet <qmo@kernel.org>
Link: https://lore.kernel.org/r/3acbe829-06f0-4df4-b57c-a8c7b802ec96@kernel.org
Acked-by: Quentin Monnet <qmo@kernel.org>
Assisted-by: LLM
Signed-off-by: Tianyi Chen <hi@tychen.cc>
---
 tools/bpf/bpftool/map.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 4b65edf74859b..b150a06a25085 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -647,6 +647,8 @@ static int do_show_subset(int argc, char **argv)
 	if (json_output && nb_fds > 1)
 		jsonw_start_array(json_wtr);	/* root array */
 	for (i = 0; i < nb_fds; i++) {
+		memset(&info, 0, sizeof(info));
+		len = sizeof(info);
 		err = bpf_map_get_info_by_fd(fds[i], &info, &len);
 		if (err) {
 			p_err("can't get map info: %s",
@@ -718,6 +720,8 @@ static int do_show(int argc, char **argv)
 			break;
 		}
 
+		memset(&info, 0, sizeof(info));
+		len = sizeof(info);
 		err = bpf_map_get_info_by_fd(fd, &info, &len);
 		if (err) {
 			p_err("can't get map info: %s", strerror(errno));
@@ -774,6 +778,8 @@ static int maps_have_btf(int *fds, int nb_fds)
 	int err, i;
 
 	for (i = 0; i < nb_fds; i++) {
+		memset(&info, 0, sizeof(info));
+		len = sizeof(info);
 		err = bpf_map_get_info_by_fd(fds[i], &info, &len);
 		if (err) {
 			p_err("can't get map info: %s", strerror(errno));
@@ -970,6 +976,8 @@ static int do_dump(int argc, char **argv)
 	if (wtr && nb_fds > 1)
 		jsonw_start_array(wtr);	/* root array */
 	for (i = 0; i < nb_fds; i++) {
+		memset(&info, 0, sizeof(info));
+		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;
-- 
2.56.0

[PATCH bpf-next v11 2/3] bpftool: Add recursive map dumping

[Tianyi Chen]

Dumping an array_of_maps or a hash_of_maps shows the IDs of the inner
maps but not their contents. Add a recursive keyword to map dump that
also dumps the inner map referenced by each entry, in that entry. JSON
entries gain an "inner_map" object next to "inner_map_id", and plain
output prints the inner map after the entry line. The output without
the keyword is unchanged.

The kernel supports a single level of map-in-map, so inner maps have
no inner maps of their own. Each inner map is opened by ID when its
entry is printed, dumped and closed, which needs one more map FD. An
inner map shared by several entries is dumped in each of them. If an
inner map cannot be opened, for example because it was replaced after
the outer lookup, the error is reported in that entry and the dump
continues.

Link: https://github.com/libbpf/bpftool/issues/58
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Assisted-by: LLM
Signed-off-by: Tianyi Chen <hi@tychen.cc>
---
 .../bpf/bpftool/Documentation/bpftool-map.rst |  9 ++-
 tools/bpf/bpftool/bash-completion/bpftool     |  3 +
 tools/bpf/bpftool/map.c                       | 59 +++++++++++++++++--
 3 files changed, 65 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-map.rst b/tools/bpf/bpftool/Documentation/bpftool-map.rst
index e4ed7e701c9e4..729baab2b7922 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-map.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-map.rst
@@ -29,7 +29,7 @@ MAP COMMANDS
 | **bpftool** **map create**     *FILE* **type** *TYPE* **key** *KEY_SIZE* **value** *VALUE_SIZE* \
 |     **entries** *MAX_ENTRIES* **name** *NAME* [**flags** *FLAGS*] [**inner_map** *MAP*] \
 |     [**offload_dev** *NAME*]
-| **bpftool** **map dump**       *MAP*
+| **bpftool** **map dump**       *MAP* [**recursive**]
 | **bpftool** **map update**     *MAP* [**key** *DATA*] [**value** *VALUE*] [*UPDATE_FLAGS*]
 | **bpftool** **map lookup**     *MAP* [**key** *DATA*]
 | **bpftool** **map getnext**    *MAP* [**key** *DATA*]
@@ -87,10 +87,15 @@ bpftool map create *FILE* type *TYPE* key *KEY_SIZE* value *VALUE_SIZE*  entries
     Keyword **offload_dev** expects a network interface name, and is used to
     request hardware offload for the map.
 
-bpftool map dump    *MAP*
+bpftool map dump    *MAP* [recursive]
     Dump all entries in a given *MAP*.  In case of **name**, *MAP* may match
     several maps which will all be dumped.
 
+    With **recursive**, each entry of an **array_of_maps** or **hash_of_maps**
+    also has the dump of the inner map that it refers to. An inner map that
+    cannot be opened, for example because it was replaced, is reported as an
+    error in that entry.
+
 bpftool map update  *MAP* [key *DATA*] [value *VALUE*] [*UPDATE_FLAGS*]
     Update map entry for a given *KEY*.
 
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index a28af31937ad4..78fdb9396e96f 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -718,6 +718,9 @@ _bpftool()
                             return 0
                             ;;
                         *)
+                            if [[ $command == dump && $cword -eq 5 ]]; then
+                                COMPREPLY=( $( compgen -W 'recursive' -- "$cur" ) )
+                            fi
                             return 0
                             ;;
                     esac
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index b150a06a25085..fc820c07e9f4f 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -22,6 +22,9 @@
 #include "main.h"
 
 static struct hashmap *map_table;
+static bool recursive_dump;
+
+static void dump_inner_map(__u32 id);
 
 static bool map_is_per_cpu(__u32 type)
 {
@@ -140,9 +143,14 @@ static void print_entry_json(struct bpf_map_info *info, unsigned char *key,
 		print_hex_data_json(key, info->key_size);
 		jsonw_name(json_wtr, "value");
 		print_hex_data_json(value, info->value_size);
-		if (map_is_map_of_maps(info->type))
+		if (map_is_map_of_maps(info->type)) {
 			jsonw_uint_field(json_wtr, "inner_map_id",
 					 *(unsigned int *)value);
+			if (recursive_dump) {
+				jsonw_name(json_wtr, "inner_map");
+				dump_inner_map(*(unsigned int *)value);
+			}
+		}
 		if (btf) {
 			struct btf_dumper d = {
 				.btf = btf,
@@ -273,6 +281,8 @@ static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
 		}
 
 		printf("\n");
+		if (recursive_dump && map_is_map_of_maps(info->type))
+			dump_inner_map(*(unsigned int *)value);
 	} else {
 		unsigned int step;
 		int i;
@@ -935,6 +945,36 @@ map_dump(int fd, struct bpf_map_info *info, json_writer_t *wtr,
 	return err;
 }
 
+/* The kernel has one level of map-in-map: inner maps have no inner maps. */
+static void dump_inner_map(__u32 id)
+{
+	LIBBPF_OPTS(bpf_get_fd_by_id_opts, opts, .open_flags = BPF_F_RDONLY);
+	struct bpf_map_info info = {};
+	json_writer_t *btf_wtr = NULL;
+	__u32 len = sizeof(info);
+	int fd;
+
+	fd = bpf_map_get_fd_by_id_opts(id, &opts);
+	if (fd < 0) {
+		p_err("can't get map by id (%u): %s", id, strerror(errno));
+		return;
+	}
+	if (bpf_map_get_info_by_fd(fd, &info, &len)) {
+		p_err("can't get map info: %s", strerror(errno));
+		goto out;
+	}
+
+	if (!json_output && info.btf_value_type_id)
+		btf_wtr = get_btf_writer();
+
+	map_dump(fd, &info, json_output ? json_wtr : btf_wtr, true);
+
+	if (btf_wtr)
+		jsonw_destroy(&btf_wtr);
+out:
+	close(fd);
+}
+
 static int do_dump(int argc, char **argv)
 {
 	json_writer_t *wtr = NULL, *btf_wtr = NULL;
@@ -944,8 +984,8 @@ static int do_dump(int argc, char **argv)
 	int *fds = NULL;
 	int err = -1;
 
-	if (argc != 2)
-		usage();
+	if (!REQ_ARGS(2))
+		return -1;
 
 	fds = malloc(sizeof(int));
 	if (!fds) {
@@ -956,6 +996,16 @@ static int do_dump(int argc, char **argv)
 	if (nb_fds < 1)
 		goto exit_free;
 
+	while (argc) {
+		if (**argv && is_prefix(*argv, "recursive")) {
+			NEXT_ARG();
+			recursive_dump = true;
+		} else {
+			p_err("expected 'recursive', got: '%s'", *argv);
+			goto exit_close;
+		}
+	}
+
 	if (json_output) {
 		wtr = json_wtr;
 	} else {
@@ -1002,6 +1052,7 @@ static int do_dump(int argc, char **argv)
 exit_free:
 	free(fds);
 	free_btf_vmlinux();
+	recursive_dump = false;
 	return err;
 }
 
@@ -1510,7 +1561,7 @@ static int do_help(int argc, char **argv)
 		"       %1$s %2$s create     FILE type TYPE key KEY_SIZE value VALUE_SIZE \\\n"
 		"                                  entries MAX_ENTRIES name NAME [flags FLAGS] \\\n"
 		"                                  [inner_map MAP] [offload_dev NAME]\n"
-		"       %1$s %2$s dump       MAP\n"
+		"       %1$s %2$s dump       MAP [recursive]\n"
 		"       %1$s %2$s update     MAP [key DATA] [value VALUE] [UPDATE_FLAGS]\n"
 		"       %1$s %2$s lookup     MAP [key DATA]\n"
 		"       %1$s %2$s getnext    MAP [key DATA]\n"
-- 
2.56.0

[PATCH bpf-next v11 3/3] selftests/bpf: Cover recursive bpftool map dumps

[Tianyi Chen]

Check that a recursive dump of an array_of_maps or a hash_of_maps
equals the default dump with the dump of each inner map added to the
entry that refers to it, in JSON and plain output. The maps include an
inner map shared by two entries, an empty inner map, an empty array
slot, inner maps with and without BTF, and two outer maps selected by
name.

Make one inner map impossible to open with an fmod_ret program on
security_bpf_map(), as if it had been replaced, and check that only its
entry reports an error while the dump succeeds. Also check that the
keyword does not change the dumps of other maps or of empty maps of
maps, and that invalid or misplaced arguments are rejected.

Assisted-by: LLM
Signed-off-by: Tianyi Chen <hi@tychen.cc>
---
 .../prog_tests/bpftool_map_dump_recursive.c   | 375 ++++++++++++++++++
 .../bpf/progs/bpftool_map_dump_recursive.c    |  17 +
 2 files changed, 392 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/bpftool_map_dump_recursive.c
 create mode 100644 tools/testing/selftests/bpf/progs/bpftool_map_dump_recursive.c

diff --git a/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump_recursive.c b/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump_recursive.c
new file mode 100644
index 0000000000000..4d0083a18ce16
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump_recursive.c
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <stdarg.h>
+#include <test_progs.h>
+#include <bpftool_helpers.h>
+#include <bpf/btf.h>
+#include "bpftool_map_dump_recursive.skel.h"
+
+#define OUTPUT_SIZE 16384
+
+static bool run(char *output, const char *fmt, ...)
+{
+	char command[MAX_BPFTOOL_CMD_LEN];
+	va_list ap;
+
+	va_start(ap, fmt);
+	vsnprintf(command, sizeof(command), fmt, ap);
+	va_end(ap);
+	output[0] = '\0';
+	return ASSERT_OK(get_bpftool_command_output(command, output, OUTPUT_SIZE),
+			 command) &&
+	       ASSERT_LT(strlen(output), OUTPUT_SIZE - 1, "output_fits");
+}
+
+static bool append(char *buf, size_t *len, const char *fmt, ...)
+{
+	va_list ap;
+	int n;
+
+	va_start(ap, fmt);
+	n = vsnprintf(buf + *len, OUTPUT_SIZE - *len, fmt, ap);
+	va_end(ap);
+	if (!ASSERT_GE(n, 0, "format") || !ASSERT_LT(n, OUTPUT_SIZE - *len, "expected_fits"))
+		return false;
+	*len += n;
+	return true;
+}
+
+static int count_token(const char *output, const char *token)
+{
+	int count = 0;
+
+	while ((output = strstr(output, token))) {
+		count++;
+		output += strlen(token);
+	}
+	return count;
+}
+
+static __u32 map_id(int fd)
+{
+	struct bpf_map_info info = {};
+	__u32 len = sizeof(info);
+
+	if (!ASSERT_OK(bpf_map_get_info_by_fd(fd, &info, &len), "map_info"))
+		return 0;
+	return info.id;
+}
+
+static bool map_info(__u32 id, struct bpf_map_info *info)
+{
+	__u32 len = sizeof(*info);
+	int fd, err;
+
+	memset(info, 0, sizeof(*info));
+	fd = bpf_map_get_fd_by_id(id);
+	if (!ASSERT_OK_FD(fd, "map_fd"))
+		return false;
+	err = bpf_map_get_info_by_fd(fd, info, &len);
+	close(fd);
+	return ASSERT_OK(err, "map_info");
+}
+
+/* Expect an "inner_map" member after each "inner_map_id" of a default dump. */
+static bool expect_json(const char *dump, __u32 hidden, char *expected)
+{
+	const char *tag = "\"inner_map_id\":", *p;
+	char inner[OUTPUT_SIZE];
+	struct bpf_map_info info;
+	size_t len = 0;
+	char *end;
+	__u32 id;
+
+	while ((p = strstr(dump, tag))) {
+		id = strtoul(p + strlen(tag), &end, 10);
+		if (!append(expected, &len, "%.*s,\"inner_map\":", (int)(end - dump), dump))
+			return false;
+		dump = end;
+		if (id == hidden) {
+			if (!append(expected, &len, "{\"error\":\"can't get map by id (%u): %s\"}",
+				    id, strerror(ENOENT)))
+				return false;
+			continue;
+		}
+		if (!map_info(id, &info) || !run(inner, "-j map dump id %u", id))
+			return false;
+		inner[strcspn(inner, "\n")] = '\0';
+		if (!append(expected, &len,
+			    "{\"id\":%u,\"type\":\"%s\",\"name\":\"%s\",\"flags\":%u,\"elements\":%s}",
+			    id, libbpf_bpf_map_type_str(info.type), info.name,
+			    info.map_flags, inner))
+			return false;
+	}
+	return append(expected, &len, "%s", dump);
+}
+
+/* Expect the inner map after each "inner_map_id" line of a default dump. */
+static bool expect_plain(const char *dump, __u32 hidden, char *expected)
+{
+	const char *tag = "inner_map_id: ", *p, *eol;
+	char inner[OUTPUT_SIZE];
+	struct bpf_map_info info;
+	size_t len = 0;
+	__u32 id;
+
+	while ((p = strstr(dump, tag))) {
+		id = strtoul(p + strlen(tag), NULL, 10);
+		eol = strchr(p, '\n');
+		if (!ASSERT_OK_PTR(eol, "entry_line") ||
+		    !append(expected, &len, "%.*s", (int)(eol + 1 - dump), dump))
+			return false;
+		dump = eol + 1;
+		if (id == hidden)
+			continue;
+		if (!map_info(id, &info) || !run(inner, "map dump id %u", id) ||
+		    !append(expected, &len, "%u: %s  name %s  flags 0x%x\n%s", id,
+			    libbpf_bpf_map_type_str(info.type), info.name,
+			    info.map_flags, inner))
+			return false;
+	}
+	return append(expected, &len, "%s", dump);
+}
+
+static void check_dumps(const char *handle, __u32 hidden, bool plain)
+{
+	char dump[OUTPUT_SIZE], expected[OUTPUT_SIZE], output[OUTPUT_SIZE];
+
+	if (run(dump, "-j map dump %s", handle) &&
+	    expect_json(dump, hidden, expected) &&
+	    run(output, "-j map dump %s recursive", handle))
+		ASSERT_STREQ(output, expected, "json");
+	if (plain && run(dump, "map dump %s", handle) &&
+	    expect_plain(dump, hidden, expected) &&
+	    run(output, "map dump %s recursive 2>/dev/null", handle))
+		ASSERT_STREQ(output, expected, "plain");
+}
+
+static int create_inner(const char *name, struct btf **btf, bool populate)
+{
+	LIBBPF_OPTS(bpf_map_create_opts, opts);
+	__u32 key = 0, value = 0x01010101;
+	int fd;
+
+	if (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"))
+			return -1;
+		opts.btf_fd = btf__fd(*btf);
+		opts.btf_key_type_id = 1;
+		opts.btf_value_type_id = 1;
+	}
+	fd = bpf_map_create(BPF_MAP_TYPE_HASH, name, sizeof(key), sizeof(value), 1, &opts);
+	if (!ASSERT_OK_FD(fd, "create_inner"))
+		return -1;
+	if (populate && !ASSERT_OK(bpf_map_update_elem(fd, &key, &value, BPF_ANY),
+				   "update_inner")) {
+		close(fd);
+		return -1;
+	}
+	return fd;
+}
+
+/* Entries 0 and 2 share @a, entry 1 refers to @b; arrays leave slot 3 empty. */
+static int create_outer(enum bpf_map_type type, const char *name, int a, int b)
+{
+	LIBBPF_OPTS(bpf_map_create_opts, opts, .inner_map_fd = a);
+	int inner[] = { a, b, a };
+	__u32 key;
+	int fd;
+
+	fd = bpf_map_create(type, name, sizeof(key), sizeof(__u32), 4, &opts);
+	if (!ASSERT_OK_FD(fd, "create_outer"))
+		return -1;
+	for (key = 0; key < ARRAY_SIZE(inner); key++) {
+		if (!ASSERT_OK(bpf_map_update_elem(fd, &key, &inner[key], BPF_ANY),
+			       "update_outer")) {
+			close(fd);
+			return -1;
+		}
+	}
+	return fd;
+}
+
+static void test_outer(enum bpf_map_type type, bool typed_inner)
+{
+	char handle[32], token[64], output[OUTPUT_SIZE];
+	int a = -1, b = -1, outer = -1;
+	struct btf *btf = NULL;
+	__u32 a_id, b_id;
+
+	a = create_inner("rec_a", typed_inner ? &btf : NULL, true);
+	b = create_inner("rec_b", NULL, false);
+	if (a < 0 || b < 0)
+		goto out;
+	outer = create_outer(type, "rec_outer", a, b);
+	if (outer < 0)
+		goto out;
+	snprintf(handle, sizeof(handle), "id %u", map_id(outer));
+	check_dumps(handle, 0, !typed_inner);
+	if (!typed_inner)
+		goto out;
+
+	/* Plain output uses BTF for the inner maps that have it. */
+	a_id = map_id(a);
+	b_id = map_id(b);
+	if (!run(output, "map dump %s recursive", handle))
+		goto out;
+	snprintf(token, sizeof(token), "\"id\": %u,", a_id);
+	ASSERT_EQ(count_token(output, token), 2, "btf_inner_dumps");
+	ASSERT_EQ(count_token(output, "\"value\": 16843009"), 2, "btf_inner_values");
+	snprintf(token, sizeof(token), "%u: hash  name rec_b  flags 0x0\nFound 0 elements\n",
+		 b_id);
+	ASSERT_EQ(count_token(output, token), 1, "plain_inner_dump");
+out:
+	if (outer >= 0)
+		close(outer);
+	if (a >= 0)
+		close(a);
+	if (b >= 0)
+		close(b);
+	btf__free(btf);
+}
+
+static void test_missing_inner(void)
+{
+	char handle[32], message[96], output[OUTPUT_SIZE];
+	struct bpftool_map_dump_recursive *skel;
+	int a = -1, b = -1, outer = -1;
+	__u32 hidden;
+
+	skel = bpftool_map_dump_recursive__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
+		return;
+	if (!ASSERT_OK(bpftool_map_dump_recursive__attach(skel), "skel_attach"))
+		goto out;
+	a = create_inner("rec_a", NULL, true);
+	b = create_inner("rec_b", NULL, true);
+	if (a < 0 || b < 0)
+		goto out;
+	outer = create_outer(BPF_MAP_TYPE_HASH_OF_MAPS, "rec_outer", a, b);
+	hidden = map_id(b);
+	if (outer < 0 || !hidden)
+		goto out;
+	snprintf(handle, sizeof(handle), "id %u", map_id(outer));
+	skel->bss->hidden_id = hidden;
+
+	/* Only the entry of the missing map has an error; the dump succeeds. */
+	check_dumps(handle, hidden, true);
+	snprintf(message, sizeof(message), "Error: can't get map by id (%u): %s\n",
+		 hidden, strerror(ENOENT));
+	if (run(output, "map dump %s recursive 2>&1 >/dev/null", handle))
+		ASSERT_STREQ(output, message, "plain_error");
+out:
+	if (outer >= 0)
+		close(outer);
+	if (a >= 0)
+		close(a);
+	if (b >= 0)
+		close(b);
+	bpftool_map_dump_recursive__destroy(skel);
+}
+
+static void test_multiple_roots(void)
+{
+	int a = -1, b = -1, outer[] = { -1, -1 };
+	char name[BPF_OBJ_NAME_LEN], handle[32];
+	size_t i;
+
+	snprintf(name, sizeof(name), "rec_root_%u", getpid() % 100000);
+	a = create_inner("rec_a", NULL, true);
+	b = create_inner("rec_b", NULL, false);
+	if (a < 0 || b < 0)
+		goto out;
+	for (i = 0; i < ARRAY_SIZE(outer); i++) {
+		outer[i] = create_outer(BPF_MAP_TYPE_ARRAY_OF_MAPS, name, a, b);
+		if (outer[i] < 0)
+			goto out;
+	}
+	snprintf(handle, sizeof(handle), "name %s", name);
+	check_dumps(handle, 0, true);
+out:
+	for (i = 0; i < ARRAY_SIZE(outer); i++)
+		if (outer[i] >= 0)
+			close(outer[i]);
+	if (a >= 0)
+		close(a);
+	if (b >= 0)
+		close(b);
+}
+
+/* The keyword has no effect on other maps or on maps of maps without entries. */
+static void test_unchanged(void)
+{
+	char dump[OUTPUT_SIZE], output[OUTPUT_SIZE];
+	const char * const options[] = { "", "-j " };
+	int fds[] = { -1, -1 };
+	size_t i, j;
+
+	fds[0] = create_inner("rec_a", NULL, true);
+	if (fds[0] < 0)
+		return;
+	fds[1] = bpf_map_create(BPF_MAP_TYPE_HASH_OF_MAPS, "rec_empty", 4, 4, 1,
+				&(struct bpf_map_create_opts){
+					.sz = sizeof(struct bpf_map_create_opts),
+					.inner_map_fd = fds[0],
+				});
+	if (!ASSERT_OK_FD(fds[1], "create_outer"))
+		goto out;
+	for (i = 0; i < ARRAY_SIZE(fds); i++) {
+		for (j = 0; j < ARRAY_SIZE(options); j++) {
+			if (run(dump, "%smap dump id %u", options[j], map_id(fds[i])) &&
+			    run(output, "%smap dump id %u recursive", options[j],
+				map_id(fds[i])))
+				ASSERT_STREQ(output, dump, "unchanged");
+		}
+	}
+out:
+	for (i = 0; i < ARRAY_SIZE(fds); i++)
+		if (fds[i] >= 0)
+			close(fds[i]);
+}
+
+static void test_arguments(void)
+{
+	const char * const invalid[] = { "''", "unknown", "recursive unknown" };
+	char command[MAX_BPFTOOL_CMD_LEN], output[OUTPUT_SIZE] = {};
+	__u32 id;
+	size_t i;
+	int fd;
+
+	fd = create_inner("rec_args", NULL, false);
+	if (fd < 0)
+		return;
+	id = map_id(fd);
+	for (i = 0; i < ARRAY_SIZE(invalid); i++) {
+		snprintf(command, sizeof(command), "map dump id %u %s", id, invalid[i]);
+		ASSERT_NEQ(run_bpftool_command(command), 0, invalid[i]);
+	}
+	snprintf(command, sizeof(command), "map dump id %u rec", id);
+	ASSERT_OK(run_bpftool_command(command), "prefix");
+	snprintf(command, sizeof(command), "-j map dump recursive id %u", id);
+	ASSERT_NEQ(get_bpftool_command_output(command, output, sizeof(output)), 0,
+		   "misplaced_recursive");
+	ASSERT_HAS_SUBSTR(output, "got: 'recursive'", "misplaced_error");
+	close(fd);
+}
+
+void test_bpftool_map_dump_recursive(void)
+{
+	if (test__start_subtest("array_of_maps"))
+		test_outer(BPF_MAP_TYPE_ARRAY_OF_MAPS, false);
+	if (test__start_subtest("hash_of_maps"))
+		test_outer(BPF_MAP_TYPE_HASH_OF_MAPS, false);
+	if (test__start_subtest("btf_inner"))
+		test_outer(BPF_MAP_TYPE_HASH_OF_MAPS, true);
+	if (test__start_subtest("missing_inner"))
+		test_missing_inner();
+	if (test__start_subtest("multiple_roots"))
+		test_multiple_roots();
+	if (test__start_subtest("unchanged"))
+		test_unchanged();
+	if (test__start_subtest("arguments"))
+		test_arguments();
+}
diff --git a/tools/testing/selftests/bpf/progs/bpftool_map_dump_recursive.c b/tools/testing/selftests/bpf/progs/bpftool_map_dump_recursive.c
new file mode 100644
index 0000000000000..baa44070bcda2
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/bpftool_map_dump_recursive.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+#define ENOENT 2
+
+char _license[] SEC("license") = "GPL";
+
+/* Opening this map fails as if it had been freed. */
+__u32 hidden_id;
+
+SEC("fmod_ret/security_bpf_map")
+int BPF_PROG(hide_map, struct bpf_map *map, int fmode)
+{
+	return map->id == hidden_id ? -ENOENT : 0;
+}
-- 
2.56.0

Re: [PATCH bpf-next v11 3/3] selftests/bpf: Cover recursive bpftool map dumps

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]

> selftests/bpf: Cover recursive bpftool map dumps

TL;DR: Nit only, in the selftest: one bpf_map_create() call uses a compound
literal with a hand-set .sz instead of LIBBPF_OPTS() like create_outer() does
in the same file.

> diff --git a/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump_recursive.c b/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump_recursive.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/bpftool_map_dump_recursive.c

[ ... ]

> +static void test_unchanged(void)
> +{

[ ... ]

> +	fds[1] = bpf_map_create(BPF_MAP_TYPE_HASH_OF_MAPS, "rec_empty", 4, 4, 1,
> +				&(struct bpf_map_create_opts){
> +					.sz = sizeof(struct bpf_map_create_opts),
> +					.inner_map_fd = fds[0],
> +				});

this isn't a bug, but could this use LIBBPF_OPTS(bpf_map_create_opts, opts,
.inner_map_fd = fds[0]) like create_outer() does in this file, rather than a
compound literal with a hand-set .sz?


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