netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/3] tools: bpftool: show filenames of pinned objects
@ 2017-10-31  7:36 Prashant Bhole
  2017-10-31  7:36 ` [PATCH net-next 1/3] tools: bpftool: open pinned object without type check Prashant Bhole
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Prashant Bhole @ 2017-10-31  7:36 UTC (permalink / raw)
  To: David S . Miller
  Cc: Prashant Bhole, netdev, Alexei Starovoitov, Daniel Borkmann,
	Quentin Monnet, Jakub Kicinski

This patchset adds support to show pinned objects in object details.

Patch1 adds a funtionality to open a path in bpf-fs regardless of its object
type.

Patch2 adds actual functionality by scanning the bpf-fs once and adding
object information in hash table, with object id as a key. One object may be
associated with multiple paths because an object can be pinned multiple times

Patch3 adds command line option to enable this functionality. Making it optional
because scanning bpf-fs can be costly.


Prashant Bhole (3):
  tools: bpftool: open pinned object without type check
  tools: bpftool: show filenames of pinned objects
  tools: bpftool: optionally show filenames of pinned objects

 tools/bpf/bpftool/common.c | 85 ++++++++++++++++++++++++++++++++++++++++++++--
 tools/bpf/bpftool/main.c   | 18 +++++++++-
 tools/bpf/bpftool/main.h   | 19 ++++++++++-
 tools/bpf/bpftool/map.c    | 23 ++++++++++++-
 tools/bpf/bpftool/prog.c   | 24 +++++++++++++
 5 files changed, 164 insertions(+), 5 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net-next 1/3] tools: bpftool: open pinned object without type check
  2017-10-31  7:36 [PATCH net-next 0/3] tools: bpftool: show filenames of pinned objects Prashant Bhole
@ 2017-10-31  7:36 ` Prashant Bhole
  2017-10-31  7:36 ` [PATCH net-next 2/3] tools: bpftool: show filenames of pinned objects Prashant Bhole
  2017-10-31  7:36 ` [PATCH net-next 3/3] tools: bpftool: optionally " Prashant Bhole
  2 siblings, 0 replies; 9+ messages in thread
From: Prashant Bhole @ 2017-10-31  7:36 UTC (permalink / raw)
  To: David S . Miller
  Cc: Prashant Bhole, netdev, Alexei Starovoitov, Daniel Borkmann,
	Quentin Monnet, Jakub Kicinski

This was needed for opening any file in bpf-fs without knowing
its object type

Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
---
 tools/bpf/bpftool/common.c | 15 +++++++++++++--
 tools/bpf/bpftool/main.h   |  1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index f028826..4556947 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -91,9 +91,8 @@ static int mnt_bpffs(const char *target, char *buff, size_t bufflen)
 	return 0;
 }
 
-int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
+int open_obj_pinned(char *path)
 {
-	enum bpf_obj_type type;
 	int fd;
 
 	fd = bpf_obj_get(path);
@@ -105,6 +104,18 @@ int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
 		return -1;
 	}
 
