From mboxrd@z Thu Jan 1 00:00:00 1970 From: Okash Khawaja Subject: Re: [PATCH bpf-next 3/3] bpf: btf: json print map dump with btf info Date: Thu, 21 Jun 2018 11:05:21 +0100 Message-ID: <20180621100520.GA1176@w1t1fb> References: <20180620203051.223156973@fb.com> <20180620203703.208599277@fb.com> <60109415-AE62-4E4A-BF7D-401F66245523@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Cc: Daniel Borkmann , Martin Lau , Alexei Starovoitov , Yonghong Song , Quentin Monnet , Jakub Kicinski , "David S. Miller" , "netdev@vger.kernel.org" , Kernel Team , "linux-kernel@vger.kernel.org" To: Song Liu Return-path: Content-Disposition: inline In-Reply-To: <60109415-AE62-4E4A-BF7D-401F66245523@fb.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org On Thu, Jun 21, 2018 at 12:22:52AM +0100, Song Liu wrote: > > > > On Jun 20, 2018, at 1:30 PM, Okash Khawaja wrote: > > > > This patch modifies `bpftool map dump [-j|-p] 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 > > Acked-by: Martin KaFai Lau > > > > --- > > 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 > > #include > > #include > > +#include > > > > #include > > > > +#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. Sure. I was wondering if we can follow this up with tests afterwards. However if they are essential now, I can add them. > > > + > > +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? We start out with 4096 and in the loop below we increase the size if needed. It seems like a sane valeu to start with. However if there are better ideas for start value, we can discuss them. > > > + 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; > > } > > >