* [PATCH bpf-next] bpf: Print error message for bpftool cgroup show
@ 2019-12-20 23:03 Hechao Li
2019-12-23 23:36 ` Andrii Nakryiko
0 siblings, 1 reply; 2+ messages in thread
From: Hechao Li @ 2019-12-20 23:03 UTC (permalink / raw)
To: bpf; +Cc: hechaol, rdna, daniel, ast
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@fb.com>
---
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)
+{
+ 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;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH bpf-next] bpf: Print error message for bpftool cgroup show
2019-12-20 23:03 [PATCH bpf-next] bpf: Print error message for bpftool cgroup show Hechao Li
@ 2019-12-23 23:36 ` Andrii Nakryiko
0 siblings, 0 replies; 2+ messages in thread
From: Andrii Nakryiko @ 2019-12-23 23:36 UTC (permalink / raw)
To: Hechao Li; +Cc: bpf, Andrey Ignatov, Daniel Borkmann, Alexei Starovoitov
On Fri, Dec 20, 2019 at 3:04 PM Hechao Li <hechaol@fb.com> 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@fb.com>
> ---
> 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;
> + }
> +
[...]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-12-23 23:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-20 23:03 [PATCH bpf-next] bpf: Print error message for bpftool cgroup show Hechao Li
2019-12-23 23:36 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).