* [PATCH 1/5] bpf tools: Improve libbpf error reporting
2015-11-05 4:26 [PATCH 0/5] perf tools: improve BPF related error delivering and testing Wang Nan
@ 2015-11-05 4:26 ` Wang Nan
2015-11-05 15:26 ` Arnaldo Carvalho de Melo
2015-11-05 4:26 ` [PATCH 2/5] perf tools: Improve BPF related error messages output Wang Nan
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Wang Nan @ 2015-11-05 4:26 UTC (permalink / raw)
To: acme, namhyung
Cc: lizefan, pi3orama, linux-kernel, Wang Nan,
Arnaldo Carvalho de Melo
In this patch, a series libbpf specific error numbers and
libbpf_strerror() are created to help reporting error to caller.
Functions are updated to pass correct error number through macro
CHECK_ERR().
All users of bpf_object__open{_buffer}() and bpf_program__title()
in perf are modified accordingly. In addition, due to error code
changing, bpf__strerror_load() also modified to use new error code.
bpf__strerror_head() is also changed accordingly so it can parse
libbpf error. bpf_loader_strerror() is introduced for it, and will
be improved by following patch.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
tools/lib/bpf/libbpf.c | 149 ++++++++++++++++++++++++++++-------------
tools/lib/bpf/libbpf.h | 12 ++++
tools/perf/tests/llvm.c | 2 +-
tools/perf/util/bpf-loader.c | 36 ++++++++--
tools/perf/util/parse-events.c | 4 +-
5 files changed, 145 insertions(+), 58 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 9f3c8cf..c4dcee0 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -61,6 +61,62 @@ void libbpf_set_print(libbpf_print_fn_t warn,
__pr_debug = debug;
}
+#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
+#define STRERR_BUFSIZE 128
+
+struct {
+ int code;
+ const char *msg;
+} libbpf_strerror_table[] = {
+ {LIBBPF_ERRNO__ELIBELF, "Something wrong in libelf"},
+ {LIBBPF_ERRNO__EFORMAT, "BPF object format invalid"},
+ {LIBBPF_ERRNO__EKVERSION, "'version' section incorrect or lost"},
+ {LIBBPF_ERRNO__EENDIAN, "Endian missmatch"},
+ {LIBBPF_ERRNO__EINTERNAL, "Internal error in libbpf"},
+ {LIBBPF_ERRNO__ERELOC, "Relocation failed"},
+ {LIBBPF_ERRNO__ELOAD, "Failed to load program"},
+};
+
+int libbpf_strerror(int err, char *buf, size_t size)
+{
+ unsigned int i;
+
+ if (!buf || !size)
+ return -1;
+
+ err = err > 0 ? err : -err;
+
+ if (err < LIBBPF_ERRNO__START) {
+ int ret;
+
+ ret = strerror_r(err, buf, size);
+ buf[size - 1] = '\0';
+ return ret;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(libbpf_strerror_table); i++) {
+ if (libbpf_strerror_table[i].code == err) {
+ const char *msg;
+
+ msg = libbpf_strerror_table[i].msg;
+ snprintf(buf, size, "%s", msg);
+ buf[size - 1] = '\0';
+ return 0;
+ }
+ }
+
+ snprintf(buf, size, "Unknown libbpf error %d", err);
+ buf[size - 1] = '\0';
+ return -1;
+}
+
+#define CHECK_ERR(action, err, out) do { \
+ err = action; \
+ if (err) \
+ goto out; \
+} while(0)
+
+
/* Copied from tools/perf/util/util.h */
#ifndef zfree
# define zfree(ptr) ({ free(*ptr); *ptr = NULL; })
@@ -258,7 +314,7 @@ static struct bpf_object *bpf_object__new(const char *path,
obj = calloc(1, sizeof(struct bpf_object) + strlen(path) + 1);
if (!obj) {
pr_warning("alloc memory failed for %s\n", path);
- return NULL;
+ return ERR_PTR(-ENOMEM);
}
strcpy(obj->path, path);
@@ -305,7 +361,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
if (obj_elf_valid(obj)) {
pr_warning("elf init: internal error\n");
- return -EEXIST;
+ return -LIBBPF_ERRNO__ELIBELF;
}
if (obj->efile.obj_buf_sz > 0) {
@@ -331,14 +387,14 @@ static int bpf_object__elf_init(struct bpf_object *obj)
if (!obj->efile.elf) {
pr_warning("failed to open %s as ELF file\n",
obj->path);
- err = -EINVAL;
+ err = -LIBBPF_ERRNO__ELIBELF;
goto errout;
}
if (!gelf_getehdr(obj->efile.elf, &obj->efile.ehdr)) {
pr_warning("failed to get EHDR from %s\n",
obj->path);
- err = -EINVAL;
+ err = -LIBBPF_ERRNO__EFORMAT;
goto errout;
}
ep = &obj->efile.ehdr;
@@ -346,7 +402,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
if ((ep->e_type != ET_REL) || (ep->e_machine != 0)) {
pr_warning("%s is not an eBPF object file\n",
obj->path);
- err = -EINVAL;
+ err = -LIBBPF_ERRNO__EFORMAT;
goto errout;
}
@@ -374,14 +430,14 @@ bpf_object__check_endianness(struct bpf_object *obj)
goto mismatch;
break;
default:
- return -EINVAL;
+ return -LIBBPF_ERRNO__EENDIAN;
}
return 0;
mismatch:
pr_warning("Error: endianness mismatch.\n");
- return -EINVAL;
+ return -LIBBPF_ERRNO__EENDIAN;
}
static int
@@ -402,7 +458,7 @@ bpf_object__init_kversion(struct bpf_object *obj,
if (size != sizeof(kver)) {
pr_warning("invalid kver section in %s\n", obj->path);
- return -EINVAL;
+ return -LIBBPF_ERRNO__EFORMAT;
}
memcpy(&kver, data, sizeof(kver));
obj->kern_version = kver;
@@ -444,7 +500,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
if (!elf_rawdata(elf_getscn(elf, ep->e_shstrndx), NULL)) {
pr_warning("failed to get e_shstrndx from %s\n",
obj->path);
- return -EINVAL;
+ return -LIBBPF_ERRNO__EFORMAT;
}
while ((scn = elf_nextscn(elf, scn)) != NULL) {
@@ -456,7 +512,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
if (gelf_getshdr(scn, &sh) != &sh) {
pr_warning("failed to get section header from %s\n",
obj->path);
- err = -EINVAL;
+ err = -LIBBPF_ERRNO__EFORMAT;
goto out;
}
@@ -464,7 +520,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
if (!name) {
pr_warning("failed to get section name from %s\n",
obj->path);
- err = -EINVAL;
+ err = -LIBBPF_ERRNO__EFORMAT;
goto out;
}
@@ -472,7 +528,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
if (!data) {
pr_warning("failed to get section data from %s(%s)\n",
name, obj->path);
- err = -EINVAL;
+ err = -LIBBPF_ERRNO__EFORMAT;
goto out;
}
pr_debug("section %s, size %ld, link %d, flags %lx, type=%d\n",
@@ -495,7 +551,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
if (obj->efile.symbols) {
pr_warning("bpf: multiple SYMTAB in %s\n",
obj->path);
- err = -EEXIST;
+ err = -LIBBPF_ERRNO__EFORMAT;
} else
obj->efile.symbols = data;
} else if ((sh.sh_type == SHT_PROGBITS) &&
@@ -504,7 +560,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
err = bpf_object__add_program(obj, data->d_buf,
data->d_size, name, idx);
if (err) {
- char errmsg[128];
+ char errmsg[STRERR_BUFSIZE];
+
strerror_r(-err, errmsg, sizeof(errmsg));
pr_warning("failed to alloc program %s (%s): %s",
name, obj->path, errmsg);
@@ -576,7 +633,7 @@ bpf_program__collect_reloc(struct bpf_program *prog,
if (!gelf_getrel(data, i, &rel)) {
pr_warning("relocation: failed to get %d reloc\n", i);
- return -EINVAL;
+ return -LIBBPF_ERRNO__EFORMAT;
}
insn_idx = rel.r_offset / sizeof(struct bpf_insn);
@@ -587,20 +644,20 @@ bpf_program__collect_reloc(struct bpf_program *prog,
&sym)) {
pr_warning("relocation: symbol %"PRIx64" not found\n",
GELF_R_SYM(rel.r_info));
- return -EINVAL;
+ return -LIBBPF_ERRNO__EFORMAT;
}
if (insns[insn_idx].code != (BPF_LD | BPF_IMM | BPF_DW)) {
pr_warning("bpf: relocation: invalid relo for insns[%d].code 0x%x\n",
insn_idx, insns[insn_idx].code);
- return -EINVAL;
+ return -LIBBPF_ERRNO__ERELOC;
}
map_idx = sym.st_value / sizeof(struct bpf_map_def);
if (map_idx >= nr_maps) {
pr_warning("bpf relocation: map_idx %d large than %d\n",
(int)map_idx, (int)nr_maps - 1);
- return -EINVAL;
+ return -LIBBPF_ERRNO__ERELOC;
}
prog->reloc_desc[i].insn_idx = insn_idx;
@@ -683,7 +740,7 @@ bpf_program__relocate(struct bpf_program *prog, int *map_fds)
if (insn_idx >= (int)prog->insns_cnt) {
pr_warning("relocation out of range: '%s'\n",
prog->section_name);
- return -ERANGE;
+ return -LIBBPF_ERRNO__ERELOC;
}
insns[insn_idx].src_reg = BPF_PSEUDO_MAP_FD;
insns[insn_idx].imm = map_fds[map_idx];
@@ -721,7 +778,7 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
if (!obj_elf_valid(obj)) {
pr_warning("Internal error: elf object is closed\n");
- return -EINVAL;
+ return -LIBBPF_ERRNO__EINTERNAL;
}
for (i = 0; i < obj->efile.nr_reloc; i++) {
@@ -734,21 +791,21 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
if (shdr->sh_type != SHT_REL) {
pr_warning("internal error at %d\n", __LINE__);
- return -EINVAL;
+ return -LIBBPF_ERRNO__EINTERNAL;
}
prog = bpf_object__find_prog_by_idx(obj, idx);
if (!prog) {
pr_warning("relocation failed: no %d section\n",
idx);
- return -ENOENT;
+ return -LIBBPF_ERRNO__ERELOC;
}
err = bpf_program__collect_reloc(prog, nr_maps,
shdr, data,
obj->efile.symbols);
if (err)
- return -EINVAL;
+ return err;
}
return 0;
}
@@ -777,7 +834,7 @@ load_program(struct bpf_insn *insns, int insns_cnt,
goto out;
}
- ret = -EINVAL;
+ ret = -LIBBPF_ERRNO__ELOAD;
pr_warning("load bpf program failed: %s\n", strerror(errno));
if (log_buf) {
@@ -831,7 +888,7 @@ static int bpf_object__validate(struct bpf_object *obj)
if (obj->kern_version == 0) {
pr_warning("%s doesn't provide kernel version\n",
obj->path);
- return -EINVAL;
+ return -LIBBPF_ERRNO__EKVERSION;
}
return 0;
}
@@ -840,32 +897,28 @@ static struct bpf_object *
__bpf_object__open(const char *path, void *obj_buf, size_t obj_buf_sz)
{
struct bpf_object *obj;
+ int err;
if (elf_version(EV_CURRENT) == EV_NONE) {
pr_warning("failed to init libelf for %s\n", path);
- return NULL;
+ return ERR_PTR(-LIBBPF_ERRNO__ELIBELF);
}
obj = bpf_object__new(path, obj_buf, obj_buf_sz);
- if (!obj)
- return NULL;
+ if (IS_ERR(obj))
+ return obj;
- if (bpf_object__elf_init(obj))
- goto out;
- if (bpf_object__check_endianness(obj))
- goto out;
- if (bpf_object__elf_collect(obj))
- goto out;
- if (bpf_object__collect_reloc(obj))
- goto out;
- if (bpf_object__validate(obj))
- goto out;
+ CHECK_ERR(bpf_object__elf_init(obj), err, out);
+ CHECK_ERR(bpf_object__check_endianness(obj), err, out);
+ CHECK_ERR(bpf_object__elf_collect(obj), err, out);
+ CHECK_ERR(bpf_object__collect_reloc(obj), err, out);
+ CHECK_ERR(bpf_object__validate(obj), err, out);
bpf_object__elf_finish(obj);
return obj;
out:
bpf_object__close(obj);
- return NULL;
+ return ERR_PTR(err);
}
struct bpf_object *bpf_object__open(const char *path)
@@ -922,6 +975,8 @@ int bpf_object__unload(struct bpf_object *obj)
int bpf_object__load(struct bpf_object *obj)
{
+ int err;
+
if (!obj)
return -EINVAL;
@@ -931,18 +986,16 @@ int bpf_object__load(struct bpf_object *obj)
}
obj->loaded = true;
- if (bpf_object__create_maps(obj))
- goto out;
- if (bpf_object__relocate(obj))
- goto out;
- if (bpf_object__load_progs(obj))
- goto out;
+
+ CHECK_ERR(bpf_object__create_maps(obj), err, out);
+ CHECK_ERR(bpf_object__relocate(obj), err, out);
+ CHECK_ERR(bpf_object__load_progs(obj), err, out);
return 0;
out:
bpf_object__unload(obj);
pr_warning("failed to load object '%s'\n", obj->path);
- return -EINVAL;
+ return err;
}
void bpf_object__close(struct bpf_object *obj)
@@ -990,7 +1043,7 @@ const char *
bpf_object__get_name(struct bpf_object *obj)
{
if (!obj)
- return NULL;
+ return ERR_PTR(-EINVAL);
return obj->path;
}
@@ -1043,7 +1096,7 @@ const char *bpf_program__title(struct bpf_program *prog, bool needs_copy)
title = strdup(title);
if (!title) {
pr_warning("failed to strdup program title\n");
- return NULL;
+ return ERR_PTR(-ENOMEM);
}
}
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index bc80af0..6491be6 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -10,6 +10,18 @@
#include <stdio.h>
#include <stdbool.h>
+#include <linux/err.h>
+
+#define LIBBPF_ERRNO__START 4000
+#define LIBBPF_ERRNO__ELIBELF 4000 /* Something wrong in libelf */
+#define LIBBPF_ERRNO__EFORMAT 4001 /* BPF object format invalid */
+#define LIBBPF_ERRNO__EKVERSION 4002 /* Incorrect or no 'version' section */
+#define LIBBPF_ERRNO__EENDIAN 4003 /* Endian missmatch */
+#define LIBBPF_ERRNO__EINTERNAL 4004 /* Internal error in libbpf */
+#define LIBBPF_ERRNO__ERELOC 4005 /* Relocation failed */
+#define LIBBPF_ERRNO__ELOAD 4006 /* Failed to load program */
+
+int libbpf_strerror(int err, char *buf, size_t size);
/*
* In include/linux/compiler-gcc.h, __printf is defined. However
diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
index 512d362..8f713f6 100644
--- a/tools/perf/tests/llvm.c
+++ b/tools/perf/tests/llvm.c
@@ -27,7 +27,7 @@ static int test__bpf_parsing(void *obj_buf, size_t obj_buf_sz)
struct bpf_object *obj;
obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, NULL);
- if (!obj)
+ if (IS_ERR(obj))
return -1;
bpf_object__close(obj);
return 0;
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 0c5d174..72ad3dc 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -59,9 +59,9 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
} else
obj = bpf_object__open(filename);
- if (!obj) {
+ if (IS_ERR(obj)) {
pr_debug("bpf: failed to load %s\n", filename);
- return ERR_PTR(-EINVAL);
+ return obj;
}
return obj;
@@ -96,9 +96,9 @@ config_bpf_program(struct bpf_program *prog)
int err;
config_str = bpf_program__title(prog, false);
- if (!config_str) {
+ if (IS_ERR(config_str)) {
pr_debug("bpf: unable to get title for program\n");
- return -EINVAL;
+ return PTR_ERR(config_str);
}
priv = calloc(sizeof(*priv), 1);
@@ -308,13 +308,34 @@ int bpf__foreach_tev(struct bpf_object *obj,
return 0;
}
+static int
+bpf_loader_strerror(int err, char *buf, size_t size)
+{
+ char sbuf[STRERR_BUFSIZE];
+ const char *msg;
+
+ if (!buf || !size)
+ return -1;
+
+ err = err > 0 ? err : -err;
+
+ if (err >= LIBBPF_ERRNO__START)
+ return libbpf_strerror(err, buf, size);
+
+ msg = strerror_r(err, sbuf, sizeof(sbuf));
+ snprintf(buf, size, "%s", msg);
+ buf[size - 1] = '\0';
+ return 0;
+}
+
#define bpf__strerror_head(err, buf, size) \
char sbuf[STRERR_BUFSIZE], *emsg;\
if (!size)\
return 0;\
if (err < 0)\
err = -err;\
- emsg = strerror_r(err, sbuf, sizeof(sbuf));\
+ bpf_loader_strerror(err, sbuf, sizeof(sbuf));\
+ emsg = sbuf;\
switch (err) {\
default:\
scnprintf(buf, size, "%s", emsg);\
@@ -345,8 +366,9 @@ int bpf__strerror_load(struct bpf_object *obj __maybe_unused,
int err, char *buf, size_t size)
{
bpf__strerror_head(err, buf, size);
- bpf__strerror_entry(EINVAL, "%s: Are you root and runing a CONFIG_BPF_SYSCALL kernel?",
- emsg)
+ bpf__strerror_entry(LIBBPF_ERRNO__ELOAD,
+ "%s: Validate your program and check 'license'/'version' sections in your object",
+ emsg);
bpf__strerror_end(buf, size);
return 0;
}
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index bee6058..c75b25d 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -632,11 +632,11 @@ int parse_events_load_bpf(struct parse_events_evlist *data,
struct bpf_object *obj;
obj = bpf__prepare_load(bpf_file_name, source);
- if (IS_ERR(obj) || !obj) {
+ if (IS_ERR(obj)) {
char errbuf[BUFSIZ];
int err;
- err = obj ? PTR_ERR(obj) : -EINVAL;
+ err = PTR_ERR(obj);
if (err == -ENOTSUP)
snprintf(errbuf, sizeof(errbuf),
--
1.8.3.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/5] bpf tools: Improve libbpf error reporting
2015-11-05 4:26 ` [PATCH 1/5] bpf tools: Improve libbpf error reporting Wang Nan
@ 2015-11-05 15:26 ` Arnaldo Carvalho de Melo
2015-11-05 15:35 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-05 15:26 UTC (permalink / raw)
To: Wang Nan
Cc: namhyung, lizefan, pi3orama, linux-kernel, Ingo Molnar, Jiri Olsa,
Alexei Starovoitov
Em Thu, Nov 05, 2015 at 04:26:58AM +0000, Wang Nan escreveu:
> In this patch, a series libbpf specific error numbers and
> libbpf_strerror() are created to help reporting error to caller.
> Functions are updated to pass correct error number through macro
> CHECK_ERR().
>
> All users of bpf_object__open{_buffer}() and bpf_program__title()
> in perf are modified accordingly. In addition, due to error code
> changing, bpf__strerror_load() also modified to use new error code.
>
> bpf__strerror_head() is also changed accordingly so it can parse
> libbpf error. bpf_loader_strerror() is introduced for it, and will
> be improved by following patch.
I am applying this, test shows a improvement in error reporting, but
please look below for some suggestions:
[root@zoo ~]# perf record -e /tmp/foo.o sleep 1
event syntax error: '/tmp/foo.o'
\___ Failed to load program: Validate your program and check 'license'/'version' sections in your object
Now this is a long error message, maybe:
event syntax error: '/tmp/foo.o'
\___ Failed to load: Check 'license'/'version' sections
And then, here we _know_ the problem is in the license, so why not use
that knowledge to help the user further and show instead:
event syntax error: '/tmp/foo.o'
\___ Failed to load: 'version'(4.2.0) doesn't match running kernel (4.3.0)
More below
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/lib/bpf/libbpf.c | 149 ++++++++++++++++++++++++++++-------------
> tools/lib/bpf/libbpf.h | 12 ++++
> tools/perf/tests/llvm.c | 2 +-
> tools/perf/util/bpf-loader.c | 36 ++++++++--
> tools/perf/util/parse-events.c | 4 +-
> 5 files changed, 145 insertions(+), 58 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 9f3c8cf..c4dcee0 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -61,6 +61,62 @@ void libbpf_set_print(libbpf_print_fn_t warn,
> __pr_debug = debug;
> }
>
> +#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
> +#define STRERR_BUFSIZE 128
> +
> +struct {
> + int code;
> + const char *msg;
> +} libbpf_strerror_table[] = {
> + {LIBBPF_ERRNO__ELIBELF, "Something wrong in libelf"},
> + {LIBBPF_ERRNO__EFORMAT, "BPF object format invalid"},
> + {LIBBPF_ERRNO__EKVERSION, "'version' section incorrect or lost"},
> + {LIBBPF_ERRNO__EENDIAN, "Endian missmatch"},
> + {LIBBPF_ERRNO__EINTERNAL, "Internal error in libbpf"},
> + {LIBBPF_ERRNO__ERELOC, "Relocation failed"},
> + {LIBBPF_ERRNO__ELOAD, "Failed to load program"},
> +};
> +
> +int libbpf_strerror(int err, char *buf, size_t size)
> +{
> + unsigned int i;
> +
> + if (!buf || !size)
> + return -1;
> +
> + err = err > 0 ? err : -err;
> +
> + if (err < LIBBPF_ERRNO__START) {
> + int ret;
> +
> + ret = strerror_r(err, buf, size);
> + buf[size - 1] = '\0';
> + return ret;
> + }
Since all those errors are in sequence, why not introduce a
LIBBPF_ERRNO__END and test against it, and then index the
libbpf_strerror_table directly using (err - LIBBPF_ERRNO__START)?
When introducing a function, take a look at similar functions and reuse
code, for instance: target__strerror() does this pretty well, I think.
> +
> + for (i = 0; i < ARRAY_SIZE(libbpf_strerror_table); i++) {
> + if (libbpf_strerror_table[i].code == err) {
> + const char *msg;
> +
> + msg = libbpf_strerror_table[i].msg;
> + snprintf(buf, size, "%s", msg);
> + buf[size - 1] = '\0';
> + return 0;
> + }
> + }
> +
> + snprintf(buf, size, "Unknown libbpf error %d", err);
> + buf[size - 1] = '\0';
> + return -1;
> +}
> +
> +#define CHECK_ERR(action, err, out) do { \
> + err = action; \
> + if (err) \
> + goto out; \
> +} while(0)
> +
> +
> /* Copied from tools/perf/util/util.h */
> #ifndef zfree
> # define zfree(ptr) ({ free(*ptr); *ptr = NULL; })
> @@ -258,7 +314,7 @@ static struct bpf_object *bpf_object__new(const char *path,
> obj = calloc(1, sizeof(struct bpf_object) + strlen(path) + 1);
> if (!obj) {
> pr_warning("alloc memory failed for %s\n", path);
> - return NULL;
> + return ERR_PTR(-ENOMEM);
> }
>
> strcpy(obj->path, path);
> @@ -305,7 +361,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
>
> if (obj_elf_valid(obj)) {
> pr_warning("elf init: internal error\n");
> - return -EEXIST;
> + return -LIBBPF_ERRNO__ELIBELF;
> }
>
> if (obj->efile.obj_buf_sz > 0) {
> @@ -331,14 +387,14 @@ static int bpf_object__elf_init(struct bpf_object *obj)
> if (!obj->efile.elf) {
> pr_warning("failed to open %s as ELF file\n",
> obj->path);
> - err = -EINVAL;
> + err = -LIBBPF_ERRNO__ELIBELF;
> goto errout;
> }
>
> if (!gelf_getehdr(obj->efile.elf, &obj->efile.ehdr)) {
> pr_warning("failed to get EHDR from %s\n",
> obj->path);
> - err = -EINVAL;
> + err = -LIBBPF_ERRNO__EFORMAT;
> goto errout;
> }
> ep = &obj->efile.ehdr;
> @@ -346,7 +402,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
> if ((ep->e_type != ET_REL) || (ep->e_machine != 0)) {
> pr_warning("%s is not an eBPF object file\n",
> obj->path);
> - err = -EINVAL;
> + err = -LIBBPF_ERRNO__EFORMAT;
> goto errout;
> }
>
> @@ -374,14 +430,14 @@ bpf_object__check_endianness(struct bpf_object *obj)
> goto mismatch;
> break;
> default:
> - return -EINVAL;
> + return -LIBBPF_ERRNO__EENDIAN;
> }
>
> return 0;
>
> mismatch:
> pr_warning("Error: endianness mismatch.\n");
> - return -EINVAL;
> + return -LIBBPF_ERRNO__EENDIAN;
> }
>
> static int
> @@ -402,7 +458,7 @@ bpf_object__init_kversion(struct bpf_object *obj,
>
> if (size != sizeof(kver)) {
> pr_warning("invalid kver section in %s\n", obj->path);
> - return -EINVAL;
> + return -LIBBPF_ERRNO__EFORMAT;
> }
> memcpy(&kver, data, sizeof(kver));
> obj->kern_version = kver;
> @@ -444,7 +500,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> if (!elf_rawdata(elf_getscn(elf, ep->e_shstrndx), NULL)) {
> pr_warning("failed to get e_shstrndx from %s\n",
> obj->path);
> - return -EINVAL;
> + return -LIBBPF_ERRNO__EFORMAT;
> }
>
> while ((scn = elf_nextscn(elf, scn)) != NULL) {
> @@ -456,7 +512,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> if (gelf_getshdr(scn, &sh) != &sh) {
> pr_warning("failed to get section header from %s\n",
> obj->path);
> - err = -EINVAL;
> + err = -LIBBPF_ERRNO__EFORMAT;
> goto out;
> }
>
> @@ -464,7 +520,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> if (!name) {
> pr_warning("failed to get section name from %s\n",
> obj->path);
> - err = -EINVAL;
> + err = -LIBBPF_ERRNO__EFORMAT;
> goto out;
> }
>
> @@ -472,7 +528,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> if (!data) {
> pr_warning("failed to get section data from %s(%s)\n",
> name, obj->path);
> - err = -EINVAL;
> + err = -LIBBPF_ERRNO__EFORMAT;
> goto out;
> }
> pr_debug("section %s, size %ld, link %d, flags %lx, type=%d\n",
> @@ -495,7 +551,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> if (obj->efile.symbols) {
> pr_warning("bpf: multiple SYMTAB in %s\n",
> obj->path);
> - err = -EEXIST;
> + err = -LIBBPF_ERRNO__EFORMAT;
> } else
> obj->efile.symbols = data;
> } else if ((sh.sh_type == SHT_PROGBITS) &&
> @@ -504,7 +560,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> err = bpf_object__add_program(obj, data->d_buf,
> data->d_size, name, idx);
> if (err) {
> - char errmsg[128];
> + char errmsg[STRERR_BUFSIZE];
> +
> strerror_r(-err, errmsg, sizeof(errmsg));
> pr_warning("failed to alloc program %s (%s): %s",
> name, obj->path, errmsg);
> @@ -576,7 +633,7 @@ bpf_program__collect_reloc(struct bpf_program *prog,
>
> if (!gelf_getrel(data, i, &rel)) {
> pr_warning("relocation: failed to get %d reloc\n", i);
> - return -EINVAL;
> + return -LIBBPF_ERRNO__EFORMAT;
> }
>
> insn_idx = rel.r_offset / sizeof(struct bpf_insn);
> @@ -587,20 +644,20 @@ bpf_program__collect_reloc(struct bpf_program *prog,
> &sym)) {
> pr_warning("relocation: symbol %"PRIx64" not found\n",
> GELF_R_SYM(rel.r_info));
> - return -EINVAL;
> + return -LIBBPF_ERRNO__EFORMAT;
> }
>
> if (insns[insn_idx].code != (BPF_LD | BPF_IMM | BPF_DW)) {
> pr_warning("bpf: relocation: invalid relo for insns[%d].code 0x%x\n",
> insn_idx, insns[insn_idx].code);
> - return -EINVAL;
> + return -LIBBPF_ERRNO__ERELOC;
> }
>
> map_idx = sym.st_value / sizeof(struct bpf_map_def);
> if (map_idx >= nr_maps) {
> pr_warning("bpf relocation: map_idx %d large than %d\n",
> (int)map_idx, (int)nr_maps - 1);
> - return -EINVAL;
> + return -LIBBPF_ERRNO__ERELOC;
> }
>
> prog->reloc_desc[i].insn_idx = insn_idx;
> @@ -683,7 +740,7 @@ bpf_program__relocate(struct bpf_program *prog, int *map_fds)
> if (insn_idx >= (int)prog->insns_cnt) {
> pr_warning("relocation out of range: '%s'\n",
> prog->section_name);
> - return -ERANGE;
> + return -LIBBPF_ERRNO__ERELOC;
> }
> insns[insn_idx].src_reg = BPF_PSEUDO_MAP_FD;
> insns[insn_idx].imm = map_fds[map_idx];
> @@ -721,7 +778,7 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
>
> if (!obj_elf_valid(obj)) {
> pr_warning("Internal error: elf object is closed\n");
> - return -EINVAL;
> + return -LIBBPF_ERRNO__EINTERNAL;
> }
>
> for (i = 0; i < obj->efile.nr_reloc; i++) {
> @@ -734,21 +791,21 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
>
> if (shdr->sh_type != SHT_REL) {
> pr_warning("internal error at %d\n", __LINE__);
> - return -EINVAL;
> + return -LIBBPF_ERRNO__EINTERNAL;
> }
>
> prog = bpf_object__find_prog_by_idx(obj, idx);
> if (!prog) {
> pr_warning("relocation failed: no %d section\n",
> idx);
> - return -ENOENT;
> + return -LIBBPF_ERRNO__ERELOC;
> }
>
> err = bpf_program__collect_reloc(prog, nr_maps,
> shdr, data,
> obj->efile.symbols);
> if (err)
> - return -EINVAL;
> + return err;
> }
> return 0;
> }
> @@ -777,7 +834,7 @@ load_program(struct bpf_insn *insns, int insns_cnt,
> goto out;
> }
>
> - ret = -EINVAL;
> + ret = -LIBBPF_ERRNO__ELOAD;
> pr_warning("load bpf program failed: %s\n", strerror(errno));
>
> if (log_buf) {
> @@ -831,7 +888,7 @@ static int bpf_object__validate(struct bpf_object *obj)
> if (obj->kern_version == 0) {
> pr_warning("%s doesn't provide kernel version\n",
> obj->path);
> - return -EINVAL;
> + return -LIBBPF_ERRNO__EKVERSION;
> }
> return 0;
> }
> @@ -840,32 +897,28 @@ static struct bpf_object *
> __bpf_object__open(const char *path, void *obj_buf, size_t obj_buf_sz)
> {
> struct bpf_object *obj;
> + int err;
>
> if (elf_version(EV_CURRENT) == EV_NONE) {
> pr_warning("failed to init libelf for %s\n", path);
> - return NULL;
> + return ERR_PTR(-LIBBPF_ERRNO__ELIBELF);
> }
>
> obj = bpf_object__new(path, obj_buf, obj_buf_sz);
> - if (!obj)
> - return NULL;
> + if (IS_ERR(obj))
> + return obj;
>
> - if (bpf_object__elf_init(obj))
> - goto out;
> - if (bpf_object__check_endianness(obj))
> - goto out;
> - if (bpf_object__elf_collect(obj))
> - goto out;
> - if (bpf_object__collect_reloc(obj))
> - goto out;
> - if (bpf_object__validate(obj))
> - goto out;
> + CHECK_ERR(bpf_object__elf_init(obj), err, out);
> + CHECK_ERR(bpf_object__check_endianness(obj), err, out);
> + CHECK_ERR(bpf_object__elf_collect(obj), err, out);
> + CHECK_ERR(bpf_object__collect_reloc(obj), err, out);
> + CHECK_ERR(bpf_object__validate(obj), err, out);
>
> bpf_object__elf_finish(obj);
> return obj;
> out:
> bpf_object__close(obj);
> - return NULL;
> + return ERR_PTR(err);
> }
>
> struct bpf_object *bpf_object__open(const char *path)
> @@ -922,6 +975,8 @@ int bpf_object__unload(struct bpf_object *obj)
>
> int bpf_object__load(struct bpf_object *obj)
> {
> + int err;
> +
> if (!obj)
> return -EINVAL;
>
> @@ -931,18 +986,16 @@ int bpf_object__load(struct bpf_object *obj)
> }
>
> obj->loaded = true;
> - if (bpf_object__create_maps(obj))
> - goto out;
> - if (bpf_object__relocate(obj))
> - goto out;
> - if (bpf_object__load_progs(obj))
> - goto out;
> +
> + CHECK_ERR(bpf_object__create_maps(obj), err, out);
> + CHECK_ERR(bpf_object__relocate(obj), err, out);
> + CHECK_ERR(bpf_object__load_progs(obj), err, out);
>
> return 0;
> out:
> bpf_object__unload(obj);
> pr_warning("failed to load object '%s'\n", obj->path);
> - return -EINVAL;
> + return err;
> }
>
> void bpf_object__close(struct bpf_object *obj)
> @@ -990,7 +1043,7 @@ const char *
> bpf_object__get_name(struct bpf_object *obj)
> {
> if (!obj)
> - return NULL;
> + return ERR_PTR(-EINVAL);
> return obj->path;
> }
>
> @@ -1043,7 +1096,7 @@ const char *bpf_program__title(struct bpf_program *prog, bool needs_copy)
> title = strdup(title);
> if (!title) {
> pr_warning("failed to strdup program title\n");
> - return NULL;
> + return ERR_PTR(-ENOMEM);
> }
> }
>
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index bc80af0..6491be6 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -10,6 +10,18 @@
>
> #include <stdio.h>
> #include <stdbool.h>
> +#include <linux/err.h>
> +
> +#define LIBBPF_ERRNO__START 4000
> +#define LIBBPF_ERRNO__ELIBELF 4000 /* Something wrong in libelf */
> +#define LIBBPF_ERRNO__EFORMAT 4001 /* BPF object format invalid */
> +#define LIBBPF_ERRNO__EKVERSION 4002 /* Incorrect or no 'version' section */
> +#define LIBBPF_ERRNO__EENDIAN 4003 /* Endian missmatch */
> +#define LIBBPF_ERRNO__EINTERNAL 4004 /* Internal error in libbpf */
> +#define LIBBPF_ERRNO__ERELOC 4005 /* Relocation failed */
> +#define LIBBPF_ERRNO__ELOAD 4006 /* Failed to load program */
> +
> +int libbpf_strerror(int err, char *buf, size_t size);
>
> /*
> * In include/linux/compiler-gcc.h, __printf is defined. However
> diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
> index 512d362..8f713f6 100644
> --- a/tools/perf/tests/llvm.c
> +++ b/tools/perf/tests/llvm.c
> @@ -27,7 +27,7 @@ static int test__bpf_parsing(void *obj_buf, size_t obj_buf_sz)
> struct bpf_object *obj;
>
> obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, NULL);
> - if (!obj)
> + if (IS_ERR(obj))
> return -1;
> bpf_object__close(obj);
> return 0;
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 0c5d174..72ad3dc 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -59,9 +59,9 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
> } else
> obj = bpf_object__open(filename);
>
> - if (!obj) {
> + if (IS_ERR(obj)) {
> pr_debug("bpf: failed to load %s\n", filename);
> - return ERR_PTR(-EINVAL);
> + return obj;
> }
>
> return obj;
> @@ -96,9 +96,9 @@ config_bpf_program(struct bpf_program *prog)
> int err;
>
> config_str = bpf_program__title(prog, false);
> - if (!config_str) {
> + if (IS_ERR(config_str)) {
> pr_debug("bpf: unable to get title for program\n");
> - return -EINVAL;
> + return PTR_ERR(config_str);
> }
>
> priv = calloc(sizeof(*priv), 1);
> @@ -308,13 +308,34 @@ int bpf__foreach_tev(struct bpf_object *obj,
> return 0;
> }
>
> +static int
> +bpf_loader_strerror(int err, char *buf, size_t size)
> +{
> + char sbuf[STRERR_BUFSIZE];
> + const char *msg;
> +
> + if (!buf || !size)
> + return -1;
> +
> + err = err > 0 ? err : -err;
> +
> + if (err >= LIBBPF_ERRNO__START)
> + return libbpf_strerror(err, buf, size);
> +
> + msg = strerror_r(err, sbuf, sizeof(sbuf));
> + snprintf(buf, size, "%s", msg);
> + buf[size - 1] = '\0';
> + return 0;
> +}
> +
> #define bpf__strerror_head(err, buf, size) \
> char sbuf[STRERR_BUFSIZE], *emsg;\
> if (!size)\
> return 0;\
> if (err < 0)\
> err = -err;\
> - emsg = strerror_r(err, sbuf, sizeof(sbuf));\
> + bpf_loader_strerror(err, sbuf, sizeof(sbuf));\
> + emsg = sbuf;\
> switch (err) {\
> default:\
> scnprintf(buf, size, "%s", emsg);\
> @@ -345,8 +366,9 @@ int bpf__strerror_load(struct bpf_object *obj __maybe_unused,
> int err, char *buf, size_t size)
> {
> bpf__strerror_head(err, buf, size);
> - bpf__strerror_entry(EINVAL, "%s: Are you root and runing a CONFIG_BPF_SYSCALL kernel?",
> - emsg)
> + bpf__strerror_entry(LIBBPF_ERRNO__ELOAD,
> + "%s: Validate your program and check 'license'/'version' sections in your object",
> + emsg);
> bpf__strerror_end(buf, size);
> return 0;
> }
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index bee6058..c75b25d 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -632,11 +632,11 @@ int parse_events_load_bpf(struct parse_events_evlist *data,
> struct bpf_object *obj;
>
> obj = bpf__prepare_load(bpf_file_name, source);
> - if (IS_ERR(obj) || !obj) {
> + if (IS_ERR(obj)) {
> char errbuf[BUFSIZ];
> int err;
>
> - err = obj ? PTR_ERR(obj) : -EINVAL;
> + err = PTR_ERR(obj);
>
> if (err == -ENOTSUP)
> snprintf(errbuf, sizeof(errbuf),
> --
> 1.8.3.4
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/5] bpf tools: Improve libbpf error reporting
2015-11-05 15:26 ` Arnaldo Carvalho de Melo
@ 2015-11-05 15:35 ` Arnaldo Carvalho de Melo
2015-11-05 15:36 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-05 15:35 UTC (permalink / raw)
To: Wang Nan
Cc: namhyung, lizefan, pi3orama, linux-kernel, Ingo Molnar, Jiri Olsa,
Alexei Starovoitov
Em Thu, Nov 05, 2015 at 12:26:07PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Nov 05, 2015 at 04:26:58AM +0000, Wang Nan escreveu:
> > In this patch, a series libbpf specific error numbers and
> > libbpf_strerror() are created to help reporting error to caller.
> > Functions are updated to pass correct error number through macro
> > CHECK_ERR().
> >
> > All users of bpf_object__open{_buffer}() and bpf_program__title()
> > in perf are modified accordingly. In addition, due to error code
> > changing, bpf__strerror_load() also modified to use new error code.
> >
> > bpf__strerror_head() is also changed accordingly so it can parse
> > libbpf error. bpf_loader_strerror() is introduced for it, and will
> > be improved by following patch.
>
> I am applying this, test shows a improvement in error reporting, but
> please look below for some suggestions:
I take that back, better fix this now, see the other message about the
BPF loader part too.
>
> [root@zoo ~]# perf record -e /tmp/foo.o sleep 1
> event syntax error: '/tmp/foo.o'
> \___ Failed to load program: Validate your program and check 'license'/'version' sections in your object
>
> Now this is a long error message, maybe:
>
> event syntax error: '/tmp/foo.o'
> \___ Failed to load: Check 'license'/'version' sections
>
> And then, here we _know_ the problem is in the license, so why not use
> that knowledge to help the user further and show instead:
>
> event syntax error: '/tmp/foo.o'
> \___ Failed to load: 'version'(4.2.0) doesn't match running kernel (4.3.0)
>
> More below
>
> > Signed-off-by: Wang Nan <wangnan0@huawei.com>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/lib/bpf/libbpf.c | 149 ++++++++++++++++++++++++++++-------------
> > tools/lib/bpf/libbpf.h | 12 ++++
> > tools/perf/tests/llvm.c | 2 +-
> > tools/perf/util/bpf-loader.c | 36 ++++++++--
> > tools/perf/util/parse-events.c | 4 +-
> > 5 files changed, 145 insertions(+), 58 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 9f3c8cf..c4dcee0 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -61,6 +61,62 @@ void libbpf_set_print(libbpf_print_fn_t warn,
> > __pr_debug = debug;
> > }
> >
> > +#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
> > +#define STRERR_BUFSIZE 128
> > +
> > +struct {
> > + int code;
> > + const char *msg;
> > +} libbpf_strerror_table[] = {
> > + {LIBBPF_ERRNO__ELIBELF, "Something wrong in libelf"},
> > + {LIBBPF_ERRNO__EFORMAT, "BPF object format invalid"},
> > + {LIBBPF_ERRNO__EKVERSION, "'version' section incorrect or lost"},
> > + {LIBBPF_ERRNO__EENDIAN, "Endian missmatch"},
> > + {LIBBPF_ERRNO__EINTERNAL, "Internal error in libbpf"},
> > + {LIBBPF_ERRNO__ERELOC, "Relocation failed"},
> > + {LIBBPF_ERRNO__ELOAD, "Failed to load program"},
> > +};
> > +
> > +int libbpf_strerror(int err, char *buf, size_t size)
> > +{
> > + unsigned int i;
> > +
> > + if (!buf || !size)
> > + return -1;
> > +
> > + err = err > 0 ? err : -err;
> > +
> > + if (err < LIBBPF_ERRNO__START) {
> > + int ret;
> > +
> > + ret = strerror_r(err, buf, size);
> > + buf[size - 1] = '\0';
> > + return ret;
> > + }
>
> Since all those errors are in sequence, why not introduce a
> LIBBPF_ERRNO__END and test against it, and then index the
> libbpf_strerror_table directly using (err - LIBBPF_ERRNO__START)?
>
> When introducing a function, take a look at similar functions and reuse
> code, for instance: target__strerror() does this pretty well, I think.
>
> > +
> > + for (i = 0; i < ARRAY_SIZE(libbpf_strerror_table); i++) {
> > + if (libbpf_strerror_table[i].code == err) {
> > + const char *msg;
> > +
> > + msg = libbpf_strerror_table[i].msg;
> > + snprintf(buf, size, "%s", msg);
> > + buf[size - 1] = '\0';
> > + return 0;
> > + }
> > + }
> > +
> > + snprintf(buf, size, "Unknown libbpf error %d", err);
> > + buf[size - 1] = '\0';
> > + return -1;
> > +}
> > +
> > +#define CHECK_ERR(action, err, out) do { \
> > + err = action; \
> > + if (err) \
> > + goto out; \
> > +} while(0)
> > +
> > +
> > /* Copied from tools/perf/util/util.h */
> > #ifndef zfree
> > # define zfree(ptr) ({ free(*ptr); *ptr = NULL; })
> > @@ -258,7 +314,7 @@ static struct bpf_object *bpf_object__new(const char *path,
> > obj = calloc(1, sizeof(struct bpf_object) + strlen(path) + 1);
> > if (!obj) {
> > pr_warning("alloc memory failed for %s\n", path);
> > - return NULL;
> > + return ERR_PTR(-ENOMEM);
> > }
> >
> > strcpy(obj->path, path);
> > @@ -305,7 +361,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
> >
> > if (obj_elf_valid(obj)) {
> > pr_warning("elf init: internal error\n");
> > - return -EEXIST;
> > + return -LIBBPF_ERRNO__ELIBELF;
> > }
> >
> > if (obj->efile.obj_buf_sz > 0) {
> > @@ -331,14 +387,14 @@ static int bpf_object__elf_init(struct bpf_object *obj)
> > if (!obj->efile.elf) {
> > pr_warning("failed to open %s as ELF file\n",
> > obj->path);
> > - err = -EINVAL;
> > + err = -LIBBPF_ERRNO__ELIBELF;
> > goto errout;
> > }
> >
> > if (!gelf_getehdr(obj->efile.elf, &obj->efile.ehdr)) {
> > pr_warning("failed to get EHDR from %s\n",
> > obj->path);
> > - err = -EINVAL;
> > + err = -LIBBPF_ERRNO__EFORMAT;
> > goto errout;
> > }
> > ep = &obj->efile.ehdr;
> > @@ -346,7 +402,7 @@ static int bpf_object__elf_init(struct bpf_object *obj)
> > if ((ep->e_type != ET_REL) || (ep->e_machine != 0)) {
> > pr_warning("%s is not an eBPF object file\n",
> > obj->path);
> > - err = -EINVAL;
> > + err = -LIBBPF_ERRNO__EFORMAT;
> > goto errout;
> > }
> >
> > @@ -374,14 +430,14 @@ bpf_object__check_endianness(struct bpf_object *obj)
> > goto mismatch;
> > break;
> > default:
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__EENDIAN;
> > }
> >
> > return 0;
> >
> > mismatch:
> > pr_warning("Error: endianness mismatch.\n");
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__EENDIAN;
> > }
> >
> > static int
> > @@ -402,7 +458,7 @@ bpf_object__init_kversion(struct bpf_object *obj,
> >
> > if (size != sizeof(kver)) {
> > pr_warning("invalid kver section in %s\n", obj->path);
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__EFORMAT;
> > }
> > memcpy(&kver, data, sizeof(kver));
> > obj->kern_version = kver;
> > @@ -444,7 +500,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> > if (!elf_rawdata(elf_getscn(elf, ep->e_shstrndx), NULL)) {
> > pr_warning("failed to get e_shstrndx from %s\n",
> > obj->path);
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__EFORMAT;
> > }
> >
> > while ((scn = elf_nextscn(elf, scn)) != NULL) {
> > @@ -456,7 +512,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> > if (gelf_getshdr(scn, &sh) != &sh) {
> > pr_warning("failed to get section header from %s\n",
> > obj->path);
> > - err = -EINVAL;
> > + err = -LIBBPF_ERRNO__EFORMAT;
> > goto out;
> > }
> >
> > @@ -464,7 +520,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> > if (!name) {
> > pr_warning("failed to get section name from %s\n",
> > obj->path);
> > - err = -EINVAL;
> > + err = -LIBBPF_ERRNO__EFORMAT;
> > goto out;
> > }
> >
> > @@ -472,7 +528,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> > if (!data) {
> > pr_warning("failed to get section data from %s(%s)\n",
> > name, obj->path);
> > - err = -EINVAL;
> > + err = -LIBBPF_ERRNO__EFORMAT;
> > goto out;
> > }
> > pr_debug("section %s, size %ld, link %d, flags %lx, type=%d\n",
> > @@ -495,7 +551,7 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> > if (obj->efile.symbols) {
> > pr_warning("bpf: multiple SYMTAB in %s\n",
> > obj->path);
> > - err = -EEXIST;
> > + err = -LIBBPF_ERRNO__EFORMAT;
> > } else
> > obj->efile.symbols = data;
> > } else if ((sh.sh_type == SHT_PROGBITS) &&
> > @@ -504,7 +560,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj)
> > err = bpf_object__add_program(obj, data->d_buf,
> > data->d_size, name, idx);
> > if (err) {
> > - char errmsg[128];
> > + char errmsg[STRERR_BUFSIZE];
> > +
> > strerror_r(-err, errmsg, sizeof(errmsg));
> > pr_warning("failed to alloc program %s (%s): %s",
> > name, obj->path, errmsg);
> > @@ -576,7 +633,7 @@ bpf_program__collect_reloc(struct bpf_program *prog,
> >
> > if (!gelf_getrel(data, i, &rel)) {
> > pr_warning("relocation: failed to get %d reloc\n", i);
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__EFORMAT;
> > }
> >
> > insn_idx = rel.r_offset / sizeof(struct bpf_insn);
> > @@ -587,20 +644,20 @@ bpf_program__collect_reloc(struct bpf_program *prog,
> > &sym)) {
> > pr_warning("relocation: symbol %"PRIx64" not found\n",
> > GELF_R_SYM(rel.r_info));
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__EFORMAT;
> > }
> >
> > if (insns[insn_idx].code != (BPF_LD | BPF_IMM | BPF_DW)) {
> > pr_warning("bpf: relocation: invalid relo for insns[%d].code 0x%x\n",
> > insn_idx, insns[insn_idx].code);
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__ERELOC;
> > }
> >
> > map_idx = sym.st_value / sizeof(struct bpf_map_def);
> > if (map_idx >= nr_maps) {
> > pr_warning("bpf relocation: map_idx %d large than %d\n",
> > (int)map_idx, (int)nr_maps - 1);
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__ERELOC;
> > }
> >
> > prog->reloc_desc[i].insn_idx = insn_idx;
> > @@ -683,7 +740,7 @@ bpf_program__relocate(struct bpf_program *prog, int *map_fds)
> > if (insn_idx >= (int)prog->insns_cnt) {
> > pr_warning("relocation out of range: '%s'\n",
> > prog->section_name);
> > - return -ERANGE;
> > + return -LIBBPF_ERRNO__ERELOC;
> > }
> > insns[insn_idx].src_reg = BPF_PSEUDO_MAP_FD;
> > insns[insn_idx].imm = map_fds[map_idx];
> > @@ -721,7 +778,7 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
> >
> > if (!obj_elf_valid(obj)) {
> > pr_warning("Internal error: elf object is closed\n");
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__EINTERNAL;
> > }
> >
> > for (i = 0; i < obj->efile.nr_reloc; i++) {
> > @@ -734,21 +791,21 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
> >
> > if (shdr->sh_type != SHT_REL) {
> > pr_warning("internal error at %d\n", __LINE__);
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__EINTERNAL;
> > }
> >
> > prog = bpf_object__find_prog_by_idx(obj, idx);
> > if (!prog) {
> > pr_warning("relocation failed: no %d section\n",
> > idx);
> > - return -ENOENT;
> > + return -LIBBPF_ERRNO__ERELOC;
> > }
> >
> > err = bpf_program__collect_reloc(prog, nr_maps,
> > shdr, data,
> > obj->efile.symbols);
> > if (err)
> > - return -EINVAL;
> > + return err;
> > }
> > return 0;
> > }
> > @@ -777,7 +834,7 @@ load_program(struct bpf_insn *insns, int insns_cnt,
> > goto out;
> > }
> >
> > - ret = -EINVAL;
> > + ret = -LIBBPF_ERRNO__ELOAD;
> > pr_warning("load bpf program failed: %s\n", strerror(errno));
> >
> > if (log_buf) {
> > @@ -831,7 +888,7 @@ static int bpf_object__validate(struct bpf_object *obj)
> > if (obj->kern_version == 0) {
> > pr_warning("%s doesn't provide kernel version\n",
> > obj->path);
> > - return -EINVAL;
> > + return -LIBBPF_ERRNO__EKVERSION;
> > }
> > return 0;
> > }
> > @@ -840,32 +897,28 @@ static struct bpf_object *
> > __bpf_object__open(const char *path, void *obj_buf, size_t obj_buf_sz)
> > {
> > struct bpf_object *obj;
> > + int err;
> >
> > if (elf_version(EV_CURRENT) == EV_NONE) {
> > pr_warning("failed to init libelf for %s\n", path);
> > - return NULL;
> > + return ERR_PTR(-LIBBPF_ERRNO__ELIBELF);
> > }
> >
> > obj = bpf_object__new(path, obj_buf, obj_buf_sz);
> > - if (!obj)
> > - return NULL;
> > + if (IS_ERR(obj))
> > + return obj;
> >
> > - if (bpf_object__elf_init(obj))
> > - goto out;
> > - if (bpf_object__check_endianness(obj))
> > - goto out;
> > - if (bpf_object__elf_collect(obj))
> > - goto out;
> > - if (bpf_object__collect_reloc(obj))
> > - goto out;
> > - if (bpf_object__validate(obj))
> > - goto out;
> > + CHECK_ERR(bpf_object__elf_init(obj), err, out);
> > + CHECK_ERR(bpf_object__check_endianness(obj), err, out);
> > + CHECK_ERR(bpf_object__elf_collect(obj), err, out);
> > + CHECK_ERR(bpf_object__collect_reloc(obj), err, out);
> > + CHECK_ERR(bpf_object__validate(obj), err, out);
> >
> > bpf_object__elf_finish(obj);
> > return obj;
> > out:
> > bpf_object__close(obj);
> > - return NULL;
> > + return ERR_PTR(err);
> > }
> >
> > struct bpf_object *bpf_object__open(const char *path)
> > @@ -922,6 +975,8 @@ int bpf_object__unload(struct bpf_object *obj)
> >
> > int bpf_object__load(struct bpf_object *obj)
> > {
> > + int err;
> > +
> > if (!obj)
> > return -EINVAL;
> >
> > @@ -931,18 +986,16 @@ int bpf_object__load(struct bpf_object *obj)
> > }
> >
> > obj->loaded = true;
> > - if (bpf_object__create_maps(obj))
> > - goto out;
> > - if (bpf_object__relocate(obj))
> > - goto out;
> > - if (bpf_object__load_progs(obj))
> > - goto out;
> > +
> > + CHECK_ERR(bpf_object__create_maps(obj), err, out);
> > + CHECK_ERR(bpf_object__relocate(obj), err, out);
> > + CHECK_ERR(bpf_object__load_progs(obj), err, out);
> >
> > return 0;
> > out:
> > bpf_object__unload(obj);
> > pr_warning("failed to load object '%s'\n", obj->path);
> > - return -EINVAL;
> > + return err;
> > }
> >
> > void bpf_object__close(struct bpf_object *obj)
> > @@ -990,7 +1043,7 @@ const char *
> > bpf_object__get_name(struct bpf_object *obj)
> > {
> > if (!obj)
> > - return NULL;
> > + return ERR_PTR(-EINVAL);
> > return obj->path;
> > }
> >
> > @@ -1043,7 +1096,7 @@ const char *bpf_program__title(struct bpf_program *prog, bool needs_copy)
> > title = strdup(title);
> > if (!title) {
> > pr_warning("failed to strdup program title\n");
> > - return NULL;
> > + return ERR_PTR(-ENOMEM);
> > }
> > }
> >
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index bc80af0..6491be6 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -10,6 +10,18 @@
> >
> > #include <stdio.h>
> > #include <stdbool.h>
> > +#include <linux/err.h>
> > +
> > +#define LIBBPF_ERRNO__START 4000
> > +#define LIBBPF_ERRNO__ELIBELF 4000 /* Something wrong in libelf */
> > +#define LIBBPF_ERRNO__EFORMAT 4001 /* BPF object format invalid */
> > +#define LIBBPF_ERRNO__EKVERSION 4002 /* Incorrect or no 'version' section */
> > +#define LIBBPF_ERRNO__EENDIAN 4003 /* Endian missmatch */
> > +#define LIBBPF_ERRNO__EINTERNAL 4004 /* Internal error in libbpf */
> > +#define LIBBPF_ERRNO__ERELOC 4005 /* Relocation failed */
> > +#define LIBBPF_ERRNO__ELOAD 4006 /* Failed to load program */
> > +
> > +int libbpf_strerror(int err, char *buf, size_t size);
> >
> > /*
> > * In include/linux/compiler-gcc.h, __printf is defined. However
> > diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
> > index 512d362..8f713f6 100644
> > --- a/tools/perf/tests/llvm.c
> > +++ b/tools/perf/tests/llvm.c
> > @@ -27,7 +27,7 @@ static int test__bpf_parsing(void *obj_buf, size_t obj_buf_sz)
> > struct bpf_object *obj;
> >
> > obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, NULL);
> > - if (!obj)
> > + if (IS_ERR(obj))
> > return -1;
> > bpf_object__close(obj);
> > return 0;
> > diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> > index 0c5d174..72ad3dc 100644
> > --- a/tools/perf/util/bpf-loader.c
> > +++ b/tools/perf/util/bpf-loader.c
> > @@ -59,9 +59,9 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
> > } else
> > obj = bpf_object__open(filename);
> >
> > - if (!obj) {
> > + if (IS_ERR(obj)) {
> > pr_debug("bpf: failed to load %s\n", filename);
> > - return ERR_PTR(-EINVAL);
> > + return obj;
> > }
> >
> > return obj;
> > @@ -96,9 +96,9 @@ config_bpf_program(struct bpf_program *prog)
> > int err;
> >
> > config_str = bpf_program__title(prog, false);
> > - if (!config_str) {
> > + if (IS_ERR(config_str)) {
> > pr_debug("bpf: unable to get title for program\n");
> > - return -EINVAL;
> > + return PTR_ERR(config_str);
> > }
> >
> > priv = calloc(sizeof(*priv), 1);
> > @@ -308,13 +308,34 @@ int bpf__foreach_tev(struct bpf_object *obj,
> > return 0;
> > }
> >
> > +static int
> > +bpf_loader_strerror(int err, char *buf, size_t size)
> > +{
> > + char sbuf[STRERR_BUFSIZE];
> > + const char *msg;
> > +
> > + if (!buf || !size)
> > + return -1;
> > +
> > + err = err > 0 ? err : -err;
> > +
> > + if (err >= LIBBPF_ERRNO__START)
> > + return libbpf_strerror(err, buf, size);
> > +
> > + msg = strerror_r(err, sbuf, sizeof(sbuf));
> > + snprintf(buf, size, "%s", msg);
> > + buf[size - 1] = '\0';
> > + return 0;
> > +}
> > +
> > #define bpf__strerror_head(err, buf, size) \
> > char sbuf[STRERR_BUFSIZE], *emsg;\
> > if (!size)\
> > return 0;\
> > if (err < 0)\
> > err = -err;\
> > - emsg = strerror_r(err, sbuf, sizeof(sbuf));\
> > + bpf_loader_strerror(err, sbuf, sizeof(sbuf));\
> > + emsg = sbuf;\
> > switch (err) {\
> > default:\
> > scnprintf(buf, size, "%s", emsg);\
> > @@ -345,8 +366,9 @@ int bpf__strerror_load(struct bpf_object *obj __maybe_unused,
> > int err, char *buf, size_t size)
> > {
> > bpf__strerror_head(err, buf, size);
> > - bpf__strerror_entry(EINVAL, "%s: Are you root and runing a CONFIG_BPF_SYSCALL kernel?",
> > - emsg)
> > + bpf__strerror_entry(LIBBPF_ERRNO__ELOAD,
> > + "%s: Validate your program and check 'license'/'version' sections in your object",
> > + emsg);
> > bpf__strerror_end(buf, size);
> > return 0;
> > }
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index bee6058..c75b25d 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -632,11 +632,11 @@ int parse_events_load_bpf(struct parse_events_evlist *data,
> > struct bpf_object *obj;
> >
> > obj = bpf__prepare_load(bpf_file_name, source);
> > - if (IS_ERR(obj) || !obj) {
> > + if (IS_ERR(obj)) {
> > char errbuf[BUFSIZ];
> > int err;
> >
> > - err = obj ? PTR_ERR(obj) : -EINVAL;
> > + err = PTR_ERR(obj);
> >
> > if (err == -ENOTSUP)
> > snprintf(errbuf, sizeof(errbuf),
> > --
> > 1.8.3.4
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/5] bpf tools: Improve libbpf error reporting
2015-11-05 15:35 ` Arnaldo Carvalho de Melo
@ 2015-11-05 15:36 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-05 15:36 UTC (permalink / raw)
To: Wang Nan
Cc: namhyung, lizefan, pi3orama, linux-kernel, Ingo Molnar, Jiri Olsa,
Alexei Starovoitov
Em Thu, Nov 05, 2015 at 12:35:11PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Nov 05, 2015 at 12:26:07PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Thu, Nov 05, 2015 at 04:26:58AM +0000, Wang Nan escreveu:
> > > In this patch, a series libbpf specific error numbers and
> > > libbpf_strerror() are created to help reporting error to caller.
> > > Functions are updated to pass correct error number through macro
> > > CHECK_ERR().
> > >
> > > All users of bpf_object__open{_buffer}() and bpf_program__title()
> > > in perf are modified accordingly. In addition, due to error code
> > > changing, bpf__strerror_load() also modified to use new error code.
> > >
> > > bpf__strerror_head() is also changed accordingly so it can parse
> > > libbpf error. bpf_loader_strerror() is introduced for it, and will
> > > be improved by following patch.
> >
> > I am applying this, test shows a improvement in error reporting, but
> > please look below for some suggestions:
>
> I take that back, better fix this now, see the other message about the
> BPF loader part too.
And please add something like this so that we can see how it was before
your change and how it became, to make it clear the value of your patch,
i.e. what is that the user will see before and after your patch:
Committer note:
Before:
the foo.o object has an invalid kernel version:
# /tmp/oldperf record -e /tmp/foo.o sleep 1
event syntax error: '/tmp/foo.o'
\___ Invalid argument: Are you root and runing a CONFIG_BPF_SYSCALL kernel?
(add -v to see detail)
Run 'perf list' for a list of valid events
Usage: perf record [<options>] [<command>]
or: perf record [<options>] -- <command> [<options>]
-e, --event <event> event selector. use 'perf list' to list available events
After:
# perf record -e /tmp/foo.o sleep 1
event syntax error: '/tmp/foo.o'
\___ Failed to load program: Validate your program and check 'license'/'version' sections in your object
(add -v to see detail)
Run 'perf list' for a list of valid events
Usage: perf record [<options>] [<command>]
or: perf record [<options>] -- <command> [<options>]
-e, --event <event> event selector. use 'perf list' to list available events
#
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/5] perf tools: Improve BPF related error messages output
2015-11-05 4:26 [PATCH 0/5] perf tools: improve BPF related error delivering and testing Wang Nan
2015-11-05 4:26 ` [PATCH 1/5] bpf tools: Improve libbpf error reporting Wang Nan
@ 2015-11-05 4:26 ` Wang Nan
2015-11-05 15:34 ` Arnaldo Carvalho de Melo
2015-11-05 4:27 ` [PATCH 3/5] perf test: Enforce LLVM test: update basic BPF test program Wang Nan
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Wang Nan @ 2015-11-05 4:26 UTC (permalink / raw)
To: acme, namhyung
Cc: lizefan, pi3orama, linux-kernel, Wang Nan,
Arnaldo Carvalho de Melo
A series of bpf loader related error code is introduced to help error
delivering. Functions are improved to return those new error code.
Functions which return pointers are adjusted to encode error code into
return value using "ERR_PTR".
bpf_loader_strerror() is improved to convert those error message to
string. It detected the value of error code and calls libbpf_strerror()
and strerror_r() accordingly, so caller don't need to consider checking
the range of error code.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/bpf-loader.c | 70 +++++++++++++++++++++++++++++++++++++-----
tools/perf/util/bpf-loader.h | 18 +++++++++++
tools/perf/util/parse-events.c | 7 +++--
3 files changed, 84 insertions(+), 11 deletions(-)
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 72ad3dc..4b9bb2a 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -14,6 +14,10 @@
#include "probe-finder.h" // for MAX_PROBES
#include "llvm-utils.h"
+#if BPF_LOADER_ERRNO__END >= LIBBPF_ERRNO__START
+# error Too many BPF loader error code
+#endif
+
#define DEFINE_PRINT_FN(name, level) \
static int libbpf_##name(const char *fmt, ...) \
{ \
@@ -53,7 +57,7 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
err = llvm__compile_bpf(filename, &obj_buf, &obj_buf_sz);
if (err)
- return ERR_PTR(err);
+ return ERR_PTR(-BPF_LOADER_ERRNO__ECOMPILE);
obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, filename);
free(obj_buf);
} else
@@ -113,14 +117,14 @@ config_bpf_program(struct bpf_program *prog)
if (err < 0) {
pr_debug("bpf: '%s' is not a valid config string\n",
config_str);
- err = -EINVAL;
+ err = -BPF_LOADER_ERRNO__ECONFIG;
goto errout;
}
if (pev->group && strcmp(pev->group, PERF_BPF_PROBE_GROUP)) {
pr_debug("bpf: '%s': group for event is set and not '%s'.\n",
config_str, PERF_BPF_PROBE_GROUP);
- err = -EINVAL;
+ err = -BPF_LOADER_ERRNO__EGROUP;
goto errout;
} else if (!pev->group)
pev->group = strdup(PERF_BPF_PROBE_GROUP);
@@ -132,9 +136,9 @@ config_bpf_program(struct bpf_program *prog)
}
if (!pev->event) {
- pr_debug("bpf: '%s': event name is missing\n",
+ pr_debug("bpf: '%s': event name is missing. Section name should be 'key=value'\n",
config_str);
- err = -EINVAL;
+ err = -BPF_LOADER_ERRNO__EEVENTNAME;
goto errout;
}
pr_debug("bpf: config '%s' is ok\n", config_str);
@@ -285,7 +289,7 @@ int bpf__foreach_tev(struct bpf_object *obj,
(void **)&priv);
if (err || !priv) {
pr_debug("bpf: failed to get private field\n");
- return -EINVAL;
+ return -BPF_LOADER_ERRNO__EINTERNAL;
}
pev = &priv->pev;
@@ -308,11 +312,25 @@ int bpf__foreach_tev(struct bpf_object *obj,
return 0;
}
+#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
+
+struct {
+ int code;
+ const char *msg;
+} bpf_loader_strerror_table[] = {
+ {BPF_LOADER_ERRNO__ECONFIG, "Invalid config string"},
+ {BPF_LOADER_ERRNO__EGROUP, "Invalid group name"},
+ {BPF_LOADER_ERRNO__EEVENTNAME, "No event name found in config string"},
+ {BPF_LOADER_ERRNO__EINTERNAL, "BPF loader internal error"},
+ {BPF_LOADER_ERRNO__ECOMPILE, "Error when compiling BPF scriptlet"},
+};
+
static int
bpf_loader_strerror(int err, char *buf, size_t size)
{
char sbuf[STRERR_BUFSIZE];
const char *msg;
+ unsigned int i;
if (!buf || !size)
return -1;
@@ -322,6 +340,21 @@ bpf_loader_strerror(int err, char *buf, size_t size)
if (err >= LIBBPF_ERRNO__START)
return libbpf_strerror(err, buf, size);
+ if (err >= BPF_LOADER_ERRNO__START) {
+ for (i = 0; i < ARRAY_SIZE(bpf_loader_strerror_table); i++) {
+ if (bpf_loader_strerror_table[i].code == err) {
+ msg = bpf_loader_strerror_table[i].msg;
+ snprintf(buf, size, "%s", msg);
+ buf[size - 1] = '\0';
+ return 0;
+ }
+ }
+
+ snprintf(buf, size, "Unknown bpf loader error %d", err);
+ buf[size - 1] = '\0';
+ return -1;
+ }
+
msg = strerror_r(err, sbuf, sizeof(sbuf));
snprintf(buf, size, "%s", msg);
buf[size - 1] = '\0';
@@ -351,13 +384,34 @@ bpf_loader_strerror(int err, char *buf, size_t size)
}\
buf[size - 1] = '\0';
+int bpf__strerror_prepare_load(const char *filename, bool source,
+ int err, char *buf, size_t size)
+{
+ size_t n;
+ int ret;
+
+ n = snprintf(buf, size, "Failed to load %s%s: ",
+ filename, source ? " from source" : "");
+ if (n >= size) {
+ buf[size - 1] = '\0';
+ return 0;
+ }
+ buf += n;
+ size -= n;
+
+ ret = bpf_loader_strerror(err, buf, size);
+ buf[size - 1] = '\0';
+ return ret;
+}
+
int bpf__strerror_probe(struct bpf_object *obj __maybe_unused,
int err, char *buf, size_t size)
{
bpf__strerror_head(err, buf, size);
bpf__strerror_entry(EEXIST, "Probe point exist. Try use 'perf probe -d \"*\"'");
- bpf__strerror_entry(EPERM, "You need to be root, and /proc/sys/kernel/kptr_restrict should be 0\n");
- bpf__strerror_entry(ENOENT, "You need to check probing points in BPF file\n");
+ bpf__strerror_entry(EACCES, "You need to be root");
+ bpf__strerror_entry(EPERM, "You need to be root, and /proc/sys/kernel/kptr_restrict should be 0");
+ bpf__strerror_entry(ENOENT, "You need to check probing points in BPF file");
bpf__strerror_end(buf, size);
return 0;
}
diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
index ccd8d7f..490c78c 100644
--- a/tools/perf/util/bpf-loader.h
+++ b/tools/perf/util/bpf-loader.h
@@ -11,6 +11,14 @@
#include "probe-event.h"
#include "debug.h"
+#define BPF_LOADER_ERRNO__START 3900
+#define BPF_LOADER_ERRNO__ECONFIG 3900 /* Invalid config string */
+#define BPF_LOADER_ERRNO__EGROUP 3901 /* Invalid group name */
+#define BPF_LOADER_ERRNO__EEVENTNAME 3902 /* Event name is missing */
+#define BPF_LOADER_ERRNO__EINTERNAL 3903 /* BPF loader internal error */
+#define BPF_LOADER_ERRNO__ECOMPILE 3904 /* Error when compiling BPF scriptlet */
+#define BPF_LOADER_ERRNO__END 3905
+
struct bpf_object;
#define PERF_BPF_PROBE_GROUP "perf_bpf_probe"
@@ -19,6 +27,8 @@ typedef int (*bpf_prog_iter_callback_t)(struct probe_trace_event *tev,
#ifdef HAVE_LIBBPF_SUPPORT
struct bpf_object *bpf__prepare_load(const char *filename, bool source);
+int bpf__strerror_prepare_load(const char *filename, bool source,
+ int err, char *buf, size_t size);
void bpf__clear(void);
@@ -67,6 +77,14 @@ __bpf_strerror(char *buf, size_t size)
return 0;
}
+int bpf__strerror_prepare_load(const char *filename __maybe_unused,
+ bool source __maybe_unused,
+ int err __maybe_unused,
+ char *buf, size_t size)
+{
+ return __bpf_strerror(buf, size);
+}
+
static inline int
bpf__strerror_probe(struct bpf_object *obj __maybe_unused,
int err __maybe_unused,
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index c75b25d..e48d9da 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -642,9 +642,10 @@ int parse_events_load_bpf(struct parse_events_evlist *data,
snprintf(errbuf, sizeof(errbuf),
"BPF support is not compiled");
else
- snprintf(errbuf, sizeof(errbuf),
- "BPF object file '%s' is invalid",
- bpf_file_name);
+ bpf__strerror_prepare_load(bpf_file_name,
+ source,
+ -err, errbuf,
+ sizeof(errbuf));
data->error->help = strdup("(add -v to see detail)");
data->error->str = strdup(errbuf);
--
1.8.3.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/5] perf tools: Improve BPF related error messages output
2015-11-05 4:26 ` [PATCH 2/5] perf tools: Improve BPF related error messages output Wang Nan
@ 2015-11-05 15:34 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-05 15:34 UTC (permalink / raw)
To: Wang Nan; +Cc: namhyung, lizefan, pi3orama, linux-kernel
Em Thu, Nov 05, 2015 at 04:26:59AM +0000, Wang Nan escreveu:
> A series of bpf loader related error code is introduced to help error
> delivering. Functions are improved to return those new error code.
> Functions which return pointers are adjusted to encode error code into
> return value using "ERR_PTR".
So:
> +#define BPF_LOADER_ERRNO__START 3900
This is done in terms of LIBBPF_ERRNO__START, right?
Why not do this as an enum and also, again, look at
tools/perf/util/target.h and see how it is done, then you can do
something like:
> +#define BPF_LOADER_ERRNO__START 3900
> +#define BPF_LOADER_ERRNO__ECONFIG 3900 /* Invalid config string */
> +#define BPF_LOADER_ERRNO__EGROUP 3901 /* Invalid group name */
> +#define BPF_LOADER_ERRNO__EEVENTNAME 3902 /* Event name is missing */
> +#define BPF_LOADER_ERRNO__EINTERNAL 3903 /* BPF loader internal
> error */
> +#define BPF_LOADER_ERRNO__ECOMPILE 3904 /* Error when compiling BPF
> scriptlet */
> +#define BPF_LOADER_ERRNO__END 3905
enum bpf_loader_errno {
__BPF_LOADER_ERRNO__START = (LIBBPF_ERRNO__START - 100),
BPF_LOADER_ERRNO__CONFIG = __BPF_LOADER_ERRNO__START,
BPF_LOADER_ERRNO__CONFIG /* Invalid config string */
BPF_LOADER_ERRNO__GROUP /* Invalid group name */
BPF_LOADER_ERRNO__EVENTNAME /* Event name is missing */
BPF_LOADER_ERRNO__INTERNAL /* BPF loader internal error */
BPF_LOADER_ERRNO__COMPILE /* Error when compiling BPF > scriptlet */
__BPF_LOADER_ERRNO__END,
};
In the process I removed that extra 'E', heck, we have
BPF_LOADER_ERRNO__, that should be enough indication that this is an
ERRNO code, no? 8-)
So, I'm removing the first patch as well, better get that one right from
start.
- Arnaldo
> bpf_loader_strerror() is improved to convert those error message to
> string. It detected the value of error code and calls libbpf_strerror()
> and strerror_r() accordingly, so caller don't need to consider checking
> the range of error code.
>
> Signed-off-by: Wang Nan <wangnan0@huawei.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/bpf-loader.c | 70 +++++++++++++++++++++++++++++++++++++-----
> tools/perf/util/bpf-loader.h | 18 +++++++++++
> tools/perf/util/parse-events.c | 7 +++--
> 3 files changed, 84 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index 72ad3dc..4b9bb2a 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -14,6 +14,10 @@
> #include "probe-finder.h" // for MAX_PROBES
> #include "llvm-utils.h"
>
> +#if BPF_LOADER_ERRNO__END >= LIBBPF_ERRNO__START
> +# error Too many BPF loader error code
> +#endif
> +
> #define DEFINE_PRINT_FN(name, level) \
> static int libbpf_##name(const char *fmt, ...) \
> { \
> @@ -53,7 +57,7 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source)
>
> err = llvm__compile_bpf(filename, &obj_buf, &obj_buf_sz);
> if (err)
> - return ERR_PTR(err);
> + return ERR_PTR(-BPF_LOADER_ERRNO__ECOMPILE);
> obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, filename);
> free(obj_buf);
> } else
> @@ -113,14 +117,14 @@ config_bpf_program(struct bpf_program *prog)
> if (err < 0) {
> pr_debug("bpf: '%s' is not a valid config string\n",
> config_str);
> - err = -EINVAL;
> + err = -BPF_LOADER_ERRNO__ECONFIG;
> goto errout;
> }
>
> if (pev->group && strcmp(pev->group, PERF_BPF_PROBE_GROUP)) {
> pr_debug("bpf: '%s': group for event is set and not '%s'.\n",
> config_str, PERF_BPF_PROBE_GROUP);
> - err = -EINVAL;
> + err = -BPF_LOADER_ERRNO__EGROUP;
> goto errout;
> } else if (!pev->group)
> pev->group = strdup(PERF_BPF_PROBE_GROUP);
> @@ -132,9 +136,9 @@ config_bpf_program(struct bpf_program *prog)
> }
>
> if (!pev->event) {
> - pr_debug("bpf: '%s': event name is missing\n",
> + pr_debug("bpf: '%s': event name is missing. Section name should be 'key=value'\n",
> config_str);
> - err = -EINVAL;
> + err = -BPF_LOADER_ERRNO__EEVENTNAME;
> goto errout;
> }
> pr_debug("bpf: config '%s' is ok\n", config_str);
> @@ -285,7 +289,7 @@ int bpf__foreach_tev(struct bpf_object *obj,
> (void **)&priv);
> if (err || !priv) {
> pr_debug("bpf: failed to get private field\n");
> - return -EINVAL;
> + return -BPF_LOADER_ERRNO__EINTERNAL;
> }
>
> pev = &priv->pev;
> @@ -308,11 +312,25 @@ int bpf__foreach_tev(struct bpf_object *obj,
> return 0;
> }
>
> +#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
> +
> +struct {
> + int code;
> + const char *msg;
> +} bpf_loader_strerror_table[] = {
> + {BPF_LOADER_ERRNO__ECONFIG, "Invalid config string"},
> + {BPF_LOADER_ERRNO__EGROUP, "Invalid group name"},
> + {BPF_LOADER_ERRNO__EEVENTNAME, "No event name found in config string"},
> + {BPF_LOADER_ERRNO__EINTERNAL, "BPF loader internal error"},
> + {BPF_LOADER_ERRNO__ECOMPILE, "Error when compiling BPF scriptlet"},
> +};
> +
> static int
> bpf_loader_strerror(int err, char *buf, size_t size)
> {
> char sbuf[STRERR_BUFSIZE];
> const char *msg;
> + unsigned int i;
>
> if (!buf || !size)
> return -1;
> @@ -322,6 +340,21 @@ bpf_loader_strerror(int err, char *buf, size_t size)
> if (err >= LIBBPF_ERRNO__START)
> return libbpf_strerror(err, buf, size);
>
> + if (err >= BPF_LOADER_ERRNO__START) {
> + for (i = 0; i < ARRAY_SIZE(bpf_loader_strerror_table); i++) {
> + if (bpf_loader_strerror_table[i].code == err) {
> + msg = bpf_loader_strerror_table[i].msg;
> + snprintf(buf, size, "%s", msg);
> + buf[size - 1] = '\0';
> + return 0;
> + }
> + }
> +
> + snprintf(buf, size, "Unknown bpf loader error %d", err);
> + buf[size - 1] = '\0';
> + return -1;
> + }
> +
> msg = strerror_r(err, sbuf, sizeof(sbuf));
> snprintf(buf, size, "%s", msg);
> buf[size - 1] = '\0';
> @@ -351,13 +384,34 @@ bpf_loader_strerror(int err, char *buf, size_t size)
> }\
> buf[size - 1] = '\0';
>
> +int bpf__strerror_prepare_load(const char *filename, bool source,
> + int err, char *buf, size_t size)
> +{
> + size_t n;
> + int ret;
> +
> + n = snprintf(buf, size, "Failed to load %s%s: ",
> + filename, source ? " from source" : "");
> + if (n >= size) {
> + buf[size - 1] = '\0';
> + return 0;
> + }
> + buf += n;
> + size -= n;
> +
> + ret = bpf_loader_strerror(err, buf, size);
> + buf[size - 1] = '\0';
> + return ret;
> +}
> +
> int bpf__strerror_probe(struct bpf_object *obj __maybe_unused,
> int err, char *buf, size_t size)
> {
> bpf__strerror_head(err, buf, size);
> bpf__strerror_entry(EEXIST, "Probe point exist. Try use 'perf probe -d \"*\"'");
> - bpf__strerror_entry(EPERM, "You need to be root, and /proc/sys/kernel/kptr_restrict should be 0\n");
> - bpf__strerror_entry(ENOENT, "You need to check probing points in BPF file\n");
> + bpf__strerror_entry(EACCES, "You need to be root");
> + bpf__strerror_entry(EPERM, "You need to be root, and /proc/sys/kernel/kptr_restrict should be 0");
> + bpf__strerror_entry(ENOENT, "You need to check probing points in BPF file");
> bpf__strerror_end(buf, size);
> return 0;
> }
> diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
> index ccd8d7f..490c78c 100644
> --- a/tools/perf/util/bpf-loader.h
> +++ b/tools/perf/util/bpf-loader.h
> @@ -11,6 +11,14 @@
> #include "probe-event.h"
> #include "debug.h"
>
> +#define BPF_LOADER_ERRNO__START 3900
> +#define BPF_LOADER_ERRNO__ECONFIG 3900 /* Invalid config string */
> +#define BPF_LOADER_ERRNO__EGROUP 3901 /* Invalid group name */
> +#define BPF_LOADER_ERRNO__EEVENTNAME 3902 /* Event name is missing */
> +#define BPF_LOADER_ERRNO__EINTERNAL 3903 /* BPF loader internal error */
> +#define BPF_LOADER_ERRNO__ECOMPILE 3904 /* Error when compiling BPF scriptlet */
> +#define BPF_LOADER_ERRNO__END 3905
> +
> struct bpf_object;
> #define PERF_BPF_PROBE_GROUP "perf_bpf_probe"
>
> @@ -19,6 +27,8 @@ typedef int (*bpf_prog_iter_callback_t)(struct probe_trace_event *tev,
>
> #ifdef HAVE_LIBBPF_SUPPORT
> struct bpf_object *bpf__prepare_load(const char *filename, bool source);
> +int bpf__strerror_prepare_load(const char *filename, bool source,
> + int err, char *buf, size_t size);
>
> void bpf__clear(void);
>
> @@ -67,6 +77,14 @@ __bpf_strerror(char *buf, size_t size)
> return 0;
> }
>
> +int bpf__strerror_prepare_load(const char *filename __maybe_unused,
> + bool source __maybe_unused,
> + int err __maybe_unused,
> + char *buf, size_t size)
> +{
> + return __bpf_strerror(buf, size);
> +}
> +
> static inline int
> bpf__strerror_probe(struct bpf_object *obj __maybe_unused,
> int err __maybe_unused,
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index c75b25d..e48d9da 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -642,9 +642,10 @@ int parse_events_load_bpf(struct parse_events_evlist *data,
> snprintf(errbuf, sizeof(errbuf),
> "BPF support is not compiled");
> else
> - snprintf(errbuf, sizeof(errbuf),
> - "BPF object file '%s' is invalid",
> - bpf_file_name);
> + bpf__strerror_prepare_load(bpf_file_name,
> + source,
> + -err, errbuf,
> + sizeof(errbuf));
>
> data->error->help = strdup("(add -v to see detail)");
> data->error->str = strdup(errbuf);
> --
> 1.8.3.4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/5] perf test: Enforce LLVM test: update basic BPF test program
2015-11-05 4:26 [PATCH 0/5] perf tools: improve BPF related error delivering and testing Wang Nan
2015-11-05 4:26 ` [PATCH 1/5] bpf tools: Improve libbpf error reporting Wang Nan
2015-11-05 4:26 ` [PATCH 2/5] perf tools: Improve BPF related error messages output Wang Nan
@ 2015-11-05 4:27 ` Wang Nan
2015-11-05 4:27 ` [PATCH 4/5] perf test: Enforce LLVM test: add kbuild test Wang Nan
2015-11-05 4:27 ` [PATCH 5/5] perf test: Add 'perf test BPF' Wang Nan
4 siblings, 0 replies; 10+ messages in thread
From: Wang Nan @ 2015-11-05 4:27 UTC (permalink / raw)
To: acme, namhyung
Cc: lizefan, pi3orama, linux-kernel, Wang Nan, He Kuang,
Arnaldo Carvalho de Melo
This patch replaces the original toy BPF program with previous introduced
bpf-script-example.c. Dynamically embedded it into 'llvm-src-base.c'.
The newly introduced BPF program attaches a BPF program to
'sys_epoll_pwait()'. perf itself never use that syscall, so further test
can verify their result with it. The program would generate 1 sample
in every 2 calls of epoll_pwait() system call.
Since the resuling BPF object is useful, test_llvm__fetch_bpf_obj() is
introduced for creating BPF objects for source. llvm test is rewritten
according to it.
Signed-off-by: He Kuang <hekuang@huawei.com>
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
tools/perf/tests/Build | 9 ++-
tools/perf/tests/bpf-script-example.c | 4 ++
tools/perf/tests/llvm.c | 131 ++++++++++++++++++++++++++--------
tools/perf/tests/llvm.h | 16 +++++
4 files changed, 129 insertions(+), 31 deletions(-)
create mode 100644 tools/perf/tests/llvm.h
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 50de225..6c095b3 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -31,9 +31,16 @@ perf-y += sample-parsing.o
perf-y += parse-no-sample-id-all.o
perf-y += kmod-path.o
perf-y += thread-map.o
-perf-y += llvm.o
+perf-y += llvm.o llvm-src-base.o
perf-y += topology.o
+$(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c
+ $(call rule_mkdir)
+ $(Q)echo '#include <tests/llvm.h>' > $@
+ $(Q)echo 'const char test_llvm__bpf_base_prog[] =' >> $@
+ $(Q)sed -e 's/"/\\"/g' -e 's/\(.*\)/"\1\\n"/g' $< >> $@
+ $(Q)echo ';' >> $@
+
ifeq ($(ARCH),$(filter $(ARCH),x86 arm arm64))
perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
endif
diff --git a/tools/perf/tests/bpf-script-example.c b/tools/perf/tests/bpf-script-example.c
index 410a70b..0ec9c2c 100644
--- a/tools/perf/tests/bpf-script-example.c
+++ b/tools/perf/tests/bpf-script-example.c
@@ -1,3 +1,7 @@
+/*
+ * bpf-script-example.c
+ * Test basic LLVM building
+ */
#ifndef LINUX_VERSION_CODE
# error Need LINUX_VERSION_CODE
# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
index 8f713f6..05683c5 100644
--- a/tools/perf/tests/llvm.c
+++ b/tools/perf/tests/llvm.c
@@ -2,6 +2,7 @@
#include <bpf/libbpf.h>
#include <util/llvm-utils.h>
#include <util/cache.h>
+#include "llvm.h"
#include "tests.h"
#include "debug.h"
@@ -11,16 +12,6 @@ static int perf_config_cb(const char *var, const char *val,
return perf_default_config(var, val, arg);
}
-/*
- * Randomly give it a "version" section since we don't really load it
- * into kernel
- */
-static const char test_bpf_prog[] =
- "__attribute__((section(\"do_fork\"), used)) "
- "int fork(void *ctx) {return 0;} "
- "char _license[] __attribute__((section(\"license\"), used)) = \"GPL\";"
- "int _version __attribute__((section(\"version\"), used)) = 0x40100;";
-
#ifdef HAVE_LIBBPF_SUPPORT
static int test__bpf_parsing(void *obj_buf, size_t obj_buf_sz)
{
@@ -28,25 +19,47 @@ static int test__bpf_parsing(void *obj_buf, size_t obj_buf_sz)
obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, NULL);
if (IS_ERR(obj))
- return -1;
+ return TEST_FAIL;
bpf_object__close(obj);
- return 0;
+ return TEST_OK;
}
#else
static int test__bpf_parsing(void *obj_buf __maybe_unused,
size_t obj_buf_sz __maybe_unused)
{
pr_debug("Skip bpf parsing\n");
- return 0;
+ return TEST_OK;
}
#endif
-int test__llvm(void)
+static struct {
+ const char *source;
+ const char *desc;
+} bpf_source_table[__LLVM_TESTCASE_MAX] = {
+ [LLVM_TESTCASE_BASE] = {
+ .source = test_llvm__bpf_base_prog,
+ .desc = "Basic BPF llvm compiling test",
+ },
+};
+
+
+int
+test_llvm__fetch_bpf_obj(void **p_obj_buf,
+ size_t *p_obj_buf_sz,
+ enum test_llvm__testcase index,
+ bool force)
{
- char *tmpl_new, *clang_opt_new;
- void *obj_buf;
- size_t obj_buf_sz;
- int err, old_verbose;
+ const char *source;
+ const char *desc;
+ const char *tmpl_old, *clang_opt_old;
+ char *tmpl_new = NULL, *clang_opt_new = NULL;
+ int err, old_verbose, ret = TEST_FAIL;
+
+ if (index >= __LLVM_TESTCASE_MAX)
+ return TEST_FAIL;
+
+ source = bpf_source_table[index].source;
+ desc = bpf_source_table[index].desc;
perf_config(perf_config_cb, NULL);
@@ -54,42 +67,100 @@ int test__llvm(void)
* Skip this test if user's .perfconfig doesn't set [llvm] section
* and clang is not found in $PATH, and this is not perf test -v
*/
- if (verbose == 0 && !llvm_param.user_set_param && llvm__search_clang()) {
+ if (!force && (verbose == 0 &&
+ !llvm_param.user_set_param &&
+ llvm__search_clang())) {
pr_debug("No clang and no verbosive, skip this test\n");
return TEST_SKIP;
}
- old_verbose = verbose;
/*
* llvm is verbosity when error. Suppress all error output if
* not 'perf test -v'.
*/
+ old_verbose = verbose;
if (verbose == 0)
verbose = -1;
+ *p_obj_buf = NULL;
+ *p_obj_buf_sz = 0;
+
if (!llvm_param.clang_bpf_cmd_template)
- return -1;
+ goto out;
if (!llvm_param.clang_opt)
llvm_param.clang_opt = strdup("");
- err = asprintf(&tmpl_new, "echo '%s' | %s", test_bpf_prog,
- llvm_param.clang_bpf_cmd_template);
+ err = asprintf(&tmpl_new, "echo '%s' | %s%s", source,
+ llvm_param.clang_bpf_cmd_template,
+ old_verbose ? "" : " 2>/dev/null");
if (err < 0)
- return -1;
+ goto out;
err = asprintf(&clang_opt_new, "-xc %s", llvm_param.clang_opt);
if (err < 0)
- return -1;
+ goto out;
+ tmpl_old = llvm_param.clang_bpf_cmd_template;
llvm_param.clang_bpf_cmd_template = tmpl_new;
+ clang_opt_old = llvm_param.clang_opt;
llvm_param.clang_opt = clang_opt_new;
- err = llvm__compile_bpf("-", &obj_buf, &obj_buf_sz);
+
+ err = llvm__compile_bpf("-", p_obj_buf, p_obj_buf_sz);
+
+ llvm_param.clang_bpf_cmd_template = tmpl_old;
+ llvm_param.clang_opt = clang_opt_old;
verbose = old_verbose;
if (err)
- return TEST_FAIL;
+ goto out;
+
+ ret = TEST_OK;
+out:
+ free(tmpl_new);
+ free(clang_opt_new);
+ if (ret != TEST_OK)
+ pr_debug("Failed to compile test case: '%s'\n", desc);
+ return ret;
+}
- err = test__bpf_parsing(obj_buf, obj_buf_sz);
- free(obj_buf);
- return err;
+int test__llvm(void)
+{
+ enum test_llvm__testcase i;
+
+ for (i = 0; i < __LLVM_TESTCASE_MAX; i++) {
+ int ret;
+ void *obj_buf = NULL;
+ size_t obj_buf_sz = 0;
+
+ ret = test_llvm__fetch_bpf_obj(&obj_buf, &obj_buf_sz,
+ i, false);
+
+ if (ret == TEST_OK) {
+ ret = test__bpf_parsing(obj_buf, obj_buf_sz);
+ if (ret != TEST_OK)
+ pr_debug("Failed to parse test case '%s'\n",
+ bpf_source_table[i].desc);
+ }
+ free(obj_buf);
+
+ switch (ret) {
+ case TEST_SKIP:
+ return TEST_SKIP;
+ case TEST_OK:
+ break;
+ default:
+ /*
+ * Test 0 is the basic LLVM test. If test 0
+ * fail, the basic LLVM support not functional
+ * so the whole test should fail. If other test
+ * case fail, it can be fixed by adjusting
+ * config so don't report error.
+ */
+ if (i == 0)
+ return TEST_FAIL;
+ else
+ return TEST_SKIP;
+ }
+ }
+ return TEST_OK;
}
diff --git a/tools/perf/tests/llvm.h b/tools/perf/tests/llvm.h
new file mode 100644
index 0000000..bd63cee
--- /dev/null
+++ b/tools/perf/tests/llvm.h
@@ -0,0 +1,16 @@
+#ifndef PERF_TEST_LLVM_H
+#define PERF_TEST_LLVM_H
+
+#include <stddef.h> /* for size_t */
+#include <stdbool.h> /* for bool */
+
+extern const char test_llvm__bpf_base_prog[];
+
+enum test_llvm__testcase {
+ LLVM_TESTCASE_BASE,
+ __LLVM_TESTCASE_MAX,
+};
+
+int test_llvm__fetch_bpf_obj(void **p_obj_buf, size_t *p_obj_buf_sz,
+ enum test_llvm__testcase index, bool force);
+#endif
--
1.8.3.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/5] perf test: Enforce LLVM test: add kbuild test
2015-11-05 4:26 [PATCH 0/5] perf tools: improve BPF related error delivering and testing Wang Nan
` (2 preceding siblings ...)
2015-11-05 4:27 ` [PATCH 3/5] perf test: Enforce LLVM test: update basic BPF test program Wang Nan
@ 2015-11-05 4:27 ` Wang Nan
2015-11-05 4:27 ` [PATCH 5/5] perf test: Add 'perf test BPF' Wang Nan
4 siblings, 0 replies; 10+ messages in thread
From: Wang Nan @ 2015-11-05 4:27 UTC (permalink / raw)
To: acme, namhyung
Cc: lizefan, pi3orama, linux-kernel, Wang Nan,
Arnaldo Carvalho de Melo
This patch adds a kbuild testcase to check whether kernel headers
can be correctly found.
For example:
# mv /lib/modules/4.3.0-rc5{,.bak}
# perf test LLVM
38: Test LLVM searching and compiling : Skip
# perf test -v LLVM
...
<stdin>:11:10: fatal error: 'uapi/linux/fs.h' file not found
#include <uapi/linux/fs.h>
^
1 error generated.
ERROR: unable to compile -
Hint: Check error message shown above.
Hint: You can also pre-compile it into .o using:
clang -target bpf -O2 -c -
with proper -I and -D options.
Failed to compile test case: 'Test kbuild searching'
test child finished with -2
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
tools/perf/tests/Build | 9 ++++++++-
tools/perf/tests/bpf-script-test-kbuild.c | 21 +++++++++++++++++++++
tools/perf/tests/llvm.c | 4 ++++
tools/perf/tests/llvm.h | 2 ++
4 files changed, 35 insertions(+), 1 deletion(-)
create mode 100644 tools/perf/tests/bpf-script-test-kbuild.c
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 6c095b3..a47b211 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -31,7 +31,7 @@ perf-y += sample-parsing.o
perf-y += parse-no-sample-id-all.o
perf-y += kmod-path.o
perf-y += thread-map.o
-perf-y += llvm.o llvm-src-base.o
+perf-y += llvm.o llvm-src-base.o llvm-src-kbuild.o
perf-y += topology.o
$(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c
@@ -41,6 +41,13 @@ $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c
$(Q)sed -e 's/"/\\"/g' -e 's/\(.*\)/"\1\\n"/g' $< >> $@
$(Q)echo ';' >> $@
+$(OUTPUT)tests/llvm-src-kbuild.c: tests/bpf-script-test-kbuild.c
+ $(call rule_mkdir)
+ $(Q)echo '#include <tests/llvm.h>' > $@
+ $(Q)echo 'const char test_llvm__bpf_test_kbuild_prog[] =' >> $@
+ $(Q)sed -e 's/"/\\"/g' -e 's/\(.*\)/"\1\\n"/g' $< >> $@
+ $(Q)echo ';' >> $@
+
ifeq ($(ARCH),$(filter $(ARCH),x86 arm arm64))
perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
endif
diff --git a/tools/perf/tests/bpf-script-test-kbuild.c b/tools/perf/tests/bpf-script-test-kbuild.c
new file mode 100644
index 0000000..3626924
--- /dev/null
+++ b/tools/perf/tests/bpf-script-test-kbuild.c
@@ -0,0 +1,21 @@
+/*
+ * bpf-script-test-kbuild.c
+ * Test include from kernel header
+ */
+#ifndef LINUX_VERSION_CODE
+# error Need LINUX_VERSION_CODE
+# error Example: for 4.2 kernel, put 'clang-opt="-DLINUX_VERSION_CODE=0x40200" into llvm section of ~/.perfconfig'
+#endif
+#define SEC(NAME) __attribute__((section(NAME), used))
+
+#include <uapi/linux/fs.h>
+#include <uapi/asm/ptrace.h>
+
+SEC("func=vfs_llseek")
+int bpf_func__vfs_llseek(void *ctx)
+{
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+int _version SEC("version") = LINUX_VERSION_CODE;
diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
index 05683c5..bc4cf50 100644
--- a/tools/perf/tests/llvm.c
+++ b/tools/perf/tests/llvm.c
@@ -40,6 +40,10 @@ static struct {
.source = test_llvm__bpf_base_prog,
.desc = "Basic BPF llvm compiling test",
},
+ [LLVM_TESTCASE_KBUILD] = {
+ .source = test_llvm__bpf_test_kbuild_prog,
+ .desc = "Test kbuild searching",
+ },
};
diff --git a/tools/perf/tests/llvm.h b/tools/perf/tests/llvm.h
index bd63cee..d91d8f4 100644
--- a/tools/perf/tests/llvm.h
+++ b/tools/perf/tests/llvm.h
@@ -5,9 +5,11 @@
#include <stdbool.h> /* for bool */
extern const char test_llvm__bpf_base_prog[];
+extern const char test_llvm__bpf_test_kbuild_prog[];
enum test_llvm__testcase {
LLVM_TESTCASE_BASE,
+ LLVM_TESTCASE_KBUILD,
__LLVM_TESTCASE_MAX,
};
--
1.8.3.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/5] perf test: Add 'perf test BPF'
2015-11-05 4:26 [PATCH 0/5] perf tools: improve BPF related error delivering and testing Wang Nan
` (3 preceding siblings ...)
2015-11-05 4:27 ` [PATCH 4/5] perf test: Enforce LLVM test: add kbuild test Wang Nan
@ 2015-11-05 4:27 ` Wang Nan
4 siblings, 0 replies; 10+ messages in thread
From: Wang Nan @ 2015-11-05 4:27 UTC (permalink / raw)
To: acme, namhyung
Cc: lizefan, pi3orama, linux-kernel, Wang Nan,
Arnaldo Carvalho de Melo, Alexei Starovoitov
This patch adds BPF testcase for testing BPF event filtering.
By utilizing the result of 'perf test LLVM', this patch compiles the
eBPF sample program then test it ability. The BPF script in 'perf test
LLVM' let only 50% samples generated by epoll_pwait() be captured.
This patch runs that system call for 111 times, so the resule should
contain 56 samples.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
---
tools/perf/tests/Build | 1 +
tools/perf/tests/bpf.c | 209 ++++++++++++++++++++++++++++++++++++++++
tools/perf/tests/builtin-test.c | 4 +
tools/perf/tests/tests.h | 1 +
tools/perf/util/bpf-loader.c | 24 ++++-
tools/perf/util/bpf-loader.h | 10 ++
6 files changed, 248 insertions(+), 1 deletion(-)
create mode 100644 tools/perf/tests/bpf.c
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index a47b211..f41ebf8 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -32,6 +32,7 @@ perf-y += parse-no-sample-id-all.o
perf-y += kmod-path.o
perf-y += thread-map.o
perf-y += llvm.o llvm-src-base.o llvm-src-kbuild.o
+perf-y += bpf.o
perf-y += topology.o
$(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c
diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
new file mode 100644
index 0000000..ec16f78
--- /dev/null
+++ b/tools/perf/tests/bpf.c
@@ -0,0 +1,209 @@
+#include <stdio.h>
+#include <sys/epoll.h>
+#include <util/bpf-loader.h>
+#include <util/evlist.h>
+#include "tests.h"
+#include "llvm.h"
+#include "debug.h"
+#define NR_ITERS 111
+
+#ifdef HAVE_LIBBPF_SUPPORT
+
+static int epoll_pwait_loop(void)
+{
+ int i;
+
+ /* Should fail NR_ITERS times */
+ for (i = 0; i < NR_ITERS; i++)
+ epoll_pwait(-(i + 1), NULL, 0, 0, NULL);
+ return 0;
+}
+
+static struct {
+ enum test_llvm__testcase prog_id;
+ const char *desc;
+ const char *name;
+ const char *msg_compile_fail;
+ const char *msg_load_fail;
+ int (*target_func)(void);
+ int expect_result;
+} bpf_testcase_table[] = {
+ {
+ LLVM_TESTCASE_BASE,
+ "Test basic BPF filtering",
+ "[basic_bpf_test]",
+ "fix 'perf test LLVM' first",
+ "load bpf object failed",
+ &epoll_pwait_loop,
+ (NR_ITERS + 1) / 2,
+ },
+};
+
+static int do_test(struct bpf_object *obj, int (*func)(void),
+ int expect)
+{
+ struct record_opts opts = {
+ .target = {
+ .uid = UINT_MAX,
+ .uses_mmap = true,
+ },
+ .freq = 0,
+ .mmap_pages = 256,
+ .default_interval = 1,
+ };
+
+ char pid[16];
+ char sbuf[STRERR_BUFSIZE];
+ struct perf_evlist *evlist;
+ int i, ret = TEST_FAIL, err = 0, count = 0;
+
+ struct parse_events_evlist parse_evlist;
+ struct parse_events_error parse_error;
+
+ bzero(&parse_error, sizeof(parse_error));
+ bzero(&parse_evlist, sizeof(parse_evlist));
+ parse_evlist.error = &parse_error;
+ INIT_LIST_HEAD(&parse_evlist.list);
+
+ err = parse_events_load_bpf_obj(&parse_evlist, &parse_evlist.list, obj);
+ if (err || list_empty(&parse_evlist.list)) {
+ pr_debug("Failed to add events selected by BPF\n");
+ if (!err)
+ return TEST_FAIL;
+ }
+
+ snprintf(pid, sizeof(pid), "%d", getpid());
+ pid[sizeof(pid) - 1] = '\0';
+ opts.target.tid = opts.target.pid = pid;
+
+ /* Instead of perf_evlist__new_default, don't add default events */
+ evlist = perf_evlist__new();
+ if (!evlist) {
+ pr_debug("No ehough memory to create evlist\n");
+ return TEST_FAIL;
+ }
+
+ err = perf_evlist__create_maps(evlist, &opts.target);
+ if (err < 0) {
+ pr_debug("Not enough memory to create thread/cpu maps\n");
+ goto out_delete_evlist;
+ }
+
+ perf_evlist__splice_list_tail(evlist, &parse_evlist.list);
+ evlist->nr_groups = parse_evlist.nr_groups;
+
+ perf_evlist__config(evlist, &opts);
+
+ err = perf_evlist__open(evlist);
+ if (err < 0) {
+ pr_debug("perf_evlist__open: %s\n",
+ strerror_r(errno, sbuf, sizeof(sbuf)));
+ goto out_delete_evlist;
+ }
+
+ err = perf_evlist__mmap(evlist, opts.mmap_pages, false);
+ if (err < 0) {
+ pr_debug("perf_evlist__mmap: %s\n",
+ strerror_r(errno, sbuf, sizeof(sbuf)));
+ goto out_delete_evlist;
+ }
+
+ perf_evlist__enable(evlist);
+ (*func)();
+ perf_evlist__disable(evlist);
+
+ for (i = 0; i < evlist->nr_mmaps; i++) {
+ union perf_event *event;
+
+ while ((event = perf_evlist__mmap_read(evlist, i)) != NULL) {
+ const u32 type = event->header.type;
+
+ if (type == PERF_RECORD_SAMPLE)
+ count ++;
+ }
+ }
+
+ if (count != expect)
+ pr_debug("BPF filter result incorrect\n");
+
+ ret = TEST_OK;
+
+out_delete_evlist:
+ perf_evlist__delete(evlist);
+ return ret;
+}
+
+static struct bpf_object *
+prepare_bpf(void *obj_buf, size_t obj_buf_sz, const char *name)
+{
+ struct bpf_object *obj;
+
+ obj = bpf__prepare_load_buffer(obj_buf, obj_buf_sz, name);
+ if (IS_ERR(obj)) {
+ pr_debug("Compile BPF program failed.\n");
+ return NULL;
+ }
+ return obj;
+}
+
+static int __test__bpf(int index)
+{
+ int ret;
+ void *obj_buf;
+ size_t obj_buf_sz;
+ struct bpf_object *obj;
+
+ ret = test_llvm__fetch_bpf_obj(&obj_buf, &obj_buf_sz,
+ bpf_testcase_table[index].prog_id,
+ true);
+ if (ret != TEST_OK || !obj_buf || !obj_buf_sz) {
+ pr_debug("Unable to get BPF object, %s\n",
+ bpf_testcase_table[index].msg_compile_fail);
+ if (index == 0)
+ return TEST_SKIP;
+ else
+ return TEST_FAIL;
+ }
+
+ obj = prepare_bpf(obj_buf, obj_buf_sz,
+ bpf_testcase_table[index].name);
+ if (!obj) {
+ ret = TEST_FAIL;
+ goto out;
+ }
+
+ ret = do_test(obj,
+ bpf_testcase_table[index].target_func,
+ bpf_testcase_table[index].expect_result);
+out:
+ bpf__clear();
+ return ret;
+}
+
+int test__bpf(void)
+{
+ unsigned int i;
+ int err;
+
+ if (geteuid() != 0) {
+ pr_debug("Only root can run BPF test\n");
+ return TEST_SKIP;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(bpf_testcase_table); i++) {
+ err = __test__bpf(i);
+
+ if (err != TEST_OK)
+ return err;
+ }
+
+ return TEST_OK;
+}
+
+#else
+int test__bpf(void)
+{
+ pr_debug("Skip BPF test because BPF support is not compiled\n");
+ return TEST_SKIP;
+}
+#endif
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 66f72d3..7b0120a 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -166,6 +166,10 @@ static struct test generic_tests[] = {
.func = test_session_topology,
},
{
+ .desc = "Test BPF filter",
+ .func = test__bpf,
+ },
+ {
.func = NULL,
},
};
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index c804869..3c8734a 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -66,6 +66,7 @@ int test__fdarray__add(void);
int test__kmod_path__parse(void);
int test__thread_map(void);
int test__llvm(void);
+int test__bpf(void);
int test_session_topology(void);
#if defined(__arm__) || defined(__aarch64__)
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index 4b9bb2a..85dee69 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -38,10 +38,32 @@ struct bpf_prog_priv {
struct perf_probe_event pev;
};
+static bool libbpf_initialized;
+
+struct bpf_object *
+bpf__prepare_load_buffer(void *obj_buf, size_t obj_buf_sz, const char *name)
+{
+ struct bpf_object *obj;
+
+ if (!libbpf_initialized) {
+ libbpf_set_print(libbpf_warning,
+ libbpf_info,
+ libbpf_debug);
+ libbpf_initialized = true;
+ }
+
+ obj = bpf_object__open_buffer(obj_buf, obj_buf_sz, name);
+ if (IS_ERR(obj)) {
+ pr_debug("bpf: failed to load buffer\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ return obj;
+}
+
struct bpf_object *bpf__prepare_load(const char *filename, bool source)
{
struct bpf_object *obj;
- static bool libbpf_initialized;
if (!libbpf_initialized) {
libbpf_set_print(libbpf_warning,
diff --git a/tools/perf/util/bpf-loader.h b/tools/perf/util/bpf-loader.h
index 490c78c..c908365 100644
--- a/tools/perf/util/bpf-loader.h
+++ b/tools/perf/util/bpf-loader.h
@@ -30,6 +30,9 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source);
int bpf__strerror_prepare_load(const char *filename, bool source,
int err, char *buf, size_t size);
+struct bpf_object *bpf__prepare_load_buffer(void *obj_buf, size_t obj_buf_sz,
+ const char *name);
+
void bpf__clear(void);
int bpf__probe(struct bpf_object *obj);
@@ -51,6 +54,13 @@ bpf__prepare_load(const char *filename __maybe_unused,
return ERR_PTR(-ENOTSUP);
}
+static inline struct bpf_object *
+bpf__prepare_load_buffer(void *obj_buf __maybe_unused,
+ size_t obj_buf_sz __maybe_unused)
+{
+ return ERR_PTR(-ENOTSUP);
+}
+
static inline void bpf__clear(void) { }
static inline int bpf__probe(struct bpf_object *obj __maybe_unused) { return 0;}
--
1.8.3.4
^ permalink raw reply related [flat|nested] 10+ messages in thread