netdev - Re: [PATCH net-next 3/3] tools: bpftool: optionally show filenames of pinned objects

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6bf809f2-acb5-4615-bb63-24325b967d12@netronome.com>
Date:   Tue, 31 Oct 2017 11:54:20 +0000
From:   Quentin Monnet <quentin.monnet@...ronome.com>
To:     Prashant Bhole <bhole_prashant_q7@....ntt.co.jp>,
        "David S . Miller" <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>
Subject: Re: [PATCH net-next 3/3] tools: bpftool: optionally show filenames of
 pinned objects

2017-10-31 16:36 UTC+0900 ~ Prashant Bhole <bhole_prashant_q7@....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@....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