Skip to contentSkip to conversation

bpf: Print error message for bpftool cgroup show

PATCH Superseded Hechao Li 2 participants 2 messages Dec 20, 2019 – Dec 23, 2019

Subsystems (2)
BPF bpftool

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Hechao Li

author
#0 lore raw

PATCH 1/1 bpf: Print error message for bpftool cgroup show

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 <[email protected]>
---
 tools/bpf/bpftool/cgroup.c | 60 ++++++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 18 deletions(-)
tools/bpf/bpftool/cgroup.c +42 -18
index 1ef45e55039e..74b57e2d7524 100644
@@ -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)
+{
+	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;
 	}
 
+	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;
+	}
+
 	if (json_output)
 		jsonw_start_array(json_wtr);
 	else

@@ -212,8 +245,8 @@ static int do_show(int argc, char **argv)
 	if (json_output)
 		jsonw_end_array(json_wtr);
 
+exit_cgroup:
 	close(cgroup_fd);
-exit:
 	return ret;
 }
 

@@ -228,7 +261,7 @@ static int do_show_tree_fn(const char *fpath, const struct stat *sb,
 			   int typeflag, struct FTW *ftw)
 {
 	enum bpf_attach_type type;
-	bool skip = true;
+	int query_check;
 	int cgroup_fd;
 
 	if (typeflag != FTW_D)

@@ -240,22 +273,13 @@ static int do_show_tree_fn(const char *fpath, const struct stat *sb,
 		return SHOW_TREE_FN_ERR;
 	}
 
-	for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++) {
-		int count = count_attached_bpf_progs(cgroup_fd, type);
-
-		if (count < 0 && errno != EINVAL) {
-			p_err("can't query bpf programs attached to %s: %s",
-			      fpath, strerror(errno));
-			close(cgroup_fd);
-			return SHOW_TREE_FN_ERR;
-		}
-		if (count > 0) {
-			skip = false;
-			break;
-		}
-	}
-
-	if (skip) {
+	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",
+		      fpath, strerror(errno));
+		close(cgroup_fd);
+		return SHOW_TREE_FN_ERR;
+	} else if (query_check == QUERY_CGROUP_NO_PROG) {
 		close(cgroup_fd);
 		return 0;
 	}
Signature · 1 line
2.17.1
#1 lore raw
Quoted 1 line · Hechao Li

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.

Quoted 26 lines

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 <[email protected]>
---
 tools/bpf/bpftool/cgroup.c | 60 ++++++++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 18 deletions(-)
tools/bpf/bpftool/cgroup.c +0 -0
index 1ef45e55039e..74b57e2d7524 100644

tools/bpf/bpftool/cgroup.c static int count_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type) · line 124

Quoted 8 lines
tools/bpf/bpftool/cgroup.c +5 -0
@@ -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.

Quoted 29 lines
tools/bpf/bpftool/cgroup.c +18 -0
+       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;

tools/bpf/bpftool/cgroup.c static int do_show(int argc, char **argv) · line 215

Quoted 1 line
tools/bpf/bpftool/cgroup.c +0 -0
@@ -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

Quoted 12 lines
tools/bpf/bpftool/cgroup.c +10 -0
        }
 
+       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;
+       }
+

[...]