Messages in this thread | | | Date | Mon, 9 Jul 2018 21:06:09 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH bpf-next v3 3/3] bpf: btf: print map dump and lookup with btf info |
| |
On Sun, 8 Jul 2018 13:30:05 -0700, Okash Khawaja wrote: > This patch augments the output of bpftool's map dump and map lookup > commands to print data along side btf info, if the correspondin btf > info is available. The outputs for each of map dump and map lookup > commands are augmented in two ways: > > 1. when neither of -j and -p are supplied, btf-ful map data is printed > whose aim is human readability. This means no commitments for json- or > backward- compatibility. > > 2. when either -j or -p are supplied, a new json object named > "formatted" is added for each key-value pair. This object contains the > same data as the key-value pair, but with btf info. "formatted" object > promises json- and backward- compatibility. Below is a sample output. > > $ bpftool map dump -p id 8 > [{ > "key": ["0x0f","0x00","0x00","0x00" > ], > "value": ["0x03", "0x00", "0x00", "0x00", ... > ], > "formatted": { > "key": 15, > "value": { > "int_field": 3, > ... > } > } > } > ] > > This patch calls btf_dumper introduced in previous patch to accomplish > the above. Indeed, btf-ful info is only displayed if btf data for the > given map is available. Otherwise existing output is displayed as-is. > > Signed-off-by: Okash Khawaja <osk@fb.com>
> +static struct btf *get_btf(struct bpf_map_info *map_info) > +{ > + struct bpf_btf_info btf_info = { 0 }; > + __u32 len = sizeof(btf_info); > + struct btf *btf = NULL; > + __u32 last_size; > + int btf_fd; > + void *ptr; > + int err; > + > + btf_fd = bpf_btf_get_fd_by_id(map_info->btf_id); > + if (btf_fd < 0) > + return NULL; > + > + /* we won't know btf_size until we call bpf_obj_get_info_by_fd(). so > + * let's start with a sane default - 4KiB here - and resize it only if > + * bpf_obj_get_info_by_fd() needs a bigger buffer. > + */ > + btf_info.btf_size = 4096; > + last_size = btf_info.btf_size; > + ptr = malloc(last_size); > + if (!ptr) { > + p_err("unable to allocate memory for debug info"); > + goto exit_free;
I don't think we can continue working after a p_err() call :S Or p_info() for that matter. Something else may call p_err() again and we'll end up with multiple "error" members in JSON.
Could you return an error and make the callers fail where you do goto exit_free? The case there is no BTF is okay to return NULL, but other cases should really not happen, and I think it's OK to just error out completely.
> + } > + > + bzero(ptr, last_size); > + btf_info.btf = ptr_to_u64(ptr); > + err = bpf_obj_get_info_by_fd(btf_fd, &btf_info, &len); > + > + if (!err && btf_info.btf_size > last_size) { > + void *temp_ptr; > + > + last_size = btf_info.btf_size; > + temp_ptr = realloc(ptr, last_size); > + if (!temp_ptr) { > + p_err("unable to re-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); > + } > + > + 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((__u8 *)btf_info.btf, > + btf_info.btf_size, NULL); > + if (IS_ERR(btf)) { > + p_info("error when initialising btf: %s\n", > + strerror(PTR_ERR(btf))); > + btf = NULL; > + } > + > +exit_free: > + close(btf_fd); > + free(ptr); > + > + return btf; > +}
> @@ -549,9 +681,18 @@ 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); > + print_entry_json(&info, key, value, btf); > else > - print_entry_plain(&info, key, value); > + if (btf) { > + struct btf_dumper d = { > + .btf = btf, > + .jw = btf_wtr, > + .is_plain_text = true, > + };
nit: new line missing here and in another place
> + do_dump_btf(&d, &info, key, value); > + } else {
| |