+	return fd;
+}
+
+int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type)
+{
+	enum bpf_obj_type type;
+	int fd;
+
+	fd = open_obj_pinned(path);
+	if (fd < 0)
+		return -1;
+
 	type = get_fd_type(fd);
 	if (type < 0) {
 		close(fd);
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index d315d01..4b56850 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -86,6 +86,7 @@ int cmd_select(const struct cmd *cmds, int argc, char **argv,
 int get_fd_type(int fd);
 const char *get_fd_type_name(enum bpf_obj_type type);
 char *get_fdinfo(int fd, const char *key);
+int open_obj_pinned(char *path);
 int open_obj_pinned_any(char *path, enum bpf_obj_type exp_type);
 int do_pin_any(int argc, char **argv, int (*get_fd_by_id)(__u32));
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net-next 2/3] tools: bpftool: show filenames of pinned objects
  2017-10-31  7:36 [PATCH net-next 0/3] tools: bpftool: show filenames of pinned objects Prashant Bhole
  2017-10-31  7:36 ` [PATCH net-next 1/3] tools: bpftool: open pinned object without type check Prashant Bhole
@ 2017-10-31  7:36 ` Prashant Bhole
  2017-10-31 11:41   ` Quentin Monnet
  2017-10-31  7:36 ` [PATCH net-next 3/3] tools: bpftool: optionally " Prashant Bhole
  2 siblings, 1 reply; 9+ messages in thread
From: Prashant Bhole @ 2017-10-31  7:36 UTC (permalink / raw)
  To: David S . Miller
  Cc: Prashant Bhole, netdev, Alexei Starovoitov, Daniel Borkmann,
	Quentin Monnet, Jakub Kicinski

Added support to show filenames of pinned objects.

For example:

root@test# ./bpftool prog
3: tracepoint  name tracepoint__irq  tag f677a7dd722299a3
    loaded_at Oct 26/11:39  uid 0
    xlated 160B  not jited  memlock 4096B  map_ids 4
    pinned /sys/fs/bpf/softirq_prog

4: tracepoint  name tracepoint__irq  tag ea5dc530d00b92b6
    loaded_at Oct 26/11:39  uid 0
    xlated 392B  not jited  memlock 4096B  map_ids 4,6

root@test# ./bpftool --json --pretty prog
[{
        "id": 3,
        "type": "tracepoint",
        "name": "tracepoint__irq",
        "tag": "f677a7dd722299a3",
        "loaded_at": "Oct 26/11:39",
        "uid": 0,
        "bytes_xlated": 160,
        "jited": false,
        "bytes_memlock": 4096,
        "map_ids": [4
        ]
,
        "pinned": ["/sys/fs/bpf/softirq_prog"
        ]
    },{
        "id": 4,
        "type": "tracepoint",
        "name": "tracepoint__irq",
        "tag": "ea5dc530d00b92b6",
        "loaded_at": "Oct 26/11:39",
        "uid": 0,
        "bytes_xlated": 392,
        "jited": false,
        "bytes_memlock": 4096,
        "map_ids": [4,6
        ]
,
        "pinned": []
    }
]

root@test# ./bpftool map
4: hash  name start  flags 0x0
    key 4B  value 16B  max_entries 10240  memlock 1003520B
    pinned /sys/fs/bpf/softirq_map1
5: hash  name iptr  flags 0x0
    key 4B  value 8B  max_entries 10240  memlock 921600B

root@test# ./bpftool --json --pretty map
[{
        "id": 4,
        "type": "hash",
        "name": "start",
        "flags": 0,
        "bytes_key": 4,
        "bytes_value": 16,
        "max_entries": 10240,
        "bytes_memlock": 1003520,
        "pinned": ["/sys/fs/bpf/softirq_map1"
        ]
    },{
        "id": 5,
        "type": "hash",
        "name": "iptr",
        "flags": 0,
        "bytes_key": 4,
        "bytes_value": 8,
        "max_entries": 10240,
        "bytes_memlock": 921600,
        "pinned": []
    }
]

Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
---
 tools/bpf/bpftool/common.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/bpf/bpftool/main.c   |  8 ++++++
 tools/bpf/bpftool/main.h   | 15 ++++++++++
 tools/bpf/bpftool/map.c    | 22 ++++++++++++++-
 tools/bpf/bpftool/prog.c   | 23 +++++++++++++++
 5 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index 4556947..6b6247c 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -45,6 +45,7 @@
 #include <sys/mount.h>
 #include <sys/types.h>
 #include <sys/vfs.h>
+#include <fts.h>
 
 #include <bpf.h>
 
@@ -290,3 +291,72 @@ void print_hex_data_json(uint8_t *data, size_t len)
 		jsonw_printf(json_wtr, "\"0x%02hhx\"", data[i]);
 	jsonw_end_array(json_wtr);
 }
+
+int build_pinned_obj_table(struct pinned_obj_table *tab, enum bpf_obj_type type)
+{
+	char *path[] = {"/sys/fs/bpf/", NULL};
+	FTS *ftsp = NULL;
+	FTSENT *ftse = NULL;
+	enum bpf_obj_type objtype;
+	struct bpf_prog_info pinned_info = {};
+	__u32 len = sizeof(pinned_info);
+	int fd, err;
+	struct pinned_obj *obj_node;
+
+	if (!is_bpffs(path[0]))
+		return -1;
+
+	ftsp = fts_open(path, 0, NULL);
+	if (!ftsp)
+		return -1;
+
+	while ((ftse = fts_read(ftsp)) != NULL) {
+		if (!(ftse->fts_info & FTS_F))
+			continue;
+		fd = open_obj_pinned(ftse->fts_path);
+		if (fd < 0)
+			continue;
+
+		objtype = get_fd_type(fd);
+		if (objtype != type) {
+			close(fd);
+			continue;
+		}
+		memset(&pinned_info, 0, sizeof(pinned_info));
+		err = bpf_obj_get_info_by_fd(fd, &pinned_info, &len);
+		if (err) {
+			close(fd);
+			continue;
+		}
+
+		obj_node = malloc(sizeof(*obj_node));
+		if (!obj_node)
+			return -1;
+
+		memset(obj_node, 0, sizeof(*obj_node));
+		obj_node->id = pinned_info.id;
+		obj_node->path = strdup(ftse->fts_path);
+		hash_add(tab->table, &obj_node->hash, obj_node->id);
+
+		close(fd);
+	}
+
+	fts_close(ftsp);
+	return 0;
+}
+
+void delete_pinned_obj_table(struct pinned_obj_table *tab)
+{
+	struct pinned_obj *obj;
+	struct hlist_node *tmp;
+	unsigned int bkt;
+
+	if (hash_empty(tab->table))
+		return;
+
+	hash_for_each_safe(tab->table, bkt, tmp, obj, hash) {
+		hash_del(&obj->hash);
+		free(obj->path);
+		free(obj);
+	}
+}
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 78d9afb..6ad53f1 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -54,6 +54,8 @@
 json_writer_t *json_wtr;
 bool pretty_output;
 bool json_output;
+struct pinned_obj_table prog_table;
+struct pinned_obj_table map_table;
 
 void usage(void)
 {
@@ -272,6 +274,9 @@ int main(int argc, char **argv)
 	json_output = false;
 	bin_name = argv[0];
 
+	hash_init(prog_table.table);
+	hash_init(map_table.table);
+
 	while ((opt = getopt_long(argc, argv, "Vhpj",
 				  options, NULL)) >= 0) {
 		switch (opt) {
@@ -311,5 +316,8 @@ int main(int argc, char **argv)
 	if (json_output)
 		jsonw_destroy(&json_wtr);
 
+	delete_pinned_obj_table(&prog_table);
+	delete_pinned_obj_table(&map_table);
+
 	return ret;
 }
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 4b56850..3abaa17 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -42,6 +42,7 @@
 #include <stdio.h>
 #include <linux/bpf.h>
 #include <linux/kernel.h>
+#include <linux/hashtable.h>
 
 #include "json_writer.h"
 
@@ -70,11 +71,25 @@ enum bpf_obj_type {
 
 extern json_writer_t *json_wtr;
 extern bool json_output;
+extern struct pinned_obj_table prog_table;
+extern struct pinned_obj_table map_table;
 
 bool is_prefix(const char *pfx, const char *str);
 void fprint_hex(FILE *f, void *arg, unsigned int n, const char *sep);
 void usage(void) __attribute__((noreturn));
 
+struct pinned_obj_table {
+	DECLARE_HASHTABLE(table, 16);
+};
+
+struct pinned_obj {
+	__u32 id;
+	char *path;
+	struct hlist_node hash;
+};
+int build_pinned_obj_table(struct pinned_obj_table *table, enum bpf_obj_type type);
+void delete_pinned_obj_table(struct pinned_obj_table *tab);
+
 struct cmd {
 	const char *cmd;
 	int (*func)(int argc, char **argv);
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index e978ab2..fbe236e 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -436,6 +436,18 @@ static int show_map_close_json(int fd, struct bpf_map_info *info)
 		jsonw_int_field(json_wtr, "bytes_memlock", atoi(memlock));
 	free(memlock);
 
+	if (!hash_empty(map_table.table)) {
+		struct pinned_obj *obj;
+
+		jsonw_name(json_wtr, "pinned");
+		jsonw_start_array(json_wtr);
+		hash_for_each_possible(map_table.table, obj, hash, info->id) {
+			if (obj->id == info->id)
+				jsonw_string(json_wtr, obj->path);
+		}
+		jsonw_end_array(json_wtr);
+	}
+
 	jsonw_end_object(json_wtr);
 
 	return 0;
@@ -466,7 +478,13 @@ static int show_map_close_plain(int fd, struct bpf_map_info *info)
 	free(memlock);
 
 	printf("\n");
-
+	if (!hash_empty(map_table.table)) {
+		struct pinned_obj *obj;
+		hash_for_each_possible(map_table.table, obj, hash, info->id) {
+			if (obj->id == info->id)
+				printf("\tpinned %s\n", obj->path);
+		}
+	}
 	return 0;
 }
 
@@ -478,6 +496,8 @@ static int do_show(int argc, char **argv)
 	int err;
 	int fd;
 
+	build_pinned_obj_table(&map_table, BPF_OBJ_MAP);
+
 	if (argc == 2) {
 		fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
 		if (fd < 0)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 250f80f..cd29eae 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -207,6 +207,7 @@ static void show_prog_maps(int fd, u32 num_maps)
 			printf("%u%s", map_ids[i],
 			       i == info.nr_map_ids - 1 ? "" : ",");
 	}
+	printf("\n");
 }
 
 static void print_prog_json(struct bpf_prog_info *info, int fd)
@@ -256,6 +257,18 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
 	if (info->nr_map_ids)
 		show_prog_maps(fd, info->nr_map_ids);
 
+	if (!hash_empty(prog_table.table)) {
+		struct pinned_obj *obj;
+
+		jsonw_name(json_wtr, "pinned");
+		jsonw_start_array(json_wtr);
+		hash_for_each_possible(prog_table.table, obj, hash, info->id) {
+			if (obj->id == info->id)
+				jsonw_string(json_wtr, obj->path);
+		}
+		jsonw_end_array(json_wtr);
+	}
+
 	jsonw_end_object(json_wtr);
 }
 
@@ -300,6 +313,14 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
 	if (info->nr_map_ids)
 		show_prog_maps(fd, info->nr_map_ids);
 
+	if (!hash_empty(prog_table.table)) {
+		struct pinned_obj *obj;
+		hash_for_each_possible(prog_table.table, obj, hash, info->id) {
+			if (obj->id == info->id)
+				printf("\tpinned %s\n", obj->path);
+		}
+	}
+
 	printf("\n");
 }
 
@@ -329,6 +350,8 @@ static int do_show(int argc, char **argv)
 	int err;
 	int fd;
 
+	build_pinned_obj_table(&prog_table, BPF_OBJ_PROG);
+
 	if (argc == 2) {
 		fd = prog_parse_fd(&argc, &argv);
 		if (fd < 0)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net-next 3/3] tools: bpftool: optionally show filenames of pinned objects
  2017-10-31  7:36 [PATCH net-next 0/3] tools: bpftool: show filenames of pinned objects Prashant Bhole
  2017-10-31  7:36 ` [PATCH net-next 1/3] tools: bpftool: open pinned object without type check Prashant Bhole
  2017-10-31  7:36 ` [PATCH net-next 2/3] tools: bpftool: show filenames of pinned objects Prashant Bhole
@ 2017-10-31  7:36 ` Prashant Bhole
  2017-10-31 11:54   ` Quentin Monnet
  2 siblings, 1 reply; 9+ messages in thread
From: Prashant Bhole @ 2017-10-31  7:36 UTC (permalink / raw)
  To: David S . Miller
  Cc: Prashant Bhole, netdev, Alexei Starovoitov, Daniel Borkmann,
	Quentin Monnet, Jakub Kicinski

Making it optional to show file names of pinned objects because
it scans complete bpf-fs filesystem which is costly.
Added option -l|--pinned

Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
---
 tools/bpf/bpftool/main.c | 14 +++++++++++---
 tools/bpf/bpftool/main.h |  3 ++-
 tools/bpf/bpftool/map.c  |  3 ++-
 tools/bpf/bpftool/prog.c |  3 ++-
 4 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 6ad53f1..2ae26c6 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -54,6 +54,7 @@
 json_writer_t *json_wtr;
 bool pretty_output;
 bool json_output;
+bool show_pinned;
 struct pinned_obj_table prog_table;
 struct pinned_obj_table map_table;
 
@@ -265,6 +266,7 @@ int main(int argc, char **argv)
 		{ "help",	no_argument,	NULL,	'h' },
 		{ "pretty",	no_argument,	NULL,	'p' },
 		{ "version",	no_argument,	NULL,	'V' },
+		{ "pinned",	no_argument,	NULL,	'l' },
 		{ 0 }
 	};
 	int opt, ret;
