Re: [PATCH bpf-next v2 7/7] tools/bpftool: add perf subcommand

[Jakub Kicinski <jakub.kicinski@netronome.com>]

On Thu, 17 May 2018 22:03:10 -0700, Yonghong Song wrote:
> The new command "bpftool perf [show | list]" will traverse
> all processes under /proc, and if any fd is associated
> with a perf event, it will print out related perf event
> information. Documentation is also added.

Thanks for the changes, it looks good with some minor nits which can be
addressed as follow up if there is no other need to respin.  Please
consider it:

Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

> Below is an example to show the results using bcc commands.
> Running the following 4 bcc commands:
>   kprobe:     trace.py '__x64_sys_nanosleep'
>   kretprobe:  trace.py 'r::__x64_sys_nanosleep'
>   tracepoint: trace.py 't:syscalls:sys_enter_nanosleep'
>   uprobe:     trace.py 'p:/home/yhs/a.out:main'
> 
> The bpftool command line and result:
> 
>   $ bpftool perf
>   pid 21711  fd 5: prog_id 5  kprobe  func __x64_sys_write  offset 0
>   pid 21765  fd 5: prog_id 7  kretprobe  func __x64_sys_nanosleep  offset 0
>   pid 21767  fd 5: prog_id 8  tracepoint  sys_enter_nanosleep
>   pid 21800  fd 5: prog_id 9  uprobe  filename /home/yhs/a.out  offset 1159
> 
>   $ bpftool -j perf
>   {"pid":21711,"fd":5,"prog_id":5,"attach_info":"kprobe","func":"__x64_sys_write","offset":0}, \
>   {"pid":21765,"fd":5,"prog_id":7,"attach_info":"kretprobe","func":"__x64_sys_nanosleep","offset":0}, \
>   {"pid":21767,"fd":5,"prog_id":8,"attach_info":"tracepoint","tracepoint":"sys_enter_nanosleep"}, \
>   {"pid":21800,"fd":5,"prog_id":9,"attach_info":"uprobe","filename":"/home/yhs/a.out","offset":1159}

nit: this is now an array

>   $ bpftool prog
>   5: kprobe  name probe___x64_sys  tag e495a0c82f2c7a8d  gpl
> 	  loaded_at 2018-05-15T04:46:37-0700  uid 0
> 	  xlated 200B  not jited  memlock 4096B  map_ids 4
>   7: kprobe  name probe___x64_sys  tag f2fdee479a503abf  gpl
> 	  loaded_at 2018-05-15T04:48:32-0700  uid 0
> 	  xlated 200B  not jited  memlock 4096B  map_ids 7
>   8: tracepoint  name tracepoint__sys  tag 5390badef2395fcf  gpl
> 	  loaded_at 2018-05-15T04:48:48-0700  uid 0
> 	  xlated 200B  not jited  memlock 4096B  map_ids 8
>   9: kprobe  name probe_main_1  tag 0a87bdc2e2953b6d  gpl
> 	  loaded_at 2018-05-15T04:49:52-0700  uid 0
> 	  xlated 200B  not jited  memlock 4096B  map_ids 9
> 
>   $ ps ax | grep "python ./trace.py"
>   21711 pts/0    T      0:03 python ./trace.py __x64_sys_write
>   21765 pts/0    S+     0:00 python ./trace.py r::__x64_sys_nanosleep
>   21767 pts/2    S+     0:00 python ./trace.py t:syscalls:sys_enter_nanosleep
>   21800 pts/3    S+     0:00 python ./trace.py p:/home/yhs/a.out:main
>   22374 pts/1    S+     0:00 grep --color=auto python ./trace.py
> 
> Signed-off-by: Yonghong Song <yhs@fb.com>

> diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
> index b301c9b..3680ad4 100644
> --- a/tools/bpf/bpftool/bash-completion/bpftool
> +++ b/tools/bpf/bpftool/bash-completion/bpftool
> @@ -448,6 +448,15 @@ _bpftool()
>                      ;;
>              esac
>              ;;
> +        cgroup)

s/cgroup/perf/ :)

> +            case $command in
> +                *)
> +                    [[ $prev == $object ]] && \
> +                        COMPREPLY=( $( compgen -W 'help \
> +                            show list' -- "$cur" ) )
> +                    ;;
> +            esac
> +            ;;
>      esac
>  } &&
>  complete -F _bpftool bpftool

> +static int show_proc(const char *fpath, const struct stat *sb,
> +		     int tflag, struct FTW *ftwbuf)
> +{
> +	__u64 probe_offset, probe_addr;
> +	__u32 prog_id, attach_info;
> +	int err, pid = 0, fd = 0;
> +	const char *pch;
> +	char buf[4096];
> +
> +	/* prefix always /proc */
> +	pch = fpath + 5;
> +	if (*pch == '\0')
> +		return 0;
> +
> +	/* pid should be all numbers */
> +	pch++;
> +	while (isdigit(*pch)) {
> +		pid = pid * 10 + *pch - '0';
> +		pch++;
> +	}
> +	if (*pch == '\0')
> +		return 0;
> +	if (*pch != '/')
> +		return FTW_SKIP_SUBTREE;
> +
> +	/* check /proc/<pid>/fd directory */
> +	pch++;
> +	if (strncmp(pch, "fd", 2))
> +		return FTW_SKIP_SUBTREE;
> +	pch += 2;
> +	if (*pch == '\0')
> +		return 0;
> +	if (*pch != '/')
> +		return FTW_SKIP_SUBTREE;
> +
> +	/* check /proc/<pid>/fd/<fd_num> */
> +	pch++;
> +	while (isdigit(*pch)) {
> +		fd = fd * 10 + *pch - '0';
> +		pch++;
> +	}
> +	if (*pch != '\0')
> +		return FTW_SKIP_SUBTREE;
> +
> +	/* query (pid, fd) for potential perf events */
> +	err = bpf_task_fd_query(pid, fd, 0, buf, sizeof(buf), &prog_id,
> +				&attach_info, &probe_offset, &probe_addr);
> +	if (err < 0)
> +		return 0;

nit: it could be nice from user perspective to detect whether kernel
     supports the command and fail if not.  Otherwise user is not sure
     if there is no output because kernel lacks support or because
     there were really no attached progs.  Just a thought, not really
     a requirement.

> +	if (json_output)
> +		print_perf_json(pid, fd, prog_id, attach_info, buf, probe_offset,
> +				probe_addr);
> +	else
> +		print_perf_plain(pid, fd, prog_id, attach_info, buf, probe_offset,
> +				 probe_addr);
> +
> +	return 0;
> +}
> +
> +static int do_show(int argc, char **argv)
> +{
> +	int err = 0, nopenfd = 16;
> +	int flags = FTW_ACTIONRETVAL | FTW_PHYS;

nit: reverse xmas tree

> +	if (json_output)
> +		jsonw_start_array(json_wtr);
> +	if (nftw("/proc", show_proc, nopenfd, flags) == -1) {
> +		p_err("%s", strerror(errno));
> +		err = -1;
> +	}
> +	if (json_output)
> +		jsonw_end_array(json_wtr);
> +
> +	return err;
> +}