From bce6a14996f991e570d973179b5ff57544efaa9a Mon Sep 17 00:00:00 2001 From: Shannon Nelson Date: Mon, 29 Oct 2018 14:14:41 -0700 Subject: bpf_load: add map name to load_maps error message To help when debugging bpf/xdp load issues, have the load_map() error message include the number and name of the map that failed. Signed-off-by: Shannon Nelson Acked-by: John Fastabend Acked-by: Song Liu Signed-off-by: Daniel Borkmann --- samples/bpf/bpf_load.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'samples') diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c index e6d7e0fe155b..5c052b9ea63f 100644 --- a/samples/bpf/bpf_load.c +++ b/samples/bpf/bpf_load.c @@ -284,8 +284,8 @@ static int load_maps(struct bpf_map_data *maps, int nr_maps, numa_node); } if (map_fd[i] < 0) { - printf("failed to create a map: %d %s\n", - errno, strerror(errno)); + printf("failed to create map %d (%s): %d %s\n", + i, maps[i].name, errno, strerror(errno)); return 1; } maps[i].fd = map_fd[i]; -- cgit v1.2.3-59-g8ed1b From 9ce6ae22c8e878aee7a96836b2ed9fd9a8173e41 Mon Sep 17 00:00:00 2001 From: Yonghong Song Date: Mon, 19 Nov 2018 15:29:17 -0800 Subject: tools/bpf: do not use pahole if clang/llvm can generate BTF sections Add additional checks in tools/testing/selftests/bpf and samples/bpf such that if clang/llvm compiler can generate BTF sections, do not use pahole. Signed-off-by: Yonghong Song Signed-off-by: Martin KaFai Lau Signed-off-by: Alexei Starovoitov --- samples/bpf/Makefile | 8 ++++++++ tools/testing/selftests/bpf/Makefile | 8 ++++++++ 2 files changed, 16 insertions(+) (limited to 'samples') diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile index be0a961450bc..35444f4a846b 100644 --- a/samples/bpf/Makefile +++ b/samples/bpf/Makefile @@ -208,12 +208,20 @@ endif BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris) BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF) BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm') +BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \ + $(CLANG) -target bpf -O2 -g -c -x c - -o ./llvm_btf_verify.o; \ + readelf -S ./llvm_btf_verify.o | grep BTF; \ + /bin/rm -f ./llvm_btf_verify.o) +ifneq ($(BTF_LLVM_PROBE),) + EXTRA_CFLAGS += -g +else ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),) EXTRA_CFLAGS += -g LLC_FLAGS += -mattr=dwarfris DWARF2BTF = y endif +endif # Trick to allow make to be run from this directory all: diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 57b4712a6276..1dde03ea1484 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -126,7 +126,14 @@ $(OUTPUT)/test_stack_map.o: test_queue_stack_map.h BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris) BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF) BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm') +BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \ + $(CLANG) -target bpf -O2 -g -c -x c - -o ./llvm_btf_verify.o; \ + readelf -S ./llvm_btf_verify.o | grep BTF; \ + /bin/rm -f ./llvm_btf_verify.o) +ifneq ($(BTF_LLVM_PROBE),) + CLANG_FLAGS += -g +else ifneq ($(BTF_LLC_PROBE),) ifneq ($(BTF_PAHOLE_PROBE),) ifneq ($(BTF_OBJCOPY_PROBE),) @@ -136,6 +143,7 @@ ifneq ($(BTF_OBJCOPY_PROBE),) endif endif endif +endif # Have one program compiled without "-target bpf" to test whether libbpf loads # it successfully -- cgit v1.2.3-59-g8ed1b From 5a863813216ce79e16a8c1503b2543c528b778b6 Mon Sep 17 00:00:00 2001 From: "Daniel T. Lee" Date: Fri, 23 Nov 2018 07:14:32 +0900 Subject: samples: bpf: fix: error handling regarding kprobe_events Currently, kprobe_events failure won't be handled properly. Due to calling system() indirectly to write to kprobe_events, it can't be identified whether an error is derived from kprobe or system. // buf = "echo '%c:%s %s' >> /s/k/d/t/kprobe_events" err = system(buf); if (err < 0) { printf("failed to create kprobe .."); return -1; } For example, running ./tracex7 sample in ext4 partition, "echo p:open_ctree open_ctree >> /s/k/d/t/kprobe_events" gets 256 error code system() failure. => The error comes from kprobe, but it's not handled correctly. According to man of system(3), it's return value just passes the termination status of the child shell rather than treating the error as -1. (don't care success) Which means, currently it's not working as desired. (According to the upper code snippet) ex) running ./tracex7 with ext4 env. # Current Output sh: echo: I/O error failed to open event open_ctree # Desired Output failed to create kprobe 'open_ctree' error 'No such file or directory' The problem is, error can't be verified whether from child ps or system. But using write() directly can verify the command failure, and it will treat all error as -1. So I suggest using write() directly to 'kprobe_events' rather than calling system(). Signed-off-by: Daniel T. Lee Signed-off-by: Daniel Borkmann --- samples/bpf/bpf_load.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) (limited to 'samples') diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c index 5c052b9ea63f..434ea34a5954 100644 --- a/samples/bpf/bpf_load.c +++ b/samples/bpf/bpf_load.c @@ -54,6 +54,23 @@ static int populate_prog_array(const char *event, int prog_fd) return 0; } +static int write_kprobe_events(const char *val) +{ + int fd, ret, flags; + + if ((val != NULL) && (val[0] == '\0')) + flags = O_WRONLY | O_TRUNC; + else + flags = O_WRONLY | O_APPEND; + + fd = open("/sys/kernel/debug/tracing/kprobe_events", flags); + + ret = write(fd, val, strlen(val)); + close(fd); + + return ret; +} + static int load_and_attach(const char *event, struct bpf_insn *prog, int size) { bool is_socket = strncmp(event, "socket", 6) == 0; @@ -165,10 +182,9 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size) #ifdef __x86_64__ if (strncmp(event, "sys_", 4) == 0) { - snprintf(buf, sizeof(buf), - "echo '%c:__x64_%s __x64_%s' >> /sys/kernel/debug/tracing/kprobe_events", - is_kprobe ? 'p' : 'r', event, event); - err = system(buf); + snprintf(buf, sizeof(buf), "%c:__x64_%s __x64_%s", + is_kprobe ? 'p' : 'r', event, event); + err = write_kprobe_events(buf); if (err >= 0) { need_normal_check = false; event_prefix = "__x64_"; @@ -176,10 +192,9 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size) } #endif if (need_normal_check) { - snprintf(buf, sizeof(buf), - "echo '%c:%s %s' >> /sys/kernel/debug/tracing/kprobe_events", - is_kprobe ? 'p' : 'r', event, event); - err = system(buf); + snprintf(buf, sizeof(buf), "%c:%s %s", + is_kprobe ? 'p' : 'r', event, event); + err = write_kprobe_events(buf); if (err < 0) { printf("failed to create kprobe '%s' error '%s'\n", event, strerror(errno)); @@ -519,7 +534,7 @@ static int do_load_bpf_file(const char *path, fixup_map_cb fixup_map) return 1; /* clear all kprobes */ - i = system("echo \"\" > /sys/kernel/debug/tracing/kprobe_events"); + i = write_kprobe_events(""); /* scan over all elf sections to get license and map info */ for (i = 1; i < ehdr.e_shnum; i++) { -- cgit v1.2.3-59-g8ed1b From d606ee5c1d9a9bc4d1bba3c36fae615447f9851c Mon Sep 17 00:00:00 2001 From: Matteo Croce Date: Sat, 1 Dec 2018 01:23:05 +0100 Subject: samples: bpf: improve xdp1 example Store only the total packet count for every protocol, instead of the whole per-cpu array. Use bpf_map_get_next_key() to iterate the map, instead of looking up all the protocols. Signed-off-by: Matteo Croce Signed-off-by: Alexei Starovoitov --- samples/bpf/xdp1_user.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) (limited to 'samples') diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c index b02c531510ed..4f3d824fc044 100644 --- a/samples/bpf/xdp1_user.c +++ b/samples/bpf/xdp1_user.c @@ -34,26 +34,24 @@ static void int_exit(int sig) static void poll_stats(int map_fd, int interval) { unsigned int nr_cpus = bpf_num_possible_cpus(); - const unsigned int nr_keys = 256; - __u64 values[nr_cpus], prev[nr_keys][nr_cpus]; - __u32 key; + __u64 values[nr_cpus], prev[UINT8_MAX] = { 0 }; int i; - memset(prev, 0, sizeof(prev)); - while (1) { + __u32 key = UINT32_MAX; + sleep(interval); - for (key = 0; key < nr_keys; key++) { + while (bpf_map_get_next_key(map_fd, &key, &key) != -1) { __u64 sum = 0; assert(bpf_map_lookup_elem(map_fd, &key, values) == 0); for (i = 0; i < nr_cpus; i++) - sum += (values[i] - prev[key][i]); - if (sum) + sum += values[i]; + if (sum > prev[key]) printf("proto %u: %10llu pkt/s\n", - key, sum / interval); - memcpy(prev[key], values, sizeof(values)); + key, (sum - prev[key]) / interval); + prev[key] = sum; } } } -- cgit v1.2.3-59-g8ed1b From dc378a1ab5b6009629a3f8410f8b3030ac6b14c9 Mon Sep 17 00:00:00 2001 From: Matteo Croce Date: Sat, 1 Dec 2018 01:23:06 +0100 Subject: samples: bpf: get ifindex from ifname Find the ifindex with if_nametoindex() instead of requiring the numeric ifindex. Signed-off-by: Matteo Croce Signed-off-by: Alexei Starovoitov --- samples/bpf/xdp1_user.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'samples') diff --git a/samples/bpf/xdp1_user.c b/samples/bpf/xdp1_user.c index 4f3d824fc044..0a197f86ac43 100644 --- a/samples/bpf/xdp1_user.c +++ b/samples/bpf/xdp1_user.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "bpf_util.h" #include "bpf/bpf.h" @@ -59,7 +60,7 @@ static void poll_stats(int map_fd, int interval) static void usage(const char *prog) { fprintf(stderr, - "usage: %s [OPTS] IFINDEX\n\n" + "usage: %s [OPTS] IFACE\n\n" "OPTS:\n" " -S use skb-mode\n" " -N enforce native mode\n", @@ -102,7 +103,11 @@ int main(int argc, char **argv) return 1; } - ifindex = strtoul(argv[optind], NULL, 0); + ifindex = if_nametoindex(argv[1]); + if (!ifindex) { + perror("if_nametoindex"); + return 1; + } snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]); prog_load_attr.file = filename; -- cgit v1.2.3-59-g8ed1b From d59dd69d5576d699d7d3f5da0b4738c3a36d0133 Mon Sep 17 00:00:00 2001 From: "Daniel T. Lee" Date: Mon, 3 Dec 2018 19:39:30 +0900 Subject: samples: bpf: fix: seg fault with NULL pointer arg When NULL pointer accidentally passed to write_kprobe_events, due to strlen(NULL), segmentation fault happens. Changed code returns -1 to deal with this situation. Bug issued with Smatch, static analysis. Signed-off-by: Daniel T. Lee Acked-by: Song Liu Signed-off-by: Daniel Borkmann --- samples/bpf/bpf_load.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'samples') diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c index 434ea34a5954..eae7b635343d 100644 --- a/samples/bpf/bpf_load.c +++ b/samples/bpf/bpf_load.c @@ -58,7 +58,9 @@ static int write_kprobe_events(const char *val) { int fd, ret, flags; - if ((val != NULL) && (val[0] == '\0')) + if (val == NULL) + return -1; + else if (val[0] == '\0') flags = O_WRONLY | O_TRUNC; else flags = O_WRONLY | O_APPEND; -- cgit v1.2.3-59-g8ed1b