* [bpf-next V2 PATCH 0/5] tools/libbpf improvements and selftests @ 2018-02-06 14:54 Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 1/5] bpf: Sync kernel ABI header with tooling header for bpf_common.h Jesper Dangaard Brouer ` (4 more replies) 0 siblings, 5 replies; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-06 14:54 UTC (permalink / raw) To: netdev, Daniel Borkmann, Alexei Starovoitov, wangnan0 Cc: jakub.kicinski, joe, acme, eric, Jesper Dangaard Brouer, yhs While playing with using libbpf for the Suricata project, we had issues LLVM >= 4.0.1 generating ELF files that could not be loaded with libbpf (tools/lib/bpf/). During the troubleshooting phase, I wrote a test program and improved the debugging output in libbpf. I turned this into a selftests program, and it also serves as a code example for libbpf in itself. I discovered that there are at least three ELF load issues with libbpf. I left them as TODO comments in (tools/testing/selftests/bpf) test_libbpf.sh. I've only fixed the load issue with eh_frames. We can work on the other issues later. --- Jesper Dangaard Brouer (5): bpf: Sync kernel ABI header with tooling header for bpf_common.h tools/libbpf: improve the pr_debug statements to contain section numbers selftests/bpf: add test program for loading BPF ELF files selftests/bpf: add selftest that use test_libbpf_open tools/libbpf: handle issues with bpf ELF objects containing .eh_frames tools/include/uapi/linux/bpf_common.h | 7 + tools/lib/bpf/libbpf.c | 32 +++-- tools/testing/selftests/bpf/Makefile | 12 ++ tools/testing/selftests/bpf/test_libbpf.sh | 49 ++++++++ tools/testing/selftests/bpf/test_libbpf_open.c | 150 ++++++++++++++++++++++++ 5 files changed, 234 insertions(+), 16 deletions(-) create mode 100755 tools/testing/selftests/bpf/test_libbpf.sh create mode 100644 tools/testing/selftests/bpf/test_libbpf_open.c ^ permalink raw reply [flat|nested] 20+ messages in thread
* [bpf-next V2 PATCH 1/5] bpf: Sync kernel ABI header with tooling header for bpf_common.h 2018-02-06 14:54 [bpf-next V2 PATCH 0/5] tools/libbpf improvements and selftests Jesper Dangaard Brouer @ 2018-02-06 14:54 ` Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 2/5] tools/libbpf: improve the pr_debug statements to contain section numbers Jesper Dangaard Brouer ` (3 subsequent siblings) 4 siblings, 0 replies; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-06 14:54 UTC (permalink / raw) To: netdev, Daniel Borkmann, Alexei Starovoitov, wangnan0 Cc: jakub.kicinski, joe, acme, eric, Jesper Dangaard Brouer, yhs I recently fixed up a lot of commits that forgot to keep the tooling headers in sync. And then I forgot to do the same thing in commit cb5f7334d479 ("bpf: add comments to BPF ld/ldx sizes"). Let correct that before people notice ;-). Lawrence did partly fix/sync this for bpf.h in commit d6d4f60c3a09 ("bpf: add selftest for tcpbpf"). Fixes: cb5f7334d479 ("bpf: add comments to BPF ld/ldx sizes") Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- tools/include/uapi/linux/bpf_common.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/include/uapi/linux/bpf_common.h b/tools/include/uapi/linux/bpf_common.h index 18be90725ab0..ee97668bdadb 100644 --- a/tools/include/uapi/linux/bpf_common.h +++ b/tools/include/uapi/linux/bpf_common.h @@ -15,9 +15,10 @@ /* ld/ldx fields */ #define BPF_SIZE(code) ((code) & 0x18) -#define BPF_W 0x00 -#define BPF_H 0x08 -#define BPF_B 0x10 +#define BPF_W 0x00 /* 32-bit */ +#define BPF_H 0x08 /* 16-bit */ +#define BPF_B 0x10 /* 8-bit */ +/* eBPF BPF_DW 0x18 64-bit */ #define BPF_MODE(code) ((code) & 0xe0) #define BPF_IMM 0x00 #define BPF_ABS 0x20 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [bpf-next V2 PATCH 2/5] tools/libbpf: improve the pr_debug statements to contain section numbers 2018-02-06 14:54 [bpf-next V2 PATCH 0/5] tools/libbpf improvements and selftests Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 1/5] bpf: Sync kernel ABI header with tooling header for bpf_common.h Jesper Dangaard Brouer @ 2018-02-06 14:54 ` Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 3/5] selftests/bpf: add test program for loading BPF ELF files Jesper Dangaard Brouer ` (2 subsequent siblings) 4 siblings, 0 replies; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-06 14:54 UTC (permalink / raw) To: netdev, Daniel Borkmann, Alexei Starovoitov, wangnan0 Cc: jakub.kicinski, joe, acme, eric, Jesper Dangaard Brouer, yhs While debugging a bpf ELF loading issue, I needed to correlate the ELF section number with the failed relocation section reference. Thus, add section numbers/index to the pr_debug. In debug mode, also print section that were skipped. This helped me identify that a section (.eh_frame) was skipped, and this was the reason the relocation section (.rel.eh_frame) could not find that section number. The section numbers corresponds to the readelf tools Section Headers [Nr]. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- tools/lib/bpf/libbpf.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 30c776375118..b4eeaa3ebff5 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -315,8 +315,8 @@ bpf_program__init(void *data, size_t size, char *section_name, int idx, prog->section_name = strdup(section_name); if (!prog->section_name) { - pr_warning("failed to alloc name for prog under section %s\n", - section_name); + pr_warning("failed to alloc name for prog under section(%d) %s\n", + idx, section_name); goto errout; } @@ -759,29 +759,29 @@ static int bpf_object__elf_collect(struct bpf_object *obj) idx++; if (gelf_getshdr(scn, &sh) != &sh) { - pr_warning("failed to get section header from %s\n", - obj->path); + pr_warning("failed to get section(%d) header from %s\n", + idx, obj->path); err = -LIBBPF_ERRNO__FORMAT; goto out; } name = elf_strptr(elf, ep->e_shstrndx, sh.sh_name); if (!name) { - pr_warning("failed to get section name from %s\n", - obj->path); + pr_warning("failed to get section(%d) name from %s\n", + idx, obj->path); err = -LIBBPF_ERRNO__FORMAT; goto out; } data = elf_getdata(scn, 0); if (!data) { - pr_warning("failed to get section data from %s(%s)\n", - name, obj->path); + pr_warning("failed to get section(%d) data from %s(%s)\n", + idx, name, obj->path); err = -LIBBPF_ERRNO__FORMAT; goto out; } - pr_debug("section %s, size %ld, link %d, flags %lx, type=%d\n", - name, (unsigned long)data->d_size, + pr_debug("section(%d) %s, size %ld, link %d, flags %lx, type=%d\n", + idx, name, (unsigned long)data->d_size, (int)sh.sh_link, (unsigned long)sh.sh_flags, (int)sh.sh_type); @@ -836,6 +836,8 @@ static int bpf_object__elf_collect(struct bpf_object *obj) obj->efile.reloc[n].shdr = sh; obj->efile.reloc[n].data = data; } + } else { + pr_debug("skip section(%d) %s\n", idx, name); } if (err) goto out; @@ -1115,8 +1117,7 @@ static int bpf_object__collect_reloc(struct bpf_object *obj) prog = bpf_object__find_prog_by_idx(obj, idx); if (!prog) { - pr_warning("relocation failed: no %d section\n", - idx); + pr_warning("relocation failed: no section(%d)\n", idx); return -LIBBPF_ERRNO__RELOC; } ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [bpf-next V2 PATCH 3/5] selftests/bpf: add test program for loading BPF ELF files 2018-02-06 14:54 [bpf-next V2 PATCH 0/5] tools/libbpf improvements and selftests Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 1/5] bpf: Sync kernel ABI header with tooling header for bpf_common.h Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 2/5] tools/libbpf: improve the pr_debug statements to contain section numbers Jesper Dangaard Brouer @ 2018-02-06 14:54 ` Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 4/5] selftests/bpf: add selftest that use test_libbpf_open Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames Jesper Dangaard Brouer 4 siblings, 0 replies; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-06 14:54 UTC (permalink / raw) To: netdev, Daniel Borkmann, Alexei Starovoitov, wangnan0 Cc: jakub.kicinski, joe, acme, eric, Jesper Dangaard Brouer, yhs V2: Moved program into selftests/bpf from tools/libbpf This program can be used on its own for testing/debugging if a BPF ELF-object file can be loaded with libbpf (from tools/lib/bpf). If something is wrong with the ELF object, the program have a --debug mode that will display the ELF sections and especially the skipped sections. This allows for quickly identifying the problematic ELF section number, which can be corrolated with the readelf tool. The program signal error via return codes, and also have a --quiet mode, which is practical for use in scripts like selftests/bpf. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- tools/testing/selftests/bpf/Makefile | 2 tools/testing/selftests/bpf/test_libbpf_open.c | 150 ++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/test_libbpf_open.c diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index bf05bc5e36e5..de7b85e2e532 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -20,7 +20,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o \ sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o test_tracepoint.o \ test_l4lb_noinline.o test_xdp_noinline.o test_stacktrace_map.o \ - sample_map_ret0.o test_tcpbpf_kern.o + sample_map_ret0.o test_tcpbpf_kern.o test_libbpf_open TEST_PROGS := test_kmod.sh test_xdp_redirect.sh test_xdp_meta.sh \ test_offload.py diff --git a/tools/testing/selftests/bpf/test_libbpf_open.c b/tools/testing/selftests/bpf/test_libbpf_open.c new file mode 100644 index 000000000000..8fcd1c076add --- /dev/null +++ b/tools/testing/selftests/bpf/test_libbpf_open.c @@ -0,0 +1,150 @@ +/* SPDX-License-Identifier: GPL-2.0 + * Copyright (c) 2018 Jesper Dangaard Brouer, Red Hat Inc. + */ +static const char *__doc__ = + "Libbpf test program for loading BPF ELF object files"; + +#include <stdlib.h> +#include <stdio.h> +#include <string.h> +#include <stdarg.h> +#include <bpf/libbpf.h> +#include <getopt.h> + +static const struct option long_options[] = { + {"help", no_argument, NULL, 'h' }, + {"debug", no_argument, NULL, 'D' }, + {"quiet", no_argument, NULL, 'q' }, + {0, 0, NULL, 0 } +}; + +static void usage(char *argv[]) +{ + int i; + + printf("\nDOCUMENTATION:\n%s\n\n", __doc__); + printf(" Usage: %s (options-see-below) BPF_FILE\n", argv[0]); + printf(" Listing options:\n"); + for (i = 0; long_options[i].name != 0; i++) { + printf(" --%-12s", long_options[i].name); + printf(" short-option: -%c", + long_options[i].val); + printf("\n"); + } + printf("\n"); +} + +#define DEFINE_PRINT_FN(name, enabled) \ +static int libbpf_##name(const char *fmt, ...) \ +{ \ + va_list args; \ + int ret; \ + \ + va_start(args, fmt); \ + if (enabled) { \ + fprintf(stderr, "[" #name "] "); \ + ret = vfprintf(stderr, fmt, args); \ + } \ + va_end(args); \ + return ret; \ +} +DEFINE_PRINT_FN(warning, 1) +DEFINE_PRINT_FN(info, 1) +DEFINE_PRINT_FN(debug, 1) + +#define EXIT_FAIL_LIBBPF EXIT_FAILURE +#define EXIT_FAIL_OPTION 2 + +int test_walk_progs(struct bpf_object *obj, bool verbose) +{ + struct bpf_program *prog; + int cnt = 0; + + bpf_object__for_each_program(prog, obj) { + cnt++; + if (verbose) + printf("Prog (count:%d) section_name: %s\n", cnt, + bpf_program__title(prog, false)); + } + return 0; +} + +int test_walk_maps(struct bpf_object *obj, bool verbose) +{ + struct bpf_map *map; + int cnt = 0; + + bpf_map__for_each(map, obj) { + cnt++; + if (verbose) + printf("Map (count:%d) name: %s\n", cnt, + bpf_map__name(map)); + } + return 0; +} + +int test_open_file(char *filename, bool verbose) +{ + struct bpf_object *bpfobj = NULL; + long err; + + if (verbose) + printf("Open BPF ELF-file with libbpf: %s\n", filename); + + /* Load BPF ELF object file and check for errors */ + bpfobj = bpf_object__open(filename); + err = libbpf_get_error(bpfobj); + if (err) { + char err_buf[128]; + libbpf_strerror(err, err_buf, sizeof(err_buf)); + if (verbose) + printf("Unable to load eBPF objects in file '%s': %s\n", + filename, err_buf); + return EXIT_FAIL_LIBBPF; + } + test_walk_progs(bpfobj, verbose); + test_walk_maps(bpfobj, verbose); + + if (verbose) + printf("Close BPF ELF-file with libbpf: %s\n", + bpf_object__name(bpfobj)); + bpf_object__close(bpfobj); + + return 0; +} + +int main(int argc, char **argv) +{ + char filename[1024] = { 0 }; + bool verbose = 1; + int longindex = 0; + int opt; + + libbpf_set_print(libbpf_warning, libbpf_info, NULL); + + /* Parse commands line args */ + while ((opt = getopt_long(argc, argv, "hDq", + long_options, &longindex)) != -1) { + switch (opt) { + case 'D': + libbpf_set_print(libbpf_warning, libbpf_info, + libbpf_debug); + break; + case 'q': /* Use in scripting mode */ + verbose = 0; + break; + case 'h': + default: + usage(argv); + return EXIT_FAIL_OPTION; + } + } + if (optind >= argc) { + usage(argv); + printf("ERROR: Expected BPF_FILE argument after options\n"); + return EXIT_FAIL_OPTION; + } + snprintf(filename, sizeof(filename), "%s", argv[optind]); + + return test_open_file(filename, verbose); +} ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [bpf-next V2 PATCH 4/5] selftests/bpf: add selftest that use test_libbpf_open 2018-02-06 14:54 [bpf-next V2 PATCH 0/5] tools/libbpf improvements and selftests Jesper Dangaard Brouer ` (2 preceding siblings ...) 2018-02-06 14:54 ` [bpf-next V2 PATCH 3/5] selftests/bpf: add test program for loading BPF ELF files Jesper Dangaard Brouer @ 2018-02-06 14:54 ` Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames Jesper Dangaard Brouer 4 siblings, 0 replies; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-06 14:54 UTC (permalink / raw) To: netdev, Daniel Borkmann, Alexei Starovoitov, wangnan0 Cc: jakub.kicinski, joe, acme, eric, Jesper Dangaard Brouer, yhs This script test_libbpf.sh will be part of the 'make run_tests' invocation, but can also be invoked manually in this directory, and a verbose mode can be enabled via setting the environment variable $VERBOSE like: $ VERBOSE=yes ./test_libbpf.sh The script contains some tests that are commented out, as they currently fail. They are reminders about what we need to improve for the libbpf loader library. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- tools/testing/selftests/bpf/Makefile | 14 +++++++- tools/testing/selftests/bpf/test_libbpf.sh | 49 ++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) create mode 100755 tools/testing/selftests/bpf/test_libbpf.sh diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index de7b85e2e532..8b5667714250 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -13,6 +13,7 @@ endif CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include LDLIBS += -lcap -lelf -lrt -lpthread +# Order correspond to 'make run_tests' order TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \ test_align test_verifier_log test_dev_cgroup test_tcpbpf_user @@ -20,17 +21,26 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test test_pkt_md_access.o test_xdp_redirect.o test_xdp_meta.o sockmap_parse_prog.o \ sockmap_verdict_prog.o dev_cgroup.o sample_ret0.o test_tracepoint.o \ test_l4lb_noinline.o test_xdp_noinline.o test_stacktrace_map.o \ - sample_map_ret0.o test_tcpbpf_kern.o test_libbpf_open + sample_map_ret0.o test_tcpbpf_kern.o -TEST_PROGS := test_kmod.sh test_xdp_redirect.sh test_xdp_meta.sh \ +# Order correspond to 'make run_tests' order +TEST_PROGS := test_kmod.sh \ + test_libbpf.sh \ + test_xdp_redirect.sh \ + test_xdp_meta.sh \ test_offload.py +# Compile but not part of 'make run_tests' +TEST_GEN_PROGS_EXTENDED = test_libbpf_open + include ../lib.mk BPFOBJ := $(OUTPUT)/libbpf.a $(OUTPUT)/cgroup_helpers.c $(TEST_GEN_PROGS): $(BPFOBJ) +$(TEST_GEN_PROGS_EXTENDED): $(OUTPUT)/libbpf.a + .PHONY: force # force a rebuild of BPFOBJ when its dependencies are updated diff --git a/tools/testing/selftests/bpf/test_libbpf.sh b/tools/testing/selftests/bpf/test_libbpf.sh new file mode 100755 index 000000000000..d97dc914cd49 --- /dev/null +++ b/tools/testing/selftests/bpf/test_libbpf.sh @@ -0,0 +1,49 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 + +export TESTNAME=test_libbpf + +# Determine selftest success via shell exit code +exit_handler() +{ + if (( $? == 0 )); then + echo "selftests: $TESTNAME [PASS]"; + else + echo "$TESTNAME: failed at file $LAST_LOADED" 1>&2 + echo "selftests: $TESTNAME [FAILED]"; + fi +} + +libbpf_open_file() +{ + LAST_LOADED=$1 + if [ -n "$VERBOSE" ]; then + ./test_libbpf_open $1 + else + ./test_libbpf_open --quiet $1 + fi +} + +# Exit script immediately (well catched by trap handler) if any +# program/thing exits with a non-zero status. +set -e + +# (Use 'trap -l' to list meaning of numbers) +trap exit_handler 0 2 3 6 9 + +libbpf_open_file test_l4lb.o + +# TODO: fix libbpf to load noinline functions +# [warning] libbpf: incorrect bpf_call opcode +#libbpf_open_file test_l4lb_noinline.o + +# TODO: fix test_xdp_meta.c to load with libbpf +# [warning] libbpf: test_xdp_meta.o doesn't provide kernel version +#libbpf_open_file test_xdp_meta.o + +# TODO: fix libbpf to handle .eh_frame +# [warning] libbpf: relocation failed: no section(10) +#libbpf_open_file ../../../../samples/bpf/tracex3_kern.o + +# Success +exit 0 ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames 2018-02-06 14:54 [bpf-next V2 PATCH 0/5] tools/libbpf improvements and selftests Jesper Dangaard Brouer ` (3 preceding siblings ...) 2018-02-06 14:54 ` [bpf-next V2 PATCH 4/5] selftests/bpf: add selftest that use test_libbpf_open Jesper Dangaard Brouer @ 2018-02-06 14:54 ` Jesper Dangaard Brouer 2018-02-06 16:00 ` Alexei Starovoitov 4 siblings, 1 reply; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-06 14:54 UTC (permalink / raw) To: netdev, Daniel Borkmann, Alexei Starovoitov, wangnan0 Cc: jakub.kicinski, joe, acme, eric, Jesper Dangaard Brouer, yhs If clang >= 4.0.1 is missing the option '-target bpf', it will cause llc/llvm to create two ELF sections for "Exception Frames", with section names '.eh_frame' and '.rel.eh_frame'. The BPF ELF loader library libbpf fails when loading files with these sections. The other in-kernel BPF ELF loader in samples/bpf/bpf_load.c, handle this gracefully. And iproute2 loader also seems to work with these "eh" sections. The issue in libbpf is caused by bpf_object__elf_collect() skip the '.eh_frame' and thus doesn't create an internal data structure pointing to this ELF section index. Later when the relocation section '.rel.eh_frame' is processed, it tries to find the '.eh_frame' via the ELF section idx, which is that fails (in bpf_object__collect_reloc). I couldn't find a way to see that the '.rel.eh_frame' was irrelevant (that is only determined by looking at the section it reference, which we no longer have info available on). Thus, my solution is simply to match on the name of the relocation section, to skip that too. Note, for samples/bpf/ the '-target bpf' parameter to clang cannot be used due to incompatibility with asm embedded headers, that some of the samples include. This is explained in more details by Yonghong Song in bpf_devel_QA. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- tools/lib/bpf/libbpf.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index b4eeaa3ebff5..84e8bbe07347 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -822,6 +822,13 @@ static int bpf_object__elf_collect(struct bpf_object *obj) void *reloc = obj->efile.reloc; int nr_reloc = obj->efile.nr_reloc + 1; + /* Skip decoding of "eh" exception frames */ + if (strcmp(name, ".rel.eh_frame") == 0) { + pr_debug("skip relo section %s(%d) for section(%d)\n", + name, idx, sh.sh_info); + continue; + } + reloc = realloc(reloc, sizeof(*obj->efile.reloc) * nr_reloc); if (!reloc) { ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames 2018-02-06 14:54 ` [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames Jesper Dangaard Brouer @ 2018-02-06 16:00 ` Alexei Starovoitov 2018-02-06 17:03 ` Jesper Dangaard Brouer 0 siblings, 1 reply; 20+ messages in thread From: Alexei Starovoitov @ 2018-02-06 16:00 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: netdev, Daniel Borkmann, wangnan0, jakub.kicinski, joe, acme, eric, yhs On Tue, Feb 06, 2018 at 03:54:28PM +0100, Jesper Dangaard Brouer wrote: > If clang >= 4.0.1 is missing the option '-target bpf', it will cause > llc/llvm to create two ELF sections for "Exception Frames", with > section names '.eh_frame' and '.rel.eh_frame'. > > The BPF ELF loader library libbpf fails when loading files with these > sections. The other in-kernel BPF ELF loader in samples/bpf/bpf_load.c, > handle this gracefully. And iproute2 loader also seems to work with these > "eh" sections. > > The issue in libbpf is caused by bpf_object__elf_collect() skip the > '.eh_frame' and thus doesn't create an internal data structure > pointing to this ELF section index. Later when the relocation section > '.rel.eh_frame' is processed, it tries to find the '.eh_frame' via the > ELF section idx, which is that fails (in bpf_object__collect_reloc). > > I couldn't find a way to see that the '.rel.eh_frame' was irrelevant > (that is only determined by looking at the section it reference, which > we no longer have info available on). but does this approach work for all extra sections and relocations emitted when source is compiled with -g ? To address this case bpf_load.c does: if (shdr.sh_type == SHT_REL) { struct bpf_insn *insns; /* locate prog sec that need map fixup (relocations) */ if (get_sec(elf, shdr.sh_info, &ehdr, &shname_prog, &shdr_prog, &data_prog)) continue; if (shdr_prog.sh_type != SHT_PROGBITS || !(shdr_prog.sh_flags & SHF_EXECINSTR)) continue; why the same approach is not applicable here? I guess we can apply this workaround as-is but it looks incomplete. > Thus, my solution is simply to match on the name of the relocation > section, to skip that too. > > Note, for samples/bpf/ the '-target bpf' parameter to clang cannot be used > due to incompatibility with asm embedded headers, that some of the samples > include. This is explained in more details by Yonghong Song in bpf_devel_QA. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > --- > tools/lib/bpf/libbpf.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index b4eeaa3ebff5..84e8bbe07347 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -822,6 +822,13 @@ static int bpf_object__elf_collect(struct bpf_object *obj) > void *reloc = obj->efile.reloc; > int nr_reloc = obj->efile.nr_reloc + 1; > > + /* Skip decoding of "eh" exception frames */ > + if (strcmp(name, ".rel.eh_frame") == 0) { > + pr_debug("skip relo section %s(%d) for section(%d)\n", > + name, idx, sh.sh_info); > + continue; > + } > + > reloc = realloc(reloc, > sizeof(*obj->efile.reloc) * nr_reloc); > if (!reloc) { > ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames 2018-02-06 16:00 ` Alexei Starovoitov @ 2018-02-06 17:03 ` Jesper Dangaard Brouer 2018-02-06 19:05 ` Daniel Borkmann 0 siblings, 1 reply; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-06 17:03 UTC (permalink / raw) To: Alexei Starovoitov Cc: netdev, Daniel Borkmann, wangnan0, jakub.kicinski, joe, acme, eric, yhs, brouer On Tue, 6 Feb 2018 08:00:59 -0800 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > On Tue, Feb 06, 2018 at 03:54:28PM +0100, Jesper Dangaard Brouer wrote: > > If clang >= 4.0.1 is missing the option '-target bpf', it will cause > > llc/llvm to create two ELF sections for "Exception Frames", with > > section names '.eh_frame' and '.rel.eh_frame'. > > > > The BPF ELF loader library libbpf fails when loading files with these > > sections. The other in-kernel BPF ELF loader in samples/bpf/bpf_load.c, > > handle this gracefully. And iproute2 loader also seems to work with these > > "eh" sections. > > > > The issue in libbpf is caused by bpf_object__elf_collect() skip the > > '.eh_frame' and thus doesn't create an internal data structure > > pointing to this ELF section index. Later when the relocation section > > '.rel.eh_frame' is processed, it tries to find the '.eh_frame' via the > > ELF section idx, which is that fails (in bpf_object__collect_reloc). > > > > I couldn't find a way to see that the '.rel.eh_frame' was irrelevant > > (that is only determined by looking at the section it reference, which > > we no longer have info available on). > > but does this approach work for all extra sections and relocations emitted > when source is compiled with -g ? No, but I plan to follow up and do a more complete solution later. This is a workaround to get the Suricata use-case working and also that samples/bpf/ can be loaded. > To address this case bpf_load.c does: > if (shdr.sh_type == SHT_REL) { > struct bpf_insn *insns; > > /* locate prog sec that need map fixup (relocations) */ > if (get_sec(elf, shdr.sh_info, &ehdr, &shname_prog, > &shdr_prog, &data_prog)) > continue; > > if (shdr_prog.sh_type != SHT_PROGBITS || > !(shdr_prog.sh_flags & SHF_EXECINSTR)) > continue; > > why the same approach is not applicable here? As described above bpf_object__elf_collect() skip the "real" section that the relo-section want to lookup (based on the same kind of check), but libbpf is now missing the section idx in its internal structures... and thus the relo lookup of the idx fails. (bpf_load.c does the lookup in the ELF obj directly, thus it does not have this problem). > I guess we can apply this workaround as-is but it looks incomplete. Yes, it is a workaround to move forward... it requires a larger change to libbpf, so it stores idx'es of skipped sections. -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames 2018-02-06 17:03 ` Jesper Dangaard Brouer @ 2018-02-06 19:05 ` Daniel Borkmann 2018-02-07 12:40 ` Jesper Dangaard Brouer 0 siblings, 1 reply; 20+ messages in thread From: Daniel Borkmann @ 2018-02-06 19:05 UTC (permalink / raw) To: Jesper Dangaard Brouer, Alexei Starovoitov Cc: netdev, Daniel Borkmann, wangnan0, jakub.kicinski, joe, acme, eric, yhs On 02/06/2018 06:03 PM, Jesper Dangaard Brouer wrote: > On Tue, 6 Feb 2018 08:00:59 -0800 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: >> On Tue, Feb 06, 2018 at 03:54:28PM +0100, Jesper Dangaard Brouer wrote: >>> If clang >= 4.0.1 is missing the option '-target bpf', it will cause >>> llc/llvm to create two ELF sections for "Exception Frames", with >>> section names '.eh_frame' and '.rel.eh_frame'. >>> >>> The BPF ELF loader library libbpf fails when loading files with these >>> sections. The other in-kernel BPF ELF loader in samples/bpf/bpf_load.c, >>> handle this gracefully. And iproute2 loader also seems to work with these >>> "eh" sections. >>> >>> The issue in libbpf is caused by bpf_object__elf_collect() skip the >>> '.eh_frame' and thus doesn't create an internal data structure >>> pointing to this ELF section index. Later when the relocation section >>> '.rel.eh_frame' is processed, it tries to find the '.eh_frame' via the >>> ELF section idx, which is that fails (in bpf_object__collect_reloc). >>> >>> I couldn't find a way to see that the '.rel.eh_frame' was irrelevant >>> (that is only determined by looking at the section it reference, which >>> we no longer have info available on). >> >> but does this approach work for all extra sections and relocations emitted >> when source is compiled with -g ? > > No, but I plan to follow up and do a more complete solution later. This > is a workaround to get the Suricata use-case working and also that > samples/bpf/ can be loaded. Aside from a needed fix in any case, is there a specifc reason why Suricata cannot rely on 'clang -target bpf'? Is it asm inline headers in your case? >> To address this case bpf_load.c does: >> if (shdr.sh_type == SHT_REL) { >> struct bpf_insn *insns; >> >> /* locate prog sec that need map fixup (relocations) */ >> if (get_sec(elf, shdr.sh_info, &ehdr, &shname_prog, >> &shdr_prog, &data_prog)) >> continue; >> >> if (shdr_prog.sh_type != SHT_PROGBITS || >> !(shdr_prog.sh_flags & SHF_EXECINSTR)) >> continue; >> >> why the same approach is not applicable here? > > As described above bpf_object__elf_collect() skip the "real" section > that the relo-section want to lookup (based on the same kind of > check), but libbpf is now missing the section idx in its internal > structures... and thus the relo lookup of the idx fails. (bpf_load.c > does the lookup in the ELF obj directly, thus it does not have this > problem). Out of curiosity, I just double checked iproute2 loader (examples/bpf/): $ clang -O2 -g -emit-llvm -c bpf_cyclic.c -o - | llc -march=bpf -mcpu=probe -filetype=obj -o bpf_cyclic.o $ readelf -a bpf_cyclic.o | grep "\[" [Nr] Name Type Address Offset [ 0] NULL 0000000000000000 00000000 [ 1] .strtab STRTAB 0000000000000000 000016b0 [ 2] .text PROGBITS 0000000000000000 00000040 [ 3] 0xabccba/0 PROGBITS 0000000000000000 00000040 [ 4] .rel0xabccba/0 REL 0000000000000000 00001120 [ 5] classifier PROGBITS 0000000000000000 000000e8 [ 6] .relclassifier REL 0000000000000000 00001130 [ 7] maps PROGBITS 0000000000000000 00000118 [ 8] license PROGBITS 0000000000000000 0000013c [ 9] .debug_str PROGBITS 0000000000000000 00000140 [10] .debug_loc PROGBITS 0000000000000000 000003d5 [11] .rel.debug_loc REL 0000000000000000 00001140 [12] .debug_abbrev PROGBITS 0000000000000000 0000045a [13] .debug_info PROGBITS 0000000000000000 0000055c [14] .rel.debug_info REL 0000000000000000 000011c0 [15] .debug_ranges PROGBITS 0000000000000000 0000088c [16] .rel.debug_ranges REL 0000000000000000 000015d0 [17] .debug_macinfo PROGBITS 0000000000000000 000008ec [18] .debug_pubnames PROGBITS 0000000000000000 000008ed [19] .rel.debug_pubnam REL 0000000000000000 00001650 [20] .debug_pubtypes PROGBITS 0000000000000000 00000954 [21] .rel.debug_pubtyp REL 0000000000000000 00001660 [22] .eh_frame PROGBITS 0000000000000000 000009c0 [23] .rel.eh_frame REL 0000000000000000 00001670 [24] .debug_line PROGBITS 0000000000000000 00000a10 [25] .rel.debug_line REL 0000000000000000 00001690 [26] .symtab SYMTAB 0000000000000000 00000b08 # tc qdisc add dev lo clsact # tc filter add dev lo ingress bpf da obj bpf_cyclic.o # tc filter show dev lo ingress filter protocol all pref 49152 bpf chain 0 filter protocol all pref 49152 bpf chain 0 handle 0x1 bpf_cyclic.o:[classifier] direct-action not_in_hw id 6 tag 736a8a004dead229 So no problems. What it does internally is pretty similar to what Alexei described; for programs, they need to have ELF section header type of SHT_PROGBITS and section header flags must match on SHF_EXECINSTR in the relocation parsing. Now, picking out two, and looking at the flags: Section Headers: [Nr] Name Type Address Offset Size EntSize Flags Link Info Align [...] [ 5] classifier PROGBITS 0000000000000000 000000e8 0000000000000030 0000000000000000 AX 0 0 8 [...] [22] .eh_frame PROGBITS 0000000000000000 000009c0 0000000000000050 0000000000000000 A 0 0 8 [...] Key to Flags: W (write), A (alloc), X (execute), M (merge), S (strings) I (info), L (link order), G (group), T (TLS), E (exclude), x (unknown) O (extra OS processing required) o (OS specific), p (processor specific) So .eh_frame doesn't even have SHF_EXECINSTR set. Why it cannot test on this? Doing strcmp(name, ".rel.eh_frame") == 0 test is indeed a bit fragile in the sense that we would also need to strcmp() all the others listed above since libbpf could trip over them just as well. When you check the SHT_REL sections, the target section index sits in GElf_Shdr's .sh_info, so the only thing that would need to be done in this case is to look up the ELF section header with index from .sh_info, get the GElf_Shdr section header and check for a match on SHT_PROGBITS/SHF_EXECINSTR, otherwise skip that SHT_REL section. A direct lookup of the index in the obj would not require any complex section/index tracking or larger rework in libbpf, hmm, what am I missing? >> I guess we can apply this workaround as-is but it looks incomplete. > > Yes, it is a workaround to move forward... it requires a larger change > to libbpf, so it stores idx'es of skipped sections. Thanks, Daniel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames 2018-02-06 19:05 ` Daniel Borkmann @ 2018-02-07 12:40 ` Jesper Dangaard Brouer 2018-02-07 13:19 ` Daniel Borkmann 0 siblings, 1 reply; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-07 12:40 UTC (permalink / raw) To: Daniel Borkmann Cc: Alexei Starovoitov, netdev, Daniel Borkmann, wangnan0, jakub.kicinski, joe, acme, eric, yhs, brouer, Victor Julien On Tue, 6 Feb 2018 20:05:43 +0100 Daniel Borkmann <daniel@iogearbox.net> wrote: > On 02/06/2018 06:03 PM, Jesper Dangaard Brouer wrote: [...] > > [...] I plan to follow up and do a more complete solution later. This > > is a workaround to get the Suricata use-case working and also that > > samples/bpf/ can be loaded. > > Aside from a needed fix in any case, is there a specifc reason why Suricata > cannot rely on 'clang -target bpf'? Is it asm inline headers in your case? Below is the error I get when using 'clang' with '-target bpf' $ dirs ~/git/suricata/src/ebpf $ clang -Wall -Iinclude -O2 -D__KERNEL__ -target bpf -emit-llvm -c xdp_filter.c -o - | llc -march=bpf -filetype=obj -o xdp_filter.bpf In file included from xdp_filter.c:19: In file included from /usr/bin/../lib64/clang/4.0.1/include/stdint.h:63: In file included from /usr/include/stdint.h:26: In file included from /usr/include/bits/libc-header-start.h:33: In file included from /usr/include/features.h:434: /usr/include/gnu/stubs.h:7:11: fatal error: 'gnu/stubs-32.h' file not found # include <gnu/stubs-32.h> ^~~~~~~~~~~~~~~~ I'll leave it up to Eric Leblond to figure out that he need to change in the eBPF programs to make it compile with '-target bpf'. Maybe you can offer him some guidance here? Direct link to code: https://github.com/OISF/suricata/blob/master/ebpf/xdp_filter.c -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames 2018-02-07 12:40 ` Jesper Dangaard Brouer @ 2018-02-07 13:19 ` Daniel Borkmann 2018-02-07 14:58 ` Jesper Dangaard Brouer 0 siblings, 1 reply; 20+ messages in thread From: Daniel Borkmann @ 2018-02-07 13:19 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: Alexei Starovoitov, netdev, Daniel Borkmann, wangnan0, jakub.kicinski, joe, acme, eric, yhs, Victor Julien On 02/07/2018 01:40 PM, Jesper Dangaard Brouer wrote: > On Tue, 6 Feb 2018 20:05:43 +0100 Daniel Borkmann <daniel@iogearbox.net> wrote: >> On 02/06/2018 06:03 PM, Jesper Dangaard Brouer wrote: > [...] >>> [...] I plan to follow up and do a more complete solution later. This >>> is a workaround to get the Suricata use-case working and also that >>> samples/bpf/ can be loaded. >> >> Aside from a needed fix in any case, is there a specifc reason why Suricata >> cannot rely on 'clang -target bpf'? Is it asm inline headers in your case? > > Below is the error I get when using 'clang' with '-target bpf' > > $ dirs > ~/git/suricata/src/ebpf > > $ clang -Wall -Iinclude -O2 -D__KERNEL__ -target bpf -emit-llvm -c xdp_filter.c -o - | llc -march=bpf -filetype=obj -o xdp_filter.bpf > In file included from xdp_filter.c:19: > In file included from /usr/bin/../lib64/clang/4.0.1/include/stdint.h:63: > In file included from /usr/include/stdint.h:26: > In file included from /usr/include/bits/libc-header-start.h:33: > In file included from /usr/include/features.h:434: > /usr/include/gnu/stubs.h:7:11: fatal error: 'gnu/stubs-32.h' file not found > # include <gnu/stubs-32.h> > ^~~~~~~~~~~~~~~~ > > I'll leave it up to Eric Leblond to figure out that he need to change > in the eBPF programs to make it compile with '-target bpf'. Maybe you > can offer him some guidance here? > > Direct link to code: > https://github.com/OISF/suricata/blob/master/ebpf/xdp_filter.c Sure, you just need glibc-devel.i686, see: $ clang -Wall -Iinclude -O2 -D__KERNEL__ -target bpf -emit-llvm -c xdp_filter.c -o - | llc -march=bpf -filetype=obj -o xdp_filter.bpf In file included from xdp_filter.c:19: In file included from /home/darkstar/llvm/build/lib/clang/7.0.0/include/stdint.h:63: In file included from /usr/include/stdint.h:25: In file included from /usr/include/features.h:392: /usr/include/gnu/stubs.h:7:11: fatal error: 'gnu/stubs-32.h' file not found # include <gnu/stubs-32.h> ^~~~~~~~~~~~~~~~ 1 error generated. # yum install glibc-devel.i686 [...] $ clang -Wall -Iinclude -O2 -D__KERNEL__ -target bpf -emit-llvm -c xdp_filter.c -o - | llc -march=bpf -filetype=obj -o xdp_filter.bpf $ Alternatively, you could do something like done in selftests to provide a dummy, see commit 1c2dd16add7e ("selftests/bpf: get rid of -D__x86_64__"). Cheers, Daniel ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames 2018-02-07 13:19 ` Daniel Borkmann @ 2018-02-07 14:58 ` Jesper Dangaard Brouer 2018-02-07 16:18 ` Daniel Borkmann 2018-02-07 22:21 ` [suricata PATCH 0/3] Suricata cleanup makefile Jesper Dangaard Brouer 0 siblings, 2 replies; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-07 14:58 UTC (permalink / raw) To: Daniel Borkmann Cc: Alexei Starovoitov, netdev, Daniel Borkmann, wangnan0, jakub.kicinski, joe, acme, eric, yhs, Victor Julien, brouer On Wed, 7 Feb 2018 14:19:00 +0100 Daniel Borkmann <daniel@iogearbox.net> wrote: > On 02/07/2018 01:40 PM, Jesper Dangaard Brouer wrote: > > On Tue, 6 Feb 2018 20:05:43 +0100 Daniel Borkmann <daniel@iogearbox.net> wrote: > >> On 02/06/2018 06:03 PM, Jesper Dangaard Brouer wrote: > > [...] > >>> [...] I plan to follow up and do a more complete solution later. This > >>> is a workaround to get the Suricata use-case working and also that > >>> samples/bpf/ can be loaded. > >> > >> Aside from a needed fix in any case, is there a specifc reason why Suricata > >> cannot rely on 'clang -target bpf'? Is it asm inline headers in your case? > > > > Below is the error I get when using 'clang' with '-target bpf' > > > > $ dirs > > ~/git/suricata/src/ebpf > > > > $ clang -Wall -Iinclude -O2 -D__KERNEL__ -target bpf -emit-llvm -c xdp_filter.c -o - | llc -march=bpf -filetype=obj -o xdp_filter.bpf > > In file included from xdp_filter.c:19: > > In file included from /usr/bin/../lib64/clang/4.0.1/include/stdint.h:63: > > In file included from /usr/include/stdint.h:26: > > In file included from /usr/include/bits/libc-header-start.h:33: > > In file included from /usr/include/features.h:434: > > /usr/include/gnu/stubs.h:7:11: fatal error: 'gnu/stubs-32.h' file not found > > # include <gnu/stubs-32.h> > > ^~~~~~~~~~~~~~~~ > > > > I'll leave it up to Eric Leblond to figure out that he need to change > > in the eBPF programs to make it compile with '-target bpf'. Maybe you > > can offer him some guidance here? > > > > Direct link to code: > > https://github.com/OISF/suricata/blob/master/ebpf/xdp_filter.c > > Sure, you just need glibc-devel.i686, see: > > $ clang -Wall -Iinclude -O2 -D__KERNEL__ -target bpf -emit-llvm -c xdp_filter.c -o - | llc -march=bpf -filetype=obj -o xdp_filter.bpf > In file included from xdp_filter.c:19: > In file included from /home/darkstar/llvm/build/lib/clang/7.0.0/include/stdint.h:63: > In file included from /usr/include/stdint.h:25: > In file included from /usr/include/features.h:392: > /usr/include/gnu/stubs.h:7:11: fatal error: 'gnu/stubs-32.h' file not found > # include <gnu/stubs-32.h> > ^~~~~~~~~~~~~~~~ > 1 error generated. > # yum install glibc-devel.i686 > [...] > $ clang -Wall -Iinclude -O2 -D__KERNEL__ -target bpf -emit-llvm -c xdp_filter.c -o - | llc -march=bpf -filetype=obj -o xdp_filter.bpf > $ Could you please explain why if makes a difference to install glibc-devel.i686 ? How will people compiling suricata figure out the new dependency, that on their 64-bit (x86_64) distro's they also need to install the 32-bit (i686) variant of glibc-devel ? > Alternatively, you could do something like done in selftests to provide a > dummy, see commit 1c2dd16add7e ("selftests/bpf: get rid of -D__x86_64__"). That is a funny way to workaround the problem (having an empty <gnu/stubs.h> file in include path), but it might be a better solution to avoid frustrations for people compiling suricata. An alternative solution is to NOT: #include <stdint.h> #include <string.h> And then change: uint64_t -> __u64 uint32_t -> __u32 uint16_t -> __u16 uint8_t -> __u8 -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames 2018-02-07 14:58 ` Jesper Dangaard Brouer @ 2018-02-07 16:18 ` Daniel Borkmann 2018-02-07 22:21 ` [suricata PATCH 0/3] Suricata cleanup makefile Jesper Dangaard Brouer 1 sibling, 0 replies; 20+ messages in thread From: Daniel Borkmann @ 2018-02-07 16:18 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: Alexei Starovoitov, netdev, Daniel Borkmann, wangnan0, jakub.kicinski, joe, acme, eric, yhs, Victor Julien On 02/07/2018 03:58 PM, Jesper Dangaard Brouer wrote: > On Wed, 7 Feb 2018 14:19:00 +0100 > Daniel Borkmann <daniel@iogearbox.net> wrote: >> On 02/07/2018 01:40 PM, Jesper Dangaard Brouer wrote: >>> On Tue, 6 Feb 2018 20:05:43 +0100 Daniel Borkmann <daniel@iogearbox.net> wrote: >>>> On 02/06/2018 06:03 PM, Jesper Dangaard Brouer wrote: >>> [...] >>>>> [...] I plan to follow up and do a more complete solution later. This >>>>> is a workaround to get the Suricata use-case working and also that >>>>> samples/bpf/ can be loaded. >>>> >>>> Aside from a needed fix in any case, is there a specifc reason why Suricata >>>> cannot rely on 'clang -target bpf'? Is it asm inline headers in your case? >>> >>> Below is the error I get when using 'clang' with '-target bpf' >>> >>> $ dirs >>> ~/git/suricata/src/ebpf >>> >>> $ clang -Wall -Iinclude -O2 -D__KERNEL__ -target bpf -emit-llvm -c xdp_filter.c -o - | llc -march=bpf -filetype=obj -o xdp_filter.bpf >>> In file included from xdp_filter.c:19: >>> In file included from /usr/bin/../lib64/clang/4.0.1/include/stdint.h:63: >>> In file included from /usr/include/stdint.h:26: >>> In file included from /usr/include/bits/libc-header-start.h:33: >>> In file included from /usr/include/features.h:434: >>> /usr/include/gnu/stubs.h:7:11: fatal error: 'gnu/stubs-32.h' file not found >>> # include <gnu/stubs-32.h> >>> ^~~~~~~~~~~~~~~~ >>> >>> I'll leave it up to Eric Leblond to figure out that he need to change >>> in the eBPF programs to make it compile with '-target bpf'. Maybe you >>> can offer him some guidance here? >>> >>> Direct link to code: >>> https://github.com/OISF/suricata/blob/master/ebpf/xdp_filter.c >> >> Sure, you just need glibc-devel.i686, see: >> >> $ clang -Wall -Iinclude -O2 -D__KERNEL__ -target bpf -emit-llvm -c xdp_filter.c -o - | llc -march=bpf -filetype=obj -o xdp_filter.bpf >> In file included from xdp_filter.c:19: >> In file included from /home/darkstar/llvm/build/lib/clang/7.0.0/include/stdint.h:63: >> In file included from /usr/include/stdint.h:25: >> In file included from /usr/include/features.h:392: >> /usr/include/gnu/stubs.h:7:11: fatal error: 'gnu/stubs-32.h' file not found >> # include <gnu/stubs-32.h> >> ^~~~~~~~~~~~~~~~ >> 1 error generated. >> # yum install glibc-devel.i686 >> [...] >> $ clang -Wall -Iinclude -O2 -D__KERNEL__ -target bpf -emit-llvm -c xdp_filter.c -o - | llc -march=bpf -filetype=obj -o xdp_filter.bpf > >> $ > > Could you please explain why if makes a difference to install glibc-devel.i686 ? Well, see what /usr/include/gnu/stubs.h is doing, on x86_64 it's: #if !defined __x86_64__ # include <gnu/stubs-32.h> #endif #if defined __x86_64__ && defined __LP64__ # include <gnu/stubs-64.h> #endif #if defined __x86_64__ && defined __ILP32__ # include <gnu/stubs-x32.h> #endif If you do clang -target bpf, then clang will have '__bpf__' defined instead of '__x86_64__' hence the gnu/stubs-32.h include attempt, and the workaround used in selftests with -D__x86_64__. But the -D__x86_64__ is not portable, so yeah, either dummy stubs if you need to include the headers or also other the workaround you mention below. [...] >> Alternatively, you could do something like done in selftests to provide a >> dummy, see commit 1c2dd16add7e ("selftests/bpf: get rid of -D__x86_64__"). > > That is a funny way to workaround the problem (having an empty > <gnu/stubs.h> file in include path), but it might be a better solution > to avoid frustrations for people compiling suricata. > > An alternative solution is to NOT: > #include <stdint.h> > #include <string.h> > > And then change: > uint64_t -> __u64 > uint32_t -> __u32 > uint16_t -> __u16 > uint8_t -> __u8 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [suricata PATCH 0/3] Suricata cleanup makefile 2018-02-07 14:58 ` Jesper Dangaard Brouer 2018-02-07 16:18 ` Daniel Borkmann @ 2018-02-07 22:21 ` Jesper Dangaard Brouer 2018-02-07 22:21 ` [suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account Jesper Dangaard Brouer ` (3 more replies) 1 sibling, 4 replies; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-07 22:21 UTC (permalink / raw) To: eric Cc: victor, netdev, Jesper Dangaard Brouer, yhs, Daniel Borkmann, Alexei Starovoitov Hi Eric, I've improved the Suricata ebpf makefile, in-order to avoid generating the .eh_frame sections. This required changing the code a bit, to allow using clang -target bpf. The makefile have also been improved to stop on clang compile errors, instead of generating an almost empty BPF ELF file. Could I ask you to get these changes into Suricata, through correct process for this Open Source project? --Jesper --- Jesper Dangaard Brouer (3): suricata/ebpf: take clang -target bpf include issue of stdint.h into account suricata/ebpf: compile with clang -target bpf suricata/ebpf: improving the ebpf makefile ebpf/Makefile.am | 22 ++++++++++++++++++---- ebpf/bypass_filter.c | 27 +++++++++++++-------------- ebpf/filter.c | 3 +-- ebpf/hash_func01.h | 12 ++++++------ ebpf/lb.c | 11 +++++------ ebpf/vlan_filter.c | 5 ++--- ebpf/xdp_filter.c | 42 ++++++++++++++++++++---------------------- 7 files changed, 65 insertions(+), 57 deletions(-) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account 2018-02-07 22:21 ` [suricata PATCH 0/3] Suricata cleanup makefile Jesper Dangaard Brouer @ 2018-02-07 22:21 ` Jesper Dangaard Brouer 2018-02-07 23:52 ` Eric Leblond 2018-02-07 22:21 ` [suricata PATCH 2/3] suricata/ebpf: compile with clang -target bpf Jesper Dangaard Brouer ` (2 subsequent siblings) 3 siblings, 1 reply; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-07 22:21 UTC (permalink / raw) To: eric Cc: victor, netdev, Jesper Dangaard Brouer, yhs, Daniel Borkmann, Alexei Starovoitov From: Jesper Dangaard Brouer <netoptimizer@brouer.com> This patch prepares code before enabling the clang -target bpf. The clang compiler does not like #include <stdint.h> when using '-target bpf' it will fail with: fatal error: 'gnu/stubs-32.h' file not found This is because using clang -target bpf, then clang will have '__bpf__' defined instead of '__x86_64__' hence the gnu/stubs-32.h include attempt as /usr/include/gnu/stubs.h contains, on x86_64: #if !defined __x86_64__ # include <gnu/stubs-32.h> #endif #if defined __x86_64__ && defined __LP64__ # include <gnu/stubs-64.h> #endif #if defined __x86_64__ && defined __ILP32__ # include <gnu/stubs-x32.h> #endif This can be worked around by installing the 32-bit version of glibc-devel.i686 on your distribution. But the BPF programs does not really need to include stdint.h, if converting: uint64_t -> __u64 uint32_t -> __u32 uint16_t -> __u16 uint8_t -> __u8 This patch does this type syntax conversion. Signed-off-by: Jesper Dangaard Brouer <netoptimizer@brouer.com> --- ebpf/bypass_filter.c | 27 +++++++++++++-------------- ebpf/filter.c | 3 +-- ebpf/hash_func01.h | 12 ++++++------ ebpf/lb.c | 11 +++++------ ebpf/vlan_filter.c | 5 ++--- ebpf/xdp_filter.c | 42 ++++++++++++++++++++---------------------- 6 files changed, 47 insertions(+), 53 deletions(-) diff --git a/ebpf/bypass_filter.c b/ebpf/bypass_filter.c index d2ce12aa1cd5..be81032b16cf 100644 --- a/ebpf/bypass_filter.c +++ b/ebpf/bypass_filter.c @@ -15,7 +15,6 @@ * 02110-1301, USA. */ -#include <stdint.h> #include <stddef.h> #include <linux/bpf.h> @@ -51,9 +50,9 @@ struct flowv6_keys { } __attribute__((__aligned__(8))); struct pair { - uint64_t time; - uint64_t packets; - uint64_t bytes; + __u64 time; + __u64 packets; + __u64 bytes; } __attribute__((__aligned__(8))); struct bpf_map_def SEC("maps") flow_table_v4 = { @@ -77,10 +76,10 @@ struct bpf_map_def SEC("maps") flow_table_v6 = { */ static __always_inline int ipv4_filter(struct __sk_buff *skb) { - uint32_t nhoff, verlen; + __u32 nhoff, verlen; struct flowv4_keys tuple; struct pair *value; - uint16_t port; + __u16 port; nhoff = skb->cb[0]; @@ -107,8 +106,8 @@ static __always_inline int ipv4_filter(struct __sk_buff *skb) #if 0 if ((tuple.port16[0] == 22) || (tuple.port16[1] == 22)) { - uint16_t sp = tuple.port16[0]; - //uint16_t dp = tuple.port16[1]; + __u16 sp = tuple.port16[0]; + //__u16 dp = tuple.port16[1]; char fmt[] = "Parsed SSH flow: %u %d -> %u\n"; bpf_trace_printk(fmt, sizeof(fmt), tuple.src, sp, tuple.dst); } @@ -118,8 +117,8 @@ static __always_inline int ipv4_filter(struct __sk_buff *skb) if (value) { #if 0 { - uint16_t sp = tuple.port16[0]; - //uint16_t dp = tuple.port16[1]; + __u16 sp = tuple.port16[0]; + //__u16 dp = tuple.port16[1]; char bfmt[] = "Found flow: %u %d -> %u\n"; bpf_trace_printk(bfmt, sizeof(bfmt), tuple.src, sp, tuple.dst); } @@ -139,11 +138,11 @@ static __always_inline int ipv4_filter(struct __sk_buff *skb) */ static __always_inline int ipv6_filter(struct __sk_buff *skb) { - uint32_t nhoff; - uint8_t nhdr; + __u32 nhoff; + __u8 nhdr; struct flowv6_keys tuple; struct pair *value; - uint16_t port; + __u16 port; nhoff = skb->cb[0]; @@ -223,4 +222,4 @@ int SEC("filter") hashfilter(struct __sk_buff *skb) { char __license[] SEC("license") = "GPL"; -uint32_t __version SEC("version") = LINUX_VERSION_CODE; +__u32 __version SEC("version") = LINUX_VERSION_CODE; diff --git a/ebpf/filter.c b/ebpf/filter.c index 1976ffcf188f..4fe95d4fb005 100644 --- a/ebpf/filter.c +++ b/ebpf/filter.c @@ -15,7 +15,6 @@ * 02110-1301, USA. */ -#include <stdint.h> #include <stddef.h> #include <linux/bpf.h> @@ -56,4 +55,4 @@ int SEC("filter") hashfilter(struct __sk_buff *skb) { char __license[] SEC("license") = "GPL"; -uint32_t __version SEC("version") = LINUX_VERSION_CODE; +__u32 __version SEC("version") = LINUX_VERSION_CODE; diff --git a/ebpf/hash_func01.h b/ebpf/hash_func01.h index 060346f67a6a..38255812e376 100644 --- a/ebpf/hash_func01.h +++ b/ebpf/hash_func01.h @@ -4,12 +4,12 @@ * From: http://www.azillionmonkeys.com/qed/hash.html */ -#define get16bits(d) (*((const uint16_t *) (d))) +#define get16bits(d) (*((const __u16 *) (d))) static __always_inline -uint32_t SuperFastHash (const char *data, int len, uint32_t initval) { - uint32_t hash = initval; - uint32_t tmp; +__u32 SuperFastHash (const char *data, int len, __u32 initval) { + __u32 hash = initval; + __u32 tmp; int rem; if (len <= 0 || data == NULL) return 0; @@ -23,7 +23,7 @@ uint32_t SuperFastHash (const char *data, int len, uint32_t initval) { hash += get16bits (data); tmp = (get16bits (data+2) << 11) ^ hash; hash = (hash << 16) ^ tmp; - data += 2*sizeof (uint16_t); + data += 2*sizeof (__u16); hash += hash >> 11; } @@ -31,7 +31,7 @@ uint32_t SuperFastHash (const char *data, int len, uint32_t initval) { switch (rem) { case 3: hash += get16bits (data); hash ^= hash << 16; - hash ^= ((signed char)data[sizeof (uint16_t)]) << 18; + hash ^= ((signed char)data[sizeof (__u16)]) << 18; hash += hash >> 11; break; case 2: hash += get16bits (data); diff --git a/ebpf/lb.c b/ebpf/lb.c index 14974784d925..7551781c5f80 100644 --- a/ebpf/lb.c +++ b/ebpf/lb.c @@ -15,7 +15,6 @@ * 02110-1301, USA. */ -#include <stdint.h> #include <stddef.h> #include <linux/bpf.h> @@ -36,8 +35,8 @@ static __always_inline int ipv4_hash(struct __sk_buff *skb) { - uint32_t nhoff; - uint32_t src, dst; + __u32 nhoff; + __u32 src, dst; nhoff = skb->cb[0]; src = load_word(skb, nhoff + offsetof(struct iphdr, saddr)); @@ -54,8 +53,8 @@ static __always_inline int ipv4_hash(struct __sk_buff *skb) static __always_inline int ipv6_hash(struct __sk_buff *skb) { - uint32_t nhoff; - uint32_t src, dst, hash; + __u32 nhoff; + __u32 src, dst, hash; nhoff = skb->cb[0]; hash = 0; @@ -107,4 +106,4 @@ char __license[] __section("license") = "GPL"; /* libbpf needs version section to check sync of eBPF code and kernel * but socket filter don't need it */ -uint32_t __version __section("version") = LINUX_VERSION_CODE; +__u32 __version __section("version") = LINUX_VERSION_CODE; diff --git a/ebpf/vlan_filter.c b/ebpf/vlan_filter.c index 5c3865273d3c..d797b94bfbd5 100644 --- a/ebpf/vlan_filter.c +++ b/ebpf/vlan_filter.c @@ -15,7 +15,6 @@ * 02110-1301, USA. */ -#include <stdint.h> #include <stddef.h> #include <linux/bpf.h> @@ -24,7 +23,7 @@ #define LINUX_VERSION_CODE 263682 int SEC("filter") hashfilter(struct __sk_buff *skb) { - uint16_t vlan_id = skb->vlan_tci & 0x0fff; + __u16 vlan_id = skb->vlan_tci & 0x0fff; /* accept VLAN 2 and 4 and drop the rest */ switch (vlan_id) { case 2: @@ -38,4 +37,4 @@ int SEC("filter") hashfilter(struct __sk_buff *skb) { char __license[] SEC("license") = "GPL"; -uint32_t __version SEC("version") = LINUX_VERSION_CODE; +__u32 __version SEC("version") = LINUX_VERSION_CODE; diff --git a/ebpf/xdp_filter.c b/ebpf/xdp_filter.c index 9d3ff0910c30..3ee9dd0b114c 100644 --- a/ebpf/xdp_filter.c +++ b/ebpf/xdp_filter.c @@ -16,8 +16,6 @@ */ #define KBUILD_MODNAME "foo" -#include <stdint.h> -#include <string.h> #include <stddef.h> #include <linux/bpf.h> @@ -70,9 +68,9 @@ struct flowv6_keys { } __attribute__((__aligned__(8))); struct pair { - uint64_t time; - uint64_t packets; - uint64_t bytes; + __u64 time; + __u64 packets; + __u64 bytes; } __attribute__((__aligned__(8))); struct bpf_map_def SEC("maps") flow_table_v4 = { @@ -128,7 +126,7 @@ struct bpf_map_def SEC("maps") tx_peer_int = { }; static __always_inline int get_sport(void *trans_data, void *data_end, - uint8_t protocol) + __u8 protocol) { struct tcphdr *th; struct udphdr *uh; @@ -150,7 +148,7 @@ static __always_inline int get_sport(void *trans_data, void *data_end, } static __always_inline int get_dport(void *trans_data, void *data_end, - uint8_t protocol) + __u8 protocol) { struct tcphdr *th; struct udphdr *uh; @@ -178,12 +176,12 @@ static int __always_inline filter_ipv4(void *data, __u64 nh_off, void *data_end) int sport; struct flowv4_keys tuple; struct pair *value; - uint32_t key0 = 0; + __u32 key0 = 0; #if BUILD_CPUMAP - uint32_t cpu_dest; - uint32_t *cpu_max = bpf_map_lookup_elem(&cpus_count, &key0); - uint32_t *cpu_selected; - uint32_t cpu_hash; + __u32 cpu_dest; + __u32 *cpu_max = bpf_map_lookup_elem(&cpus_count, &key0); + __u32 *cpu_selected; + __u32 cpu_hash; #endif int *iface_peer; int tx_port = 0; @@ -191,7 +189,7 @@ static int __always_inline filter_ipv4(void *data, __u64 nh_off, void *data_end) if ((void *)(iph + 1) > data_end) return XDP_PASS; - tuple.ip_proto = (uint32_t) iph->protocol; + tuple.ip_proto = (__u32) iph->protocol; tuple.src = iph->saddr; tuple.dst = iph->daddr; @@ -203,8 +201,8 @@ static int __always_inline filter_ipv4(void *data, __u64 nh_off, void *data_end) if (sport == -1) return XDP_PASS; - tuple.port16[0] = (uint16_t)sport; - tuple.port16[1] = (uint16_t)dport; + tuple.port16[0] = (__u16)sport; + tuple.port16[1] = (__u16)dport; value = bpf_map_lookup_elem(&flow_table_v4, &tuple); #if 0 { @@ -260,12 +258,12 @@ static int __always_inline filter_ipv6(void *data, __u64 nh_off, void *data_end) int sport; struct flowv6_keys tuple; struct pair *value; - uint32_t key0 = 0; + __u32 key0 = 0; #if BUILD_CPUMAP - uint32_t cpu_dest; + __u32 cpu_dest; int *cpu_max = bpf_map_lookup_elem(&cpus_count, &key0); - uint32_t *cpu_selected; - uint32_t cpu_hash; + __u32 *cpu_selected; + __u32 cpu_hash; #endif int tx_port = 0; int *iface_peer; @@ -336,8 +334,8 @@ int SEC("xdp") xdp_hashfilter(struct xdp_md *ctx) void *data = (void *)(long)ctx->data; struct ethhdr *eth = data; int rc = XDP_PASS; - uint16_t h_proto; - uint64_t nh_off; + __u16 h_proto; + __u64 nh_off; nh_off = sizeof(*eth); if (data + nh_off > data_end) @@ -376,4 +374,4 @@ int SEC("xdp") xdp_hashfilter(struct xdp_md *ctx) char __license[] SEC("license") = "GPL"; -uint32_t __version SEC("version") = LINUX_VERSION_CODE; +__u32 __version SEC("version") = LINUX_VERSION_CODE; ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account 2018-02-07 22:21 ` [suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account Jesper Dangaard Brouer @ 2018-02-07 23:52 ` Eric Leblond 2018-02-08 8:42 ` Jesper Dangaard Brouer 0 siblings, 1 reply; 20+ messages in thread From: Eric Leblond @ 2018-02-07 23:52 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: victor, netdev, yhs, Daniel Borkmann, Alexei Starovoitov Hi, On Wed, 2018-02-07 at 23:21 +0100, Jesper Dangaard Brouer wrote: > From: Jesper Dangaard Brouer <netoptimizer@brouer.com> > > This patch prepares code before enabling the clang -target bpf. > > The clang compiler does not like #include <stdint.h> when > using '-target bpf' it will fail with: > > fatal error: 'gnu/stubs-32.h' file not found ... > This can be worked around by installing the 32-bit version of > glibc-devel.i686 on your distribution. > > But the BPF programs does not really need to include stdint.h, > if converting: > uint64_t -> __u64 > uint32_t -> __u32 > uint16_t -> __u16 > uint8_t -> __u8 > > This patch does this type syntax conversion. There is an issue for system like Debian because they don't have a asm/types.h in the include path if the architecture is not defined which is the case due to target bpf. This results in: clang-5.0 -Wall -Iinclude -O2 \ -D__KERNEL__ -D__ASM_SYSREG_H \ -target bpf -S -emit-llvm vlan_filter.c -o vlan_filter.ll In file included from vlan_filter.c:19: In file included from include/linux/bpf.h:11: /usr/include/linux/types.h:5:10: fatal error: 'asm/types.h' file not found #include <asm/types.h> ^~~~~~~~~~~~~ 1 error generated. Makefile:523: recipe for target 'vlan_filter.bpf' failed To go into details, the Debian package providing the 'asm/typs.h' include is the the headers or linux-libc-dev. But this package comes with a flavor and thus we have a prefix: linux-libc-dev:amd64: /usr/include/x86_64-linux-gnu/asm/types.h "Fun" part here is that if you build a debian package of the via make in Linux tree then the linux-libc-dev package is correct. So I propose the following patch that fixes the issue for me: diff --git a/ebpf/Makefile.am b/ebpf/Makefile.am index 89a3304e9..712b05343 100644 --- a/ebpf/Makefile.am +++ b/ebpf/Makefile.am @@ -16,6 +16,7 @@ all: $(BPF_TARGETS) $(BPF_TARGETS): %.bpf: %.c # From C-code to LLVM-IR format suffix .ll (clang -S -emit-llvm) ${CLANG} -Wall $(BPF_CFLAGS) -O2 \ + -I/usr/include/$(host_cpu)-$(host_os)/ \ -D__KERNEL__ -D__ASM_SYSREG_H \ -target bpf -S -emit-llvm $< -o ${@:.bpf=.ll} # From LLVM-IR to BPF-bytecode in ELF-obj file Let me know if it is ok for you. Best regards, -- Eric Leblond <eric@regit.org> ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account 2018-02-07 23:52 ` Eric Leblond @ 2018-02-08 8:42 ` Jesper Dangaard Brouer 0 siblings, 0 replies; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-08 8:42 UTC (permalink / raw) To: Eric Leblond Cc: victor, netdev, yhs, Daniel Borkmann, Alexei Starovoitov, brouer On Thu, 08 Feb 2018 00:52:09 +0100 Eric Leblond <eric@regit.org> wrote: > Hi, > > On Wed, 2018-02-07 at 23:21 +0100, Jesper Dangaard Brouer wrote: > > From: Jesper Dangaard Brouer <netoptimizer@brouer.com> > > > > This patch prepares code before enabling the clang -target bpf. > > > > The clang compiler does not like #include <stdint.h> when > > using '-target bpf' it will fail with: > > > > fatal error: 'gnu/stubs-32.h' file not found > ... > > This can be worked around by installing the 32-bit version of > > glibc-devel.i686 on your distribution. > > > > But the BPF programs does not really need to include stdint.h, > > if converting: > > uint64_t -> __u64 > > uint32_t -> __u32 > > uint16_t -> __u16 > > uint8_t -> __u8 > > > > This patch does this type syntax conversion. > > There is an issue for system like Debian because they don't have a > asm/types.h in the include path if the architecture is not defined > which is the case due to target bpf. This results in: > > clang-5.0 -Wall -Iinclude -O2 \ > -D__KERNEL__ -D__ASM_SYSREG_H \ > -target bpf -S -emit-llvm vlan_filter.c -o vlan_filter.ll > In file included from vlan_filter.c:19: > In file included from include/linux/bpf.h:11: > /usr/include/linux/types.h:5:10: fatal error: 'asm/types.h' file not > found > #include <asm/types.h> > ^~~~~~~~~~~~~ > 1 error generated. > Makefile:523: recipe for target 'vlan_filter.bpf' failed > > To go into details, the Debian package providing the 'asm/typs.h' > include is the the headers or linux-libc-dev. But this package comes > with a flavor and thus we have a prefix: > linux-libc-dev:amd64: /usr/include/x86_64-linux-gnu/asm/types.h Oh, the joy of distro choices. > "Fun" part here is that if you build a debian package of the via make > in Linux tree then the linux-libc-dev package is correct. > > So I propose the following patch that fixes the issue for me: > > diff --git a/ebpf/Makefile.am b/ebpf/Makefile.am > index 89a3304e9..712b05343 100644 > --- a/ebpf/Makefile.am > +++ b/ebpf/Makefile.am > @@ -16,6 +16,7 @@ all: $(BPF_TARGETS) > $(BPF_TARGETS): %.bpf: %.c > # From C-code to LLVM-IR format suffix .ll (clang -S -emit-llvm) > ${CLANG} -Wall $(BPF_CFLAGS) -O2 \ > + -I/usr/include/$(host_cpu)-$(host_os)/ \ Cool solution. These variables originate from configure/automake. Would it be more technical correct to use(?): $(build_cpu)-$(build_os) I verified that the variables are the same (notice 'make -p' trick): $ make -p | egrep '_os' build_os = linux-gnu host_os = linux-gnu $ make -p | egrep '_cpu' host_cpu = x86_64 build_cpu = x86_64 > -D__KERNEL__ -D__ASM_SYSREG_H \ > -target bpf -S -emit-llvm $< -o ${@:.bpf=.ll} > # From LLVM-IR to BPF-bytecode in ELF-obj file > > Let me know if it is ok for you. I'm fine with this fix. I wonder if we should check other distros? -- Best regards, Jesper Dangaard Brouer MSc.CS, Principal Kernel Engineer at Red Hat LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 20+ messages in thread
* [suricata PATCH 2/3] suricata/ebpf: compile with clang -target bpf 2018-02-07 22:21 ` [suricata PATCH 0/3] Suricata cleanup makefile Jesper Dangaard Brouer 2018-02-07 22:21 ` [suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account Jesper Dangaard Brouer @ 2018-02-07 22:21 ` Jesper Dangaard Brouer 2018-02-07 22:21 ` [suricata PATCH 3/3] suricata/ebpf: improving the ebpf makefile Jesper Dangaard Brouer 2018-02-07 22:38 ` [suricata PATCH 0/3] Suricata cleanup makefile Eric Leblond 3 siblings, 0 replies; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-07 22:21 UTC (permalink / raw) To: eric Cc: victor, netdev, Jesper Dangaard Brouer, yhs, Daniel Borkmann, Alexei Starovoitov From: Jesper Dangaard Brouer <netoptimizer@brouer.com> Enable compiling eBPF programs with clang -target bpf. This is mostly to workaround a bug in libbpf, where clang > ver 4.0.0 generates some ELF sections (.eh_frame) when -target bpf is NOT specified, and libbpf fails loading such files. Notice libbpf is provided by the kernel, and in kernel v4.16 the library will contain the needed function for attatching to the XDP hook. Kernel commit 949abbe88436 ("libbpf: add function to setup XDP") https://git.kernel.org/torvalds/c/949abbe88436 As it looks now, the library fix will not get into kernel v4.16. Thus, we need this workaround for Suricata. In-order to recommend people installing the library libbpf from kernel v4.16. Signed-off-by: Jesper Dangaard Brouer <netoptimizer@brouer.com> --- ebpf/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ebpf/Makefile.am b/ebpf/Makefile.am index 66db35f80c7e..d0ee44ae668e 100644 --- a/ebpf/Makefile.am +++ b/ebpf/Makefile.am @@ -6,7 +6,7 @@ BPF_CFLAGS = -Iinclude all: lb.bpf filter.bpf bypass_filter.bpf xdp_filter.bpf vlan_filter.bpf %.bpf: %.c - ${CC} -Wall $(BPF_CFLAGS) -O2 -D__KERNEL__ -D__ASM_SYSREG_H -emit-llvm -c $< -o - | ${LLC} -march=bpf -filetype=obj -o $@ + ${CC} -Wall $(BPF_CFLAGS) -O2 -D__KERNEL__ -D__ASM_SYSREG_H -target bpf -emit-llvm -c $< -o - | ${LLC} -march=bpf -filetype=obj -o $@ CLEANFILES = *.bpf ^ permalink raw reply related [flat|nested] 20+ messages in thread
* [suricata PATCH 3/3] suricata/ebpf: improving the ebpf makefile 2018-02-07 22:21 ` [suricata PATCH 0/3] Suricata cleanup makefile Jesper Dangaard Brouer 2018-02-07 22:21 ` [suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account Jesper Dangaard Brouer 2018-02-07 22:21 ` [suricata PATCH 2/3] suricata/ebpf: compile with clang -target bpf Jesper Dangaard Brouer @ 2018-02-07 22:21 ` Jesper Dangaard Brouer 2018-02-07 22:38 ` [suricata PATCH 0/3] Suricata cleanup makefile Eric Leblond 3 siblings, 0 replies; 20+ messages in thread From: Jesper Dangaard Brouer @ 2018-02-07 22:21 UTC (permalink / raw) To: eric Cc: victor, netdev, Jesper Dangaard Brouer, yhs, Daniel Borkmann, Alexei Starovoitov From: Jesper Dangaard Brouer <netoptimizer@brouer.com> The current ebpf/Makefile.am have the problem that clang compile errors still result in an ELF .bpf output file. This is obviously problematic as the the problem is first seen runtime when loading the bpf-prog. This this is cause by the uses of a pipe from clang to llc. To address this problem, split up the clang and llc invocations up into two separate commands, to get proper reaction based on the compiler exit code. The clang compiler is used as a frontend (+ optimizer) and instructed (via -S -emit-llvm) to generate LLVM IR (Intermediate Representation) with suffix .ll. The LLVM llc command is used as a compiler backend taking IR and producing BPF machine bytecode, and storing this into a ELF object. In the last step the IR .ll suffix code it removed. The official documentation of the IR language: http://llvm.org/docs/LangRef.html Also fix the previous make portability warning: '%-style pattern rules are a GNU make extension' I instead use some static pattern rules: https://www.gnu.org/software/make/manual/html_node/Static-Usage.html Signed-off-by: Jesper Dangaard Brouer <netoptimizer@brouer.com> --- ebpf/Makefile.am | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/ebpf/Makefile.am b/ebpf/Makefile.am index d0ee44ae668e..89a3304e953b 100644 --- a/ebpf/Makefile.am +++ b/ebpf/Makefile.am @@ -3,11 +3,25 @@ if BUILD_EBPF # Maintaining a local copy of UAPI linux/bpf.h BPF_CFLAGS = -Iinclude -all: lb.bpf filter.bpf bypass_filter.bpf xdp_filter.bpf vlan_filter.bpf +CLANG = ${CC} -%.bpf: %.c - ${CC} -Wall $(BPF_CFLAGS) -O2 -D__KERNEL__ -D__ASM_SYSREG_H -target bpf -emit-llvm -c $< -o - | ${LLC} -march=bpf -filetype=obj -o $@ +BPF_TARGETS = lb.bpf +BPF_TARGETS += filter.bpf +BPF_TARGETS += bypass_filter.bpf +BPF_TARGETS += xdp_filter.bpf +BPF_TARGETS += vlan_filter.bpf -CLEANFILES = *.bpf +all: $(BPF_TARGETS) + +$(BPF_TARGETS): %.bpf: %.c +# From C-code to LLVM-IR format suffix .ll (clang -S -emit-llvm) + ${CLANG} -Wall $(BPF_CFLAGS) -O2 \ + -D__KERNEL__ -D__ASM_SYSREG_H \ + -target bpf -S -emit-llvm $< -o ${@:.bpf=.ll} +# From LLVM-IR to BPF-bytecode in ELF-obj file + ${LLC} -march=bpf -filetype=obj ${@:.bpf=.ll} -o $@ + ${RM} ${@:.bpf=.ll} + +CLEANFILES = *.bpf *.ll endif ^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [suricata PATCH 0/3] Suricata cleanup makefile 2018-02-07 22:21 ` [suricata PATCH 0/3] Suricata cleanup makefile Jesper Dangaard Brouer ` (2 preceding siblings ...) 2018-02-07 22:21 ` [suricata PATCH 3/3] suricata/ebpf: improving the ebpf makefile Jesper Dangaard Brouer @ 2018-02-07 22:38 ` Eric Leblond 3 siblings, 0 replies; 20+ messages in thread From: Eric Leblond @ 2018-02-07 22:38 UTC (permalink / raw) To: Jesper Dangaard Brouer Cc: victor, netdev, yhs, Daniel Borkmann, Alexei Starovoitov Hello Jesper, On Wed, 2018-02-07 at 23:21 +0100, Jesper Dangaard Brouer wrote: > Hi Eric, > > I've improved the Suricata ebpf makefile, in-order to avoid > generating > the .eh_frame sections. This required changing the code a bit, to > allow using clang -target bpf. > > The makefile have also been improved to stop on clang compile errors, > instead of generating an almost empty BPF ELF file. > > Could I ask you to get these changes into Suricata, through correct > process for this Open Source project? Sure, I'm reviewing the code, testing it and I will do a Pull Request on github. Thanks a lot for that, that's a really valuable help! BR, -- Eric Leblond <eric@regit.org> ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-02-08 8:42 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-02-06 14:54 [bpf-next V2 PATCH 0/5] tools/libbpf improvements and selftests Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 1/5] bpf: Sync kernel ABI header with tooling header for bpf_common.h Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 2/5] tools/libbpf: improve the pr_debug statements to contain section numbers Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 3/5] selftests/bpf: add test program for loading BPF ELF files Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 4/5] selftests/bpf: add selftest that use test_libbpf_open Jesper Dangaard Brouer 2018-02-06 14:54 ` [bpf-next V2 PATCH 5/5] tools/libbpf: handle issues with bpf ELF objects containing .eh_frames Jesper Dangaard Brouer 2018-02-06 16:00 ` Alexei Starovoitov 2018-02-06 17:03 ` Jesper Dangaard Brouer 2018-02-06 19:05 ` Daniel Borkmann 2018-02-07 12:40 ` Jesper Dangaard Brouer 2018-02-07 13:19 ` Daniel Borkmann 2018-02-07 14:58 ` Jesper Dangaard Brouer 2018-02-07 16:18 ` Daniel Borkmann 2018-02-07 22:21 ` [suricata PATCH 0/3] Suricata cleanup makefile Jesper Dangaard Brouer 2018-02-07 22:21 ` [suricata PATCH 1/3] suricata/ebpf: take clang -target bpf include issue of stdint.h into account Jesper Dangaard Brouer 2018-02-07 23:52 ` Eric Leblond 2018-02-08 8:42 ` Jesper Dangaard Brouer 2018-02-07 22:21 ` [suricata PATCH 2/3] suricata/ebpf: compile with clang -target bpf Jesper Dangaard Brouer 2018-02-07 22:21 ` [suricata PATCH 3/3] suricata/ebpf: improving the ebpf makefile Jesper Dangaard Brouer 2018-02-07 22:38 ` [suricata PATCH 0/3] Suricata cleanup makefile Eric Leblond
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).