netdev - Re: [PATCH bpf-next] bpftool: Improve handling of ENOENT on map dumps

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]

Date:   Thu, 8 Nov 2018 13:25:18 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     David Ahern <dsahern@...nel.org>
Cc:     netdev@...r.kernel.org, David Ahern <dsahern@...il.com>
Subject: Re: [PATCH bpf-next] bpftool: Improve handling of ENOENT on map
 dumps

On Thu,  8 Nov 2018 13:00:07 -0800, David Ahern wrote:
> From: David Ahern <dsahern@...il.com>
> 
> bpftool output is not user friendly when dumping a map with only a few
> populated entries:
> 
>     $ bpftool map
>     1: devmap  name tx_devmap  flags 0x0
>             key 4B  value 4B  max_entries 64  memlock 4096B
>     2: array  name tx_idxmap  flags 0x0
>             key 4B  value 4B  max_entries 64  memlock 4096B
> 
>     $ bpftool map dump id 1
>     key:
>     00 00 00 00
>     value:
>     No such file or directory
>     key:
>     01 00 00 00
>     value:
>     No such file or directory
>     key:
>     02 00 00 00
>     value:
>     No such file or directory
>     key: 03 00 00 00  value: 03 00 00 00
> 
> Handle ENOENT by keeping the line format sane and dumping
> "<no entry>" for the value
> 
>     $ bpftool map dump id 1
>     key: 00 00 00 00  value: <no entry>
>     key: 01 00 00 00  value: <no entry>
>     key: 02 00 00 00  value: <no entry>
>     key: 03 00 00 00  value: 03 00 00 00
>     ...
> 
> Signed-off-by: David Ahern <dsahern@...il.com>

Seems good.  I wonder why "fd" maps report all indexes in get_next..

Acked-by: Jakub Kicinski <jakub.kicinski@...ronome.com>

> Alternatively, could just omit the value, so:
> key: 00 00 00 00  value:
> key: 01 00 00 00  value:
> key: 02 00 00 00  value:
> key: 03 00 00 00  value: 03 00 00 00
> 
>  tools/bpf/bpftool/map.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index 101b8a881225..1f0060644e0c 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -383,7 +383,10 @@ static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
>  		printf(single_line ? "  " : "\n");
>  
>  		printf("value:%c", break_names ? '\n' : ' ');
> -		fprint_hex(stdout, value, info->value_size, " ");
> +		if (value)
> +			fprint_hex(stdout, value, info->value_size, " ");
> +		else
> +			printf("<no entry>");
>  
>  		printf("\n");
>  	} else {
> @@ -398,8 +401,12 @@ static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
>  		for (i = 0; i < n; i++) {
>  			printf("value (CPU %02d):%c",
>  			       i, info->value_size > 16 ? '\n' : ' ');
> -			fprint_hex(stdout, value + i * step,
> -				   info->value_size, " ");
> +			if (value) {
> +				fprint_hex(stdout, value + i * step,
> +					   info->value_size, " ");
> +			} else {
> +				printf("<no entry>");
> +			}

nit: in other places you don't add { }

>  			printf("\n");
>  		}
>  	}
> @@ -731,7 +738,11 @@ static int dump_map_elem(int fd, void *key, void *value,
>  		jsonw_string_field(json_wtr, "error", strerror(lookup_errno));
>  		jsonw_end_object(json_wtr);
>  	} else {
> -		print_entry_error(map_info, key, strerror(lookup_errno));
> +		if (errno == ENOENT)
> +			print_entry_plain(map_info, key, NULL);
> +		else
> +			print_entry_error(map_info, key,
> +					  strerror(lookup_errno));
>  	}
>  
>  	return 0;