* [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes
@ 2020-07-03 9:51 Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 1/9] bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object Jiri Olsa
` (8 more replies)
0 siblings, 9 replies; 22+ messages in thread
From: Jiri Olsa @ 2020-07-03 9:51 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, bpf, Song Liu, Yonghong Song, Martin KaFai Lau,
David Miller, John Fastabend, Wenbo Zhang, KP Singh,
Andrii Nakryiko, Brendan Gregg, Florent Revest, Al Viro
hi,
this patchset does preparation work for adding d_path helper,
which still needs more work, but the initial set of patches
is ready and useful to have.
This patchset adds:
- support to generate BTF ID lists that are resolved during
kernel linking and usable within kernel code with following
macros:
BTF_ID_LIST(bpf_skb_output_btf_ids)
BTF_ID(struct, sk_buff)
and access it in kernel code via:
extern u32 bpf_skb_output_btf_ids[];
- resolve_btfids tool that scans elf object for .BTF_ids
section and resolves its symbols with BTF ID values
- resolving of bpf_ctx_convert struct and several other
objects with BTF_ID_LIST
v5 changes:
- added acks to patches I did not change in this version
- split the original patchset into 2 parts and sending the first one
where the support to define BTF list is added, the BTF set support
will be posted later on [Andrii]
- used u32 instead of int in btf_ids.h [Andrii]
- changed the btf_ids.h define guard [Andrii]
- added resolve_btfids_clean target [Andrii]
- moved resolve_btfids test into prog_tests suite [Andrii]
- fixed BTF type iteration index in test [Andrii]
- removed btf_id checks in bpf_ctx_convert resolve code [Yonghong]
- removed WARN_ON_ONCE from btf_resolve_helper_id [Yonghong]
- added BTF_ID_UNUSED macro [Yonghong]
- fixed out of the tree build failure [0day bot]
Also available at:
https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
bpf/d_path
thanks,
jirka
---
Jiri Olsa (9):
bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object
bpf: Compile resolve_btfids tool at kernel compilation start
bpf: Add BTF_ID_LIST/BTF_ID/BTF_ID_UNUSED macros
bpf: Resolve BTF IDs in vmlinux image
bpf: Remove btf_id helpers resolving
bpf: Use BTF_ID to resolve bpf_ctx_convert struct
bpf: Add info about .BTF.ids section to btf.rst
tools headers: Adopt verbatim copy of btf_ids.h from kernel sources
selftests/bpf: Add test for resolve_btfids
Documentation/bpf/btf.rst | 36 +++++
Makefile | 25 +++-
include/asm-generic/vmlinux.lds.h | 4 +
include/linux/btf_ids.h | 87 ++++++++++++
kernel/bpf/btf.c | 103 ++------------
kernel/trace/bpf_trace.c | 9 +-
net/core/filter.c | 9 +-
scripts/link-vmlinux.sh | 6 +
tools/Makefile | 3 +
tools/bpf/Makefile | 9 +-
tools/bpf/resolve_btfids/Build | 26 ++++
tools/bpf/resolve_btfids/Makefile | 77 +++++++++++
tools/bpf/resolve_btfids/main.c | 716 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
tools/include/linux/btf_ids.h | 87 ++++++++++++
tools/include/linux/compiler.h | 4 +
tools/testing/selftests/bpf/Makefile | 22 ++-
tools/testing/selftests/bpf/prog_tests/resolve_btfids.c | 170 +++++++++++++++++++++++
17 files changed, 1289 insertions(+), 104 deletions(-)
create mode 100644 include/linux/btf_ids.h
create mode 100644 tools/bpf/resolve_btfids/Build
create mode 100644 tools/bpf/resolve_btfids/Makefile
create mode 100644 tools/bpf/resolve_btfids/main.c
create mode 100644 tools/include/linux/btf_ids.h
create mode 100644 tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 bpf-next 1/9] bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object
2020-07-03 9:51 [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes Jiri Olsa
@ 2020-07-03 9:51 ` Jiri Olsa
2020-07-07 0:34 ` Andrii Nakryiko
2020-07-03 9:51 ` [PATCH v5 bpf-next 2/9] bpf: Compile resolve_btfids tool at kernel compilation start Jiri Olsa
` (7 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Jiri Olsa @ 2020-07-03 9:51 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, bpf, Song Liu, Yonghong Song, Martin KaFai Lau,
David Miller, John Fastabend, Wenbo Zhang, KP Singh,
Andrii Nakryiko, Brendan Gregg, Florent Revest, Al Viro
The resolve_btfids tool scans elf object for .BTF_ids section
and resolves its symbols with BTF ID values.
It will be used to during linking time to resolve arrays of BTF
ID values used in verifier, so these IDs do not need to be
resolved in runtime.
The expected layout of .BTF_ids section is described in btfid.c
header. Related kernel changes are coming in following changes.
Build issue reported by 0-DAY CI Kernel Test Service.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/bpf/resolve_btfids/Build | 26 ++
tools/bpf/resolve_btfids/Makefile | 77 ++++
tools/bpf/resolve_btfids/main.c | 716 ++++++++++++++++++++++++++++++
3 files changed, 819 insertions(+)
create mode 100644 tools/bpf/resolve_btfids/Build
create mode 100644 tools/bpf/resolve_btfids/Makefile
create mode 100644 tools/bpf/resolve_btfids/main.c
diff --git a/tools/bpf/resolve_btfids/Build b/tools/bpf/resolve_btfids/Build
new file mode 100644
index 000000000000..c7318cc55341
--- /dev/null
+++ b/tools/bpf/resolve_btfids/Build
@@ -0,0 +1,26 @@
+resolve_btfids-y += main.o
+resolve_btfids-y += rbtree.o
+resolve_btfids-y += zalloc.o
+resolve_btfids-y += string.o
+resolve_btfids-y += ctype.o
+resolve_btfids-y += str_error_r.o
+
+$(OUTPUT)rbtree.o: ../../lib/rbtree.c FORCE
+ $(call rule_mkdir)
+ $(call if_changed_dep,cc_o_c)
+
+$(OUTPUT)zalloc.o: ../../lib/zalloc.c FORCE
+ $(call rule_mkdir)
+ $(call if_changed_dep,cc_o_c)
+
+$(OUTPUT)string.o: ../../lib/string.c FORCE
+ $(call rule_mkdir)
+ $(call if_changed_dep,cc_o_c)
+
+$(OUTPUT)ctype.o: ../../lib/ctype.c FORCE
+ $(call rule_mkdir)
+ $(call if_changed_dep,cc_o_c)
+
+$(OUTPUT)str_error_r.o: ../../lib/str_error_r.c FORCE
+ $(call rule_mkdir)
+ $(call if_changed_dep,cc_o_c)
diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
new file mode 100644
index 000000000000..948378ca73d4
--- /dev/null
+++ b/tools/bpf/resolve_btfids/Makefile
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: GPL-2.0-only
+include ../../scripts/Makefile.include
+
+ifeq ($(srctree),)
+srctree := $(patsubst %/,%,$(dir $(CURDIR)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+endif
+
+ifeq ($(V),1)
+ Q =
+ msg =
+else
+ Q = @
+ msg = @printf ' %-8s %s%s\n' "$(1)" "$(notdir $(2))" "$(if $(3), $(3))";
+ MAKEFLAGS=--no-print-directory
+endif
+
+OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
+
+LIBBPF_SRC := $(srctree)/tools/lib/bpf/
+SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
+
+BPFOBJ := $(OUTPUT)/libbpf.a
+SUBCMDOBJ := $(OUTPUT)/libsubcmd.a
+
+BINARY := $(OUTPUT)/resolve_btfids
+BINARY_IN := $(BINARY)-in.o
+
+all: $(BINARY)
+
+$(OUTPUT):
+ $(call msg,MKDIR,,$@)
+ $(Q)mkdir -p $(OUTPUT)
+
+$(SUBCMDOBJ): fixdep FORCE
+ $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(OUTPUT)
+
+$(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(OUTPUT)
+ $(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(abspath $(dir $@))/ $(abspath $@)
+
+CFLAGS := -g \
+ -I$(srctree)/tools/include \
+ -I$(srctree)/tools/include/uapi \
+ -I$(LIBBPF_SRC) \
+ -I$(SUBCMD_SRC)
+
+LIBS = -lelf -lz
+
+export srctree OUTPUT CFLAGS Q
+include $(srctree)/tools/build/Makefile.include
+
+$(BINARY_IN): fixdep FORCE
+ $(Q)$(MAKE) $(build)=resolve_btfids
+
+$(BINARY): $(BPFOBJ) $(SUBCMDOBJ) $(BINARY_IN)
+ $(call msg,LINK,$@)
+ $(Q)$(CC) $(BINARY_IN) $(LDFLAGS) -o $@ $(BPFOBJ) $(SUBCMDOBJ) $(LIBS)
+
+libsubcmd-clean:
+ $(Q)$(MAKE) -C $(SUBCMD_SRC) OUTPUT=$(OUTPUT) clean
+
+libbpf-clean:
+ $(Q)$(MAKE) -C $(LIBBPF_SRC) OUTPUT=$(OUTPUT) clean
+
+clean: libsubcmd-clean libbpf-clean fixdep-clean
+ $(call msg,CLEAN,$(BINARY))
+ $(Q)$(RM) -f $(BINARY); \
+ find $(if $(OUTPUT),$(OUTPUT),.) -name \*.o -or -name \*.o.cmd -or -name \*.o.d | xargs $(RM)
+
+tags:
+ $(call msg,GEN,,tags)
+ $(Q)ctags -R . $(LIBBPF_SRC) $(SUBCMD_SRC)
+
+FORCE:
+
+.PHONY: all FORCE clean tags
diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
new file mode 100644
index 000000000000..030c3da8a8e5
--- /dev/null
+++ b/tools/bpf/resolve_btfids/main.c
@@ -0,0 +1,716 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+
+/*
+ * resolve_btfids scans Elf object for .BTF_ids section and resolves
+ * its symbols with BTF ID values.
+ *
+ * Each symbol points to 4 bytes data and is expected to have
+ * following name syntax:
+ *
+ * __BTF_ID__<type>__<symbol>[__<id>]
+ *
+ * type is:
+ *
+ * func - lookup BTF_KIND_FUNC symbol with <symbol> name
+ * and store its ID into the data:
+ *
+ * __BTF_ID__func__vfs_close__1:
+ * .zero 4
+ *
+ * struct - lookup BTF_KIND_STRUCT symbol with <symbol> name
+ * and store its ID into the data:
+ *
+ * __BTF_ID__struct__sk_buff__1:
+ * .zero 4
+ *
+ * union - lookup BTF_KIND_UNION symbol with <symbol> name
+ * and store its ID into the data:
+ *
+ * __BTF_ID__union__thread_union__1:
+ * .zero 4
+ *
+ * typedef - lookup BTF_KIND_TYPEDEF symbol with <symbol> name
+ * and store its ID into the data:
+ *
+ * __BTF_ID__typedef__pid_t__1:
+ * .zero 4
+ *
+ * set - store symbol size into first 4 bytes and sort following
+ * ID list
+ *
+ * __BTF_ID__set__list:
+ * .zero 4
+ * list:
+ * __BTF_ID__func__vfs_getattr__3:
+ * .zero 4
+ * __BTF_ID__func__vfs_fallocate__4:
+ * .zero 4
+ */
+
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <libelf.h>
+#include <gelf.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <linux/rbtree.h>
+#include <linux/zalloc.h>
+#include <linux/err.h>
+#include <btf.h>
+#include <libbpf.h>
+#include <parse-options.h>
+
+#define BTF_IDS_SECTION ".BTF.ids"
+#define BTF_ID "__BTF_ID__"
+
+#define BTF_STRUCT "struct"
+#define BTF_UNION "union"
+#define BTF_TYPEDEF "typedef"
+#define BTF_FUNC "func"
+#define BTF_SET "set"
+
+#define ADDR_CNT 100
+
+struct btf_id {
+ struct rb_node rb_node;
+ char *name;
+ union {
+ int id;
+ int cnt;
+ };
+ int addr_cnt;
+ Elf64_Addr addr[ADDR_CNT];
+};
+
+struct object {
+ const char *path;
+ const char *btf;
+
+ struct {
+ int fd;
+ Elf *elf;
+ Elf_Data *symbols;
+ Elf_Data *idlist;
+ int symbols_shndx;
+ int idlist_shndx;
+ size_t strtabidx;
+ unsigned long idlist_addr;
+ } efile;
+
+ struct rb_root sets;
+ struct rb_root symbols;
+
+ int nr_funcs;
+ int nr_structs;
+ int nr_unions;
+ int nr_typedefs;
+};
+
+static int verbose;
+
+int eprintf(int level, int var, const char *fmt, ...)
+{
+ va_list args;
+ int ret;
+
+ if (var >= level) {
+ va_start(args, fmt);
+ ret = vfprintf(stderr, fmt, args);
+ va_end(args);
+ }
+ return ret;
+}
+
+#ifndef pr_fmt
+#define pr_fmt(fmt) fmt
+#endif
+
+#define pr_debug(fmt, ...) \
+ eprintf(1, verbose, pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_debugN(n, fmt, ...) \
+ eprintf(n, verbose, pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_debug2(fmt, ...) pr_debugN(2, pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_err(fmt, ...) \
+ eprintf(0, verbose, pr_fmt(fmt), ##__VA_ARGS__)
+
+static bool is_btf_id(const char *name)
+{
+ return name && !strncmp(name, BTF_ID, sizeof(BTF_ID) - 1);
+}
+
+static struct btf_id *btf_id__find(struct rb_root *root, const char *name)
+{
+ struct rb_node *p = root->rb_node;
+ struct btf_id *id;
+ int cmp;
+
+ while (p) {
+ id = rb_entry(p, struct btf_id, rb_node);
+ cmp = strcmp(id->name, name);
+ if (cmp < 0)
+ p = p->rb_left;
+ else if (cmp > 0)
+ p = p->rb_right;
+ else
+ return id;
+ }
+ return NULL;
+}
+
+static struct btf_id*
+btf_id__add(struct rb_root *root, char *name, bool unique)
+{
+ struct rb_node **p = &root->rb_node;
+ struct rb_node *parent = NULL;
+ struct btf_id *id;
+ int cmp;
+
+ while (*p != NULL) {
+ parent = *p;
+ id = rb_entry(parent, struct btf_id, rb_node);
+ cmp = strcmp(id->name, name);
+ if (cmp < 0)
+ p = &(*p)->rb_left;
+ else if (cmp > 0)
+ p = &(*p)->rb_right;
+ else
+ return unique ? NULL : id;
+ }
+
+ id = zalloc(sizeof(*id));
+ if (id) {
+ pr_debug("adding symbol %s\n", name);
+ id->name = name;
+ rb_link_node(&id->rb_node, parent, p);
+ rb_insert_color(&id->rb_node, root);
+ }
+ return id;
+}
+
+static char *get_id(const char *prefix_end)
+{
+ /*
+ * __BTF_ID__func__vfs_truncate__0
+ * prefix_end = ^
+ */
+ char *p, *id = strdup(prefix_end + sizeof("__") - 1);
+
+ if (id) {
+ /*
+ * __BTF_ID__func__vfs_truncate__0
+ * id = ^
+ *
+ * cut the unique id part
+ */
+ p = strrchr(id, '_');
+ p--;
+ if (*p != '_') {
+ free(id);
+ return NULL;
+ }
+ *p = '\0';
+ }
+ return id;
+}
+
+static struct btf_id *add_set(struct object *obj, char *name)
+{
+ char *id;
+
+ id = strdup(name + sizeof(BTF_SET) + sizeof("__") - 2);
+ if (!id) {
+ pr_err("FAILED to parse cnt name: %s\n", name);
+ return NULL;
+ }
+
+ return btf_id__add(&obj->sets, id, true);
+}
+
+static struct btf_id *add_symbol(struct object *obj, char *name, size_t size)
+{
+ char *id;
+
+ id = get_id(name + size);
+ if (!id) {
+ pr_err("FAILED to parse symbol name: %s\n", name);
+ return NULL;
+ }
+
+ return btf_id__add(&obj->symbols, id, false);
+}
+
+static int elf_collect(struct object *obj)
+{
+ Elf_Scn *scn = NULL;
+ size_t shdrstrndx;
+ int idx = 0;
+ Elf *elf;
+ int fd;
+
+ fd = open(obj->path, O_RDWR, 0666);
+ if (fd == -1) {
+ pr_err("FAILED cannot open %s: %s\n",
+ obj->path, strerror(errno));
+ return -1;
+ }
+
+ elf_version(EV_CURRENT);
+
+ elf = elf_begin(fd, ELF_C_RDWR_MMAP, NULL);
+ if (!elf) {
+ pr_err("FAILED cannot create ELF descriptor: %s\n",
+ elf_errmsg(-1));
+ return -1;
+ }
+
+ obj->efile.fd = fd;
+ obj->efile.elf = elf;
+
+ elf_flagelf(elf, ELF_C_SET, ELF_F_LAYOUT);
+
+ if (elf_getshdrstrndx(elf, &shdrstrndx) != 0) {
+ pr_err("FAILED cannot get shdr str ndx\n");
+ return -1;
+ }
+
+ /*
+ * Scan all the elf sections and look for save data
+ * from .BTF_ids section and symbols.
+ */
+ while ((scn = elf_nextscn(elf, scn)) != NULL) {
+ Elf_Data *data;
+ GElf_Shdr sh;
+ char *name;
+
+ idx++;
+ if (gelf_getshdr(scn, &sh) != &sh) {
+ pr_err("FAILED get section(%d) header\n", idx);
+ return -1;
+ }
+
+ name = elf_strptr(elf, shdrstrndx, sh.sh_name);
+ if (!name) {
+ pr_err("FAILED get section(%d) name\n", idx);
+ return -1;
+ }
+
+ data = elf_getdata(scn, 0);
+ if (!data) {
+ pr_err("FAILED to get section(%d) data from %s\n",
+ idx, name);
+ return -1;
+ }
+
+ pr_debug2("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);
+
+ if (sh.sh_type == SHT_SYMTAB) {
+ obj->efile.symbols = data;
+ obj->efile.symbols_shndx = idx;
+ obj->efile.strtabidx = sh.sh_link;
+ } else if (!strcmp(name, BTF_IDS_SECTION)) {
+ obj->efile.idlist = data;
+ obj->efile.idlist_shndx = idx;
+ obj->efile.idlist_addr = sh.sh_addr;
+ }
+ }
+
+ /*
+ * We did not find .BTF_ids section or
+ * symbols section, nothing to do..
+ */
+ if (obj->efile.idlist_shndx == -1 ||
+ obj->efile.symbols_shndx == -1) {
+ pr_err("FAILED to find needed sections\n");
+ return -1;
+ }
+
+ return 0;
+}
+
+static int symbols_collect(struct object *obj)
+{
+ Elf_Scn *scn = NULL;
+ int n, i, err = 0;
+ GElf_Shdr sh;
+ char *name;
+
+ scn = elf_getscn(obj->efile.elf, obj->efile.symbols_shndx);
+ if (!scn)
+ return -1;
+
+ if (gelf_getshdr(scn, &sh) != &sh)
+ return -1;
+
+ n = sh.sh_size / sh.sh_entsize;
+
+ /*
+ * Scan symbols and look for the ones starting with
+ * __BTF_ID__* over .BTF_ids section.
+ */
+ for (i = 0; !err && i < n; i++) {
+ char *tmp, *prefix;
+ struct btf_id *id;
+ GElf_Sym sym;
+ int err = -1;
+
+ if (!gelf_getsym(obj->efile.symbols, i, &sym))
+ return -1;
+
+ if (sym.st_shndx != obj->efile.idlist_shndx)
+ continue;
+
+ name = elf_strptr(obj->efile.elf, obj->efile.strtabidx,
+ sym.st_name);
+
+ if (!is_btf_id(name))
+ continue;
+
+ /*
+ * __BTF_ID__TYPE__vfs_truncate__0
+ * prefix = ^
+ */
+ prefix = name + sizeof(BTF_ID) - 1;
+
+ /* struct */
+ if (!strncmp(prefix, BTF_STRUCT, sizeof(BTF_STRUCT) - 1)) {
+ obj->nr_structs++;
+ id = add_symbol(obj, prefix, sizeof(BTF_STRUCT) - 1);
+ /* union */
+ } else if (!strncmp(prefix, BTF_UNION, sizeof(BTF_UNION) - 1)) {
+ obj->nr_unions++;
+ id = add_symbol(obj, prefix, sizeof(BTF_UNION) - 1);
+ /* typedef */
+ } else if (!strncmp(prefix, BTF_TYPEDEF, sizeof(BTF_TYPEDEF) - 1)) {
+ obj->nr_typedefs++;
+ id = add_symbol(obj, prefix, sizeof(BTF_TYPEDEF) - 1);
+ /* func */
+ } else if (!strncmp(prefix, BTF_FUNC, sizeof(BTF_FUNC) - 1)) {
+ obj->nr_funcs++;
+ id = add_symbol(obj, prefix, sizeof(BTF_FUNC) - 1);
+ /* set */
+ } else if (!strncmp(prefix, BTF_SET, sizeof(BTF_SET) - 1)) {
+ id = add_set(obj, prefix);
+ /*
+ * SET objects store list's count, which is encoded
+ * in symbol's size, together with 'cnt' field hence
+ * that - 1.
+ */
+ if (id)
+ id->cnt = sym.st_size / sizeof(int) - 1;
+ } else {
+ pr_err("FAILED unsupported prefix %s\n", prefix);
+ return -1;
+ }
+
+ if (!id)
+ return -ENOMEM;
+
+ if (id->addr_cnt >= ADDR_CNT) {
+ pr_err("FAILED symbol %s crossed the number of allowed lists",
+ id->name);
+ return -1;
+ }
+ id->addr[id->addr_cnt++] = sym.st_value;
+ }
+
+ return 0;
+}
+
+static struct btf *btf__parse_raw(const char *file)
+{
+ struct btf *btf;
+ struct stat st;
+ __u8 *buf;
+ FILE *f;
+
+ if (stat(file, &st))
+ return NULL;
+
+ f = fopen(file, "rb");
+ if (!f)
+ return NULL;
+
+ buf = malloc(st.st_size);
+ if (!buf) {
+ btf = ERR_PTR(-ENOMEM);
+ goto exit_close;
+ }
+
+ if ((size_t) st.st_size != fread(buf, 1, st.st_size, f)) {
+ btf = ERR_PTR(-EINVAL);
+ goto exit_free;
+ }
+
+ btf = btf__new(buf, st.st_size);
+
+exit_free:
+ free(buf);
+exit_close:
+ fclose(f);
+ return btf;
+}
+
+static bool is_btf_raw(const char *file)
+{
+ __u16 magic = 0;
+ int fd, nb_read;
+
+ fd = open(file, O_RDONLY);
+ if (fd < 0)
+ return false;
+
+ nb_read = read(fd, &magic, sizeof(magic));
+ close(fd);
+ return nb_read == sizeof(magic) && magic == BTF_MAGIC;
+}
+
+static struct btf *btf_open(const char *path)
+{
+ if (is_btf_raw(path))
+ return btf__parse_raw(path);
+ else
+ return btf__parse_elf(path, NULL);
+}
+
+static int symbols_resolve(struct object *obj)
+{
+ int nr_typedefs = obj->nr_typedefs;
+ int nr_structs = obj->nr_structs;
+ int nr_unions = obj->nr_unions;
+ int nr_funcs = obj->nr_funcs;
+ int err, type_id;
+ struct btf *btf;
+ __u32 nr;
+
+ btf = btf_open(obj->btf ?: obj->path);
+ err = libbpf_get_error(btf);
+ if (err) {
+ pr_err("FAILED: load BTF from %s: %s",
+ obj->path, strerror(err));
+ return -1;
+ }
+
+ err = -1;
+ nr = btf__get_nr_types(btf);
+
+ /*
+ * Iterate all the BTF types and search for collected symbol IDs.
+ */
+ for (type_id = 1; type_id <= nr; type_id++) {
+ const struct btf_type *type;
+ struct rb_root *root = NULL;
+ struct btf_id *id;
+ const char *str;
+ int *nr;
+
+ type = btf__type_by_id(btf, type_id);
+ if (!type) {
+ pr_err("FAILED: malformed BTF, can't resolve type for ID %d\n",
+ type_id);
+ goto out;
+ }
+
+ if (btf_is_func(type) && nr_funcs)
+ nr = &nr_funcs;
+ else if (btf_is_struct(type) && nr_structs)
+ nr = &nr_structs;
+ else if (btf_is_union(type) && nr_unions)
+ nr = &nr_unions;
+ else if (btf_is_typedef(type) && nr_typedefs)
+ nr = &nr_typedefs;
+ else
+ continue;
+
+ str = btf__name_by_offset(btf, type->name_off);
+ if (!str) {
+ pr_err("FAILED: malformed BTF, can't resolve name for ID %d\n",
+ type_id);
+ goto out;
+ }
+
+ id = btf_id__find(&obj->symbols, str);
+ if (id) {
+ id->id = type_id;
+ (*nr)--;
+ }
+ }
+
+ err = 0;
+out:
+ btf__free(btf);
+ return err;
+}
+
+static int id_patch(struct object *obj, struct btf_id *id)
+{
+ Elf_Data *data = obj->efile.idlist;
+ int *ptr = data->d_buf;
+ int i;
+
+ if (!id->id) {
+ pr_err("FAILED unresolved symbol %s\n", id->name);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < id->addr_cnt; i++) {
+ unsigned long addr = id->addr[i];
+ unsigned long idx = addr - obj->efile.idlist_addr;
+
+ pr_debug("patching addr %5lu: ID %7d [%s]\n",
+ idx, id->id, id->name);
+
+ if (idx >= data->d_size) {
+ pr_err("FAILED patching index %lu out of bounds %lu\n",
+ idx, data->d_size);
+ return -1;
+ }
+
+ idx = idx / sizeof(int);
+ ptr[idx] = id->id;
+ }
+
+ return 0;
+}
+
+static int __symbols_patch(struct object *obj, struct rb_root *root)
+{
+ struct rb_node *next;
+ struct btf_id *id;
+
+ next = rb_first(root);
+ while (next) {
+ id = rb_entry(next, struct btf_id, rb_node);
+
+ if (id_patch(obj, id))
+ return -1;
+
+ next = rb_next(next);
+ }
+ return 0;
+}
+
+static int cmp_id(const void *pa, const void *pb)
+{
+ const int *a = pa, *b = pb;
+
+ return *a - *b;
+}
+
+static int sets_patch(struct object *obj)
+{
+ Elf_Data *data = obj->efile.idlist;
+ int *ptr = data->d_buf;
+ struct rb_node *next;
+
+ next = rb_first(&obj->sets);
+ while (next) {
+ unsigned long addr, idx;
+ struct btf_id *id;
+ int *base;
+ int cnt;
+
+ id = rb_entry(next, struct btf_id, rb_node);
+ addr = id->addr[0];
+ idx = addr - obj->efile.idlist_addr;
+
+ /* sets are unique */
+ if (id->addr_cnt != 1) {
+ pr_err("FAILED malformed data for set '%s'\n",
+ id->name);
+ return -1;
+ }
+
+ idx = idx / sizeof(int);
+ base = &ptr[idx] + 1;
+ cnt = ptr[idx];
+
+ pr_debug("sorting addr %5lu: cnt %6d [%s]\n",
+ (idx + 1) * sizeof(int), cnt, id->name);
+
+ qsort(base, cnt, sizeof(int), cmp_id);
+
+ next = rb_next(next);
+ }
+}
+
+static int symbols_patch(struct object *obj)
+{
+ int err;
+
+ if (__symbols_patch(obj, &obj->symbols) ||
+ __symbols_patch(obj, &obj->sets))
+ return -1;
+
+ if (sets_patch(obj))
+ return -1;
+
+ elf_flagdata(obj->efile.idlist, ELF_C_SET, ELF_F_DIRTY);
+
+ err = elf_update(obj->efile.elf, ELF_C_WRITE);
+ if (err < 0) {
+ pr_err("FAILED elf_update(WRITE): %s\n",
+ elf_errmsg(-1));
+ }
+
+ pr_debug("update %s for %s\n",
+ err >= 0 ? "ok" : "failed", obj->path);
+ return err < 0 ? -1 : 0;
+}
+
+static const char * const resolve_btfids_usage[] = {
+ "resolve_btfids [<options>] <ELF object>",
+ NULL
+};
+
+int main(int argc, const char **argv)
+{
+ struct object obj = {
+ .efile = {
+ .idlist_shndx = -1,
+ .symbols_shndx = -1,
+ },
+ .symbols = RB_ROOT,
+ .sets = RB_ROOT,
+ };
+ struct option btfid_options[] = {
+ OPT_INCR('v', "verbose", &verbose,
+ "be more verbose (show errors, etc)"),
+ OPT_STRING(0, "btf", &obj.btf, "BTF data",
+ "BTF data"),
+ OPT_END()
+ };
+ int err = -1;
+
+ argc = parse_options(argc, argv, btfid_options, resolve_btfids_usage,
+ PARSE_OPT_STOP_AT_NON_OPTION);
+ if (argc != 1)
+ usage_with_options(resolve_btfids_usage, btfid_options);
+
+ obj.path = argv[0];
+
+ if (elf_collect(&obj))
+ goto out;
+
+ if (symbols_collect(&obj))
+ goto out;
+
+ if (symbols_resolve(&obj))
+ goto out;
+
+ if (symbols_patch(&obj))
+ goto out;
+
+ err = 0;
+out:
+ if (obj.efile.elf)
+ elf_end(obj.efile.elf);
+ close(obj.efile.fd);
+ return err;
+}
--
2.25.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 bpf-next 2/9] bpf: Compile resolve_btfids tool at kernel compilation start
2020-07-03 9:51 [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 1/9] bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object Jiri Olsa
@ 2020-07-03 9:51 ` Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 3/9] bpf: Add BTF_ID_LIST/BTF_ID/BTF_ID_UNUSED macros Jiri Olsa
` (6 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Jiri Olsa @ 2020-07-03 9:51 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Andrii Nakryiko, netdev, bpf, Song Liu, Yonghong Song,
Martin KaFai Lau, David Miller, John Fastabend, Wenbo Zhang,
KP Singh, Brendan Gregg, Florent Revest, Al Viro
The resolve_btfids tool will be used during the vmlinux linking,
so it's necessary it's ready for it.
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
Makefile | 22 ++++++++++++++++++----
tools/Makefile | 3 +++
tools/bpf/Makefile | 9 ++++++++-
3 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/Makefile b/Makefile
index ae5d8220f431..8db4fd8097e0 100644
--- a/Makefile
+++ b/Makefile
@@ -1053,9 +1053,10 @@ export mod_sign_cmd
HOST_LIBELF_LIBS = $(shell pkg-config libelf --libs 2>/dev/null || echo -lelf)
+has_libelf = $(call try-run,\
+ echo "int main() {}" | $(HOSTCC) -xc -o /dev/null $(HOST_LIBELF_LIBS) -,1,0)
+
ifdef CONFIG_STACK_VALIDATION
- has_libelf := $(call try-run,\
- echo "int main() {}" | $(HOSTCC) -xc -o /dev/null $(HOST_LIBELF_LIBS) -,1,0)
ifeq ($(has_libelf),1)
objtool_target := tools/objtool FORCE
else
@@ -1064,6 +1065,14 @@ ifdef CONFIG_STACK_VALIDATION
endif
endif
+ifdef CONFIG_DEBUG_INFO_BTF
+ ifeq ($(has_libelf),1)
+ resolve_btfids_target := tools/bpf/resolve_btfids FORCE
+ else
+ ERROR_RESOLVE_BTFIDS := 1
+ endif
+endif
+
PHONY += prepare0
export MODORDER := $(extmod-prefix)modules.order
@@ -1175,7 +1184,7 @@ prepare0: archprepare
$(Q)$(MAKE) $(build)=.
# All the preparing..
-prepare: prepare0 prepare-objtool
+prepare: prepare0 prepare-objtool prepare-resolve_btfids
# Support for using generic headers in asm-generic
asm-generic := -f $(srctree)/scripts/Makefile.asm-generic obj
@@ -1188,7 +1197,7 @@ uapi-asm-generic:
$(Q)$(MAKE) $(asm-generic)=arch/$(SRCARCH)/include/generated/uapi/asm \
generic=include/uapi/asm-generic
-PHONY += prepare-objtool
+PHONY += prepare-objtool prepare-resolve_btfids
prepare-objtool: $(objtool_target)
ifeq ($(SKIP_STACK_VALIDATION),1)
ifdef CONFIG_UNWINDER_ORC
@@ -1199,6 +1208,11 @@ else
endif
endif
+prepare-resolve_btfids: $(resolve_btfids_target)
+ifeq ($(ERROR_RESOLVE_BTFIDS),1)
+ @echo "error: Cannot resolve BTF IDs for CONFIG_DEBUG_INFO_BTF, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
+ @false
+endif
# Generate some files
# ---------------------------------------------------------------------------
diff --git a/tools/Makefile b/tools/Makefile
index bd778812e915..85af6ebbce91 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -67,6 +67,9 @@ cpupower: FORCE
cgroup firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging: FORCE
$(call descend,$@)
+bpf/%: FORCE
+ $(call descend,$@)
+
liblockdep: FORCE
$(call descend,lib/lockdep)
diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index 6df1850f8353..74bc9a105225 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -123,5 +123,12 @@ runqslower_install:
runqslower_clean:
$(call descend,runqslower,clean)
+resolve_btfids:
+ $(call descend,resolve_btfids)
+
+resolve_btfids_clean:
+ $(call descend,resolve_btfids,clean)
+
.PHONY: all install clean bpftool bpftool_install bpftool_clean \
- runqslower runqslower_install runqslower_clean
+ runqslower runqslower_install runqslower_clean \
+ resolve_btfids resolve_btfids_clean
--
2.25.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 bpf-next 3/9] bpf: Add BTF_ID_LIST/BTF_ID/BTF_ID_UNUSED macros
2020-07-03 9:51 [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 1/9] bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 2/9] bpf: Compile resolve_btfids tool at kernel compilation start Jiri Olsa
@ 2020-07-03 9:51 ` Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 4/9] bpf: Resolve BTF IDs in vmlinux image Jiri Olsa
` (5 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Jiri Olsa @ 2020-07-03 9:51 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Andrii Nakryiko, netdev, bpf, Song Liu, Yonghong Song,
Martin KaFai Lau, David Miller, John Fastabend, Wenbo Zhang,
KP Singh, Brendan Gregg, Florent Revest, Al Viro
Adding support to generate .BTF_ids section that will hold BTF
ID lists for verifier.
Adding macros that will help to define lists of BTF ID values
placed in .BTF_ids section. They are initially filled with zeros
(during compilation) and resolved later during the linking phase
by resolve_btfids tool.
Following defines list of one BTF ID value:
BTF_ID_LIST(bpf_skb_output_btf_ids)
BTF_ID(struct, sk_buff)
It also defines following variable to access the list:
extern u32 bpf_skb_output_btf_ids[];
The BTF_ID_UNUSED macro defines 4 zero bytes. It's used when we
want to define 'unused' entry in BTF_ID_LIST, like:
BTF_ID_LIST(bpf_skb_output_btf_ids)
BTF_ID(struct, sk_buff)
BTF_ID_UNUSED
BTF_ID(struct, task_struct)
Suggested-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
include/asm-generic/vmlinux.lds.h | 4 ++
include/linux/btf_ids.h | 87 +++++++++++++++++++++++++++++++
2 files changed, 91 insertions(+)
create mode 100644 include/linux/btf_ids.h
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index db600ef218d7..a9b77a4314a8 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -641,6 +641,10 @@
__start_BTF = .; \
*(.BTF) \
__stop_BTF = .; \
+ } \
+ . = ALIGN(4); \
+ .BTF.ids : AT(ADDR(.BTF.ids) - LOAD_OFFSET) { \
+ *(.BTF.ids) \
}
#else
#define BTF
diff --git a/include/linux/btf_ids.h b/include/linux/btf_ids.h
new file mode 100644
index 000000000000..d317150bc9e3
--- /dev/null
+++ b/include/linux/btf_ids.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_BTF_IDS_H
+#define _LINUX_BTF_IDS_H
+
+#include <linux/compiler.h> /* for __PASTE */
+
+/*
+ * Following macros help to define lists of BTF IDs placed
+ * in .BTF_ids section. They are initially filled with zeros
+ * (during compilation) and resolved later during the
+ * linking phase by btfid tool.
+ *
+ * Any change in list layout must be reflected in btfid
+ * tool logic.
+ */
+
+#define BTF_IDS_SECTION ".BTF.ids"
+
+#define ____BTF_ID(symbol) \
+asm( \
+".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
+".local " #symbol " ; \n" \
+".type " #symbol ", @object; \n" \
+".size " #symbol ", 4; \n" \
+#symbol ": \n" \
+".zero 4 \n" \
+".popsection; \n");
+
+#define __BTF_ID(symbol) \
+ ____BTF_ID(symbol)
+
+#define __ID(prefix) \
+ __PASTE(prefix, __COUNTER__)
+
+/*
+ * The BTF_ID defines unique symbol for each ID pointing
+ * to 4 zero bytes.
+ */
+#define BTF_ID(prefix, name) \
+ __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__))
+
+/*
+ * The BTF_ID_LIST macro defines pure (unsorted) list
+ * of BTF IDs, with following layout:
+ *
+ * BTF_ID_LIST(list1)
+ * BTF_ID(type1, name1)
+ * BTF_ID(type2, name2)
+ *
+ * list1:
+ * __BTF_ID__type1__name1__1:
+ * .zero 4
+ * __BTF_ID__type2__name2__2:
+ * .zero 4
+ *
+ */
+#define __BTF_ID_LIST(name) \
+asm( \
+".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
+".local " #name "; \n" \
+#name ":; \n" \
+".popsection; \n"); \
+
+#define BTF_ID_LIST(name) \
+__BTF_ID_LIST(name) \
+extern u32 name[];
+
+/*
+ * The BTF_ID_UNUSED macro defines 4 zero bytes.
+ * It's used when we want to define 'unused' entry
+ * in BTF_ID_LIST, like:
+ *
+ * BTF_ID_LIST(bpf_skb_output_btf_ids)
+ * BTF_ID(struct, sk_buff)
+ * BTF_ID_UNUSED
+ * BTF_ID(struct, task_struct)
+ */
+
+#define BTF_ID_UNUSED \
+asm( \
+".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
+".zero 4 \n" \
+".popsection; \n");
+
+
+#endif
--
2.25.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 bpf-next 4/9] bpf: Resolve BTF IDs in vmlinux image
2020-07-03 9:51 [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes Jiri Olsa
` (2 preceding siblings ...)
2020-07-03 9:51 ` [PATCH v5 bpf-next 3/9] bpf: Add BTF_ID_LIST/BTF_ID/BTF_ID_UNUSED macros Jiri Olsa
@ 2020-07-03 9:51 ` Jiri Olsa
2020-07-07 0:38 ` Andrii Nakryiko
2020-07-03 9:51 ` [PATCH v5 bpf-next 5/9] bpf: Remove btf_id helpers resolving Jiri Olsa
` (4 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Jiri Olsa @ 2020-07-03 9:51 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Andrii Nakryiko, netdev, bpf, Song Liu, Yonghong Song,
Martin KaFai Lau, David Miller, John Fastabend, Wenbo Zhang,
KP Singh, Brendan Gregg, Florent Revest, Al Viro
Using BTF_ID_LIST macro to define lists for several helpers
using BTF arguments.
And running resolve_btfids on vmlinux elf object during linking,
so the .BTF_ids section gets the IDs resolved.
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
Makefile | 3 ++-
kernel/trace/bpf_trace.c | 9 +++++++--
net/core/filter.c | 9 +++++++--
scripts/link-vmlinux.sh | 6 ++++++
4 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/Makefile b/Makefile
index 8db4fd8097e0..def58d4f9ed7 100644
--- a/Makefile
+++ b/Makefile
@@ -448,6 +448,7 @@ OBJSIZE = $(CROSS_COMPILE)size
STRIP = $(CROSS_COMPILE)strip
endif
PAHOLE = pahole
+RESOLVE_BTFIDS = $(srctree)/tools/bpf/resolve_btfids/resolve_btfids
LEX = flex
YACC = bison
AWK = awk
@@ -510,7 +511,7 @@ GCC_PLUGINS_CFLAGS :=
CLANG_FLAGS :=
export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC
-export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
+export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
export PERL PYTHON PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ
export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1d874d8e4384..e26fbb1dad9c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -13,6 +13,7 @@
#include <linux/kprobes.h>
#include <linux/syscalls.h>
#include <linux/error-injection.h>
+#include <linux/btf_ids.h>
#include <asm/tlb.h>
@@ -710,7 +711,9 @@ BPF_CALL_5(bpf_seq_printf, struct seq_file *, m, char *, fmt, u32, fmt_size,
return err;
}
-static int bpf_seq_printf_btf_ids[5];
+BTF_ID_LIST(bpf_seq_printf_btf_ids)
+BTF_ID(struct, seq_file)
+
static const struct bpf_func_proto bpf_seq_printf_proto = {
.func = bpf_seq_printf,
.gpl_only = true,
@@ -728,7 +731,9 @@ BPF_CALL_3(bpf_seq_write, struct seq_file *, m, const void *, data, u32, len)
return seq_write(m, data, len) ? -EOVERFLOW : 0;
}
-static int bpf_seq_write_btf_ids[5];
+BTF_ID_LIST(bpf_seq_write_btf_ids)
+BTF_ID(struct, seq_file)
+
static const struct bpf_func_proto bpf_seq_write_proto = {
.func = bpf_seq_write,
.gpl_only = true,
diff --git a/net/core/filter.c b/net/core/filter.c
index c5e696e6c315..a859ad681988 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -75,6 +75,7 @@
#include <net/ipv6_stubs.h>
#include <net/bpf_sk_storage.h>
#include <net/transp_v6.h>
+#include <linux/btf_ids.h>
/**
* sk_filter_trim_cap - run a packet through a socket filter
@@ -3779,7 +3780,9 @@ static const struct bpf_func_proto bpf_skb_event_output_proto = {
.arg5_type = ARG_CONST_SIZE_OR_ZERO,
};
-static int bpf_skb_output_btf_ids[5];
+BTF_ID_LIST(bpf_skb_output_btf_ids)
+BTF_ID(struct, sk_buff)
+
const struct bpf_func_proto bpf_skb_output_proto = {
.func = bpf_skb_event_output,
.gpl_only = true,
@@ -4173,7 +4176,9 @@ static const struct bpf_func_proto bpf_xdp_event_output_proto = {
.arg5_type = ARG_CONST_SIZE_OR_ZERO,
};
-static int bpf_xdp_output_btf_ids[5];
+BTF_ID_LIST(bpf_xdp_output_btf_ids)
+BTF_ID(struct, xdp_buff)
+
const struct bpf_func_proto bpf_xdp_output_proto = {
.func = bpf_xdp_event_output,
.gpl_only = true,
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 92dd745906f4..e26f02dbedee 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -336,6 +336,12 @@ fi
vmlinux_link vmlinux "${kallsymso}" ${btf_vmlinux_bin_o}
+# fill in BTF IDs
+if [ -n "${CONFIG_DEBUG_INFO_BTF}" ]; then
+info BTFIDS vmlinux
+${RESOLVE_BTFIDS} vmlinux
+fi
+
if [ -n "${CONFIG_BUILDTIME_TABLE_SORT}" ]; then
info SORTTAB vmlinux
if ! sorttable vmlinux; then
--
2.25.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 bpf-next 5/9] bpf: Remove btf_id helpers resolving
2020-07-03 9:51 [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes Jiri Olsa
` (3 preceding siblings ...)
2020-07-03 9:51 ` [PATCH v5 bpf-next 4/9] bpf: Resolve BTF IDs in vmlinux image Jiri Olsa
@ 2020-07-03 9:51 ` Jiri Olsa
2020-07-07 0:46 ` Andrii Nakryiko
2020-07-03 9:51 ` [PATCH v5 bpf-next 6/9] bpf: Use BTF_ID to resolve bpf_ctx_convert struct Jiri Olsa
` (3 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Jiri Olsa @ 2020-07-03 9:51 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, bpf, Song Liu, Yonghong Song, Martin KaFai Lau,
David Miller, John Fastabend, Wenbo Zhang, KP Singh,
Andrii Nakryiko, Brendan Gregg, Florent Revest, Al Viro
Now when we moved the helpers btf_id arrays into .BTF_ids section,
we can remove the code that resolve those IDs in runtime.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/bpf/btf.c | 89 +++---------------------------------------------
1 file changed, 5 insertions(+), 84 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 4c3007f428b1..71140b73ae3c 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4079,96 +4079,17 @@ int btf_struct_access(struct bpf_verifier_log *log,
return -EINVAL;
}
-static int __btf_resolve_helper_id(struct bpf_verifier_log *log, void *fn,
- int arg)
-{
- char fnname[KSYM_SYMBOL_LEN + 4] = "btf_";
- const struct btf_param *args;
- const struct btf_type *t;
- const char *tname, *sym;
- u32 btf_id, i;
-
- if (IS_ERR(btf_vmlinux)) {
- bpf_log(log, "btf_vmlinux is malformed\n");
- return -EINVAL;
- }
-
- sym = kallsyms_lookup((long)fn, NULL, NULL, NULL, fnname + 4);
- if (!sym) {
- bpf_log(log, "kernel doesn't have kallsyms\n");
- return -EFAULT;
- }
-
- for (i = 1; i <= btf_vmlinux->nr_types; i++) {
- t = btf_type_by_id(btf_vmlinux, i);
- if (BTF_INFO_KIND(t->info) != BTF_KIND_TYPEDEF)
- continue;
- tname = __btf_name_by_offset(btf_vmlinux, t->name_off);
- if (!strcmp(tname, fnname))
- break;
- }
- if (i > btf_vmlinux->nr_types) {
- bpf_log(log, "helper %s type is not found\n", fnname);
- return -ENOENT;
- }
-
- t = btf_type_by_id(btf_vmlinux, t->type);
- if (!btf_type_is_ptr(t))
- return -EFAULT;
- t = btf_type_by_id(btf_vmlinux, t->type);
- if (!btf_type_is_func_proto(t))
- return -EFAULT;
-
- args = (const struct btf_param *)(t + 1);
- if (arg >= btf_type_vlen(t)) {
- bpf_log(log, "bpf helper %s doesn't have %d-th argument\n",
- fnname, arg);
- return -EINVAL;
- }
-
- t = btf_type_by_id(btf_vmlinux, args[arg].type);
- if (!btf_type_is_ptr(t) || !t->type) {
- /* anything but the pointer to struct is a helper config bug */
- bpf_log(log, "ARG_PTR_TO_BTF is misconfigured\n");
- return -EFAULT;
- }
- btf_id = t->type;
- t = btf_type_by_id(btf_vmlinux, t->type);
- /* skip modifiers */
- while (btf_type_is_modifier(t)) {
- btf_id = t->type;
- t = btf_type_by_id(btf_vmlinux, t->type);
- }
- if (!btf_type_is_struct(t)) {
- bpf_log(log, "ARG_PTR_TO_BTF is not a struct\n");
- return -EFAULT;
- }
- bpf_log(log, "helper %s arg%d has btf_id %d struct %s\n", fnname + 4,
- arg, btf_id, __btf_name_by_offset(btf_vmlinux, t->name_off));
- return btf_id;
-}
-
int btf_resolve_helper_id(struct bpf_verifier_log *log,
const struct bpf_func_proto *fn, int arg)
{
- int *btf_id = &fn->btf_id[arg];
- int ret;
+ int id;
if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID)
return -EINVAL;
-
- ret = READ_ONCE(*btf_id);
- if (ret)
- return ret;
- /* ok to race the search. The result is the same */
- ret = __btf_resolve_helper_id(log, fn->func, arg);
- if (!ret) {
- /* Function argument cannot be type 'void' */
- bpf_log(log, "BTF resolution bug\n");
- return -EFAULT;
- }
- WRITE_ONCE(*btf_id, ret);
- return ret;
+ id = fn->btf_id[arg];
+ if (!id || id > btf_vmlinux->nr_types)
+ return -EINVAL;
+ return id;
}
static int __get_type_size(struct btf *btf, u32 btf_id,
--
2.25.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 bpf-next 6/9] bpf: Use BTF_ID to resolve bpf_ctx_convert struct
2020-07-03 9:51 [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes Jiri Olsa
` (4 preceding siblings ...)
2020-07-03 9:51 ` [PATCH v5 bpf-next 5/9] bpf: Remove btf_id helpers resolving Jiri Olsa
@ 2020-07-03 9:51 ` Jiri Olsa
2020-07-07 0:47 ` Andrii Nakryiko
2020-07-03 9:51 ` [PATCH v5 bpf-next 7/9] bpf: Add info about .BTF.ids section to btf.rst Jiri Olsa
` (2 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Jiri Olsa @ 2020-07-03 9:51 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, bpf, Song Liu, Yonghong Song, Martin KaFai Lau,
David Miller, John Fastabend, Wenbo Zhang, KP Singh,
Andrii Nakryiko, Brendan Gregg, Florent Revest, Al Viro
This way the ID is resolved during compile time,
and we can remove the runtime name search.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/bpf/btf.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 71140b73ae3c..a710e3ee1f18 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -18,6 +18,7 @@
#include <linux/sort.h>
#include <linux/bpf_verifier.h>
#include <linux/btf.h>
+#include <linux/btf_ids.h>
#include <linux/skmsg.h>
#include <linux/perf_event.h>
#include <net/sock.h>
@@ -3621,12 +3622,15 @@ static int btf_translate_to_vmlinux(struct bpf_verifier_log *log,
return kern_ctx_type->type;
}
+BTF_ID_LIST(bpf_ctx_convert_btf_id)
+BTF_ID(struct, bpf_ctx_convert)
+
struct btf *btf_parse_vmlinux(void)
{
struct btf_verifier_env *env = NULL;
struct bpf_verifier_log *log;
struct btf *btf = NULL;
- int err, btf_id;
+ int err;
env = kzalloc(sizeof(*env), GFP_KERNEL | __GFP_NOWARN);
if (!env)
@@ -3659,14 +3663,8 @@ struct btf *btf_parse_vmlinux(void)
if (err)
goto errout;
- /* find struct bpf_ctx_convert for type checking later */
- btf_id = btf_find_by_name_kind(btf, "bpf_ctx_convert", BTF_KIND_STRUCT);
- if (btf_id < 0) {
- err = btf_id;
- goto errout;
- }
/* btf_parse_vmlinux() runs under bpf_verifier_lock */
- bpf_ctx_convert.t = btf_type_by_id(btf, btf_id);
+ bpf_ctx_convert.t = btf_type_by_id(btf, bpf_ctx_convert_btf_id[0]);
/* find bpf map structs for map_ptr access checking */
err = btf_vmlinux_map_ids_init(btf, log);
--
2.25.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 bpf-next 7/9] bpf: Add info about .BTF.ids section to btf.rst
2020-07-03 9:51 [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes Jiri Olsa
` (5 preceding siblings ...)
2020-07-03 9:51 ` [PATCH v5 bpf-next 6/9] bpf: Use BTF_ID to resolve bpf_ctx_convert struct Jiri Olsa
@ 2020-07-03 9:51 ` Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 8/9] tools headers: Adopt verbatim copy of btf_ids.h from kernel sources Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 9/9] selftests/bpf: Add test for resolve_btfids Jiri Olsa
8 siblings, 0 replies; 22+ messages in thread
From: Jiri Olsa @ 2020-07-03 9:51 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, bpf, Song Liu, Yonghong Song, Martin KaFai Lau,
David Miller, John Fastabend, Wenbo Zhang, KP Singh,
Andrii Nakryiko, Brendan Gregg, Florent Revest, Al Viro
Updating btf.rst doc with info about .BTF.ids section
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
Documentation/bpf/btf.rst | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/Documentation/bpf/btf.rst b/Documentation/bpf/btf.rst
index 4d565d202ce3..17ebd78d2c9e 100644
--- a/Documentation/bpf/btf.rst
+++ b/Documentation/bpf/btf.rst
@@ -691,6 +691,42 @@ kernel API, the ``insn_off`` is the instruction offset in the unit of ``struct
bpf_insn``. For ELF API, the ``insn_off`` is the byte offset from the
beginning of section (``btf_ext_info_sec->sec_name_off``).
+4.2 .BTF.ids section
+====================
+
+The .BTF.ids section encodes BTF ID values that are used within the kernel.
+
+This section is created during the kernel compilation with the help of
+macros defined in ``include/linux/btf_ids.h`` header file. Kernel code can
+use them to create lists and sets (sorted lists) of BTF ID values.
+
+The ``BTF_ID_LIST`` and ``BTF_ID`` macros define unsorted list of BTF ID values,
+with following syntax::
+
+ BTF_ID_LIST(list)
+ BTF_ID(type1, name1)
+ BTF_ID(type2, name2)
+
+resulting in following layout in .BTF.ids section::
+
+ __BTF_ID__type1__name1__1:
+ .zero 4
+ __BTF_ID__type2__name2__2:
+ .zero 4
+
+The ``int list[];`` variable is defined to access the list.
+
+The ``BTF_ID_UNUSED`` macro defines 4 zero bytes. It's used when we
+want to define unused entry in BTF_ID_LIST, like::
+
+ BTF_ID_LIST(bpf_skb_output_btf_ids)
+ BTF_ID(struct, sk_buff)
+ BTF_ID_UNUSED
+ BTF_ID(struct, task_struct)
+
+All the BTF ID lists and sets are compiled in the .BTF.ids section and
+resolved during the linking phase of kernel build by ``resolve_btfids`` tool.
+
5. Using BTF
************
--
2.25.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 bpf-next 8/9] tools headers: Adopt verbatim copy of btf_ids.h from kernel sources
2020-07-03 9:51 [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes Jiri Olsa
` (6 preceding siblings ...)
2020-07-03 9:51 ` [PATCH v5 bpf-next 7/9] bpf: Add info about .BTF.ids section to btf.rst Jiri Olsa
@ 2020-07-03 9:51 ` Jiri Olsa
2020-07-07 0:56 ` Andrii Nakryiko
2020-07-03 9:51 ` [PATCH v5 bpf-next 9/9] selftests/bpf: Add test for resolve_btfids Jiri Olsa
8 siblings, 1 reply; 22+ messages in thread
From: Jiri Olsa @ 2020-07-03 9:51 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: Andrii Nakryiko, netdev, bpf, Song Liu, Yonghong Song,
Martin KaFai Lau, David Miller, John Fastabend, Wenbo Zhang,
KP Singh, Brendan Gregg, Florent Revest, Al Viro
It will be needed by bpf selftest for resolve_btfids tool.
Also adding __PASTE macro as btf_ids.h dependency, which is
defined in:
include/linux/compiler_types.h
but because tools/include do not have this header, I'm putting
the macro into linux/compiler.h header.
Acked-by: Andrii Nakryiko <andriin@fb.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/include/linux/btf_ids.h | 87 ++++++++++++++++++++++++++++++++++
tools/include/linux/compiler.h | 4 ++
2 files changed, 91 insertions(+)
create mode 100644 tools/include/linux/btf_ids.h
diff --git a/tools/include/linux/btf_ids.h b/tools/include/linux/btf_ids.h
new file mode 100644
index 000000000000..d317150bc9e3
--- /dev/null
+++ b/tools/include/linux/btf_ids.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_BTF_IDS_H
+#define _LINUX_BTF_IDS_H
+
+#include <linux/compiler.h> /* for __PASTE */
+
+/*
+ * Following macros help to define lists of BTF IDs placed
+ * in .BTF_ids section. They are initially filled with zeros
+ * (during compilation) and resolved later during the
+ * linking phase by btfid tool.
+ *
+ * Any change in list layout must be reflected in btfid
+ * tool logic.
+ */
+
+#define BTF_IDS_SECTION ".BTF.ids"
+
+#define ____BTF_ID(symbol) \
+asm( \
+".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
+".local " #symbol " ; \n" \
+".type " #symbol ", @object; \n" \
+".size " #symbol ", 4; \n" \
+#symbol ": \n" \
+".zero 4 \n" \
+".popsection; \n");
+
+#define __BTF_ID(symbol) \
+ ____BTF_ID(symbol)
+
+#define __ID(prefix) \
+ __PASTE(prefix, __COUNTER__)
+
+/*
+ * The BTF_ID defines unique symbol for each ID pointing
+ * to 4 zero bytes.
+ */
+#define BTF_ID(prefix, name) \
+ __BTF_ID(__ID(__BTF_ID__##prefix##__##name##__))
+
+/*
+ * The BTF_ID_LIST macro defines pure (unsorted) list
+ * of BTF IDs, with following layout:
+ *
+ * BTF_ID_LIST(list1)
+ * BTF_ID(type1, name1)
+ * BTF_ID(type2, name2)
+ *
+ * list1:
+ * __BTF_ID__type1__name1__1:
+ * .zero 4
+ * __BTF_ID__type2__name2__2:
+ * .zero 4
+ *
+ */
+#define __BTF_ID_LIST(name) \
+asm( \
+".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
+".local " #name "; \n" \
+#name ":; \n" \
+".popsection; \n"); \
+
+#define BTF_ID_LIST(name) \
+__BTF_ID_LIST(name) \
+extern u32 name[];
+
+/*
+ * The BTF_ID_UNUSED macro defines 4 zero bytes.
+ * It's used when we want to define 'unused' entry
+ * in BTF_ID_LIST, like:
+ *
+ * BTF_ID_LIST(bpf_skb_output_btf_ids)
+ * BTF_ID(struct, sk_buff)
+ * BTF_ID_UNUSED
+ * BTF_ID(struct, task_struct)
+ */
+
+#define BTF_ID_UNUSED \
+asm( \
+".pushsection " BTF_IDS_SECTION ",\"a\"; \n" \
+".zero 4 \n" \
+".popsection; \n");
+
+
+#endif
diff --git a/tools/include/linux/compiler.h b/tools/include/linux/compiler.h
index 9f9002734e19..6eac24d44e81 100644
--- a/tools/include/linux/compiler.h
+++ b/tools/include/linux/compiler.h
@@ -201,4 +201,8 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
# define __fallthrough
#endif
+/* Indirect macros required for expanded argument pasting, eg. __LINE__. */
+#define ___PASTE(a, b) a##b
+#define __PASTE(a, b) ___PASTE(a, b)
+
#endif /* _TOOLS_LINUX_COMPILER_H */
--
2.25.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 bpf-next 9/9] selftests/bpf: Add test for resolve_btfids
2020-07-03 9:51 [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes Jiri Olsa
` (7 preceding siblings ...)
2020-07-03 9:51 ` [PATCH v5 bpf-next 8/9] tools headers: Adopt verbatim copy of btf_ids.h from kernel sources Jiri Olsa
@ 2020-07-03 9:51 ` Jiri Olsa
2020-07-07 1:26 ` Andrii Nakryiko
8 siblings, 1 reply; 22+ messages in thread
From: Jiri Olsa @ 2020-07-03 9:51 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: netdev, bpf, Song Liu, Yonghong Song, Martin KaFai Lau,
David Miller, John Fastabend, Wenbo Zhang, KP Singh,
Andrii Nakryiko, Brendan Gregg, Florent Revest, Al Viro
Adding resolve_btfids test under test_progs suite.
It's possible to use btf_ids.h header and its logic in
user space application, so we can add easy test for it.
The test defines BTF_ID_LIST and checks it gets properly
resolved.
For this reason the test_progs binary (and other binaries
that use TRUNNER* macros) is processed with resolve_btfids
tool, which resolves BTF IDs in .BTF.ids section.
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/Makefile | 22 ++-
.../selftests/bpf/prog_tests/resolve_btfids.c | 170 ++++++++++++++++++
2 files changed, 190 insertions(+), 2 deletions(-)
create mode 100644 tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 1f9c696b3edf..b47a685d12bd 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -190,6 +190,16 @@ else
cp "$(VMLINUX_H)" $@
endif
+$(SCRATCH_DIR)/resolve_btfids: $(BPFOBJ) \
+ $(TOOLSDIR)/bpf/resolve_btfids/main.c \
+ $(TOOLSDIR)/lib/rbtree.c \
+ $(TOOLSDIR)/lib/zalloc.c \
+ $(TOOLSDIR)/lib/string.c \
+ $(TOOLSDIR)/lib/ctype.c \
+ $(TOOLSDIR)/lib/str_error_r.c
+ $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/resolve_btfids \
+ OUTPUT=$(SCRATCH_DIR)/ BPFOBJ=$(BPFOBJ)
+
# Get Clang's default includes on this system, as opposed to those seen by
# '-target bpf'. This fixes "missing" files on some architectures/distros,
# such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc.
@@ -333,7 +343,8 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \
$(TRUNNER_BPF_SKELS) \
$$(BPFOBJ) | $(TRUNNER_OUTPUT)
$$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@)
- cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
+ cd $$(@D) && $$(CC) -I. $$(CFLAGS) $(TRUNNER_EXTRA_CFLAGS) \
+ -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
$(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \
%.c \
@@ -355,6 +366,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \
| $(TRUNNER_BINARY)-extras
$$(call msg,BINARY,,$$@)
$$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@
+ $(TRUNNER_BINARY_EXTRA_CMD)
endef
@@ -365,7 +377,10 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \
network_helpers.c testing_helpers.c \
flow_dissector_load.h
TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \
- $(wildcard progs/btf_dump_test_case_*.c)
+ $(wildcard progs/btf_dump_test_case_*.c) \
+ $(SCRATCH_DIR)/resolve_btfids
+TRUNNER_EXTRA_CFLAGS := -D"BUILD_STR(s)=\#s" -DVMLINUX_BTF="BUILD_STR($(VMLINUX_BTF))"
+TRUNNER_BINARY_EXTRA_CMD := $(SCRATCH_DIR)/resolve_btfids --btf $(VMLINUX_BTF) test_progs
TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
TRUNNER_BPF_LDFLAGS := -mattr=+alu32
@@ -373,6 +388,7 @@ $(eval $(call DEFINE_TEST_RUNNER,test_progs))
# Define test_progs-no_alu32 test runner.
TRUNNER_BPF_BUILD_RULE := CLANG_NOALU32_BPF_BUILD_RULE
+TRUNNER_BINARY_EXTRA_CMD := $(SCRATCH_DIR)/resolve_btfids --btf $(VMLINUX_BTF) test_progs-no_alu32
TRUNNER_BPF_LDFLAGS :=
$(eval $(call DEFINE_TEST_RUNNER,test_progs,no_alu32))
@@ -392,6 +408,8 @@ TRUNNER_EXTRA_FILES :=
TRUNNER_BPF_BUILD_RULE := $$(error no BPF objects should be built)
TRUNNER_BPF_CFLAGS :=
TRUNNER_BPF_LDFLAGS :=
+TRUNNER_EXTRA_CFLAGS :=
+TRUNNER_BINARY_EXTRA_CMD :=
$(eval $(call DEFINE_TEST_RUNNER,test_maps))
# Define test_verifier test runner.
diff --git a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
new file mode 100644
index 000000000000..6b7b5f736181
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <string.h>
+#include <stdio.h>
+#include <sys/stat.h>
+#include <stdio.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <linux/err.h>
+#include <stdlib.h>
+#include <bpf/btf.h>
+#include <bpf/libbpf.h>
+#include <linux/btf.h>
+#include <linux/kernel.h>
+#include <linux/btf_ids.h>
+#include "test_progs.h"
+
+static int duration;
+
+static struct btf *btf__parse_raw(const char *file)
+{
+ struct btf *btf;
+ struct stat st;
+ __u8 *buf;
+ FILE *f;
+
+ if (stat(file, &st))
+ return NULL;
+
+ f = fopen(file, "rb");
+ if (!f)
+ return NULL;
+
+ buf = malloc(st.st_size);
+ if (!buf) {
+ btf = ERR_PTR(-ENOMEM);
+ goto exit_close;
+ }
+
+ if ((size_t) st.st_size != fread(buf, 1, st.st_size, f)) {
+ btf = ERR_PTR(-EINVAL);
+ goto exit_free;
+ }
+
+ btf = btf__new(buf, st.st_size);
+
+exit_free:
+ free(buf);
+exit_close:
+ fclose(f);
+ return btf;
+}
+
+static bool is_btf_raw(const char *file)
+{
+ __u16 magic = 0;
+ int fd, nb_read;
+
+ fd = open(file, O_RDONLY);
+ if (fd < 0)
+ return false;
+
+ nb_read = read(fd, &magic, sizeof(magic));
+ close(fd);
+ return nb_read == sizeof(magic) && magic == BTF_MAGIC;
+}
+
+static struct btf *btf_open(const char *path)
+{
+ if (is_btf_raw(path))
+ return btf__parse_raw(path);
+ else
+ return btf__parse_elf(path, NULL);
+}
+
+BTF_ID_LIST(test_list)
+BTF_ID_UNUSED
+BTF_ID(typedef, pid_t)
+BTF_ID(struct, sk_buff)
+BTF_ID(union, thread_union)
+BTF_ID(func, memcpy)
+
+struct symbol {
+ const char *name;
+ int type;
+ int id;
+};
+
+struct symbol test_symbols[] = {
+ { "unused", -1, 0 },
+ { "pid_t", BTF_KIND_TYPEDEF, -1 },
+ { "sk_buff", BTF_KIND_STRUCT, -1 },
+ { "thread_union", BTF_KIND_UNION, -1 },
+ { "memcpy", BTF_KIND_FUNC, -1 },
+};
+
+static int
+__resolve_symbol(struct btf *btf, int type_id)
+{
+ const struct btf_type *type;
+ const char *str;
+ unsigned int i;
+
+ type = btf__type_by_id(btf, type_id);
+ if (!type) {
+ PRINT_FAIL("Failed to get type for ID %d\n", type_id);
+ return -1;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(test_symbols); i++) {
+ if (test_symbols[i].id != -1)
+ continue;
+
+ if (BTF_INFO_KIND(type->info) != test_symbols[i].type)
+ continue;
+
+ str = btf__name_by_offset(btf, type->name_off);
+ if (!str) {
+ PRINT_FAIL("Failed to get name for BTF ID %d\n", type_id);
+ return -1;
+ }
+
+ if (!strcmp(str, test_symbols[i].name))
+ test_symbols[i].id = type_id;
+ }
+
+ return 0;
+}
+
+static int resolve_symbols(void)
+{
+ const char *path = VMLINUX_BTF;
+ struct btf *btf;
+ int type_id;
+ __u32 nr;
+
+ btf = btf_open(path);
+ if (CHECK(libbpf_get_error(btf), "resolve",
+ "Failed to load BTF from %s\n", path))
+ return -1;
+
+ nr = btf__get_nr_types(btf);
+
+ for (type_id = 1; type_id <= nr; type_id++) {
+ if (__resolve_symbol(btf, type_id))
+ break;
+ }
+
+ btf__free(btf);
+ return 0;
+}
+
+int test_resolve_btfids(void)
+{
+ unsigned int i;
+ int ret = 0;
+
+ if (resolve_symbols())
+ return -1;
+
+ /* Check BTF_ID_LIST(test_list) IDs */
+ for (i = 0; i < ARRAY_SIZE(test_symbols) && !ret; i++) {
+ ret = CHECK(test_list[i] != test_symbols[i].id,
+ "id_check",
+ "wrong ID for %s (%d != %d)\n", test_symbols[i].name,
+ test_list[i], test_symbols[i].id);
+ }
+
+ return 0;
+}
--
2.25.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 1/9] bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object
2020-07-03 9:51 ` [PATCH v5 bpf-next 1/9] bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object Jiri Olsa
@ 2020-07-07 0:34 ` Andrii Nakryiko
2020-07-07 15:43 ` Jiri Olsa
0 siblings, 1 reply; 22+ messages in thread
From: Andrii Nakryiko @ 2020-07-07 0:34 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Networking, bpf, Song Liu,
Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
Wenbo Zhang, KP Singh, Andrii Nakryiko, Brendan Gregg,
Florent Revest, Al Viro
On Fri, Jul 3, 2020 at 2:52 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> The resolve_btfids tool scans elf object for .BTF_ids section
> and resolves its symbols with BTF ID values.
>
> It will be used to during linking time to resolve arrays of BTF
> ID values used in verifier, so these IDs do not need to be
> resolved in runtime.
>
> The expected layout of .BTF_ids section is described in btfid.c
> header. Related kernel changes are coming in following changes.
>
> Build issue reported by 0-DAY CI Kernel Test Service.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/bpf/resolve_btfids/Build | 26 ++
> tools/bpf/resolve_btfids/Makefile | 77 ++++
> tools/bpf/resolve_btfids/main.c | 716 ++++++++++++++++++++++++++++++
> 3 files changed, 819 insertions(+)
> create mode 100644 tools/bpf/resolve_btfids/Build
> create mode 100644 tools/bpf/resolve_btfids/Makefile
> create mode 100644 tools/bpf/resolve_btfids/main.c
>
> diff --git a/tools/bpf/resolve_btfids/Build b/tools/bpf/resolve_btfids/Build
> new file mode 100644
> index 000000000000..c7318cc55341
> --- /dev/null
> +++ b/tools/bpf/resolve_btfids/Build
> @@ -0,0 +1,26 @@
> +resolve_btfids-y += main.o
> +resolve_btfids-y += rbtree.o
> +resolve_btfids-y += zalloc.o
> +resolve_btfids-y += string.o
> +resolve_btfids-y += ctype.o
> +resolve_btfids-y += str_error_r.o
> +
> +$(OUTPUT)rbtree.o: ../../lib/rbtree.c FORCE
> + $(call rule_mkdir)
> + $(call if_changed_dep,cc_o_c)
> +
> +$(OUTPUT)zalloc.o: ../../lib/zalloc.c FORCE
> + $(call rule_mkdir)
> + $(call if_changed_dep,cc_o_c)
> +
> +$(OUTPUT)string.o: ../../lib/string.c FORCE
> + $(call rule_mkdir)
> + $(call if_changed_dep,cc_o_c)
> +
> +$(OUTPUT)ctype.o: ../../lib/ctype.c FORCE
> + $(call rule_mkdir)
> + $(call if_changed_dep,cc_o_c)
> +
> +$(OUTPUT)str_error_r.o: ../../lib/str_error_r.c FORCE
> + $(call rule_mkdir)
> + $(call if_changed_dep,cc_o_c)
Is Build also a Makefile? If that's the case, why not:
$(output)%.o: ../../lib/%.c FORCE
$(call rule_mkdir)
$(call if_changed_dep,cc_o_c)
?
> diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> new file mode 100644
> index 000000000000..948378ca73d4
> --- /dev/null
> +++ b/tools/bpf/resolve_btfids/Makefile
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +include ../../scripts/Makefile.include
> +
> +ifeq ($(srctree),)
> +srctree := $(patsubst %/,%,$(dir $(CURDIR)))
> +srctree := $(patsubst %/,%,$(dir $(srctree)))
> +srctree := $(patsubst %/,%,$(dir $(srctree)))
> +endif
> +
> +ifeq ($(V),1)
> + Q =
> + msg =
> +else
> + Q = @
> + msg = @printf ' %-8s %s%s\n' "$(1)" "$(notdir $(2))" "$(if $(3), $(3))";
> + MAKEFLAGS=--no-print-directory
> +endif
> +
> +OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
Ok, so this builds nicely for in-tree build, but when I did
out-of-tree build (I use KBUILD_OUTPUT, haven't checked specifying
O=whatever), I get:
LD vmlinux
BTFIDS vmlinux
/data/users/andriin/linux/scripts/link-vmlinux.sh: line 342:
/data/users/andriin/linux/tools/bpf/resolve_btfids/resolve_btfids: No
such file or directory
I suspect because you are assuming OUTPUT to be in srctree? You
probably need to adjust for out-of-tree mode.
> +
> +LIBBPF_SRC := $(srctree)/tools/lib/bpf/
> +SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
> +
> +BPFOBJ := $(OUTPUT)/libbpf.a
> +SUBCMDOBJ := $(OUTPUT)/libsubcmd.a
[...]
> +
> +#define BTF_IDS_SECTION ".BTF.ids"
You haven't updated a bunch of places (cover letter, this patch commit
message, maybe somewhere else) after renaming from .BTF_ids, please
keep them in sync. Also, while I'm not too strongly against this name,
it does sound like this section is part of generic BTF format (as is
.BTF and .BTF.ext), which it is not, because it's so kernel-specific.
So I'm mildly against it and pro .BTF_ids.
> +#define BTF_ID "__BTF_ID__"
> +
> +#define BTF_STRUCT "struct"
> +#define BTF_UNION "union"
> +#define BTF_TYPEDEF "typedef"
> +#define BTF_FUNC "func"
> +#define BTF_SET "set"
> +
[...]
> +}
> +
> +static struct btf *btf__parse_raw(const char *file)
I thought you were going to add this to libbpf itself? Or you planned
to do a follow up patch later?
> +{
> + struct btf *btf;
> + struct stat st;
> + __u8 *buf;
> + FILE *f;
> +
> + if (stat(file, &st))
> + return NULL;
> +
> + f = fopen(file, "rb");
> + if (!f)
> + return NULL;
> +
> + buf = malloc(st.st_size);
> + if (!buf) {
> + btf = ERR_PTR(-ENOMEM);
> + goto exit_close;
> + }
> +
> + if ((size_t) st.st_size != fread(buf, 1, st.st_size, f)) {
> + btf = ERR_PTR(-EINVAL);
> + goto exit_free;
> + }
> +
> + btf = btf__new(buf, st.st_size);
> +
> +exit_free:
> + free(buf);
> +exit_close:
> + fclose(f);
> + return btf;
> +}
> +
[...]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 4/9] bpf: Resolve BTF IDs in vmlinux image
2020-07-03 9:51 ` [PATCH v5 bpf-next 4/9] bpf: Resolve BTF IDs in vmlinux image Jiri Olsa
@ 2020-07-07 0:38 ` Andrii Nakryiko
2020-07-07 15:35 ` Jiri Olsa
0 siblings, 1 reply; 22+ messages in thread
From: Andrii Nakryiko @ 2020-07-07 0:38 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking,
bpf, Song Liu, Yonghong Song, Martin KaFai Lau, David Miller,
John Fastabend, Wenbo Zhang, KP Singh, Brendan Gregg,
Florent Revest, Al Viro
On Fri, Jul 3, 2020 at 2:52 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Using BTF_ID_LIST macro to define lists for several helpers
> using BTF arguments.
>
> And running resolve_btfids on vmlinux elf object during linking,
> so the .BTF_ids section gets the IDs resolved.
>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> Makefile | 3 ++-
> kernel/trace/bpf_trace.c | 9 +++++++--
> net/core/filter.c | 9 +++++++--
> scripts/link-vmlinux.sh | 6 ++++++
> 4 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 8db4fd8097e0..def58d4f9ed7 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -448,6 +448,7 @@ OBJSIZE = $(CROSS_COMPILE)size
> STRIP = $(CROSS_COMPILE)strip
> endif
> PAHOLE = pahole
> +RESOLVE_BTFIDS = $(srctree)/tools/bpf/resolve_btfids/resolve_btfids
Oh, this is probably wrong and why out-of-tree build fails. Why don't
you follow how this is done for objtool?
> LEX = flex
> YACC = bison
> AWK = awk
> @@ -510,7 +511,7 @@ GCC_PLUGINS_CFLAGS :=
> CLANG_FLAGS :=
>
> export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC
> -export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
> +export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
> export PERL PYTHON PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
> export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ
> export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE
[...]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 5/9] bpf: Remove btf_id helpers resolving
2020-07-03 9:51 ` [PATCH v5 bpf-next 5/9] bpf: Remove btf_id helpers resolving Jiri Olsa
@ 2020-07-07 0:46 ` Andrii Nakryiko
0 siblings, 0 replies; 22+ messages in thread
From: Andrii Nakryiko @ 2020-07-07 0:46 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Networking, bpf, Song Liu,
Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
Wenbo Zhang, KP Singh, Andrii Nakryiko, Brendan Gregg,
Florent Revest, Al Viro
On Fri, Jul 3, 2020 at 2:52 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Now when we moved the helpers btf_id arrays into .BTF_ids section,
> we can remove the code that resolve those IDs in runtime.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> kernel/bpf/btf.c | 89 +++---------------------------------------------
> 1 file changed, 5 insertions(+), 84 deletions(-)
>
LGTM.
Acked-by: Andrii Nakryiko <andriin@fb.com>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 4c3007f428b1..71140b73ae3c 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -4079,96 +4079,17 @@ int btf_struct_access(struct bpf_verifier_log *log,
> return -EINVAL;
> }
>
[...]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 6/9] bpf: Use BTF_ID to resolve bpf_ctx_convert struct
2020-07-03 9:51 ` [PATCH v5 bpf-next 6/9] bpf: Use BTF_ID to resolve bpf_ctx_convert struct Jiri Olsa
@ 2020-07-07 0:47 ` Andrii Nakryiko
0 siblings, 0 replies; 22+ messages in thread
From: Andrii Nakryiko @ 2020-07-07 0:47 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Networking, bpf, Song Liu,
Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
Wenbo Zhang, KP Singh, Andrii Nakryiko, Brendan Gregg,
Florent Revest, Al Viro
On Fri, Jul 3, 2020 at 2:52 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> This way the ID is resolved during compile time,
> and we can remove the runtime name search.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
Acked-by: Andrii Nakryiko <andriin@fb.com>
> kernel/bpf/btf.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 8/9] tools headers: Adopt verbatim copy of btf_ids.h from kernel sources
2020-07-03 9:51 ` [PATCH v5 bpf-next 8/9] tools headers: Adopt verbatim copy of btf_ids.h from kernel sources Jiri Olsa
@ 2020-07-07 0:56 ` Andrii Nakryiko
2020-07-07 15:44 ` Jiri Olsa
0 siblings, 1 reply; 22+ messages in thread
From: Andrii Nakryiko @ 2020-07-07 0:56 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking,
bpf, Song Liu, Yonghong Song, Martin KaFai Lau, David Miller,
John Fastabend, Wenbo Zhang, KP Singh, Brendan Gregg,
Florent Revest, Al Viro
On Fri, Jul 3, 2020 at 2:53 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> It will be needed by bpf selftest for resolve_btfids tool.
>
> Also adding __PASTE macro as btf_ids.h dependency, which is
> defined in:
>
> include/linux/compiler_types.h
>
> but because tools/include do not have this header, I'm putting
> the macro into linux/compiler.h header.
>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/include/linux/btf_ids.h | 87 ++++++++++++++++++++++++++++++++++
> tools/include/linux/compiler.h | 4 ++
> 2 files changed, 91 insertions(+)
> create mode 100644 tools/include/linux/btf_ids.h
>
> diff --git a/tools/include/linux/btf_ids.h b/tools/include/linux/btf_ids.h
> new file mode 100644
> index 000000000000..d317150bc9e3
> --- /dev/null
> +++ b/tools/include/linux/btf_ids.h
> @@ -0,0 +1,87 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _LINUX_BTF_IDS_H
> +#define _LINUX_BTF_IDS_H
> +
> +#include <linux/compiler.h> /* for __PASTE */
> +
> +/*
> + * Following macros help to define lists of BTF IDs placed
> + * in .BTF_ids section. They are initially filled with zeros
One more inconsistency with .BTF_ids vs .BTF.ids (probably same in the
original header, which I missed).
> + * (during compilation) and resolved later during the
> + * linking phase by btfid tool.
typo: resolve_btfids tool
> + *
> + * Any change in list layout must be reflected in btfid
> + * tool logic.
> + */
> +
[...]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 9/9] selftests/bpf: Add test for resolve_btfids
2020-07-03 9:51 ` [PATCH v5 bpf-next 9/9] selftests/bpf: Add test for resolve_btfids Jiri Olsa
@ 2020-07-07 1:26 ` Andrii Nakryiko
2020-07-07 15:57 ` Jiri Olsa
0 siblings, 1 reply; 22+ messages in thread
From: Andrii Nakryiko @ 2020-07-07 1:26 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, Networking, bpf, Song Liu,
Yonghong Song, Martin KaFai Lau, David Miller, John Fastabend,
Wenbo Zhang, KP Singh, Andrii Nakryiko, Brendan Gregg,
Florent Revest, Al Viro
On Fri, Jul 3, 2020 at 2:54 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding resolve_btfids test under test_progs suite.
>
> It's possible to use btf_ids.h header and its logic in
> user space application, so we can add easy test for it.
>
> The test defines BTF_ID_LIST and checks it gets properly
> resolved.
>
> For this reason the test_progs binary (and other binaries
> that use TRUNNER* macros) is processed with resolve_btfids
> tool, which resolves BTF IDs in .BTF.ids section.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
> tools/testing/selftests/bpf/Makefile | 22 ++-
> .../selftests/bpf/prog_tests/resolve_btfids.c | 170 ++++++++++++++++++
> 2 files changed, 190 insertions(+), 2 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 1f9c696b3edf..b47a685d12bd 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -190,6 +190,16 @@ else
> cp "$(VMLINUX_H)" $@
> endif
>
> +$(SCRATCH_DIR)/resolve_btfids: $(BPFOBJ) \
> + $(TOOLSDIR)/bpf/resolve_btfids/main.c \
> + $(TOOLSDIR)/lib/rbtree.c \
> + $(TOOLSDIR)/lib/zalloc.c \
> + $(TOOLSDIR)/lib/string.c \
> + $(TOOLSDIR)/lib/ctype.c \
> + $(TOOLSDIR)/lib/str_error_r.c
> + $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/resolve_btfids \
> + OUTPUT=$(SCRATCH_DIR)/ BPFOBJ=$(BPFOBJ)
> +
please indent OUTPUT, so it doesn't look like it's a separate command
> # Get Clang's default includes on this system, as opposed to those seen by
> # '-target bpf'. This fixes "missing" files on some architectures/distros,
> # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc.
> @@ -333,7 +343,8 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \
> $(TRUNNER_BPF_SKELS) \
> $$(BPFOBJ) | $(TRUNNER_OUTPUT)
> $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@)
> - cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
> + cd $$(@D) && $$(CC) -I. $$(CFLAGS) $(TRUNNER_EXTRA_CFLAGS) \
> + -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
>
> $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \
> %.c \
> @@ -355,6 +366,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \
> | $(TRUNNER_BINARY)-extras
> $$(call msg,BINARY,,$$@)
> $$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@
> + $(TRUNNER_BINARY_EXTRA_CMD)
no need to make this generic, just write out resolve_btfids here explicitly
>
> endef
>
> @@ -365,7 +377,10 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \
> network_helpers.c testing_helpers.c \
> flow_dissector_load.h
> TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \
> - $(wildcard progs/btf_dump_test_case_*.c)
> + $(wildcard progs/btf_dump_test_case_*.c) \
> + $(SCRATCH_DIR)/resolve_btfids
> +TRUNNER_EXTRA_CFLAGS := -D"BUILD_STR(s)=\#s" -DVMLINUX_BTF="BUILD_STR($(VMLINUX_BTF))"
> +TRUNNER_BINARY_EXTRA_CMD := $(SCRATCH_DIR)/resolve_btfids --btf $(VMLINUX_BTF) test_progs
I hope we can get rid of this, see suggestion below.
> TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
> TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
> TRUNNER_BPF_LDFLAGS := -mattr=+alu32
> @@ -373,6 +388,7 @@ $(eval $(call DEFINE_TEST_RUNNER,test_progs))
>
[...]
> +
> +static int duration;
> +
> +static struct btf *btf__parse_raw(const char *file)
another copy here...
> +{
> + struct btf *btf;
> + struct stat st;
> + __u8 *buf;
> + FILE *f;
> +
[...]
> +
> +BTF_ID_LIST(test_list)
> +BTF_ID_UNUSED
> +BTF_ID(typedef, pid_t)
> +BTF_ID(struct, sk_buff)
> +BTF_ID(union, thread_union)
> +BTF_ID(func, memcpy)
> +
> +struct symbol {
> + const char *name;
> + int type;
> + int id;
> +};
> +
> +struct symbol test_symbols[] = {
> + { "unused", -1, 0 },
could use BTF_KIND_UNKN here instead of -1
> + { "pid_t", BTF_KIND_TYPEDEF, -1 },
> + { "sk_buff", BTF_KIND_STRUCT, -1 },
> + { "thread_union", BTF_KIND_UNION, -1 },
> + { "memcpy", BTF_KIND_FUNC, -1 },
> +};
> +
[...]
> +
> +static int resolve_symbols(void)
> +{
> + const char *path = VMLINUX_BTF;
This build-time parameter passing to find the original VMLINUX_BTF
really sucks, IMO.
Why not use the btf_dump tests approach and have our own small
"vmlinux BTF", which resolve_btfids would use to resolve these IDs?
See how btf_dump_xxx.c files define BTFs that are used in tests. You
can do something similar here, and use a well-known BPF object file as
a source of BTF, both here in a test and in Makefile for --btf param
to resolve_btfids?
> + struct btf *btf;
> + int type_id;
> + __u32 nr;
> +
[...]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 4/9] bpf: Resolve BTF IDs in vmlinux image
2020-07-07 0:38 ` Andrii Nakryiko
@ 2020-07-07 15:35 ` Jiri Olsa
0 siblings, 0 replies; 22+ messages in thread
From: Jiri Olsa @ 2020-07-07 15:35 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Networking, bpf, Song Liu, Yonghong Song, Martin KaFai Lau,
David Miller, John Fastabend, Wenbo Zhang, KP Singh,
Brendan Gregg, Florent Revest, Al Viro
On Mon, Jul 06, 2020 at 05:38:54PM -0700, Andrii Nakryiko wrote:
> On Fri, Jul 3, 2020 at 2:52 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Using BTF_ID_LIST macro to define lists for several helpers
> > using BTF arguments.
> >
> > And running resolve_btfids on vmlinux elf object during linking,
> > so the .BTF_ids section gets the IDs resolved.
> >
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > Makefile | 3 ++-
> > kernel/trace/bpf_trace.c | 9 +++++++--
> > net/core/filter.c | 9 +++++++--
> > scripts/link-vmlinux.sh | 6 ++++++
> > 4 files changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 8db4fd8097e0..def58d4f9ed7 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -448,6 +448,7 @@ OBJSIZE = $(CROSS_COMPILE)size
> > STRIP = $(CROSS_COMPILE)strip
> > endif
> > PAHOLE = pahole
> > +RESOLVE_BTFIDS = $(srctree)/tools/bpf/resolve_btfids/resolve_btfids
>
> Oh, this is probably wrong and why out-of-tree build fails. Why don't
> you follow how this is done for objtool?
right, should be $(objtree) in there
jirka
>
> > LEX = flex
> > YACC = bison
> > AWK = awk
> > @@ -510,7 +511,7 @@ GCC_PLUGINS_CFLAGS :=
> > CLANG_FLAGS :=
> >
> > export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE LD CC
> > -export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
> > +export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE RESOLVE_BTFIDS LEX YACC AWK INSTALLKERNEL
> > export PERL PYTHON PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
> > export KGZIP KBZIP2 KLZOP LZMA LZ4 XZ
> > export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE
>
> [...]
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 1/9] bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object
2020-07-07 0:34 ` Andrii Nakryiko
@ 2020-07-07 15:43 ` Jiri Olsa
0 siblings, 0 replies; 22+ messages in thread
From: Jiri Olsa @ 2020-07-07 15:43 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
Song Liu, Yonghong Song, Martin KaFai Lau, David Miller,
John Fastabend, Wenbo Zhang, KP Singh, Andrii Nakryiko,
Brendan Gregg, Florent Revest, Al Viro
On Mon, Jul 06, 2020 at 05:34:52PM -0700, Andrii Nakryiko wrote:
SNIP
> > +$(OUTPUT)rbtree.o: ../../lib/rbtree.c FORCE
> > + $(call rule_mkdir)
> > + $(call if_changed_dep,cc_o_c)
> > +
> > +$(OUTPUT)zalloc.o: ../../lib/zalloc.c FORCE
> > + $(call rule_mkdir)
> > + $(call if_changed_dep,cc_o_c)
> > +
> > +$(OUTPUT)string.o: ../../lib/string.c FORCE
> > + $(call rule_mkdir)
> > + $(call if_changed_dep,cc_o_c)
> > +
> > +$(OUTPUT)ctype.o: ../../lib/ctype.c FORCE
> > + $(call rule_mkdir)
> > + $(call if_changed_dep,cc_o_c)
> > +
> > +$(OUTPUT)str_error_r.o: ../../lib/str_error_r.c FORCE
> > + $(call rule_mkdir)
> > + $(call if_changed_dep,cc_o_c)
>
> Is Build also a Makefile? If that's the case, why not:
>
> $(output)%.o: ../../lib/%.c FORCE
> $(call rule_mkdir)
> $(call if_changed_dep,cc_o_c)
hum, it is ... I'll try that
>
> ?
>
> > diff --git a/tools/bpf/resolve_btfids/Makefile b/tools/bpf/resolve_btfids/Makefile
> > new file mode 100644
> > index 000000000000..948378ca73d4
> > --- /dev/null
> > +++ b/tools/bpf/resolve_btfids/Makefile
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +include ../../scripts/Makefile.include
> > +
> > +ifeq ($(srctree),)
> > +srctree := $(patsubst %/,%,$(dir $(CURDIR)))
> > +srctree := $(patsubst %/,%,$(dir $(srctree)))
> > +srctree := $(patsubst %/,%,$(dir $(srctree)))
> > +endif
> > +
> > +ifeq ($(V),1)
> > + Q =
> > + msg =
> > +else
> > + Q = @
> > + msg = @printf ' %-8s %s%s\n' "$(1)" "$(notdir $(2))" "$(if $(3), $(3))";
> > + MAKEFLAGS=--no-print-directory
> > +endif
> > +
> > +OUTPUT ?= $(srctree)/tools/bpf/resolve_btfids/
>
> Ok, so this builds nicely for in-tree build, but when I did
> out-of-tree build (I use KBUILD_OUTPUT, haven't checked specifying
> O=whatever), I get:
>
> LD vmlinux
> BTFIDS vmlinux
> /data/users/andriin/linux/scripts/link-vmlinux.sh: line 342:
> /data/users/andriin/linux/tools/bpf/resolve_btfids/resolve_btfids: No
> such file or directory
>
> I suspect because you are assuming OUTPUT to be in srctree? You
> probably need to adjust for out-of-tree mode.
ok, make clean did not clean resolve_btfids, so it was
still there when I tried the out of the tree build..
>
> > +
> > +LIBBPF_SRC := $(srctree)/tools/lib/bpf/
> > +SUBCMD_SRC := $(srctree)/tools/lib/subcmd/
> > +
> > +BPFOBJ := $(OUTPUT)/libbpf.a
> > +SUBCMDOBJ := $(OUTPUT)/libsubcmd.a
>
> [...]
>
> > +
> > +#define BTF_IDS_SECTION ".BTF.ids"
>
> You haven't updated a bunch of places (cover letter, this patch commit
> message, maybe somewhere else) after renaming from .BTF_ids, please
> keep them in sync. Also, while I'm not too strongly against this name,
> it does sound like this section is part of generic BTF format (as is
> .BTF and .BTF.ext), which it is not, because it's so kernel-specific.
> So I'm mildly against it and pro .BTF_ids.
.BTF_ids it is.. I'll change all the places
>
> > +#define BTF_ID "__BTF_ID__"
> > +
> > +#define BTF_STRUCT "struct"
> > +#define BTF_UNION "union"
> > +#define BTF_TYPEDEF "typedef"
> > +#define BTF_FUNC "func"
> > +#define BTF_SET "set"
> > +
>
> [...]
>
> > +}
> > +
> > +static struct btf *btf__parse_raw(const char *file)
>
> I thought you were going to add this to libbpf itself? Or you planned
> to do a follow up patch later?
yes, I did not want to complicate this patchset more,
and send this change after.. also then there'll be one
more reason to make it a library function ;-)
thanks,
jirka
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 8/9] tools headers: Adopt verbatim copy of btf_ids.h from kernel sources
2020-07-07 0:56 ` Andrii Nakryiko
@ 2020-07-07 15:44 ` Jiri Olsa
0 siblings, 0 replies; 22+ messages in thread
From: Jiri Olsa @ 2020-07-07 15:44 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Networking, bpf, Song Liu, Yonghong Song, Martin KaFai Lau,
David Miller, John Fastabend, Wenbo Zhang, KP Singh,
Brendan Gregg, Florent Revest, Al Viro
On Mon, Jul 06, 2020 at 05:56:09PM -0700, Andrii Nakryiko wrote:
> On Fri, Jul 3, 2020 at 2:53 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > It will be needed by bpf selftest for resolve_btfids tool.
> >
> > Also adding __PASTE macro as btf_ids.h dependency, which is
> > defined in:
> >
> > include/linux/compiler_types.h
> >
> > but because tools/include do not have this header, I'm putting
> > the macro into linux/compiler.h header.
> >
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/include/linux/btf_ids.h | 87 ++++++++++++++++++++++++++++++++++
> > tools/include/linux/compiler.h | 4 ++
> > 2 files changed, 91 insertions(+)
> > create mode 100644 tools/include/linux/btf_ids.h
> >
> > diff --git a/tools/include/linux/btf_ids.h b/tools/include/linux/btf_ids.h
> > new file mode 100644
> > index 000000000000..d317150bc9e3
> > --- /dev/null
> > +++ b/tools/include/linux/btf_ids.h
> > @@ -0,0 +1,87 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef _LINUX_BTF_IDS_H
> > +#define _LINUX_BTF_IDS_H
> > +
> > +#include <linux/compiler.h> /* for __PASTE */
> > +
> > +/*
> > + * Following macros help to define lists of BTF IDs placed
> > + * in .BTF_ids section. They are initially filled with zeros
>
> One more inconsistency with .BTF_ids vs .BTF.ids (probably same in the
> original header, which I missed).
ok
>
> > + * (during compilation) and resolved later during the
> > + * linking phase by btfid tool.
>
> typo: resolve_btfids tool
will fix
thanks,
jirka
>
> > + *
> > + * Any change in list layout must be reflected in btfid
> > + * tool logic.
> > + */
> > +
>
> [...]
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 9/9] selftests/bpf: Add test for resolve_btfids
2020-07-07 1:26 ` Andrii Nakryiko
@ 2020-07-07 15:57 ` Jiri Olsa
2020-07-07 17:49 ` Andrii Nakryiko
0 siblings, 1 reply; 22+ messages in thread
From: Jiri Olsa @ 2020-07-07 15:57 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
Song Liu, Yonghong Song, Martin KaFai Lau, David Miller,
John Fastabend, Wenbo Zhang, KP Singh, Andrii Nakryiko,
Brendan Gregg, Florent Revest, Al Viro
On Mon, Jul 06, 2020 at 06:26:28PM -0700, Andrii Nakryiko wrote:
> On Fri, Jul 3, 2020 at 2:54 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding resolve_btfids test under test_progs suite.
> >
> > It's possible to use btf_ids.h header and its logic in
> > user space application, so we can add easy test for it.
> >
> > The test defines BTF_ID_LIST and checks it gets properly
> > resolved.
> >
> > For this reason the test_progs binary (and other binaries
> > that use TRUNNER* macros) is processed with resolve_btfids
> > tool, which resolves BTF IDs in .BTF.ids section.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> > tools/testing/selftests/bpf/Makefile | 22 ++-
> > .../selftests/bpf/prog_tests/resolve_btfids.c | 170 ++++++++++++++++++
> > 2 files changed, 190 insertions(+), 2 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 1f9c696b3edf..b47a685d12bd 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -190,6 +190,16 @@ else
> > cp "$(VMLINUX_H)" $@
> > endif
> >
> > +$(SCRATCH_DIR)/resolve_btfids: $(BPFOBJ) \
> > + $(TOOLSDIR)/bpf/resolve_btfids/main.c \
> > + $(TOOLSDIR)/lib/rbtree.c \
> > + $(TOOLSDIR)/lib/zalloc.c \
> > + $(TOOLSDIR)/lib/string.c \
> > + $(TOOLSDIR)/lib/ctype.c \
> > + $(TOOLSDIR)/lib/str_error_r.c
> > + $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/resolve_btfids \
> > + OUTPUT=$(SCRATCH_DIR)/ BPFOBJ=$(BPFOBJ)
> > +
>
> please indent OUTPUT, so it doesn't look like it's a separate command
ok
>
> > # Get Clang's default includes on this system, as opposed to those seen by
> > # '-target bpf'. This fixes "missing" files on some architectures/distros,
> > # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc.
> > @@ -333,7 +343,8 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \
> > $(TRUNNER_BPF_SKELS) \
> > $$(BPFOBJ) | $(TRUNNER_OUTPUT)
> > $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@)
> > - cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
> > + cd $$(@D) && $$(CC) -I. $$(CFLAGS) $(TRUNNER_EXTRA_CFLAGS) \
> > + -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
> >
> > $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \
> > %.c \
> > @@ -355,6 +366,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \
> > | $(TRUNNER_BINARY)-extras
> > $$(call msg,BINARY,,$$@)
> > $$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@
> > + $(TRUNNER_BINARY_EXTRA_CMD)
>
> no need to make this generic, just write out resolve_btfids here explicitly
currently resolve_btfids fails if there's no .BTF.ids section found,
but we can make it silently pass i nthis case and then we can invoke
it for all the binaries
>
> >
> > endef
> >
> > @@ -365,7 +377,10 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \
> > network_helpers.c testing_helpers.c \
> > flow_dissector_load.h
> > TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \
> > - $(wildcard progs/btf_dump_test_case_*.c)
> > + $(wildcard progs/btf_dump_test_case_*.c) \
> > + $(SCRATCH_DIR)/resolve_btfids
> > +TRUNNER_EXTRA_CFLAGS := -D"BUILD_STR(s)=\#s" -DVMLINUX_BTF="BUILD_STR($(VMLINUX_BTF))"
> > +TRUNNER_BINARY_EXTRA_CMD := $(SCRATCH_DIR)/resolve_btfids --btf $(VMLINUX_BTF) test_progs
>
> I hope we can get rid of this, see suggestion below.
>
> > TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
> > TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
> > TRUNNER_BPF_LDFLAGS := -mattr=+alu32
> > @@ -373,6 +388,7 @@ $(eval $(call DEFINE_TEST_RUNNER,test_progs))
> >
>
> [...]
>
> > +
> > +static int duration;
> > +
> > +static struct btf *btf__parse_raw(const char *file)
>
> another copy here...
ok
>
> > +{
> > + struct btf *btf;
> > + struct stat st;
> > + __u8 *buf;
> > + FILE *f;
> > +
>
> [...]
>
> > +
> > +BTF_ID_LIST(test_list)
> > +BTF_ID_UNUSED
> > +BTF_ID(typedef, pid_t)
> > +BTF_ID(struct, sk_buff)
> > +BTF_ID(union, thread_union)
> > +BTF_ID(func, memcpy)
> > +
> > +struct symbol {
> > + const char *name;
> > + int type;
> > + int id;
> > +};
> > +
> > +struct symbol test_symbols[] = {
> > + { "unused", -1, 0 },
>
> could use BTF_KIND_UNKN here instead of -1
ok
>
> > + { "pid_t", BTF_KIND_TYPEDEF, -1 },
> > + { "sk_buff", BTF_KIND_STRUCT, -1 },
> > + { "thread_union", BTF_KIND_UNION, -1 },
> > + { "memcpy", BTF_KIND_FUNC, -1 },
> > +};
> > +
>
> [...]
>
> > +
> > +static int resolve_symbols(void)
> > +{
> > + const char *path = VMLINUX_BTF;
>
>
> This build-time parameter passing to find the original VMLINUX_BTF
> really sucks, IMO.
>
> Why not use the btf_dump tests approach and have our own small
> "vmlinux BTF", which resolve_btfids would use to resolve these IDs?
> See how btf_dump_xxx.c files define BTFs that are used in tests. You
> can do something similar here, and use a well-known BPF object file as
> a source of BTF, both here in a test and in Makefile for --btf param
> to resolve_btfids?
well VMLINUX_BTF is there and those types are used are not going
away any time soon ;-) but yea, we can do that.. we do this also
for bpftrace, it's nicer
jirka
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 9/9] selftests/bpf: Add test for resolve_btfids
2020-07-07 15:57 ` Jiri Olsa
@ 2020-07-07 17:49 ` Andrii Nakryiko
2020-07-08 21:18 ` Jiri Olsa
0 siblings, 1 reply; 22+ messages in thread
From: Andrii Nakryiko @ 2020-07-07 17:49 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
Song Liu, Yonghong Song, Martin KaFai Lau, David Miller,
John Fastabend, Wenbo Zhang, KP Singh, Andrii Nakryiko,
Brendan Gregg, Florent Revest, Al Viro
On Tue, Jul 7, 2020 at 8:57 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Jul 06, 2020 at 06:26:28PM -0700, Andrii Nakryiko wrote:
> > On Fri, Jul 3, 2020 at 2:54 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > Adding resolve_btfids test under test_progs suite.
> > >
> > > It's possible to use btf_ids.h header and its logic in
> > > user space application, so we can add easy test for it.
> > >
> > > The test defines BTF_ID_LIST and checks it gets properly
> > > resolved.
> > >
> > > For this reason the test_progs binary (and other binaries
> > > that use TRUNNER* macros) is processed with resolve_btfids
> > > tool, which resolves BTF IDs in .BTF.ids section.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > ---
> > > tools/testing/selftests/bpf/Makefile | 22 ++-
> > > .../selftests/bpf/prog_tests/resolve_btfids.c | 170 ++++++++++++++++++
> > > 2 files changed, 190 insertions(+), 2 deletions(-)
> > > create mode 100644 tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index 1f9c696b3edf..b47a685d12bd 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -190,6 +190,16 @@ else
> > > cp "$(VMLINUX_H)" $@
> > > endif
> > >
> > > +$(SCRATCH_DIR)/resolve_btfids: $(BPFOBJ) \
> > > + $(TOOLSDIR)/bpf/resolve_btfids/main.c \
> > > + $(TOOLSDIR)/lib/rbtree.c \
> > > + $(TOOLSDIR)/lib/zalloc.c \
> > > + $(TOOLSDIR)/lib/string.c \
> > > + $(TOOLSDIR)/lib/ctype.c \
> > > + $(TOOLSDIR)/lib/str_error_r.c
> > > + $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/resolve_btfids \
> > > + OUTPUT=$(SCRATCH_DIR)/ BPFOBJ=$(BPFOBJ)
> > > +
> >
> > please indent OUTPUT, so it doesn't look like it's a separate command
>
> ok
>
> >
> > > # Get Clang's default includes on this system, as opposed to those seen by
> > > # '-target bpf'. This fixes "missing" files on some architectures/distros,
> > > # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc.
> > > @@ -333,7 +343,8 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \
> > > $(TRUNNER_BPF_SKELS) \
> > > $$(BPFOBJ) | $(TRUNNER_OUTPUT)
> > > $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@)
> > > - cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
> > > + cd $$(@D) && $$(CC) -I. $$(CFLAGS) $(TRUNNER_EXTRA_CFLAGS) \
> > > + -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
> > >
> > > $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \
> > > %.c \
> > > @@ -355,6 +366,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \
> > > | $(TRUNNER_BINARY)-extras
> > > $$(call msg,BINARY,,$$@)
> > > $$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@
> > > + $(TRUNNER_BINARY_EXTRA_CMD)
> >
> > no need to make this generic, just write out resolve_btfids here explicitly
>
> currently resolve_btfids fails if there's no .BTF.ids section found,
> but we can make it silently pass i nthis case and then we can invoke
> it for all the binaries
ah, I see. Yeah, either we can add an option to resolve_btfids to not
error when .BTF_ids is missing (probably best), or we can check
whether the test has .BTF_ids section, and if it does - run
resolve_btfids on it. Just ignoring errors always is more error-prone,
because we won't know if it's a real problem we are ignoring, or
missing .BTF_ids.
>
> >
> > >
> > > endef
> > >
> > > @@ -365,7 +377,10 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \
> > > network_helpers.c testing_helpers.c \
> > > flow_dissector_load.h
> > > TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read \
> > > - $(wildcard progs/btf_dump_test_case_*.c)
> > > + $(wildcard progs/btf_dump_test_case_*.c) \
> > > + $(SCRATCH_DIR)/resolve_btfids
> > > +TRUNNER_EXTRA_CFLAGS := -D"BUILD_STR(s)=\#s" -DVMLINUX_BTF="BUILD_STR($(VMLINUX_BTF))"
> > > +TRUNNER_BINARY_EXTRA_CMD := $(SCRATCH_DIR)/resolve_btfids --btf $(VMLINUX_BTF) test_progs
> >
> > I hope we can get rid of this, see suggestion below.
> >
> > > TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE
> > > TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS)
> > > TRUNNER_BPF_LDFLAGS := -mattr=+alu32
> > > @@ -373,6 +388,7 @@ $(eval $(call DEFINE_TEST_RUNNER,test_progs))
> > >
> >
> > [...]
> >
> > > +
> > > +static int duration;
> > > +
> > > +static struct btf *btf__parse_raw(const char *file)
> >
> > another copy here...
>
> ok
>
> >
> > > +{
> > > + struct btf *btf;
> > > + struct stat st;
> > > + __u8 *buf;
> > > + FILE *f;
> > > +
> >
> > [...]
> >
> > > +
> > > +BTF_ID_LIST(test_list)
> > > +BTF_ID_UNUSED
> > > +BTF_ID(typedef, pid_t)
> > > +BTF_ID(struct, sk_buff)
> > > +BTF_ID(union, thread_union)
> > > +BTF_ID(func, memcpy)
> > > +
> > > +struct symbol {
> > > + const char *name;
> > > + int type;
> > > + int id;
> > > +};
> > > +
> > > +struct symbol test_symbols[] = {
> > > + { "unused", -1, 0 },
> >
> > could use BTF_KIND_UNKN here instead of -1
>
> ok
>
> >
> > > + { "pid_t", BTF_KIND_TYPEDEF, -1 },
> > > + { "sk_buff", BTF_KIND_STRUCT, -1 },
> > > + { "thread_union", BTF_KIND_UNION, -1 },
> > > + { "memcpy", BTF_KIND_FUNC, -1 },
> > > +};
> > > +
> >
> > [...]
> >
> > > +
> > > +static int resolve_symbols(void)
> > > +{
> > > + const char *path = VMLINUX_BTF;
> >
> >
> > This build-time parameter passing to find the original VMLINUX_BTF
> > really sucks, IMO.
> >
> > Why not use the btf_dump tests approach and have our own small
> > "vmlinux BTF", which resolve_btfids would use to resolve these IDs?
> > See how btf_dump_xxx.c files define BTFs that are used in tests. You
> > can do something similar here, and use a well-known BPF object file as
> > a source of BTF, both here in a test and in Makefile for --btf param
> > to resolve_btfids?
>
> well VMLINUX_BTF is there and those types are used are not going
> away any time soon ;-) but yea, we can do that.. we do this also
> for bpftrace, it's nicer
"VMLINUX_BTF is there" is not really true in a lot of more complicated
setups, which is why I'd like to avoid that assumption. E.g., for
libbpf Travis CI, we build self-tests in one VM, but run the binary in
a different VM. So either vmlinux itself or the path to it might
change.
Also, having full control over **small** BTF allows to create various
test situations that might be harder to pinpoint in real vmlinux BTF,
e.g., same-named entities with different KINDS (typedef vs struct,
etc). Then if that fails, debugging this on a small BTF is much-much
easier than on a real thing. Real vmlinux BTF is being tested each
time you build a kernel and run selftests inside VM either way, so I
don't think we lose anything in terms of coverage.
>
> jirka
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 bpf-next 9/9] selftests/bpf: Add test for resolve_btfids
2020-07-07 17:49 ` Andrii Nakryiko
@ 2020-07-08 21:18 ` Jiri Olsa
0 siblings, 0 replies; 22+ messages in thread
From: Jiri Olsa @ 2020-07-08 21:18 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, Networking, bpf,
Song Liu, Yonghong Song, Martin KaFai Lau, David Miller,
John Fastabend, Wenbo Zhang, KP Singh, Andrii Nakryiko,
Brendan Gregg, Florent Revest, Al Viro
On Tue, Jul 07, 2020 at 10:49:22AM -0700, Andrii Nakryiko wrote:
SNIP
> > > > # Get Clang's default includes on this system, as opposed to those seen by
> > > > # '-target bpf'. This fixes "missing" files on some architectures/distros,
> > > > # such as asm/byteorder.h, asm/socket.h, asm/sockios.h, sys/cdefs.h etc.
> > > > @@ -333,7 +343,8 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o: \
> > > > $(TRUNNER_BPF_SKELS) \
> > > > $$(BPFOBJ) | $(TRUNNER_OUTPUT)
> > > > $$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@)
> > > > - cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
> > > > + cd $$(@D) && $$(CC) -I. $$(CFLAGS) $(TRUNNER_EXTRA_CFLAGS) \
> > > > + -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
> > > >
> > > > $(TRUNNER_EXTRA_OBJS): $(TRUNNER_OUTPUT)/%.o: \
> > > > %.c \
> > > > @@ -355,6 +366,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS) \
> > > > | $(TRUNNER_BINARY)-extras
> > > > $$(call msg,BINARY,,$$@)
> > > > $$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) -o $$@
> > > > + $(TRUNNER_BINARY_EXTRA_CMD)
> > >
> > > no need to make this generic, just write out resolve_btfids here explicitly
> >
> > currently resolve_btfids fails if there's no .BTF.ids section found,
> > but we can make it silently pass i nthis case and then we can invoke
> > it for all the binaries
>
> ah, I see. Yeah, either we can add an option to resolve_btfids to not
> error when .BTF_ids is missing (probably best), or we can check
> whether the test has .BTF_ids section, and if it does - run
> resolve_btfids on it. Just ignoring errors always is more error-prone,
> because we won't know if it's a real problem we are ignoring, or
> missing .BTF_ids.
ok, sounds good
> > > > +static int resolve_symbols(void)
> > > > +{
> > > > + const char *path = VMLINUX_BTF;
> > >
> > >
> > > This build-time parameter passing to find the original VMLINUX_BTF
> > > really sucks, IMO.
> > >
> > > Why not use the btf_dump tests approach and have our own small
> > > "vmlinux BTF", which resolve_btfids would use to resolve these IDs?
> > > See how btf_dump_xxx.c files define BTFs that are used in tests. You
> > > can do something similar here, and use a well-known BPF object file as
> > > a source of BTF, both here in a test and in Makefile for --btf param
> > > to resolve_btfids?
> >
> > well VMLINUX_BTF is there and those types are used are not going
> > away any time soon ;-) but yea, we can do that.. we do this also
> > for bpftrace, it's nicer
>
>
> "VMLINUX_BTF is there" is not really true in a lot of more complicated
> setups, which is why I'd like to avoid that assumption. E.g., for
> libbpf Travis CI, we build self-tests in one VM, but run the binary in
> a different VM. So either vmlinux itself or the path to it might
> change.
ok
>
> Also, having full control over **small** BTF allows to create various
> test situations that might be harder to pinpoint in real vmlinux BTF,
> e.g., same-named entities with different KINDS (typedef vs struct,
> etc). Then if that fails, debugging this on a small BTF is much-much
> easier than on a real thing. Real vmlinux BTF is being tested each
> time you build a kernel and run selftests inside VM either way, so I
> don't think we lose anything in terms of coverage.
agreed, will add that
thanks,
jirka
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2020-07-08 21:18 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-03 9:51 [PATCH v5 bpf-next 0/9] bpf: Add d_path helper - preparation changes Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 1/9] bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object Jiri Olsa
2020-07-07 0:34 ` Andrii Nakryiko
2020-07-07 15:43 ` Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 2/9] bpf: Compile resolve_btfids tool at kernel compilation start Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 3/9] bpf: Add BTF_ID_LIST/BTF_ID/BTF_ID_UNUSED macros Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 4/9] bpf: Resolve BTF IDs in vmlinux image Jiri Olsa
2020-07-07 0:38 ` Andrii Nakryiko
2020-07-07 15:35 ` Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 5/9] bpf: Remove btf_id helpers resolving Jiri Olsa
2020-07-07 0:46 ` Andrii Nakryiko
2020-07-03 9:51 ` [PATCH v5 bpf-next 6/9] bpf: Use BTF_ID to resolve bpf_ctx_convert struct Jiri Olsa
2020-07-07 0:47 ` Andrii Nakryiko
2020-07-03 9:51 ` [PATCH v5 bpf-next 7/9] bpf: Add info about .BTF.ids section to btf.rst Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 8/9] tools headers: Adopt verbatim copy of btf_ids.h from kernel sources Jiri Olsa
2020-07-07 0:56 ` Andrii Nakryiko
2020-07-07 15:44 ` Jiri Olsa
2020-07-03 9:51 ` [PATCH v5 bpf-next 9/9] selftests/bpf: Add test for resolve_btfids Jiri Olsa
2020-07-07 1:26 ` Andrii Nakryiko
2020-07-07 15:57 ` Jiri Olsa
2020-07-07 17:49 ` Andrii Nakryiko
2020-07-08 21:18 ` Jiri Olsa
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).