Re: [PATCH bpf-next 3/3] bpf: btf: json print map dump with btf info

[Song Liu <songliubraving@fb.com>]

> On Jun 20, 2018, at 1:30 PM, Okash Khawaja <osk@fb.com> wrote:
> 
> This patch modifies `bpftool map dump [-j|-p] id <map-id>` to json-
> print and pretty-json-print map dump. It calls btf_dumper introduced in
> previous patch to accomplish this.
> 
> The patch only prints debug info when -j or -p flags are supplied. Then
> too, if the map has associated btf data loaded. Otherwise the usual
> debug-less output is printed.
> 
> Signed-off-by: Okash Khawaja <osk@fb.com>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
> 
> ---
> tools/bpf/bpftool/map.c |   94 ++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 91 insertions(+), 3 deletions(-)
> 
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -43,9 +43,13 @@
> #include <unistd.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> +#include <linux/err.h>
> 
> #include <bpf.h>
> 
> +#include "json_writer.h"
> +#include "btf.h"
> +#include "btf_dumper.h"
> #include "main.h"
> 
> static const char * const map_type_name[] = {
> @@ -508,6 +512,83 @@ static int do_show(int argc, char **argv
> 	return errno == ENOENT ? 0 : -1;
> }
> 
> +
> +static int do_dump_btf(struct btf *btf, struct bpf_map_info *map_info,
> +		void *key, void *value)
> +{
> +	int ret;
> +
> +	jsonw_start_object(json_wtr);
> +	jsonw_name(json_wtr, "key");
> +
> +	ret = btf_dumper_type(btf, json_wtr, map_info->btf_key_type_id, key);
> +	if (ret)
> +		goto out;
> +
> +	jsonw_end_object(json_wtr);
> +
> +	jsonw_start_object(json_wtr);
> +	jsonw_name(json_wtr, "value");
> +
> +	ret = btf_dumper_type(btf, json_wtr, map_info->btf_value_type_id,
> +			value);
> +
> +out:
> +	/* end of root object */
> +	jsonw_end_object(json_wtr);
> +
> +	return ret;
> +}

Please add some tests for do_dump_btf(), including some invalid data type/kind. 

> +
> +static struct btf *get_btf(struct bpf_map_info *map_info)
> +{
> +	int btf_fd = bpf_btf_get_fd_by_id(map_info->btf_id);
> +	struct bpf_btf_info btf_info = { 0 };
> +	__u32 len = sizeof(btf_info);
> +	uint32_t last_size;
> +	int err;
> +	struct btf *btf = NULL;
> +	void *ptr = NULL, *temp_ptr;
> +
> +	if (btf_fd < 0)
> +		return NULL;
> +
> +	btf_info.btf_size = 4096;

Is btf_size always a constant of 4096? 

> +	do {
> +		last_size = btf_info.btf_size;
> +		temp_ptr = realloc(ptr, last_size);
> +		if (!temp_ptr) {
> +			p_err("unable allocate memory for debug info.");
> +			goto exit_free;
> +		}
> +
> +		ptr = temp_ptr;
> +		bzero(ptr, last_size);
> +		btf_info.btf = ptr_to_u64(ptr);
> +		err = bpf_obj_get_info_by_fd(btf_fd, &btf_info, &len);
> +	} while (!err && btf_info.btf_size > last_size && last_size == 4096);
> +
> +	if (err || btf_info.btf_size > last_size) {
> +		p_info("can't get btf info. debug info won't be displayed. error: %s",
> +				err ? strerror(errno) : "exceeds size retry");
> +		goto exit_free;
> +	}
> +
> +	btf = btf__new((uint8_t *) btf_info.btf,
> +			btf_info.btf_size, NULL);
> +	if (IS_ERR(btf)) {
> +		printf("error when initialising btf: %s\n",
> +				strerror(PTR_ERR(btf)));
> +		btf = NULL;
> +	}
> +
> +exit_free:
> +	close(btf_fd);
> +	free(ptr);
> +
> +	return btf;
> +}
> +
> static int do_dump(int argc, char **argv)
> {
> 	void *key, *value, *prev_key;
> @@ -516,6 +597,7 @@ static int do_dump(int argc, char **argv
> 	__u32 len = sizeof(info);
> 	int err;
> 	int fd;
> +	struct btf *btf = NULL;
> 
> 	if (argc != 2)
> 		usage();
> @@ -538,6 +620,8 @@ static int do_dump(int argc, char **argv
> 		goto exit_free;
> 	}
> 
> +	btf = get_btf(&info);
> +
> 	prev_key = NULL;
> 	if (json_output)
> 		jsonw_start_array(json_wtr);
> @@ -550,9 +634,12 @@ static int do_dump(int argc, char **argv
> 		}
> 
> 		if (!bpf_map_lookup_elem(fd, key, value)) {
> -			if (json_output)
> -				print_entry_json(&info, key, value);
> -			else
> +			if (json_output) {
> +				if (btf)
> +					do_dump_btf(btf, &info, key, value);
> +				else
> +					print_entry_json(&info, key, value);
> +			} else
> 				print_entry_plain(&info, key, value);
> 		} else {
> 			if (json_output) {
> @@ -584,6 +671,7 @@ exit_free:
> 	free(key);
> 	free(value);
> 	close(fd);
> +	btf__free(btf);
> 
> 	return err;
> }
>