On Fri, 7 Dec 2018 16:42:32 -0800, Martin KaFai Lau wrote:

> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst 
> b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index 5524b6dccd85..7c30731a9b73 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -22,8 +22,8 @@ MAP COMMANDS
>  =============
>  
>  |    **bpftool** **prog { show | list }** [*PROG*]
> -|    **bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes** 
> | **visual**}]
> -|    **bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | 
> **opcodes**}]
> +|    **bpftool** **prog dump xlated** *PROG* [{**file** *FILE* | **opcodes** 
> | **visual** | **linum**}]
> +|    **bpftool** **prog dump jited**  *PROG* [{**file** *FILE* | **opcodes** 
> | **linum**}]
>  |    **bpftool** **prog pin** *PROG* *FILE*
>  |    **bpftool** **prog { load | loadall }** *OBJ* *PATH* [**type** *TYPE*] 
> [**map** {**idx** *IDX* | **name** *NAME*} *MAP*] [**dev** *NAME*]
>  |    **bpftool** **prog attach** *PROG* *ATTACH_TYPE* [*MAP*]
> @@ -56,7 +56,7 @@ DESCRIPTION
>                 Output will start with program ID followed by program type and
>                 zero or more named attributes (depending on kernel version).
>  
> -     **bpftool prog dump xlated** *PROG* [{ **file** *FILE* | **opcodes** | 
> **visual** }]
> +     **bpftool prog dump xlated** *PROG* [{ **file** *FILE* | **opcodes** | 
> **visual** | **linum** }]
>                 Dump eBPF instructions of the program from the kernel. By
>                 default, eBPF will be disassembled and printed to standard
>                 output in human-readable format. In this case, **opcodes**
> @@ -69,13 +69,21 @@ DESCRIPTION
>                 built instead, and eBPF instructions will be presented with
>                 CFG in DOT format, on standard output.
>  
> -     **bpftool prog dump jited**  *PROG* [{ **file** *FILE* | **opcodes** }]
> +               If the prog has line_info available, the source line will
> +               be displayed by default.  If **linum** is specified,
> +               the filename, line number and line column will also be
> +               displayed on top of the source line.
> +     **bpftool prog dump jited**  *PROG* [{ **file** *FILE* | **opcodes** | 
> **linum** }]

why eat the new line?

>                 Dump jited image (host machine code) of the program.
>                 If *FILE* is specified image will be written to a file,
>                 otherwise it will be disassembled and printed to stdout.
>  
>                 **opcodes** controls if raw opcodes will be printed.
>  
> +               If the prog has line_info available, the source line will
> +               be displayed by default.  If **linum** is specified,
> +               the filename, line number and line column will also be
> +               displayed on top of the source line.
>       **bpftool prog pin** *PROG* *FILE*

why eat the new line?

>                 Pin program *PROG* as *FILE*.
>  

> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 0be0dd8f467f..d9393abdba78 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -138,6 +138,9 @@ struct pinned_obj {
>       struct hlist_node hash;
>  };
>  
> +struct btf;
> +struct bpf_line_info;
> +
>  int build_pinned_obj_table(struct pinned_obj_table *table,
>                          enum bpf_obj_type type);
>  void delete_pinned_obj_table(struct pinned_obj_table *tab);
> @@ -175,13 +178,23 @@ int map_parse_fd(int *argc, char ***argv);
>  int map_parse_fd_and_info(int *argc, char ***argv, void *info, __u32 
> *info_len);
>  
>  #ifdef HAVE_LIBBFD_SUPPORT
> +struct bpf_prog_linfo;
>  void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
> -                    const char *arch, const char *disassembler_options);
> +                    const char *arch, const char *disassembler_options,
> +                    const struct btf *btf,
> +                    const struct bpf_prog_linfo *prog_linfo,
> +                    __u64 func_ksym, unsigned int func_idx,
> +                    bool linum);
>  int disasm_init(void);
>  #else
>  static inline
>  void disasm_print_insn(unsigned char *image, ssize_t len, int opcodes,
> -                    const char *arch, const char *disassembler_options)
> +                    const char *arch, const char *disassembler_options,
> +                    const struct btf *btf,
> +                    const struct bpf_prog_linfo *prog_linfo,
> +                    __u64 func_ksym, unsigned int func_idx,
> +                    bool linum)
> +

extra new line

>  {
>  }
>  static inline int disasm_init(void)
> @@ -554,6 +585,13 @@ static int do_dump(int argc, char **argv)
>       info.func_info_cnt = finfo_cnt;
>       info.func_info_rec_size = finfo_rec_size;
>       info.func_info = ptr_to_u64(func_info);
> +     info.line_info_cnt = linfo_cnt;
> +     info.line_info_rec_size = linfo_rec_size;
> +     info.line_info = ptr_to_u64(linfo);
> +     info.jited_line_info_cnt = jited_linfo_cnt;
> +     info.jited_line_info_rec_size = jited_linfo_rec_size;
> +     info.jited_line_info = ptr_to_u64(jited_linfo);
> +
>  

extra new line

>       err = bpf_obj_get_info_by_fd(fd, &info, &len);
>       close(fd);
> @@ -596,6 +634,30 @@ static int do_dump(int argc, char **argv)
>               finfo_cnt = 0;
>       }
>  
> +     if (linfo && info.line_info_cnt != linfo_cnt) {
> +             p_err("incorrect line_info_cnt %u vs. expected %u",
> +                   info.line_info_cnt, linfo_cnt);
> +             goto err_free;
> +     }
> +
> +     if (info.line_info_rec_size != linfo_rec_size) {
> +             p_err("incorrect line_info_rec_size %u vs. expected %u",
> +                   info.line_info_rec_size, linfo_rec_size);
> +             goto err_free;
> +     }
> +
> +     if (jited_linfo && info.jited_line_info_cnt != jited_linfo_cnt) {
> +             p_err("incorrect jited_line_info_cnt %u vs. expected %u",
> +                   info.jited_line_info_cnt, jited_linfo_cnt);
> +             goto err_free;
> +     }
> +
> +     if (info.jited_line_info_rec_size != jited_linfo_rec_size) {
> +             p_err("incorrect jited_line_info_rec_size %u vs. expected %u",
> +                   info.jited_line_info_rec_size, jited_linfo_rec_size);
> +             goto err_free;
> +     }
> +
>       if ((member_len == &info.jited_prog_len &&
>            info.jited_prog_insns == 0) ||
>           (member_len == &info.xlated_prog_len &&
> @@ -609,6 +671,12 @@ static int do_dump(int argc, char **argv)
>               goto err_free;
>       }
>  
> +     if (linfo_cnt) {
> +             prog_linfo = bpf_prog_linfo__new(&info);
> +             if (!prog_linfo)
> +                     p_err("error in processing bpf_line_info.  continue 
> without it.");

double p_err() will cause trouble in JSON output, p_info() is better in
such cases, but what kind of errors are expected here that we'd want to
continue with?

> +     }
> +
>       if (filepath) {
>               fd = open(filepath, O_WRONLY | O_CREAT | O_TRUNC, 0600);
>               if (fd < 0) {

Please fix, thanks!

Reply via email to