@@ -272,12 +274,13 @@ int main(int argc, char **argv)
 	last_do_help = do_help;
 	pretty_output = false;
 	json_output = false;
+	show_pinned = false;
 	bin_name = argv[0];
 
 	hash_init(prog_table.table);
 	hash_init(map_table.table);
 
-	while ((opt = getopt_long(argc, argv, "Vhpj",
+	while ((opt = getopt_long(argc, argv, "Vhpjl",
 				  options, NULL)) >= 0) {
 		switch (opt) {
 		case 'V':
@@ -290,6 +293,9 @@ int main(int argc, char **argv)
 		case 'j':
 			json_output = true;
 			break;
+		case 'l':
+			show_pinned = true;
+			break;
 		default:
 			usage();
 		}
@@ -316,8 +322,10 @@ int main(int argc, char **argv)
 	if (json_output)
 		jsonw_destroy(&json_wtr);
 
-	delete_pinned_obj_table(&prog_table);
-	delete_pinned_obj_table(&map_table);
+	if (show_pinned) {
+		delete_pinned_obj_table(&prog_table);
+		delete_pinned_obj_table(&map_table);
+	}
 
 	return ret;
 }
diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
index 3abaa17..6c668a6 100644
--- a/tools/bpf/bpftool/main.h
+++ b/tools/bpf/bpftool/main.h
@@ -59,7 +59,7 @@
 #define HELP_SPEC_PROGRAM						\
 	"PROG := { id PROG_ID | pinned FILE | tag PROG_TAG }"
 #define HELP_SPEC_OPTIONS						\
-	"OPTIONS := { {-j|--json} [{-p|--pretty}] }"
+	"OPTIONS := { {-j|--json} [{-p|--pretty}] {-l |--pinned} }"
 
 enum bpf_obj_type {
 	BPF_OBJ_UNKNOWN,
@@ -71,6 +71,7 @@ enum bpf_obj_type {
 
 extern json_writer_t *json_wtr;
 extern bool json_output;
+extern bool show_pinned;
 extern struct pinned_obj_table prog_table;
 extern struct pinned_obj_table map_table;
 
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index fbe236e..c4354f8 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -496,7 +496,8 @@ static int do_show(int argc, char **argv)
 	int err;
 	int fd;
 
-	build_pinned_obj_table(&map_table, BPF_OBJ_MAP);
+	if (show_pinned)
+		build_pinned_obj_table(&map_table, BPF_OBJ_MAP);
 
 	if (argc == 2) {
 		fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index cd29eae..c97c954 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -350,7 +350,8 @@ static int do_show(int argc, char **argv)
 	int err;
 	int fd;
 
-	build_pinned_obj_table(&prog_table, BPF_OBJ_PROG);
+	if (show_pinned)
+		build_pinned_obj_table(&prog_table, BPF_OBJ_PROG);
 
 	if (argc == 2) {
 		fd = prog_parse_fd(&argc, &argv);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 2/3] tools: bpftool: show filenames of pinned objects
  2017-10-31  7:36 ` [PATCH net-next 2/3] tools: bpftool: show filenames of pinned objects Prashant Bhole
@ 2017-10-31 11:41   ` Quentin Monnet
  2017-10-31 17:35     ` Jakub Kicinski
  2017-11-02  7:57     ` Prashant Bhole
  0 siblings, 2 replies; 9+ messages in thread
From: Quentin Monnet @ 2017-10-31 11:41 UTC (permalink / raw)
  To: Prashant Bhole, David S . Miller
  Cc: netdev, Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski

2017-10-31 16:36 UTC+0900 ~ Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> Added support to show filenames of pinned objects.
>
> For example:
>
> root@test# ./bpftool prog
> 3: tracepoint  name tracepoint__irq  tag f677a7dd722299a3
>     loaded_at Oct 26/11:39  uid 0
>     xlated 160B  not jited  memlock 4096B  map_ids 4
>     pinned /sys/fs/bpf/softirq_prog
>
> 4: tracepoint  name tracepoint__irq  tag ea5dc530d00b92b6
>     loaded_at Oct 26/11:39  uid 0
>     xlated 392B  not jited  memlock 4096B  map_ids 4,6
>
> root@test# ./bpftool --json --pretty prog
> [{
>         "id": 3,
>         "type": "tracepoint",
>         "name": "tracepoint__irq",
>         "tag": "f677a7dd722299a3",
>         "loaded_at": "Oct 26/11:39",
>         "uid": 0,
>         "bytes_xlated": 160,
>         "jited": false,
>         "bytes_memlock": 4096,
>         "map_ids": [4
>         ]
> ,
>         "pinned": ["/sys/fs/bpf/softirq_prog"
>         ]
>     },{
>         "id": 4,
>         "type": "tracepoint",
>         "name": "tracepoint__irq",
>         "tag": "ea5dc530d00b92b6",
>         "loaded_at": "Oct 26/11:39",
>         "uid": 0,
>         "bytes_xlated": 392,
>         "jited": false,
>         "bytes_memlock": 4096,
>         "map_ids": [4,6
>         ]
> ,
>         "pinned": []
>     }
> ]
>
> root@test# ./bpftool map
> 4: hash  name start  flags 0x0
>     key 4B  value 16B  max_entries 10240  memlock 1003520B
>     pinned /sys/fs/bpf/softirq_map1
> 5: hash  name iptr  flags 0x0
>     key 4B  value 8B  max_entries 10240  memlock 921600B
>
> root@test# ./bpftool --json --pretty map
> [{
>         "id": 4,
>         "type": "hash",
>         "name": "start",
>         "flags": 0,
>         "bytes_key": 4,
>         "bytes_value": 16,
>         "max_entries": 10240,
>         "bytes_memlock": 1003520,
>         "pinned": ["/sys/fs/bpf/softirq_map1"
>         ]
>     },{
>         "id": 5,
>         "type": "hash",
>         "name": "iptr",
>         "flags": 0,
>         "bytes_key": 4,
>         "bytes_value": 8,
>         "max_entries": 10240,
>         "bytes_memlock": 921600,
>         "pinned": []
>     }
> ]
>
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> ---
>  tools/bpf/bpftool/common.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++
>  tools/bpf/bpftool/main.c   |  8 ++++++
>  tools/bpf/bpftool/main.h   | 15 ++++++++++
>  tools/bpf/bpftool/map.c    | 22 ++++++++++++++-
>  tools/bpf/bpftool/prog.c   | 23 +++++++++++++++
>  5 files changed, 137 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index 4556947..6b6247c 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> @@ -45,6 +45,7 @@
>  #include <sys/mount.h>
>  #include <sys/types.h>
>  #include <sys/vfs.h>
> +#include <fts.h>
>  
>  #include <bpf.h>
>  
> @@ -290,3 +291,72 @@ void print_hex_data_json(uint8_t *data, size_t len)
>  		jsonw_printf(json_wtr, "\"0x%02hhx\"", data[i]);
>  	jsonw_end_array(json_wtr);
>  }
> +
> +int build_pinned_obj_table(struct pinned_obj_table *tab, enum bpf_obj_type type)

Nit: line over 80 characters, please wrap.

> +{
> +	char *path[] = {"/sys/fs/bpf/", NULL};

This will not work for progs/maps pinned elsewhere on the system. Might
be worth mentioning in the documentation?

> +	FTS *ftsp = NULL;
> +	FTSENT *ftse = NULL;
> +	enum bpf_obj_type objtype;
> +	struct bpf_prog_info pinned_info = {};
> +	__u32 len = sizeof(pinned_info);
> +	int fd, err;
> +	struct pinned_obj *obj_node;

Please use reversed-Christmas tree style.

> +
> +	if (!is_bpffs(path[0]))
> +		return -1;
> +
> +	ftsp = fts_open(path, 0, NULL);
> +	if (!ftsp)
> +		return -1;
> +
> +	while ((ftse = fts_read(ftsp)) != NULL) {
> +		if (!(ftse->fts_info & FTS_F))
> +			continue;
> +		fd = open_obj_pinned(ftse->fts_path);
> +		if (fd < 0)
> +			continue;
> +
> +		objtype = get_fd_type(fd);
> +		if (objtype != type) {
> +			close(fd);
> +			continue;
> +		}
> +		memset(&pinned_info, 0, sizeof(pinned_info));
> +		err = bpf_obj_get_info_by_fd(fd, &pinned_info, &len);
> +		if (err) {
> +			close(fd);
> +			continue;
> +		}
> +
> +		obj_node = malloc(sizeof(*obj_node));
> +		if (!obj_node)
> +			return -1;

Maybe fd and ftsp should be closed before returning?

> +
> +		memset(obj_node, 0, sizeof(*obj_node));
> +		obj_node->id = pinned_info.id;
> +		obj_node->path = strdup(ftse->fts_path);
> +		hash_add(tab->table, &obj_node->hash, obj_node->id);
> +
> +		close(fd);
> +	}
> +
> +	fts_close(ftsp);
> +	return 0;
> +}
> +
> +void delete_pinned_obj_table(struct pinned_obj_table *tab)
> +{
> +	struct pinned_obj *obj;
> +	struct hlist_node *tmp;
> +	unsigned int bkt;
> +
> +	if (hash_empty(tab->table))
> +		return;
> +
> +	hash_for_each_safe(tab->table, bkt, tmp, obj, hash) {
> +		hash_del(&obj->hash);
> +		free(obj->path);
> +		free(obj);
> +	}
> +}
> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index 78d9afb..6ad53f1 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -54,6 +54,8 @@
>  json_writer_t *json_wtr;
>  bool pretty_output;
>  bool json_output;
> +struct pinned_obj_table prog_table;
> +struct pinned_obj_table map_table;
>  
>  void usage(void)
>  {
> @@ -272,6 +274,9 @@ int main(int argc, char **argv)
>  	json_output = false;
>  	bin_name = argv[0];
>  
> +	hash_init(prog_table.table);
> +	hash_init(map_table.table);
> +
>  	while ((opt = getopt_long(argc, argv, "Vhpj",
>  				  options, NULL)) >= 0) {
>  		switch (opt) {
> @@ -311,5 +316,8 @@ int main(int argc, char **argv)
>  	if (json_output)
>  		jsonw_destroy(&json_wtr);
>  
> +	delete_pinned_obj_table(&prog_table);
> +	delete_pinned_obj_table(&map_table);
> +
>  	return ret;
>  }
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 4b56850..3abaa17 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -42,6 +42,7 @@
>  #include <stdio.h>
>  #include <linux/bpf.h>
>  #include <linux/kernel.h>
> +#include <linux/hashtable.h>
>  
>  #include "json_writer.h"
>  
> @@ -70,11 +71,25 @@ enum bpf_obj_type {
>  
>  extern json_writer_t *json_wtr;
>  extern bool json_output;
> +extern struct pinned_obj_table prog_table;
> +extern struct pinned_obj_table map_table;
>  
>  bool is_prefix(const char *pfx, const char *str);
>  void fprint_hex(FILE *f, void *arg, unsigned int n, const char *sep);
>  void usage(void) __attribute__((noreturn));
>  
> +struct pinned_obj_table {
> +	DECLARE_HASHTABLE(table, 16);
> +};
> +
> +struct pinned_obj {
> +	__u32 id;
> +	char *path;
> +	struct hlist_node hash;
> +};

Nit: add a blank line here?

> +int build_pinned_obj_table(struct pinned_obj_table *table, enum bpf_obj_type type);

Please wrap this line as well.

> +void delete_pinned_obj_table(struct pinned_obj_table *tab);
> +
>  struct cmd {
>  	const char *cmd;
>  	int (*func)(int argc, char **argv);
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index e978ab2..fbe236e 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -436,6 +436,18 @@ static int show_map_close_json(int fd, struct bpf_map_info *info)
>  		jsonw_int_field(json_wtr, "bytes_memlock", atoi(memlock));
>  	free(memlock);
>  
> +	if (!hash_empty(map_table.table)) {
> +		struct pinned_obj *obj;
> +
> +		jsonw_name(json_wtr, "pinned");
> +		jsonw_start_array(json_wtr);
> +		hash_for_each_possible(map_table.table, obj, hash, info->id) {
> +			if (obj->id == info->id)
> +				jsonw_string(json_wtr, obj->path);
> +		}
> +		jsonw_end_array(json_wtr);
> +	}
> +
>  	jsonw_end_object(json_wtr);
>  
>  	return 0;
> @@ -466,7 +478,13 @@ static int show_map_close_plain(int fd, struct bpf_map_info *info)
>  	free(memlock);
>  
>  	printf("\n");
> -
> +	if (!hash_empty(map_table.table)) {
> +		struct pinned_obj *obj;

Nit: add a blank line after declarations.

> +		hash_for_each_possible(map_table.table, obj, hash, info->id) {
> +			if (obj->id == info->id)
> +				printf("\tpinned %s\n", obj->path);
> +		}
> +	}
>  	return 0;
>  }
>  
> @@ -478,6 +496,8 @@ static int do_show(int argc, char **argv)
>  	int err;
>  	int fd;
>  
> +	build_pinned_obj_table(&map_table, BPF_OBJ_MAP);
> +
>  	if (argc == 2) {
>  		fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
>  		if (fd < 0)
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 250f80f..cd29eae 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -207,6 +207,7 @@ static void show_prog_maps(int fd, u32 num_maps)
>  			printf("%u%s", map_ids[i],
>  			       i == info.nr_map_ids - 1 ? "" : ",");
>  	}
> +	printf("\n");

This printf() should not be here, because it would add a new line even
if there is no pinned path to add, and regardless of plain / JSON output
selection. It is responsible for the line breaks before the commas you
have in your commit log example.

>  }
>  
>  static void print_prog_json(struct bpf_prog_info *info, int fd)
> @@ -256,6 +257,18 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
>  	if (info->nr_map_ids)
>  		show_prog_maps(fd, info->nr_map_ids);
>  
> +	if (!hash_empty(prog_table.table)) {
> +		struct pinned_obj *obj;
> +
> +		jsonw_name(json_wtr, "pinned");
> +		jsonw_start_array(json_wtr);
> +		hash_for_each_possible(prog_table.table, obj, hash, info->id) {
> +			if (obj->id == info->id)
> +				jsonw_string(json_wtr, obj->path);
> +		}
> +		jsonw_end_array(json_wtr);
> +	}
> +
>  	jsonw_end_object(json_wtr);
>  }
>  
> @@ -300,6 +313,14 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
>  	if (info->nr_map_ids)
>  		show_prog_maps(fd, info->nr_map_ids);
>  
> +	if (!hash_empty(prog_table.table)) {
> +		struct pinned_obj *obj;

Nit: add a blank line after declarations.
Here might be a better place for the printf("\n");?

> +		hash_for_each_possible(prog_table.table, obj, hash, info->id) {
> +			if (obj->id == info->id)
> +				printf("\tpinned %s\n", obj->path);
> +		}
> +	}
> +
>  	printf("\n");
>  }
>  
> @@ -329,6 +350,8 @@ static int do_show(int argc, char **argv)
>  	int err;
>  	int fd;
>  
> +	build_pinned_obj_table(&prog_table, BPF_OBJ_PROG);
> +
>  	if (argc == 2) {
>  		fd = prog_parse_fd(&argc, &argv);
>  		if (fd < 0)

Quentin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 3/3] tools: bpftool: optionally show filenames of pinned objects
  2017-10-31  7:36 ` [PATCH net-next 3/3] tools: bpftool: optionally " Prashant Bhole
@ 2017-10-31 11:54   ` Quentin Monnet
  2017-10-31 17:37     ` Jakub Kicinski
  0 siblings, 1 reply; 9+ messages in thread
From: Quentin Monnet @ 2017-10-31 11:54 UTC (permalink / raw)
  To: Prashant Bhole, David S . Miller
  Cc: netdev, Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski

2017-10-31 16:36 UTC+0900 ~ Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> Making it optional to show file names of pinned objects because
> it scans complete bpf-fs filesystem which is costly.
> Added option -l|--pinned
>
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> ---
>  tools/bpf/bpftool/main.c | 14 +++++++++++---
>  tools/bpf/bpftool/main.h |  3 ++-
>  tools/bpf/bpftool/map.c  |  3 ++-
>  tools/bpf/bpftool/prog.c |  3 ++-
>  4 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> index 6ad53f1..2ae26c6 100644
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -54,6 +54,7 @@
>  json_writer_t *json_wtr;
>  bool pretty_output;
>  bool json_output;
> +bool show_pinned;
>  struct pinned_obj_table prog_table;
>  struct pinned_obj_table map_table;
>  
> @@ -265,6 +266,7 @@ int main(int argc, char **argv)
>  		{ "help",	no_argument,	NULL,	'h' },
>  		{ "pretty",	no_argument,	NULL,	'p' },
>  		{ "version",	no_argument,	NULL,	'V' },
> +		{ "pinned",	no_argument,	NULL,	'l' },

“l” is not especially easy to remember for “pinned”, but I don't see a
good alternative, so fine by me (uppercase “P” may not make things any
better I suppose).

>  		{ 0 }
>  	};
>  	int opt, ret;
> @@ -272,12 +274,13 @@ int main(int argc, char **argv)
>  	last_do_help = do_help;
>  	pretty_output = false;
>  	json_output = false;
> +	show_pinned = false;
>  	bin_name = argv[0];
>  
>  	hash_init(prog_table.table);
>  	hash_init(map_table.table);
>  
> -	while ((opt = getopt_long(argc, argv, "Vhpj",
> +	while ((opt = getopt_long(argc, argv, "Vhpjl",
>  				  options, NULL)) >= 0) {
>  		switch (opt) {
>  		case 'V':
> @@ -290,6 +293,9 @@ int main(int argc, char **argv)
>  		case 'j':
>  			json_output = true;
>  			break;
> +		case 'l':
> +			show_pinned = true;
> +			break;
>  		default:
>  			usage();
>  		}
> @@ -316,8 +322,10 @@ int main(int argc, char **argv)
>  	if (json_output)
>  		jsonw_destroy(&json_wtr);
>  
> -	delete_pinned_obj_table(&prog_table);
> -	delete_pinned_obj_table(&map_table);
> +	if (show_pinned) {
> +		delete_pinned_obj_table(&prog_table);
> +		delete_pinned_obj_table(&map_table);
> +	}
>  
>  	return ret;
>  }
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 3abaa17..6c668a6 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -59,7 +59,7 @@
>  #define HELP_SPEC_PROGRAM						\
>  	"PROG := { id PROG_ID | pinned FILE | tag PROG_TAG }"
>  #define HELP_SPEC_OPTIONS						\
> -	"OPTIONS := { {-j|--json} [{-p|--pretty}] }"
> +	"OPTIONS := { {-j|--json} [{-p|--pretty}] {-l |--pinned} }"

Please add a vertical bar as separator between options, and remove space
after -l:
OPTIONS := { {-j|--json} [{-p|--pretty}] | {-l|--pinned} }

(There is no separator between --json and --pretty because --pretty is
supposed to be used in addition to --json.)

>  
>  enum bpf_obj_type {
>  	BPF_OBJ_UNKNOWN,
> @@ -71,6 +71,7 @@ enum bpf_obj_type {
>  
>  extern json_writer_t *json_wtr;
>  extern bool json_output;
> +extern bool show_pinned;
>  extern struct pinned_obj_table prog_table;
>  extern struct pinned_obj_table map_table;
>  
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index fbe236e..c4354f8 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -496,7 +496,8 @@ static int do_show(int argc, char **argv)
>  	int err;
>  	int fd;
>  
> -	build_pinned_obj_table(&map_table, BPF_OBJ_MAP);
> +	if (show_pinned)
> +		build_pinned_obj_table(&map_table, BPF_OBJ_MAP);
>  
>  	if (argc == 2) {
>  		fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index cd29eae..c97c954 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -350,7 +350,8 @@ static int do_show(int argc, char **argv)
>  	int err;
>  	int fd;
>  
> -	build_pinned_obj_table(&prog_table, BPF_OBJ_PROG);
> +	if (show_pinned)
> +		build_pinned_obj_table(&prog_table, BPF_OBJ_PROG);
>  
>  	if (argc == 2) {
>  		fd = prog_parse_fd(&argc, &argv);


Please also add documentation for this option in
Documentation/bpftool-prog.rst and Documentation/bpftool-map.rst.

Quentin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 2/3] tools: bpftool: show filenames of pinned objects
  2017-10-31 11:41   ` Quentin Monnet
@ 2017-10-31 17:35     ` Jakub Kicinski
  2017-11-02  7:57     ` Prashant Bhole
  1 sibling, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2017-10-31 17:35 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Prashant Bhole, David S . Miller, netdev, Alexei Starovoitov,
	Daniel Borkmann

On Tue, 31 Oct 2017 11:41:34 +0000, Quentin Monnet wrote:
> > +{
> > +	char *path[] = {"/sys/fs/bpf/", NULL};  
> 
> This will not work for progs/maps pinned elsewhere on the system. Might
> be worth mentioning in the documentation?

Or should the code just scan all the mount points to find the bpffs
ones?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net-next 3/3] tools: bpftool: optionally show filenames of pinned objects
  2017-10-31 11:54   ` Quentin Monnet
@ 2017-10-31 17:37     ` Jakub Kicinski
  0 siblings, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2017-10-31 17:37 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Prashant Bhole, David S . Miller, netdev, Alexei Starovoitov,
	Daniel Borkmann

On Tue, 31 Oct 2017 11:54:20 +0000, Quentin Monnet wrote:
> > @@ -265,6 +266,7 @@ int main(int argc, char **argv)
> >  		{ "help",	no_argument,	NULL,	'h' },
> >  		{ "pretty",	no_argument,	NULL,	'p' },
> >  		{ "version",	no_argument,	NULL,	'V' },
> > +		{ "pinned",	no_argument,	NULL,	'l' },  
> 
> “l” is not especially easy to remember for “pinned”, but I don't see a
> good alternative, so fine by me (uppercase “P” may not make things any
> better I suppose).

Since -p is taken maybe something to do with bpffs instead of pinned?
--bpffs | -b or -f?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH net-next 2/3] tools: bpftool: show filenames of pinned objects
  2017-10-31 11:41   ` Quentin Monnet
  2017-10-31 17:35     ` Jakub Kicinski
@ 2017-11-02  7:57     ` Prashant Bhole
  1 sibling, 0 replies; 9+ messages in thread
From: Prashant Bhole @ 2017-11-02  7:57 UTC (permalink / raw)
  To: 'Quentin Monnet'
  Cc: netdev, 'Alexei Starovoitov', 'Daniel Borkmann',
	'Jakub Kicinski', 'David S . Miller'



> From: Quentin Monnet [mailto:quentin.monnet@netronome.com]
>	 
> 2017-10-31 16:36 UTC+0900 ~ Prashant Bhole
> <bhole_prashant_q7@lab.ntt.co.jp>
> > Added support to show filenames of pinned objects.
> >
> > For example:
> >
> > root@test# ./bpftool prog
> > 3: tracepoint  name tracepoint__irq  tag f677a7dd722299a3
> >     loaded_at Oct 26/11:39  uid 0
> >     xlated 160B  not jited  memlock 4096B  map_ids 4
> >     pinned /sys/fs/bpf/softirq_prog
> >
> > 4: tracepoint  name tracepoint__irq  tag ea5dc530d00b92b6
> >     loaded_at Oct 26/11:39  uid 0
> >     xlated 392B  not jited  memlock 4096B  map_ids 4,6
> >
> > root@test# ./bpftool --json --pretty prog [{
> >         "id": 3,
> >         "type": "tracepoint",
> >         "name": "tracepoint__irq",
> >         "tag": "f677a7dd722299a3",
> >         "loaded_at": "Oct 26/11:39",
> >         "uid": 0,
> >         "bytes_xlated": 160,
> >         "jited": false,
> >         "bytes_memlock": 4096,
> >         "map_ids": [4
> >         ]
> > ,
> >         "pinned": ["/sys/fs/bpf/softirq_prog"
> >         ]
> >     },{
> >         "id": 4,
> >         "type": "tracepoint",
> >         "name": "tracepoint__irq",
> >         "tag": "ea5dc530d00b92b6",
> >         "loaded_at": "Oct 26/11:39",
> >         "uid": 0,
> >         "bytes_xlated": 392,
> >         "jited": false,
> >         "bytes_memlock": 4096,
> >         "map_ids": [4,6
> >         ]
> > ,
> >         "pinned": []
> >     }
> > ]
> >
> > root@test# ./bpftool map
> > 4: hash  name start  flags 0x0
> >     key 4B  value 16B  max_entries 10240  memlock 1003520B
> >     pinned /sys/fs/bpf/softirq_map1
> > 5: hash  name iptr  flags 0x0
> >     key 4B  value 8B  max_entries 10240  memlock 921600B
> >
> > root@test# ./bpftool --json --pretty map [{
> >         "id": 4,
> >         "type": "hash",
> >         "name": "start",
> >         "flags": 0,
> >         "bytes_key": 4,
> >         "bytes_value": 16,
> >         "max_entries": 10240,
> >         "bytes_memlock": 1003520,
> >         "pinned": ["/sys/fs/bpf/softirq_map1"
> >         ]
> >     },{
> >         "id": 5,
> >         "type": "hash",
> >         "name": "iptr",
> >         "flags": 0,
> >         "bytes_key": 4,
> >         "bytes_value": 8,
> >         "max_entries": 10240,
> >         "bytes_memlock": 921600,
> >         "pinned": []
> >     }
> > ]
> >
> > Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> > ---
> >  tools/bpf/bpftool/common.c | 70
> ++++++++++++++++++++++++++++++++++++++++++++++
> >  tools/bpf/bpftool/main.c   |  8 ++++++
> >  tools/bpf/bpftool/main.h   | 15 ++++++++++
> >  tools/bpf/bpftool/map.c    | 22 ++++++++++++++-
> >  tools/bpf/bpftool/prog.c   | 23 +++++++++++++++
> >  5 files changed, 137 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> > index 4556947..6b6247c 100644
> > --- a/tools/bpf/bpftool/common.c
> > +++ b/tools/bpf/bpftool/common.c
> > @@ -45,6 +45,7 @@
> >  #include <sys/mount.h>
> >  #include <sys/types.h>
> >  #include <sys/vfs.h>
> > +#include <fts.h>
> >
> >  #include <bpf.h>
> >
> > @@ -290,3 +291,72 @@ void print_hex_data_json(uint8_t *data, size_t len)
> >  		jsonw_printf(json_wtr, "\"0x%02hhx\"", data[i]);
> >  	jsonw_end_array(json_wtr);
> >  }
> > +
> > +int build_pinned_obj_table(struct pinned_obj_table *tab, enum
> > +bpf_obj_type type)
> 
> Nit: line over 80 characters, please wrap.
> 
> > +{
> > +	char *path[] = {"/sys/fs/bpf/", NULL};
> 
> This will not work for progs/maps pinned elsewhere on the system. Might be
> worth mentioning in the documentation?
> 
> > +	FTS *ftsp = NULL;
> > +	FTSENT *ftse = NULL;
> > +	enum bpf_obj_type objtype;
> > +	struct bpf_prog_info pinned_info = {};
> > +	__u32 len = sizeof(pinned_info);
> > +	int fd, err;
> > +	struct pinned_obj *obj_node;
> 
> Please use reversed-Christmas tree style.
> 
> > +
> > +	if (!is_bpffs(path[0]))
> > +		return -1;
> > +
> > +	ftsp = fts_open(path, 0, NULL);
> > +	if (!ftsp)
> > +		return -1;
> > +
> > +	while ((ftse = fts_read(ftsp)) != NULL) {
> > +		if (!(ftse->fts_info & FTS_F))
> > +			continue;
> > +		fd = open_obj_pinned(ftse->fts_path);
> > +		if (fd < 0)
> > +			continue;
> > +
> > +		objtype = get_fd_type(fd);
> > +		if (objtype != type) {
> > +			close(fd);
> > +			continue;
> > +		}
> > +		memset(&pinned_info, 0, sizeof(pinned_info));
> > +		err = bpf_obj_get_info_by_fd(fd, &pinned_info, &len);
> > +		if (err) {
> > +			close(fd);
> > +			continue;
> > +		}
> > +
> > +		obj_node = malloc(sizeof(*obj_node));
> > +		if (!obj_node)
> > +			return -1;
> 
> Maybe fd and ftsp should be closed before returning?
> 
> > +
> > +		memset(obj_node, 0, sizeof(*obj_node));
> > +		obj_node->id = pinned_info.id;
> > +		obj_node->path = strdup(ftse->fts_path);
> > +		hash_add(tab->table, &obj_node->hash, obj_node->id);
> > +
> > +		close(fd);
> > +	}
> > +
> > +	fts_close(ftsp);
> > +	return 0;
> > +}
> > +
> > +void delete_pinned_obj_table(struct pinned_obj_table *tab) {
> > +	struct pinned_obj *obj;
> > +	struct hlist_node *tmp;
> > +	unsigned int bkt;
> > +
> > +	if (hash_empty(tab->table))
> > +		return;
> > +
> > +	hash_for_each_safe(tab->table, bkt, tmp, obj, hash) {
> > +		hash_del(&obj->hash);
> > +		free(obj->path);
> > +		free(obj);
> > +	}
> > +}
> > diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index
> > 78d9afb..6ad53f1 100644
> > --- a/tools/bpf/bpftool/main.c
> > +++ b/tools/bpf/bpftool/main.c
> > @@ -54,6 +54,8 @@
> >  json_writer_t *json_wtr;
> >  bool pretty_output;
> >  bool json_output;
> > +struct pinned_obj_table prog_table;
> > +struct pinned_obj_table map_table;
> >
> >  void usage(void)
> >  {
> > @@ -272,6 +274,9 @@ int main(int argc, char **argv)
> >  	json_output = false;
> >  	bin_name = argv[0];
> >
> > +	hash_init(prog_table.table);
> > +	hash_init(map_table.table);
> > +
> >  	while ((opt = getopt_long(argc, argv, "Vhpj",
> >  				  options, NULL)) >= 0) {
> >  		switch (opt) {
> > @@ -311,5 +316,8 @@ int main(int argc, char **argv)
> >  	if (json_output)
> >  		jsonw_destroy(&json_wtr);
> >
> > +	delete_pinned_obj_table(&prog_table);
> > +	delete_pinned_obj_table(&map_table);
> > +
> >  	return ret;
> >  }
> > diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h index
> > 4b56850..3abaa17 100644
> > --- a/tools/bpf/bpftool/main.h
> > +++ b/tools/bpf/bpftool/main.h
> > @@ -42,6 +42,7 @@
> >  #include <stdio.h>
> >  #include <linux/bpf.h>
> >  #include <linux/kernel.h>
> > +#include <linux/hashtable.h>
> >
> >  #include "json_writer.h"
> >
> > @@ -70,11 +71,25 @@ enum bpf_obj_type {
> >
> >  extern json_writer_t *json_wtr;
> >  extern bool json_output;
> > +extern struct pinned_obj_table prog_table; extern struct
> > +pinned_obj_table map_table;
> >
> >  bool is_prefix(const char *pfx, const char *str);  void
> > fprint_hex(FILE *f, void *arg, unsigned int n, const char *sep);  void
> > usage(void) __attribute__((noreturn));
> >
> > +struct pinned_obj_table {
> > +	DECLARE_HASHTABLE(table, 16);
> > +};
> > +
> > +struct pinned_obj {
> > +	__u32 id;
> > +	char *path;
> > +	struct hlist_node hash;
> > +};
> 
> Nit: add a blank line here?
> 
> > +int build_pinned_obj_table(struct pinned_obj_table *table, enum
> > +bpf_obj_type type);
> 
> Please wrap this line as well.
> 
> > +void delete_pinned_obj_table(struct pinned_obj_table *tab);
> > +
> >  struct cmd {
> >  	const char *cmd;
> >  	int (*func)(int argc, char **argv);
> > diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c index
> > e978ab2..fbe236e 100644
> > --- a/tools/bpf/bpftool/map.c
> > +++ b/tools/bpf/bpftool/map.c
> > @@ -436,6 +436,18 @@ static int show_map_close_json(int fd, struct
> bpf_map_info *info)
> >  		jsonw_int_field(json_wtr, "bytes_memlock", atoi(memlock));
> >  	free(memlock);
> >
> > +	if (!hash_empty(map_table.table)) {
> > +		struct pinned_obj *obj;
> > +
> > +		jsonw_name(json_wtr, "pinned");
> > +		jsonw_start_array(json_wtr);
> > +		hash_for_each_possible(map_table.table, obj, hash, info->id) {
> > +			if (obj->id == info->id)
> > +				jsonw_string(json_wtr, obj->path);
> > +		}
> > +		jsonw_end_array(json_wtr);
> > +	}
> > +
> >  	jsonw_end_object(json_wtr);
> >
> >  	return 0;
> > @@ -466,7 +478,13 @@ static int show_map_close_plain(int fd, struct
> bpf_map_info *info)
> >  	free(memlock);
> >
> >  	printf("\n");
> > -
> > +	if (!hash_empty(map_table.table)) {
> > +		struct pinned_obj *obj;
> 
> Nit: add a blank line after declarations.
> 
> > +		hash_for_each_possible(map_table.table, obj, hash, info->id) {
> > +			if (obj->id == info->id)
> > +				printf("\tpinned %s\n", obj->path);
> > +		}
> > +	}
> >  	return 0;
> >  }
> >
> > @@ -478,6 +496,8 @@ static int do_show(int argc, char **argv)
> >  	int err;
> >  	int fd;
> >
> > +	build_pinned_obj_table(&map_table, BPF_OBJ_MAP);
> > +
> >  	if (argc == 2) {
> >  		fd = map_parse_fd_and_info(&argc, &argv, &info, &len);
> >  		if (fd < 0)
> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c index
> > 250f80f..cd29eae 100644
> > --- a/tools/bpf/bpftool/prog.c
> > +++ b/tools/bpf/bpftool/prog.c
> > @@ -207,6 +207,7 @@ static void show_prog_maps(int fd, u32 num_maps)
> >  			printf("%u%s", map_ids[i],
> >  			       i == info.nr_map_ids - 1 ? "" : ",");
> >  	}
> > +	printf("\n");
> 
> This printf() should not be here, because it would add a new line even if there is
> no pinned path to add, and regardless of plain / JSON output selection. It is
> responsible for the line breaks before the commas you have in your commit log
> example.
> 
> >  }
> >
> >  static void print_prog_json(struct bpf_prog_info *info, int fd) @@
> > -256,6 +257,18 @@ static void print_prog_json(struct bpf_prog_info *info, int
> fd)
> >  	if (info->nr_map_ids)
> >  		show_prog_maps(fd, info->nr_map_ids);
> >
> > +	if (!hash_empty(prog_table.table)) {
> > +		struct pinned_obj *obj;
> > +
> > +		jsonw_name(json_wtr, "pinned");
> > +		jsonw_start_array(json_wtr);
> > +		hash_for_each_possible(prog_table.table, obj, hash, info->id) {
> > +			if (obj->id == info->id)
> > +				jsonw_string(json_wtr, obj->path);
> > +		}
> > +		jsonw_end_array(json_wtr);
> > +	}
> > +
> >  	jsonw_end_object(json_wtr);
> >  }
> >
> > @@ -300,6 +313,14 @@ static void print_prog_plain(struct bpf_prog_info
> *info, int fd)
> >  	if (info->nr_map_ids)
> >  		show_prog_maps(fd, info->nr_map_ids);
> >
> > +	if (!hash_empty(prog_table.table)) {
> > +		struct pinned_obj *obj;
> 
> Nit: add a blank line after declarations.
> Here might be a better place for the printf("\n");?
> 
> > +		hash_for_each_possible(prog_table.table, obj, hash, info->id) {
> > +			if (obj->id == info->id)
> > +				printf("\tpinned %s\n", obj->path);
> > +		}
> > +	}
> > +
> >  	printf("\n");
> >  }
> >
> > @@ -329,6 +350,8 @@ static int do_show(int argc, char **argv)
> >  	int err;
> >  	int fd;
> >
> > +	build_pinned_obj_table(&prog_table, BPF_OBJ_PROG);
> > +
> >  	if (argc == 2) {
> >  		fd = prog_parse_fd(&argc, &argv);
> >  		if (fd < 0)
> 
> Quentin

All requested changes by you and Jakub are done in v2 patch series. Posting soon. Thanks.

Prashant

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-11-02  7:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-31  7:36 [PATCH net-next 0/3] tools: bpftool: show filenames of pinned objects Prashant Bhole
2017-10-31  7:36 ` [PATCH net-next 1/3] tools: bpftool: open pinned object without type check Prashant Bhole
2017-10-31  7:36 ` [PATCH net-next 2/3] tools: bpftool: show filenames of pinned objects Prashant Bhole
2017-10-31 11:41   ` Quentin Monnet
2017-10-31 17:35     ` Jakub Kicinski
2017-11-02  7:57     ` Prashant Bhole
2017-10-31  7:36 ` [PATCH net-next 3/3] tools: bpftool: optionally " Prashant Bhole
2017-10-31 11:54   ` Quentin Monnet
2017-10-31 17:37     ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).