On 10/10/21 10:27 PM, Dave Marchevsky wrote:
> To prepare for impending deprecation of libbpf's
> bpf_program__get_prog_info_linear, migrate uses of this function to use
> bpf_obj_get_info_by_fd.
>
> Since the profile_target_name and dump_prog_id_as_func_ptr helpers were
> only looking at the first func_info, avoid grabbing the rest to save a
> malloc. For do_dump, add a more full-featured helper, but avoid
> free/realloc of buffer when possible for multi-prog dumps.
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx>
> ---
> tools/bpf/bpftool/btf_dumper.c | 40 +++++----
> tools/bpf/bpftool/prog.c | 153 +++++++++++++++++++++++++--------
> 2 files changed, 143 insertions(+), 50 deletions(-)
[...]
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index a24ea7e26aa4..a7507cc165eb 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -98,6 +98,72 @@ static enum bpf_attach_type parse_attach_type(const char *str)
> return __MAX_BPF_ATTACH_TYPE;
> }
>
> +#define holder_prep_needed_rec_sz(nr, rec_size)\
> +({ \
> + holder.nr = info->nr; \
> + needed += holder.nr * rec_size; \
> +})
> +
> +#define holder_prep_needed(nr, rec_size) \
> +({ \
> + holder.nr = info->nr; \
> + holder.rec_size = info->rec_size; \
> + needed += holder.nr * holder.rec_size; \
> +})
> +
> +#define holder_set_ptr(field, nr, rec_size) \
> +({ \
> + holder.field = ptr_to_u64(ptr); \
> + ptr += nr * rec_size; \
> +})
> +
> +static int prep_prog_info(struct bpf_prog_info *const info, enum dump_mode mode,
> + void *info_data, size_t *const info_data_sz)
> +{
> + struct bpf_prog_info holder = {};
> + size_t needed = 0;
> + void *ptr;
> +
> + if (mode == DUMP_JITED)
> + holder_prep_needed_rec_sz(jited_prog_len, 1);
> + else
> + holder_prep_needed_rec_sz(xlated_prog_len, 1);
> +
> + holder_prep_needed_rec_sz(nr_jited_ksyms, sizeof(__u64));
> + holder_prep_needed_rec_sz(nr_jited_func_lens, sizeof(__u32));
> + holder_prep_needed(nr_func_info, func_info_rec_size);
> + holder_prep_needed(nr_line_info, line_info_rec_size);
> + holder_prep_needed(nr_jited_line_info, jited_line_info_rec_size);
> +
> + if (needed > *info_data_sz) {
> + info_data = realloc(info_data, needed);
This breaks 'bpftool prog dump' for multiple progs, need to pass info_data as
a void ** so that the original ptr is updated on realloc. Will send v2 shortly.
> + if (!info_data)
> + return -1;
> + *info_data_sz = needed;
> + }
[...]
> @@ -791,16 +857,18 @@ prog_dump(struct bpf_prog_info *info, enum dump_mode mode,
>
> static int do_dump(int argc, char **argv)
> {
> - struct bpf_prog_info_linear *info_linear;
> + struct bpf_prog_info info = {};
> + __u32 info_len = sizeof(info);
> + size_t info_data_sz = 0;
> + void *info_data = NULL;
> char *filepath = NULL;
> bool opcodes = false;
> bool visual = false;
> enum dump_mode mode;
> bool linum = false;
> - int *fds = NULL;
> int nb_fds, i = 0;
> + int *fds = NULL;
> int err = -1;
> - __u64 arrays;
>
> if (is_prefix(*argv, "jited")) {
> if (disasm_init())
> @@ -860,43 +928,42 @@ static int do_dump(int argc, char **argv)
> goto exit_close;
> }
>
> - if (mode == DUMP_JITED)
> - arrays = 1UL << BPF_PROG_INFO_JITED_INSNS;
> - else
> - arrays = 1UL << BPF_PROG_INFO_XLATED_INSNS;
> -
> - arrays |= 1UL << BPF_PROG_INFO_JITED_KSYMS;
> - arrays |= 1UL << BPF_PROG_INFO_JITED_FUNC_LENS;
> - arrays |= 1UL << BPF_PROG_INFO_FUNC_INFO;
> - arrays |= 1UL << BPF_PROG_INFO_LINE_INFO;
> - arrays |= 1UL << BPF_PROG_INFO_JITED_LINE_INFO;
> -
> if (json_output && nb_fds > 1)
> jsonw_start_array(json_wtr); /* root array */
> for (i = 0; i < nb_fds; i++) {
> - info_linear = bpf_program__get_prog_info_linear(fds[i], arrays);
> - if (IS_ERR_OR_NULL(info_linear)) {
> + err = bpf_obj_get_info_by_fd(fds[i], &info, &info_len);
> + if (err) {
> + p_err("can't get prog info: %s", strerror(errno));
> + break;
> + }
> +
> + err = prep_prog_info(&info, mode, info_data, &info_data_sz);
> + if (err) {
> + p_err("can't grow prog info_data");
> + break;
> + }
> +
> + err = bpf_obj_get_info_by_fd(fds[i], &info, &info_len);
> + if (err) {
> p_err("can't get prog info: %s", strerror(errno));
> break;
> }
There should be a memset(&info, 0, sizeof(info)) at the end of this loop. In
current state, when dumping multiple progs, previous iteration's populated
info will be passed to first bpf_obj_get_info_by_fd call in next iteration,
resulting in unnecessary data copying.
> if (json_output && nb_fds > 1) {
> jsonw_start_object(json_wtr); /* prog object */
> - print_prog_header_json(&info_linear->info);
> + print_prog_header_json(&info);
> jsonw_name(json_wtr, "insns");
> } else if (nb_fds > 1) {
> - print_prog_header_plain(&info_linear->info);
> + print_prog_header_plain(&info);
> }
>
> - err = prog_dump(&info_linear->info, mode, filepath, opcodes,
> - visual, linum);
> + err = prog_dump(&info, mode, filepath, opcodes, visual, linum);
>
> if (json_output && nb_fds > 1)
> jsonw_end_object(json_wtr); /* prog object */
> else if (i != nb_fds - 1 && nb_fds > 1)
> printf("\n");
>
> - free(info_linear);
> if (err)
> break;
> close(fds[i]);
> @@ -908,6 +975,7 @@ static int do_dump(int argc, char **argv)
> for (; i < nb_fds; i++)
> close(fds[i]);
> exit_free:
> + free(info_data);
> free(fds);
> return err;
> }
