[PATCH bpf-next 0/3] bpf: add bpffs pretty print for hash/lru_hash maps

[PATCH bpf-next 0/3] bpf: add bpffs pretty print for hash/lru_hash maps

[Yonghong Song]

Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
the basic arraymap") added pretty print support to array map.
This patch adds pretty print for hash and lru_hash maps.

The following example shows the pretty-print result of a pinned hashmap.
Without this patch set, user will get an error instead.

    struct map_value {
            int count_a;
            int count_b;
    };

    cat /sys/fs/bpf/pinned_hash_map:

    87907: {87907,87908}
    57354: {37354,57355}
    76625: {76625,76626}
    ...

Patch #1 fixed a bug in bpffs map_seq_next() function so that
all elements in the hash table will be traversed.
Patch #2 implemented map_seq_show_elem() and map_check_btf()
callback functions for hash and lru hash maps.
Patch #3 enhanced tools/testing/selftests/bpf/test_btf.c to
test bpffs hash and lru hash map pretty print.

Yonghong Song (3):
  bpf: fix bpffs non-array map seq_show issue
  bpf: btf: add pretty print for hash/lru_hash maps
  tools/bpf: add bpffs pretty print btf test for hash/lru_hash maps

 kernel/bpf/hashtab.c                   | 44 +++++++++++++++++
 kernel/bpf/inode.c                     |  8 ++--
 tools/testing/selftests/bpf/test_btf.c | 87 ++++++++++++++++++++++++++++------
 3 files changed, 121 insertions(+), 18 deletions(-)

-- 
2.14.3

[PATCH bpf-next 1/3] bpf: fix bpffs non-array map seq_show issue

[Yonghong Song]

In function map_seq_next() of kernel/bpf/inode.c,
the first key will be the "0" regardless of the map type.
This works for array. But for hash type, if it happens
key "0" is in the map, the bpffs map show will miss
some items if the key "0" is not the first element of
the first bucket.

This patch fixed the issue by guaranteeing to get
the first element, if the seq_show is just started,
by passing NULL pointer key to map_get_next_key() callback.
This way, no missing elements will occur for
bpffs hash table show even if key "0" is in the map.

Fixes: a26ca7c982cb5 ("bpf: btf: Add pretty print support to the basic arraymap")
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 kernel/bpf/inode.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
index 76efe9a183f5..fc5b103512e7 100644
--- a/kernel/bpf/inode.c
+++ b/kernel/bpf/inode.c
@@ -196,19 +196,21 @@ static void *map_seq_next(struct seq_file *m, void *v, loff_t *pos)
 {
 	struct bpf_map *map = seq_file_to_map(m);
 	void *key = map_iter(m)->key;
+	void *prev_key;
 
 	if (map_iter(m)->done)
 		return NULL;
 
 	if (unlikely(v == SEQ_START_TOKEN))
-		goto done;
+		prev_key = NULL;
+	else
+		prev_key = key;
 
-	if (map->ops->map_get_next_key(map, key, key)) {
+	if (map->ops->map_get_next_key(map, prev_key, key)) {
 		map_iter(m)->done = true;
 		return NULL;
 	}
 
-done:
 	++(*pos);
 	return key;
 }
-- 
2.14.3

[PATCH bpf-next 2/3] bpf: btf: add pretty print for hash/lru_hash maps

[Yonghong Song]

Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
the basic arraymap") added pretty print support to array map.
This patch adds pretty print for hash and lru_hash maps.
The following example shows the pretty-print result of
a pinned hashmap:

    struct map_value {
            int count_a;
            int count_b;
    };

    cat /sys/fs/bpf/pinned_hash_map:

    87907: {87907,87908}
    57354: {37354,57355}
    76625: {76625,76626}
    ...

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 kernel/bpf/hashtab.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 513d9dfcf4ee..d6110042e0d9 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -11,9 +11,11 @@
  * General Public License for more details.
  */
 #include <linux/bpf.h>
+#include <linux/btf.h>
 #include <linux/jhash.h>
 #include <linux/filter.h>
 #include <linux/rculist_nulls.h>
+#include <uapi/linux/btf.h>
 #include "percpu_freelist.h"
 #include "bpf_lru_list.h"
 #include "map_in_map.h"
@@ -1162,6 +1164,44 @@ static void htab_map_free(struct bpf_map *map)
 	kfree(htab);
 }
 
