On Fri, Dec 20, 2019 at 3:04 PM Hechao Li <hechaol@xxxxxx> wrote:
>
For next revision, please don't forget to add "PATCH" in patch prefix.
So you'll have: [PATCH v2 bpf-next] <subject here> for you v2.
> Currently, when bpftool cgroup show <path> has an error, no error
> message is printed. This is confusing because the user may think the
> result is empty.
>
> Before the change:
>
> $ bpftool cgroup show /sys/fs/cgroup
> ID AttachType AttachFlags Name
> $ echo $?
> 255
>
> After the change:
> $ ./bpftool cgroup show /sys/fs/cgroup
> Error: can't query bpf programs attached to /sys/fs/cgroup: Operation
> not permitted
>
> Signed-off-by: Hechao Li <hechaol@xxxxxx>
> ---
> tools/bpf/bpftool/cgroup.c | 60 ++++++++++++++++++++++++++------------
> 1 file changed, 42 insertions(+), 18 deletions(-)
>
> diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
> index 1ef45e55039e..74b57e2d7524 100644
> --- a/tools/bpf/bpftool/cgroup.c
> +++ b/tools/bpf/bpftool/cgroup.c
> @@ -117,6 +117,28 @@ static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type)
> return prog_cnt;
> }
>
> +#define QUERY_CGROUP_ERR (-1)
> +#define QUERY_CGROUP_NO_PROG (0)
> +#define QUERY_CGROUP_SUCCESS (1)
> +static int check_query_cgroup_progs(int cgroup_fd)
> +{
How about calling it a bit differently and use typical bool + error
return values. E.g., "cgroup_has_attached_progs" (or something along
those lines), then 1 means "true, it has", 0 means "false, doesn't
have any BPF progs", <0 means error, as usual. No need for special
QUERY* constants.
> + enum bpf_attach_type type;
> + bool no_prog = true;
> +
> + for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
> + int count = count_attached_bpf_progs(cgroup_fd, type);
> +
> + if (count < 0 && errno != EINVAL)
> + return QUERY_CGROUP_ERR;
> +
> + if (count > 0) {
> + no_prog = false;
> + break;
> + }
> + }
> +
> + return no_prog ? QUERY_CGROUP_NO_PROG : QUERY_CGROUP_SUCCESS;
> +}
> static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
> int level)
> {
> @@ -162,6 +184,7 @@ static int do_show(int argc, char **argv)
> {
> enum bpf_attach_type type;
> const char *path;
> + int query_check;
> int cgroup_fd;
> int ret = -1;
>
> @@ -192,6 +215,16 @@ static int do_show(int argc, char **argv)
> goto exit;
you just removed exit label below, this causes compilation error here
> }
>
> + query_check = check_query_cgroup_progs(cgroup_fd);
> + if (query_check == QUERY_CGROUP_ERR) {
> + p_err("can't query bpf programs attached to %s: %s",
> + path, strerror(errno));
> + goto exit_cgroup;
> + } else if (query_check == QUERY_CGROUP_NO_PROG) {
> + ret = 0;
> + goto exit_cgroup;
> + }
> +
[...]
