This patchset is based on tip/core.
Arnaldo found some confusing error reports when reviewing BPF related
patches. This patch set makes some changes:
- perf test only print ok, failed or skip. Extra messages can be seen
by '-v'.
- libbpf not output anything when called from perf without '-v'.
- Create libbpf and bpf-loader specific error number group to deliver
precise error. New strerror framework makes adding new error code
easier.
- Improve error messages.
After applying this patch:
When target object not exist:
$ ls ./foo.o
ls: cannot access ./foo.o: No such file or directory
$ ./perf record --event foo.o sleep
event syntax error: 'foo.o'
\___ Failed to load foo.o: No such file or directory
(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
When target object format invalid:
$ cp /etc/passwd ./badbpf.o
$ ./perf record --event ./badbpf.o sleep
event syntax error: './badbpf.o'
\___ Failed to load ./badbpf.o: BPF object format invalid
(... skip ...)
When run by normal user and /proc/sys/kernel/kptr_restrict is 1:
$ cat /proc/sys/kernel/kptr_restrict
1
$ ./perf record --event ./bpf-script-example.o sleep
Failed to init vmlinux path.
event syntax error: './bpf-script-example.o'
\___ You need to be root, and /proc/sys/kernel/kptr_restrict should be 0
(... skip ...)
After fixing /proc/sys/kernel/kptr_restrict:
$ sudo -s
# echo 0 > /proc/sys/kernel/kptr_restrict
# exit
$ ./perf record --event ./bpf-script-example.o sleep
Failed to open kprobe_events: Permission denied
event syntax error: './bpf-script-example.o'
\___ You need to be root
(... skip ...)
Load an object with no 'version' section:
# ./perf record --event ./bpf-script-example.o sleep
event syntax error: './bpf-script-example.o'
\___ Failed to load ./bpf-script-example.o: 'version' section incorrect or lost
(... skip ...)
Load an object with incorrect 'version' section:
# ./perf record --event ./bpf-script-example.o sleep
event syntax error: './bpf-script-example.o'
\___ Failed to load program: Validate your program and check 'license'/'version' sections in your object
(... skip ...)
When event name not set:
# ./perf record --event ./bpf-script-example.o sleep
event syntax error: './bpf-script-example.o'
\___ No event name found in config string
(... skip ...)
Wang Nan (5):
perf test: Keep test result clean if '-v' not set
perf tools: Mute libbpf when '-v' not set
perf tools: Parsing libbpf return value using err.h
bpf tools: Improve libbpf error reporting
perf tools: Improve BPF related error messages output
tools/lib/bpf/libbpf.c | 149 +++++++++++++++++++++++++------------
tools/lib/bpf/libbpf.h | 12 +++
tools/perf/tests/attr.c | 3 +-
tools/perf/tests/code-reading.c | 8 +-
tools/perf/tests/keep-tracking.c | 4 +-
tools/perf/tests/llvm.c | 13 ++--
tools/perf/tests/switch-tracking.c | 4 +-
tools/perf/util/bpf-loader.c | 110 +++++++++++++++++++++++----
tools/perf/util/bpf-loader.h | 18 +++++
tools/perf/util/parse-events.c | 7 +-
10 files changed, 243 insertions(+), 85 deletions(-)
--
1.8.3.4
According to [1], 'perf state' should avoid output too much information
if '-v' not set, only 'Ok', 'FAIL' or 'Skip' need to be printed.
This patch removes sereval stderr output to make output clean.
Before this patch:
# perf test dummy
23: Test using a dummy software event to keep tracking : (not supported) Ok
After this patch:
# perf test dummy
23: Test using a dummy software event to keep tracking : Skip
[1] http://lkml.kernel.org/r/[email protected]
Signed-off-by: Wang Nan <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Namhyung Kim <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
---
tools/perf/tests/attr.c | 3 +--
tools/perf/tests/code-reading.c | 8 ++++----
tools/perf/tests/keep-tracking.c | 4 ++--
tools/perf/tests/llvm.c | 11 ++++-------
tools/perf/tests/switch-tracking.c | 4 ++--
5 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/tools/perf/tests/attr.c b/tools/perf/tests/attr.c
index 2dfc9ad..638875a 100644
--- a/tools/perf/tests/attr.c
+++ b/tools/perf/tests/attr.c
@@ -171,6 +171,5 @@ int test__attr(void)
!lstat(path_perf, &st))
return run_dir(path_dir, path_perf);
- fprintf(stderr, " (omitted)");
- return 0;
+ return TEST_SKIP;
}
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 49b1959..a767a64 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -613,16 +613,16 @@ int test__code_reading(void)
case TEST_CODE_READING_OK:
return 0;
case TEST_CODE_READING_NO_VMLINUX:
- fprintf(stderr, " (no vmlinux)");
+ pr_debug("no vmlinux\n");
return 0;
case TEST_CODE_READING_NO_KCORE:
- fprintf(stderr, " (no kcore)");
+ pr_debug("no kcore\n");
return 0;
case TEST_CODE_READING_NO_ACCESS:
- fprintf(stderr, " (no access)");
+ pr_debug("no access\n");
return 0;
case TEST_CODE_READING_NO_KERNEL_OBJ:
- fprintf(stderr, " (no kernel obj)");
+ pr_debug("no kernel obj\n");
return 0;
default:
return -1;
diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
index 4d4b983..a2e2269 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -90,8 +90,8 @@ int test__keep_tracking(void)
evsel->attr.enable_on_exec = 0;
if (perf_evlist__open(evlist) < 0) {
- fprintf(stderr, " (not supported)");
- err = 0;
+ pr_debug("Unable to open dummy and cycles event\n");
+ err = TEST_SKIP;
goto out_err;
}
diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
index 52d5597..512d362 100644
--- a/tools/perf/tests/llvm.c
+++ b/tools/perf/tests/llvm.c
@@ -36,7 +36,7 @@ static int test__bpf_parsing(void *obj_buf, size_t obj_buf_sz)
static int test__bpf_parsing(void *obj_buf __maybe_unused,
size_t obj_buf_sz __maybe_unused)
{
- fprintf(stderr, " (skip bpf parsing)");
+ pr_debug("Skip bpf parsing\n");
return 0;
}
#endif
@@ -55,7 +55,7 @@ int test__llvm(void)
* 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()) {
- fprintf(stderr, " (no clang, try 'perf test -v LLVM')");
+ pr_debug("No clang and no verbosive, skip this test\n");
return TEST_SKIP;
}
@@ -86,11 +86,8 @@ int test__llvm(void)
err = llvm__compile_bpf("-", &obj_buf, &obj_buf_sz);
verbose = old_verbose;
- if (err) {
- if (!verbose)
- fprintf(stderr, " (use -v to see error message)");
- return -1;
- }
+ if (err)
+ return TEST_FAIL;
err = test__bpf_parsing(obj_buf, obj_buf_sz);
free(obj_buf);
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index e698742..a02af50 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -366,7 +366,7 @@ int test__switch_tracking(void)
/* Third event */
if (!perf_evlist__can_select_event(evlist, sched_switch)) {
- fprintf(stderr, " (no sched_switch)");
+ pr_debug("No sched_switch\n");
err = 0;
goto out;
}
@@ -442,7 +442,7 @@ int test__switch_tracking(void)
}
if (perf_evlist__open(evlist) < 0) {
- fprintf(stderr, " (not supported)");
+ pr_debug("Not supported\n");
err = 0;
goto out;
}
--
1.8.3.4
According to [1], libbpf should be muted. This patch reset info and
warning message level to ensure libbpf doesn't output anything even
if error happened.
Signed-off-by: Wang Nan <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/bpf-loader.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index ba6f752..0c5d174 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -26,8 +26,8 @@ static int libbpf_##name(const char *fmt, ...) \
return ret; \
}
-DEFINE_PRINT_FN(warning, 0)
-DEFINE_PRINT_FN(info, 0)
+DEFINE_PRINT_FN(warning, 1)
+DEFINE_PRINT_FN(info, 1)
DEFINE_PRINT_FN(debug, 1)
struct bpf_prog_priv {
--
1.8.3.4
In following patches libbpf would encode error code using return
pointer instead of returnning a NULL pointer to indicate error. This
patch makes a preperation to enable perf correctly receive error code
from libbpf.
Signed-off-by: Wang Nan <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/tests/llvm.c | 2 +-
tools/perf/util/bpf-loader.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
index 512d362..38e0d9a 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) || !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..dd6fa27 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) || !obj) {
pr_debug("bpf: failed to load %s\n", filename);
- return ERR_PTR(-EINVAL);
+ return !obj ? ERR_PTR(-EINVAL) : 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) || !config_str) {
pr_debug("bpf: unable to get title for program\n");
- return -EINVAL;
+ return !config_str ? -EINVAL : PTR_ERR(config_str);
}
priv = calloc(sizeof(*priv), 1);
--
1.8.3.4
In this patch, a series libbpf specific error numbers and
libbpf_strerror() are created to help report error to caller. Functions
are updated to pass correct error number through macro CHECK_ERR().
Signed-off-by: Wang Nan <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/bpf/libbpf.c | 149 +++++++++++++++++++++++++++++++++----------------
tools/lib/bpf/libbpf.h | 12 ++++
2 files changed, 113 insertions(+), 48 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 4252fc2..74c64b1 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 dup)
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 f16170c..c2606ae 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
--
1.8.3.4
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() are introduced 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. bpf__strerror_head() is updated so existing
strerror functions can support these error code automatically.
Signed-off-by: Wang Nan <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/bpf-loader.c | 98 +++++++++++++++++++++++++++++++++++++-----
tools/perf/util/bpf-loader.h | 18 ++++++++
tools/perf/util/parse-events.c | 7 +--
3 files changed, 110 insertions(+), 13 deletions(-)
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index dd6fa27..1e406f4 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,13 +312,65 @@ 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)
+{
+ unsigned int i;
+
+ if (!buf || !size)
+ return -1;
+
+ err = err > 0 ? err : -err;
+
+ if (err > LIBBPF_ERRNO__START)
+ return libbpf_strerror(err, buf, size);
+
+ if (err < BPF_LOADER_ERRNO__START) {
+ char sbuf[STRERR_BUFSIZE], *msg;
+
+ msg = strerror_r(err, sbuf, sizeof(sbuf));
+ snprintf(buf, size, "%s", msg);
+ buf[size - 1] = '\0';
+ return 0;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(bpf_loader_strerror_table); i++) {
+ if (bpf_loader_strerror_table[i].code == err) {
+ const char *msg;
+
+ 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;
+}
+
#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);\
@@ -330,13 +386,34 @@ int bpf__foreach_tev(struct bpf_object *obj,
}\
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;
}
@@ -345,7 +422,8 @@ 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?",
+ 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/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 bee6058..6b07027 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
On 2015/11/3 18:44, Wang Nan wrote:
> According to [1], libbpf should be muted. This patch reset info and
> warning message level to ensure libbpf doesn't output anything even
> if error happened.
[1] http://lkml.kernel.org/r/[email protected]
Sorry...
> Signed-off-by: Wang Nan <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> ---
> tools/perf/util/bpf-loader.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
> index ba6f752..0c5d174 100644
> --- a/tools/perf/util/bpf-loader.c
> +++ b/tools/perf/util/bpf-loader.c
> @@ -26,8 +26,8 @@ static int libbpf_##name(const char *fmt, ...) \
> return ret; \
> }
>
> -DEFINE_PRINT_FN(warning, 0)
> -DEFINE_PRINT_FN(info, 0)
> +DEFINE_PRINT_FN(warning, 1)
> +DEFINE_PRINT_FN(info, 1)
> DEFINE_PRINT_FN(debug, 1)
>
> struct bpf_prog_priv {
On Tue, Nov 03, 2015 at 10:44:42AM +0000, Wang Nan wrote:
> According to [1], 'perf state' should avoid output too much information
> if '-v' not set, only 'Ok', 'FAIL' or 'Skip' need to be printed.
>
> This patch removes sereval stderr output to make output clean.
>
> Before this patch:
> # perf test dummy
> 23: Test using a dummy software event to keep tracking : (not supported) Ok
>
> After this patch:
> # perf test dummy
> 23: Test using a dummy software event to keep tracking : Skip
>
> [1] http://lkml.kernel.org/r/[email protected]
>
> Signed-off-by: Wang Nan <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> Link: http://lkml.kernel.org/n/[email protected]
Acked-by: Namhyung Kim <[email protected]>
Thanks,
Namhyung
> ---
> tools/perf/tests/attr.c | 3 +--
> tools/perf/tests/code-reading.c | 8 ++++----
> tools/perf/tests/keep-tracking.c | 4 ++--
> tools/perf/tests/llvm.c | 11 ++++-------
> tools/perf/tests/switch-tracking.c | 4 ++--
> 5 files changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/tools/perf/tests/attr.c b/tools/perf/tests/attr.c
> index 2dfc9ad..638875a 100644
> --- a/tools/perf/tests/attr.c
> +++ b/tools/perf/tests/attr.c
> @@ -171,6 +171,5 @@ int test__attr(void)
> !lstat(path_perf, &st))
> return run_dir(path_dir, path_perf);
>
> - fprintf(stderr, " (omitted)");
> - return 0;
> + return TEST_SKIP;
> }
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index 49b1959..a767a64 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -613,16 +613,16 @@ int test__code_reading(void)
> case TEST_CODE_READING_OK:
> return 0;
> case TEST_CODE_READING_NO_VMLINUX:
> - fprintf(stderr, " (no vmlinux)");
> + pr_debug("no vmlinux\n");
> return 0;
> case TEST_CODE_READING_NO_KCORE:
> - fprintf(stderr, " (no kcore)");
> + pr_debug("no kcore\n");
> return 0;
> case TEST_CODE_READING_NO_ACCESS:
> - fprintf(stderr, " (no access)");
> + pr_debug("no access\n");
> return 0;
> case TEST_CODE_READING_NO_KERNEL_OBJ:
> - fprintf(stderr, " (no kernel obj)");
> + pr_debug("no kernel obj\n");
> return 0;
> default:
> return -1;
> diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
> index 4d4b983..a2e2269 100644
> --- a/tools/perf/tests/keep-tracking.c
> +++ b/tools/perf/tests/keep-tracking.c
> @@ -90,8 +90,8 @@ int test__keep_tracking(void)
> evsel->attr.enable_on_exec = 0;
>
> if (perf_evlist__open(evlist) < 0) {
> - fprintf(stderr, " (not supported)");
> - err = 0;
> + pr_debug("Unable to open dummy and cycles event\n");
> + err = TEST_SKIP;
> goto out_err;
> }
>
> diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
> index 52d5597..512d362 100644
> --- a/tools/perf/tests/llvm.c
> +++ b/tools/perf/tests/llvm.c
> @@ -36,7 +36,7 @@ static int test__bpf_parsing(void *obj_buf, size_t obj_buf_sz)
> static int test__bpf_parsing(void *obj_buf __maybe_unused,
> size_t obj_buf_sz __maybe_unused)
> {
> - fprintf(stderr, " (skip bpf parsing)");
> + pr_debug("Skip bpf parsing\n");
> return 0;
> }
> #endif
> @@ -55,7 +55,7 @@ int test__llvm(void)
> * 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()) {
> - fprintf(stderr, " (no clang, try 'perf test -v LLVM')");
> + pr_debug("No clang and no verbosive, skip this test\n");
> return TEST_SKIP;
> }
>
> @@ -86,11 +86,8 @@ int test__llvm(void)
> err = llvm__compile_bpf("-", &obj_buf, &obj_buf_sz);
>
> verbose = old_verbose;
> - if (err) {
> - if (!verbose)
> - fprintf(stderr, " (use -v to see error message)");
> - return -1;
> - }
> + if (err)
> + return TEST_FAIL;
>
> err = test__bpf_parsing(obj_buf, obj_buf_sz);
> free(obj_buf);
> diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
> index e698742..a02af50 100644
> --- a/tools/perf/tests/switch-tracking.c
> +++ b/tools/perf/tests/switch-tracking.c
> @@ -366,7 +366,7 @@ int test__switch_tracking(void)
>
> /* Third event */
> if (!perf_evlist__can_select_event(evlist, sched_switch)) {
> - fprintf(stderr, " (no sched_switch)");
> + pr_debug("No sched_switch\n");
> err = 0;
> goto out;
> }
> @@ -442,7 +442,7 @@ int test__switch_tracking(void)
> }
>
> if (perf_evlist__open(evlist) < 0) {
> - fprintf(stderr, " (not supported)");
> + pr_debug("Not supported\n");
> err = 0;
> goto out;
> }
> --
> 1.8.3.4
>
Em Tue, Nov 03, 2015 at 10:44:44AM +0000, Wang Nan escreveu:
> In following patches libbpf would encode error code using return
> pointer instead of returnning a NULL pointer to indicate error. This
> patch makes a preperation to enable perf correctly receive error code
> from libbpf.
>
> Signed-off-by: Wang Nan <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> ---
> tools/perf/tests/llvm.c | 2 +-
> tools/perf/util/bpf-loader.c | 8 ++++----
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
> index 512d362..38e0d9a 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) || !obj)
Well, since we've adopted IS_ERR() from the kernel, we better try to
follow how it is used there, no?
Since you move to use ERR_PTR(), you probably will never return NULL,
right? So whay the (|| !obj) part?
The kernel has an IS_ERR_OR_NULL() interface tho, trying to figure out
when that would be appropriate...
- Arnaldo
> 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..dd6fa27 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) || !obj) {
> pr_debug("bpf: failed to load %s\n", filename);
> - return ERR_PTR(-EINVAL);
> + return !obj ? ERR_PTR(-EINVAL) : 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) || !config_str) {
> pr_debug("bpf: unable to get title for program\n");
> - return -EINVAL;
> + return !config_str ? -EINVAL : PTR_ERR(config_str);
> }
>
> priv = calloc(sizeof(*priv), 1);
> --
> 1.8.3.4
Em Tue, Nov 03, 2015 at 12:17:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Nov 03, 2015 at 10:44:44AM +0000, Wang Nan escreveu:
> > +++ 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) || !obj)
> Well, since we've adopted IS_ERR() from the kernel, we better try to
> follow how it is used there, no?
> Since you move to use ERR_PTR(), you probably will never return NULL,
> right? So whay the (|| !obj) part?
> The kernel has an IS_ERR_OR_NULL() interface tho, trying to figure out
> when that would be appropriate...
I think you should change the error reporting convention _and_ the users
at the same time, i.e. please merge this patch with the next and use
plain:
if (IS_ERR(obj))
Instead, ok?
- Arnaldo
Em Tue, Nov 03, 2015 at 10:44:41AM +0000, Wang Nan escreveu:
> This patchset is based on tip/core.
>
> Arnaldo found some confusing error reports when reviewing BPF related
> patches. This patch set makes some changes:
Thanks for working on this!
I applied the first two and made comments about the conversion to using
err.h, please check,
- Arnaldo
> - perf test only print ok, failed or skip. Extra messages can be seen
> by '-v'.
>
> - libbpf not output anything when called from perf without '-v'.
>
> - Create libbpf and bpf-loader specific error number group to deliver
> precise error. New strerror framework makes adding new error code
> easier.
>
> - Improve error messages.
>
> After applying this patch:
>
> When target object not exist:
>
> $ ls ./foo.o
> ls: cannot access ./foo.o: No such file or directory
> $ ./perf record --event foo.o sleep
> event syntax error: 'foo.o'
> \___ Failed to load foo.o: No such file or directory
>
> (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
>
> When target object format invalid:
>
> $ cp /etc/passwd ./badbpf.o
> $ ./perf record --event ./badbpf.o sleep
> event syntax error: './badbpf.o'
> \___ Failed to load ./badbpf.o: BPF object format invalid
> (... skip ...)
>
> When run by normal user and /proc/sys/kernel/kptr_restrict is 1:
>
> $ cat /proc/sys/kernel/kptr_restrict
> 1
> $ ./perf record --event ./bpf-script-example.o sleep
> Failed to init vmlinux path.
> event syntax error: './bpf-script-example.o'
> \___ You need to be root, and /proc/sys/kernel/kptr_restrict should be 0
> (... skip ...)
>
> After fixing /proc/sys/kernel/kptr_restrict:
>
> $ sudo -s
> # echo 0 > /proc/sys/kernel/kptr_restrict
> # exit
> $ ./perf record --event ./bpf-script-example.o sleep
> Failed to open kprobe_events: Permission denied
> event syntax error: './bpf-script-example.o'
> \___ You need to be root
> (... skip ...)
>
> Load an object with no 'version' section:
>
> # ./perf record --event ./bpf-script-example.o sleep
> event syntax error: './bpf-script-example.o'
> \___ Failed to load ./bpf-script-example.o: 'version' section incorrect or lost
> (... skip ...)
>
> Load an object with incorrect 'version' section:
> # ./perf record --event ./bpf-script-example.o sleep
> event syntax error: './bpf-script-example.o'
> \___ Failed to load program: Validate your program and check 'license'/'version' sections in your object
> (... skip ...)
>
> When event name not set:
>
> # ./perf record --event ./bpf-script-example.o sleep
> event syntax error: './bpf-script-example.o'
> \___ No event name found in config string
> (... skip ...)
>
> Wang Nan (5):
> perf test: Keep test result clean if '-v' not set
> perf tools: Mute libbpf when '-v' not set
> perf tools: Parsing libbpf return value using err.h
> bpf tools: Improve libbpf error reporting
> perf tools: Improve BPF related error messages output
>
> tools/lib/bpf/libbpf.c | 149 +++++++++++++++++++++++++------------
> tools/lib/bpf/libbpf.h | 12 +++
> tools/perf/tests/attr.c | 3 +-
> tools/perf/tests/code-reading.c | 8 +-
> tools/perf/tests/keep-tracking.c | 4 +-
> tools/perf/tests/llvm.c | 13 ++--
> tools/perf/tests/switch-tracking.c | 4 +-
> tools/perf/util/bpf-loader.c | 110 +++++++++++++++++++++++----
> tools/perf/util/bpf-loader.h | 18 +++++
> tools/perf/util/parse-events.c | 7 +-
> 10 files changed, 243 insertions(+), 85 deletions(-)
>
> --
> 1.8.3.4
On 2015/11/3 23:43, Arnaldo Carvalho de Melo wrote:
> Em Tue, Nov 03, 2015 at 10:44:41AM +0000, Wang Nan escreveu:
>> This patchset is based on tip/core.
>>
>> Arnaldo found some confusing error reports when reviewing BPF related
>> patches. This patch set makes some changes:
> Thanks for working on this!
>
> I applied the first two and made comments about the conversion to using
> err.h, please check,
>
> - Arnaldo
>
I can't find them in your git repository at git.kernel.org. Could you
please recheck?
Thank you.
Commit-ID: 597bdeb4ab7396c43935eded15f82e3e100b3ff3
Gitweb: http://git.kernel.org/tip/597bdeb4ab7396c43935eded15f82e3e100b3ff3
Author: Wang Nan <[email protected]>
AuthorDate: Tue, 3 Nov 2015 10:44:42 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 3 Nov 2015 11:45:40 -0300
perf test: Keep test result clean if '-v' not set
According to [1], 'perf test' should avoid output too much information
if '-v' is not set, only 'Ok', 'FAIL' or 'Skip' need to be printed.
This patch removes several messages sent directly to stderr to make
the output clean.
Before this patch:
# perf test dummy
23: Test using a dummy software event to keep tracking : (not supported) Ok
After this patch:
# perf test dummy
23: Test using a dummy software event to keep tracking : Skip
[1] http://lkml.kernel.org/r/[email protected]
Signed-off-by: Wang Nan <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Zefan Li <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/tests/attr.c | 3 +--
tools/perf/tests/code-reading.c | 8 ++++----
tools/perf/tests/keep-tracking.c | 4 ++--
tools/perf/tests/llvm.c | 11 ++++-------
tools/perf/tests/switch-tracking.c | 4 ++--
5 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/tools/perf/tests/attr.c b/tools/perf/tests/attr.c
index 2dfc9ad..638875a 100644
--- a/tools/perf/tests/attr.c
+++ b/tools/perf/tests/attr.c
@@ -171,6 +171,5 @@ int test__attr(void)
!lstat(path_perf, &st))
return run_dir(path_dir, path_perf);
- fprintf(stderr, " (omitted)");
- return 0;
+ return TEST_SKIP;
}
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 49b1959..a767a64 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -613,16 +613,16 @@ int test__code_reading(void)
case TEST_CODE_READING_OK:
return 0;
case TEST_CODE_READING_NO_VMLINUX:
- fprintf(stderr, " (no vmlinux)");
+ pr_debug("no vmlinux\n");
return 0;
case TEST_CODE_READING_NO_KCORE:
- fprintf(stderr, " (no kcore)");
+ pr_debug("no kcore\n");
return 0;
case TEST_CODE_READING_NO_ACCESS:
- fprintf(stderr, " (no access)");
+ pr_debug("no access\n");
return 0;
case TEST_CODE_READING_NO_KERNEL_OBJ:
- fprintf(stderr, " (no kernel obj)");
+ pr_debug("no kernel obj\n");
return 0;
default:
return -1;
diff --git a/tools/perf/tests/keep-tracking.c b/tools/perf/tests/keep-tracking.c
index 4d4b983..a2e2269 100644
--- a/tools/perf/tests/keep-tracking.c
+++ b/tools/perf/tests/keep-tracking.c
@@ -90,8 +90,8 @@ int test__keep_tracking(void)
evsel->attr.enable_on_exec = 0;
if (perf_evlist__open(evlist) < 0) {
- fprintf(stderr, " (not supported)");
- err = 0;
+ pr_debug("Unable to open dummy and cycles event\n");
+ err = TEST_SKIP;
goto out_err;
}
diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c
index 52d5597..512d362 100644
--- a/tools/perf/tests/llvm.c
+++ b/tools/perf/tests/llvm.c
@@ -36,7 +36,7 @@ static int test__bpf_parsing(void *obj_buf, size_t obj_buf_sz)
static int test__bpf_parsing(void *obj_buf __maybe_unused,
size_t obj_buf_sz __maybe_unused)
{
- fprintf(stderr, " (skip bpf parsing)");
+ pr_debug("Skip bpf parsing\n");
return 0;
}
#endif
@@ -55,7 +55,7 @@ int test__llvm(void)
* 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()) {
- fprintf(stderr, " (no clang, try 'perf test -v LLVM')");
+ pr_debug("No clang and no verbosive, skip this test\n");
return TEST_SKIP;
}
@@ -86,11 +86,8 @@ int test__llvm(void)
err = llvm__compile_bpf("-", &obj_buf, &obj_buf_sz);
verbose = old_verbose;
- if (err) {
- if (!verbose)
- fprintf(stderr, " (use -v to see error message)");
- return -1;
- }
+ if (err)
+ return TEST_FAIL;
err = test__bpf_parsing(obj_buf, obj_buf_sz);
free(obj_buf);
diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index e698742..a02af50 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -366,7 +366,7 @@ int test__switch_tracking(void)
/* Third event */
if (!perf_evlist__can_select_event(evlist, sched_switch)) {
- fprintf(stderr, " (no sched_switch)");
+ pr_debug("No sched_switch\n");
err = 0;
goto out;
}
@@ -442,7 +442,7 @@ int test__switch_tracking(void)
}
if (perf_evlist__open(evlist) < 0) {
- fprintf(stderr, " (not supported)");
+ pr_debug("Not supported\n");
err = 0;
goto out;
}
Commit-ID: 7a0119468c9c2deff24ef24e1b4d2c1bd1523fd5
Gitweb: http://git.kernel.org/tip/7a0119468c9c2deff24ef24e1b4d2c1bd1523fd5
Author: Wang Nan <[email protected]>
AuthorDate: Tue, 3 Nov 2015 10:44:43 +0000
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 3 Nov 2015 12:06:04 -0300
perf bpf: Mute libbpf when '-v' not set
According to [1], libbpf should be muted. This patch reset info and
warning message level to ensure libbpf doesn't output anything even
if error happened.
[1] http://lkml.kernel.org/r/[email protected]
Committer note:
Before:
Testing it with an incompatible kernel version in the .c file that
generated foo.o:
[root@zoo ~]# perf record -e /tmp/foo.o sleep 1
libbpf: load bpf program failed: Invalid argument
libbpf: -- BEGIN DUMP LOG ---
libbpf:
libbpf: -- END LOG --
libbpf: failed to load program 'fork=_do_fork'
libbpf: failed to load object '/tmp/foo.o'
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
[root@zoo ~]#
After:
[root@zoo ~]# perf 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
[root@zoo ~]#
This, BTW, need fixing to emit a proper message by validating the
version in the foo.o "version" ELF section against the running kernel,
warning the user instead of asking the kernel to load a binary that it
will refuse due to unmatching kernel version.
Signed-off-by: Wang Nan <[email protected]>
Tested-by: Arnaldo Carvalho de Melo <[email protected]>
Cc: Zefan Li <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/bpf-loader.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c
index ba6f752..0c5d174 100644
--- a/tools/perf/util/bpf-loader.c
+++ b/tools/perf/util/bpf-loader.c
@@ -26,8 +26,8 @@ static int libbpf_##name(const char *fmt, ...) \
return ret; \
}
-DEFINE_PRINT_FN(warning, 0)
-DEFINE_PRINT_FN(info, 0)
+DEFINE_PRINT_FN(warning, 1)
+DEFINE_PRINT_FN(info, 1)
DEFINE_PRINT_FN(debug, 1)
struct bpf_prog_priv {