+static void htab_map_seq_show_elem(struct bpf_map *map, void *key,
+				   struct seq_file *m)
+{
+	void *value;
+
+	rcu_read_lock();
+
+	value = htab_map_lookup_elem(map, key);
+	if (!value) {
+		rcu_read_unlock();
+		return;
+	}
+
+	btf_type_seq_show(map->btf, map->btf_key_type_id, key, m);
+	seq_puts(m, ": ");
+	btf_type_seq_show(map->btf, map->btf_value_type_id, value, m);
+	seq_puts(m, "\n");
+
+	rcu_read_unlock();
+}
+
+static int htab_map_check_btf(const struct bpf_map *map, const struct btf *btf,
+			      u32 btf_key_id, u32 btf_value_id)
+{
+	const struct btf_type *key_type, *value_type;
+	u32 key_size, value_size;
+
+	key_type = btf_type_id_size(btf, &btf_key_id, &key_size);
+	if (!key_type || key_size != map->key_size)
+		return -EINVAL;
+
+	value_type = btf_type_id_size(btf, &btf_value_id, &value_size);
+	if (!value_type || value_size != map->value_size)
+		return -EINVAL;
+
+	return 0;
+}
+
 const struct bpf_map_ops htab_map_ops = {
 	.map_alloc_check = htab_map_alloc_check,
 	.map_alloc = htab_map_alloc,
@@ -1171,6 +1211,8 @@ const struct bpf_map_ops htab_map_ops = {
 	.map_update_elem = htab_map_update_elem,
 	.map_delete_elem = htab_map_delete_elem,
 	.map_gen_lookup = htab_map_gen_lookup,
+	.map_seq_show_elem = htab_map_seq_show_elem,
+	.map_check_btf = htab_map_check_btf,
 };
 
 const struct bpf_map_ops htab_lru_map_ops = {
@@ -1182,6 +1224,8 @@ const struct bpf_map_ops htab_lru_map_ops = {
 	.map_update_elem = htab_lru_map_update_elem,
 	.map_delete_elem = htab_lru_map_delete_elem,
 	.map_gen_lookup = htab_lru_map_gen_lookup,
+	.map_seq_show_elem = htab_map_seq_show_elem,
+	.map_check_btf = htab_map_check_btf,
 };
 
 /* Called from eBPF program */
-- 
2.14.3

[PATCH bpf-next 3/3] tools/bpf: add bpffs pretty print btf test for hash/lru_hash maps

[Yonghong Song]

Pretty print tests for hash/lru_hash maps are added in test_btf.c.
The btf type blob is the same as pretty print array map test.
The test result:
  $ mount -t bpf bpf /sys/fs/bpf
  $ ./test_btf -p
    BTF pretty print array......OK
    BTF pretty print hash......OK
    BTF pretty print lru hash......OK
    PASS:3 SKIP:0 FAIL:0

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/test_btf.c | 87 ++++++++++++++++++++++++++++------
 1 file changed, 72 insertions(+), 15 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
index ffdd27737c9e..7fa8c800c540 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -131,6 +131,8 @@ struct btf_raw_test {
 	__u32 max_entries;
 	bool btf_load_err;
 	bool map_create_err;
+	bool ordered_map;
+	bool lossless_map;
 	int hdr_len_delta;
 	int type_off_delta;
 	int str_off_delta;
@@ -2093,8 +2095,7 @@ struct pprint_mapv {
 	} aenum;
 };
 
-static struct btf_raw_test pprint_test = {
-	.descr = "BTF pretty print test #1",
+static struct btf_raw_test pprint_test_template = {
 	.raw_types = {
 		/* unsighed char */			/* [1] */
 		BTF_TYPE_INT_ENC(NAME_TBD, 0, 0, 8, 1),
@@ -2146,8 +2147,6 @@ static struct btf_raw_test pprint_test = {
 	},
 	.str_sec = "\0unsigned char\0unsigned short\0unsigned int\0int\0unsigned long long\0uint8_t\0uint16_t\0uint32_t\0int32_t\0uint64_t\0ui64\0ui8a\0ENUM_ZERO\0ENUM_ONE\0ENUM_TWO\0ENUM_THREE\0pprint_mapv\0ui32\0ui16\0si32\0unused_bits2a\0bits28\0unused_bits2b\0aenum",
 	.str_sec_size = sizeof("\0unsigned char\0unsigned short\0unsigned int\0int\0unsigned long long\0uint8_t\0uint16_t\0uint32_t\0int32_t\0uint64_t\0ui64\0ui8a\0ENUM_ZERO\0ENUM_ONE\0ENUM_TWO\0ENUM_THREE\0pprint_mapv\0ui32\0ui16\0si32\0unused_bits2a\0bits28\0unused_bits2b\0aenum"),
-	.map_type = BPF_MAP_TYPE_ARRAY,
-	.map_name = "pprint_test",
 	.key_size = sizeof(unsigned int),
 	.value_size = sizeof(struct pprint_mapv),
 	.key_type_id = 3,	/* unsigned int */
@@ -2155,6 +2154,40 @@ static struct btf_raw_test pprint_test = {
 	.max_entries = 128 * 1024,
 };
 
+static struct btf_pprint_test_meta {
+	const char *descr;
+	enum bpf_map_type map_type;
+	const char *map_name;
+	bool ordered_map;
+	bool lossless_map;
+} pprint_tests_meta[] = {
+{
+	.descr = "BTF pretty print array",
+	.map_type = BPF_MAP_TYPE_ARRAY,
+	.map_name = "pprint_test_array",
+	.ordered_map = true,
+	.lossless_map = true,
+},
+
+{
+	.descr = "BTF pretty print hash",
+	.map_type = BPF_MAP_TYPE_HASH,
+	.map_name = "pprint_test_hash",
+	.ordered_map = false,
+	.lossless_map = true,
+},
+
+{
+	.descr = "BTF pretty print lru hash",
+	.map_type = BPF_MAP_TYPE_LRU_HASH,
+	.map_name = "pprint_test_lru_hash",
+	.ordered_map = false,
+	.lossless_map = false,
+},
+
+};
+
+
 static void set_pprint_mapv(struct pprint_mapv *v, uint32_t i)
 {
 	v->ui32 = i;
@@ -2166,10 +2199,12 @@ static void set_pprint_mapv(struct pprint_mapv *v, uint32_t i)
 	v->aenum = i & 0x03;
 }
 
-static int test_pprint(void)
+static int do_test_pprint(void)
 {
-	const struct btf_raw_test *test = &pprint_test;
+	const struct btf_raw_test *test = &pprint_test_template;
 	struct bpf_create_map_attr create_attr = {};
+	unsigned int key, nr_read_elems;
+	bool ordered_map, lossless_map;
 	int map_fd = -1, btf_fd = -1;
 	struct pprint_mapv mapv = {};
 	unsigned int raw_btf_size;
@@ -2178,7 +2213,6 @@ static int test_pprint(void)
 	char pin_path[255];
 	size_t line_len = 0;
 	char *line = NULL;
-	unsigned int key;
 	uint8_t *raw_btf;
 	ssize_t nread;
 	int err, ret;
@@ -2251,14 +2285,18 @@ static int test_pprint(void)
 		goto done;
 	}
 
-	key = 0;
+	nr_read_elems = 0;
+	ordered_map = test->ordered_map;
+	lossless_map = test->lossless_map;
 	do {
 		ssize_t nexpected_line;
+		unsigned int next_key;
 
-		set_pprint_mapv(&mapv, key);
+		next_key = ordered_map ? nr_read_elems : atoi(line);
+		set_pprint_mapv(&mapv, next_key);
 		nexpected_line = snprintf(expected_line, sizeof(expected_line),
 					  "%u: {%u,0,%d,0x%x,0x%x,0x%x,{%lu|[%u,%u,%u,%u,%u,%u,%u,%u]},%s}\n",
-					  key,
+					  next_key,
 					  mapv.ui32, mapv.si32,
 					  mapv.unused_bits2a, mapv.bits28, mapv.unused_bits2b,
 					  mapv.ui64,
@@ -2281,11 +2319,12 @@ static int test_pprint(void)
 		}
 
 		nread = getline(&line, &line_len, pin_file);
-	} while (++key < test->max_entries && nread > 0);
+	} while (++nr_read_elems < test->max_entries && nread > 0);
 
-	if (CHECK(key < test->max_entries,
-		  "Unexpected EOF. key:%u test->max_entries:%u",
-		  key, test->max_entries)) {
+	if (lossless_map &&
+	    CHECK(nr_read_elems < test->max_entries,
+		  "Unexpected EOF. nr_read_elems:%u test->max_entries:%u",
+		  nr_read_elems, test->max_entries)) {
 		err = -1;
 		goto done;
 	}
@@ -2314,6 +2353,24 @@ static int test_pprint(void)
 	return err;
 }
 
+static int test_pprint(void)
+{
+	unsigned int i;
+	int err = 0;
+
+	for (i = 0; i < ARRAY_SIZE(pprint_tests_meta); i++) {
+		pprint_test_template.descr = pprint_tests_meta[i].descr;
+		pprint_test_template.map_type = pprint_tests_meta[i].map_type;
+		pprint_test_template.map_name = pprint_tests_meta[i].map_name;
+		pprint_test_template.ordered_map = pprint_tests_meta[i].ordered_map;
+		pprint_test_template.lossless_map = pprint_tests_meta[i].lossless_map;
+
+		err |= count_result(do_test_pprint());
+	}
+
+	return err;
+}
+
 static void usage(const char *cmd)
 {
 	fprintf(stderr, "Usage: %s [-l] [[-r test_num (1 - %zu)] | [-g test_num (1 - %zu)] | [-f test_num (1 - %zu)] | [-p]]\n",
@@ -2409,7 +2466,7 @@ int main(int argc, char **argv)
 		err |= test_file();
 
 	if (args.pprint_test)
-		err |= count_result(test_pprint());
+		err |= test_pprint();
 
 	if (args.raw_test || args.get_info_test || args.file_test ||
 	    args.pprint_test)
-- 
2.14.3

Re: [PATCH bpf-next 0/3] bpf: add bpffs pretty print for hash/lru_hash maps

[Daniel Borkmann]

On 08/09/2018 05:55 PM, Yonghong Song wrote:
> Commit a26ca7c982cb ("bpf: btf: Add pretty print support to
> the basic arraymap") added pretty print support to array map.
> This patch adds pretty print for hash and lru_hash maps.
> 
> The following example shows the pretty-print result of a pinned hashmap.
> Without this patch set, user will get an error instead.
> 
>     struct map_value {
>             int count_a;
>             int count_b;
>     };
> 
>     cat /sys/fs/bpf/pinned_hash_map:
> 
>     87907: {87907,87908}
>     57354: {37354,57355}
>     76625: {76625,76626}
>     ...
> 
> Patch #1 fixed a bug in bpffs map_seq_next() function so that
> all elements in the hash table will be traversed.
> Patch #2 implemented map_seq_show_elem() and map_check_btf()
> callback functions for hash and lru hash maps.
> Patch #3 enhanced tools/testing/selftests/bpf/test_btf.c to
> test bpffs hash and lru hash map pretty print.
> 
> Yonghong Song (3):
>   bpf: fix bpffs non-array map seq_show issue
>   bpf: btf: add pretty print for hash/lru_hash maps
>   tools/bpf: add bpffs pretty print btf test for hash/lru_hash maps
> 
>  kernel/bpf/hashtab.c                   | 44 +++++++++++++++++
>  kernel/bpf/inode.c                     |  8 ++--
>  tools/testing/selftests/bpf/test_btf.c | 87 ++++++++++++++++++++++++++++------
>  3 files changed, 121 insertions(+), 18 deletions(-)

Applied to bpf-next, thanks Yonghong!