From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jakub Kicinski Subject: Re: [PATCH bpf-next v3 3/3] bpf: btf: print map dump and lookup with btf info Date: Mon, 9 Jul 2018 21:06:09 -0700 Message-ID: <20180709210609.284b29f7@cakuba.lan> References: <20180708203002.543403467@fb.com> <20180708203336.570589830@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: Daniel Borkmann , Martin KaFai Lau , Alexei Starovoitov , Yonghong Song , Quentin Monnet , "David S. Miller" , , , To: Okash Khawaja Return-path: Received: from mail-qt0-f196.google.com ([209.85.216.196]:42198 "EHLO mail-qt0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750806AbeGJEGO (ORCPT ); Tue, 10 Jul 2018 00:06:14 -0400 Received: by mail-qt0-f196.google.com with SMTP id z8-v6so8764362qto.9 for ; Mon, 09 Jul 2018 21:06:14 -0700 (PDT) In-Reply-To: <20180708203336.570589830@fb.com> Sender: netdev-owner@vger.kernel.org List-ID: 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 > +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 {