* [PATCH iproute2 0/5] Various BPF improvements
@ 2018-07-17 23:31 Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 1/5] bpf: import btf uapi kernel header Daniel Borkmann
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Daniel Borkmann @ 2018-07-17 23:31 UTC (permalink / raw)
To: dsahern; +Cc: jakub.kicinski, alexei.starovoitov, netdev, Daniel Borkmann
Main part of this set is to: i) avoid strict af_alg kernel dependency,
ii) add loader support for bpf to bpf calls and iii) add btf loader
support with an option to annotate maps. For details please see the
individual patches. Thanks!
Daniel Borkmann (5):
bpf: import btf uapi kernel header
bpf: move bpf_elf_map fixup notification under verbose
bpf: remove strict dependency on af_alg
bpf: implement bpf to bpf calls support
bpf: implement btf handling and map annotation
include/bpf_elf.h | 9 +
include/bpf_util.h | 1 +
include/uapi/linux/btf.h | 113 +++++++++
lib/bpf.c | 645 +++++++++++++++++++++++++++++++++++++----------
4 files changed, 639 insertions(+), 129 deletions(-)
create mode 100644 include/uapi/linux/btf.h
--
2.9.5
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH iproute2 1/5] bpf: import btf uapi kernel header
2018-07-17 23:31 [PATCH iproute2 0/5] Various BPF improvements Daniel Borkmann
@ 2018-07-17 23:31 ` Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 2/5] bpf: move bpf_elf_map fixup notification under verbose Daniel Borkmann
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Daniel Borkmann @ 2018-07-17 23:31 UTC (permalink / raw)
To: dsahern; +Cc: jakub.kicinski, alexei.starovoitov, netdev, Daniel Borkmann
Needed later on for BTF loader.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/uapi/linux/btf.h | 113 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 113 insertions(+)
create mode 100644 include/uapi/linux/btf.h
diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
new file mode 100644
index 0000000..0b5ddbe
--- /dev/null
+++ b/include/uapi/linux/btf.h
@@ -0,0 +1,113 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* Copyright (c) 2018 Facebook */
+#ifndef _UAPI__LINUX_BTF_H__
+#define _UAPI__LINUX_BTF_H__
+
+#include <linux/types.h>
+
+#define BTF_MAGIC 0xeB9F
+#define BTF_VERSION 1
+
+struct btf_header {
+ __u16 magic;
+ __u8 version;
+ __u8 flags;
+ __u32 hdr_len;
+
+ /* All offsets are in bytes relative to the end of this header */
+ __u32 type_off; /* offset of type section */
+ __u32 type_len; /* length of type section */
+ __u32 str_off; /* offset of string section */
+ __u32 str_len; /* length of string section */
+};
+
+/* Max # of type identifier */
+#define BTF_MAX_TYPE 0x0000ffff
+/* Max offset into the string section */
+#define BTF_MAX_NAME_OFFSET 0x0000ffff
+/* Max # of struct/union/enum members or func args */
+#define BTF_MAX_VLEN 0xffff
+
+struct btf_type {
+ __u32 name_off;
+ /* "info" bits arrangement
+ * bits 0-15: vlen (e.g. # of struct's members)
+ * bits 16-23: unused
+ * bits 24-27: kind (e.g. int, ptr, array...etc)
+ * bits 28-31: unused
+ */
+ __u32 info;
+ /* "size" is used by INT, ENUM, STRUCT and UNION.
+ * "size" tells the size of the type it is describing.
+ *
+ * "type" is used by PTR, TYPEDEF, VOLATILE, CONST and RESTRICT.
+ * "type" is a type_id referring to another type.
+ */
+ union {
+ __u32 size;
+ __u32 type;
+ };
+};
+
+#define BTF_INFO_KIND(info) (((info) >> 24) & 0x0f)
+#define BTF_INFO_VLEN(info) ((info) & 0xffff)
+
+#define BTF_KIND_UNKN 0 /* Unknown */
+#define BTF_KIND_INT 1 /* Integer */
+#define BTF_KIND_PTR 2 /* Pointer */
+#define BTF_KIND_ARRAY 3 /* Array */
+#define BTF_KIND_STRUCT 4 /* Struct */
+#define BTF_KIND_UNION 5 /* Union */
+#define BTF_KIND_ENUM 6 /* Enumeration */
+#define BTF_KIND_FWD 7 /* Forward */
+#define BTF_KIND_TYPEDEF 8 /* Typedef */
+#define BTF_KIND_VOLATILE 9 /* Volatile */
+#define BTF_KIND_CONST 10 /* Const */
+#define BTF_KIND_RESTRICT 11 /* Restrict */
+#define BTF_KIND_MAX 11
+#define NR_BTF_KINDS 12
+
+/* For some specific BTF_KIND, "struct btf_type" is immediately
+ * followed by extra data.
+ */
+
+/* BTF_KIND_INT is followed by a u32 and the following
+ * is the 32 bits arrangement:
+ */
+#define BTF_INT_ENCODING(VAL) (((VAL) & 0x0f000000) >> 24)
+#define BTF_INT_OFFSET(VAL) (((VAL & 0x00ff0000)) >> 16)
+#define BTF_INT_BITS(VAL) ((VAL) & 0x0000ffff)
+
+/* Attributes stored in the BTF_INT_ENCODING */
+#define BTF_INT_SIGNED (1 << 0)
+#define BTF_INT_CHAR (1 << 1)
+#define BTF_INT_BOOL (1 << 2)
+
+/* BTF_KIND_ENUM is followed by multiple "struct btf_enum".
+ * The exact number of btf_enum is stored in the vlen (of the
+ * info in "struct btf_type").
+ */
+struct btf_enum {
+ __u32 name_off;
+ __s32 val;
+};
+
+/* BTF_KIND_ARRAY is followed by one "struct btf_array" */
+struct btf_array {
+ __u32 type;
+ __u32 index_type;
+ __u32 nelems;
+};
+
+/* BTF_KIND_STRUCT and BTF_KIND_UNION are followed
+ * by multiple "struct btf_member". The exact number
+ * of btf_member is stored in the vlen (of the info in
+ * "struct btf_type").
+ */
+struct btf_member {
+ __u32 name_off;
+ __u32 type;
+ __u32 offset; /* offset in bits */
+};
+
+#endif /* _UAPI__LINUX_BTF_H__ */
--
2.9.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2 2/5] bpf: move bpf_elf_map fixup notification under verbose
2018-07-17 23:31 [PATCH iproute2 0/5] Various BPF improvements Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 1/5] bpf: import btf uapi kernel header Daniel Borkmann
@ 2018-07-17 23:31 ` Daniel Borkmann
2018-07-18 0:24 ` Jakub Kicinski
2018-07-17 23:31 ` [PATCH iproute2 3/5] bpf: remove strict dependency on af_alg Daniel Borkmann
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Daniel Borkmann @ 2018-07-17 23:31 UTC (permalink / raw)
To: dsahern; +Cc: jakub.kicinski, alexei.starovoitov, netdev, Daniel Borkmann
No need to spam the user with this if it can be fixed gracefully
anyway. Therefore, move it under verbose option.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
lib/bpf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/bpf.c b/lib/bpf.c
index 4e26c0d..42093db 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -1893,9 +1893,9 @@ static int bpf_fetch_maps_end(struct bpf_elf_ctx *ctx)
}
memcpy(ctx->maps, fixup, sizeof(fixup));
-
- printf("Note: %zu bytes struct bpf_elf_map fixup performed due to size mismatch!\n",
- sizeof(struct bpf_elf_map) - ctx->map_len);
+ if (ctx->verbose)
+ printf("%zu bytes struct bpf_elf_map fixup performed due to size mismatch!\n",
+ sizeof(struct bpf_elf_map) - ctx->map_len);
return 0;
}
--
2.9.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2 3/5] bpf: remove strict dependency on af_alg
2018-07-17 23:31 [PATCH iproute2 0/5] Various BPF improvements Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 1/5] bpf: import btf uapi kernel header Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 2/5] bpf: move bpf_elf_map fixup notification under verbose Daniel Borkmann
@ 2018-07-17 23:31 ` Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 4/5] bpf: implement bpf to bpf calls support Daniel Borkmann
` (2 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Daniel Borkmann @ 2018-07-17 23:31 UTC (permalink / raw)
To: dsahern; +Cc: jakub.kicinski, alexei.starovoitov, netdev, Daniel Borkmann
Do not bail out when AF_ALG is not supported by the kernel and
only do so when a map is requested in object ns where we're
calculating the hash. Otherwise, the loader can operate just
fine, therefore lets not fail early when it's not needed.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
lib/bpf.c | 74 ++++++++++++++++++++++++---------------------------------------
1 file changed, 28 insertions(+), 46 deletions(-)
diff --git a/lib/bpf.c b/lib/bpf.c
index 42093db..135f496 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -1125,6 +1125,7 @@ struct bpf_elf_ctx {
GElf_Ehdr elf_hdr;
Elf_Data *sym_tab;
Elf_Data *str_tab;
+ char obj_uid[64];
int obj_fd;
int map_fds[ELF_MAX_MAPS];
struct bpf_elf_map maps[ELF_MAX_MAPS];
@@ -1138,6 +1139,7 @@ struct bpf_elf_ctx {
enum bpf_prog_type type;
__u32 ifindex;
bool verbose;
+ bool noafalg;
struct bpf_elf_st stat;
struct bpf_hash_entry *ht[256];
char *log;
@@ -1253,22 +1255,15 @@ static int bpf_obj_hash(const char *object, uint8_t *out, size_t len)
return -EINVAL;
cfd = socket(AF_ALG, SOCK_SEQPACKET, 0);
- if (cfd < 0) {
- fprintf(stderr, "Cannot get AF_ALG socket: %s\n",
- strerror(errno));
+ if (cfd < 0)
return cfd;
- }
ret = bind(cfd, (struct sockaddr *)&alg, sizeof(alg));
- if (ret < 0) {
- fprintf(stderr, "Error binding socket: %s\n", strerror(errno));
+ if (ret < 0)
goto out_cfd;
- }
ofd = accept(cfd, NULL, 0);
if (ofd < 0) {
- fprintf(stderr, "Error accepting socket: %s\n",
- strerror(errno));
ret = ofd;
goto out_cfd;
}
@@ -1313,29 +1308,7 @@ out_cfd:
return ret;
}
-static const char *bpf_get_obj_uid(const char *pathname)
-{
- static bool bpf_uid_cached;
- static char bpf_uid[64];
- uint8_t tmp[20];
- int ret;
-
- if (bpf_uid_cached)
- goto done;
-
- ret = bpf_obj_hash(pathname, tmp, sizeof(tmp));
- if (ret) {
- fprintf(stderr, "Object hashing failed!\n");
- return NULL;
- }
-
- hexstring_n2a(tmp, sizeof(tmp), bpf_uid, sizeof(bpf_uid));
- bpf_uid_cached = true;
-done:
- return bpf_uid;
-}
-
-static int bpf_init_env(const char *pathname)
+static void bpf_init_env(void)
{
struct rlimit limit = {
.rlim_cur = RLIM_INFINITY,
@@ -1345,15 +1318,8 @@ static int bpf_init_env(const char *pathname)
/* Don't bother in case we fail! */
setrlimit(RLIMIT_MEMLOCK, &limit);
- if (!bpf_get_work_dir(BPF_PROG_TYPE_UNSPEC)) {
+ if (!bpf_get_work_dir(BPF_PROG_TYPE_UNSPEC))
fprintf(stderr, "Continuing without mounted eBPF fs. Too old kernel?\n");
- return 0;
- }
-
- if (!bpf_get_obj_uid(pathname))
- return -1;
-
- return 0;
}
static const char *bpf_custom_pinning(const struct bpf_elf_ctx *ctx,
@@ -1389,7 +1355,7 @@ static void bpf_make_pathname(char *pathname, size_t len, const char *name,
case PIN_OBJECT_NS:
snprintf(pathname, len, "%s/%s/%s",
bpf_get_work_dir(ctx->type),
- bpf_get_obj_uid(NULL), name);
+ ctx->obj_uid, name);
break;
case PIN_GLOBAL_NS:
snprintf(pathname, len, "%s/%s/%s",
@@ -1422,7 +1388,7 @@ static int bpf_make_obj_path(const struct bpf_elf_ctx *ctx)
int ret;
snprintf(tmp, sizeof(tmp), "%s/%s", bpf_get_work_dir(ctx->type),
- bpf_get_obj_uid(NULL));
+ ctx->obj_uid);
ret = mkdir(tmp, S_IRWXU);
if (ret && errno != EEXIST) {
@@ -1691,6 +1657,12 @@ static int bpf_maps_attach_all(struct bpf_elf_ctx *ctx)
const char *map_name;
for (i = 0; i < ctx->map_num; i++) {
+ if (ctx->maps[i].pinning == PIN_OBJECT_NS &&
+ ctx->noafalg) {
+ fprintf(stderr, "Missing kernel AF_ALG support for PIN_OBJECT_NS!\n");
+ return -ENOTSUP;
+ }
+
map_name = bpf_map_fetch_name(ctx, i);
if (!map_name)
return -EIO;
@@ -2446,14 +2418,24 @@ static int bpf_elf_ctx_init(struct bpf_elf_ctx *ctx, const char *pathname,
enum bpf_prog_type type, __u32 ifindex,
bool verbose)
{
- int ret = -EINVAL;
+ uint8_t tmp[20];
+ int ret;
- if (elf_version(EV_CURRENT) == EV_NONE ||
- bpf_init_env(pathname))
- return ret;
+ if (elf_version(EV_CURRENT) == EV_NONE)
+ return -EINVAL;
+
+ bpf_init_env();
memset(ctx, 0, sizeof(*ctx));
bpf_get_cfg(ctx);
+
+ ret = bpf_obj_hash(pathname, tmp, sizeof(tmp));
+ if (ret)
+ ctx->noafalg = true;
+ else
+ hexstring_n2a(tmp, sizeof(tmp), ctx->obj_uid,
+ sizeof(ctx->obj_uid));
+
ctx->verbose = verbose;
ctx->type = type;
ctx->ifindex = ifindex;
--
2.9.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2 4/5] bpf: implement bpf to bpf calls support
2018-07-17 23:31 [PATCH iproute2 0/5] Various BPF improvements Daniel Borkmann
` (2 preceding siblings ...)
2018-07-17 23:31 ` [PATCH iproute2 3/5] bpf: remove strict dependency on af_alg Daniel Borkmann
@ 2018-07-17 23:31 ` Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 5/5] bpf: implement btf handling and map annotation Daniel Borkmann
2018-07-18 2:43 ` [PATCH iproute2 0/5] Various BPF improvements David Ahern
5 siblings, 0 replies; 17+ messages in thread
From: Daniel Borkmann @ 2018-07-17 23:31 UTC (permalink / raw)
To: dsahern; +Cc: jakub.kicinski, alexei.starovoitov, netdev, Daniel Borkmann
Implement missing bpf to bpf calls support. The loader will
recognize .text section and handle relocation entries that
are emitted by LLVM.
First step is processing of map related relocation entries
for .text section, and in a second step loader will copy .text
section into program section and adjust call instruction
offset accordingly.
Example with test_xdp_noinline.o from kernel selftests:
1) Every function as __attribute__ ((always_inline)), rest
left unchanged:
# ip -force link set dev lo xdp obj test_xdp_noinline.o sec xdp-test
# ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 xdpgeneric/id:233 qdisc noqueue state UNKNOWN group default qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
inet 127.0.0.1/8 scope host lo
valid_lft forever preferred_lft forever
inet6 ::1/128 scope host
valid_lft forever preferred_lft forever
[...]
# bpftool prog dump xlated id 233
[...]
1669: (2d) if r3 > r2 goto pc+4
1670: (79) r2 = *(u64 *)(r10 -136)
1671: (61) r2 = *(u32 *)(r2 +0)
1672: (63) *(u32 *)(r1 +0) = r2
1673: (b7) r0 = 1
1674: (95) exit <-- 1674 insns total
2) Every function as __attribute__ ((noinline)), rest
left unchanged:
# ip -force link set dev lo xdp obj test_xdp_noinline.o sec xdp-test
# ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 xdpgeneric/id:236 qdisc noqueue state UNKNOWN group default qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
inet 127.0.0.1/8 scope host lo
valid_lft forever preferred_lft forever
inet6 ::1/128 scope host
valid_lft forever preferred_lft forever
[...]
# bpftool prog dump xlated id 236
[...]
1000: (bf) r1 = r6
1001: (b7) r2 = 24
1002: (85) call pc+3 <-- pc-relative call insns
1003: (1f) r7 -= r0
1004: (bf) r0 = r7
1005: (95) exit
1006: (bf) r0 = r1
1007: (bf) r1 = r2
1008: (67) r1 <<= 32
1009: (77) r1 >>= 32
1010: (bf) r3 = r0
1011: (6f) r3 <<= r1
1012: (87) r2 = -r2
1013: (57) r2 &= 31
1014: (67) r0 <<= 32
1015: (77) r0 >>= 32
1016: (7f) r0 >>= r2
1017: (4f) r0 |= r3
1018: (95) exit <-- 1018 insns total
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
lib/bpf.c | 233 ++++++++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 157 insertions(+), 76 deletions(-)
diff --git a/lib/bpf.c b/lib/bpf.c
index 135f496..cea1273 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -1104,7 +1104,8 @@ int bpf_prog_load(enum bpf_prog_type type, const struct bpf_insn *insns,
#ifdef HAVE_ELF
struct bpf_elf_prog {
enum bpf_prog_type type;
- const struct bpf_insn *insns;
+ struct bpf_insn *insns;
+ unsigned int insns_num;
size_t size;
const char *license;
};
@@ -1130,11 +1131,13 @@ struct bpf_elf_ctx {
int map_fds[ELF_MAX_MAPS];
struct bpf_elf_map maps[ELF_MAX_MAPS];
struct bpf_map_ext maps_ext[ELF_MAX_MAPS];
+ struct bpf_elf_prog prog_text;
int sym_num;
int map_num;
int map_len;
bool *sec_done;
int sec_maps;
+ int sec_text;
char license[ELF_MAX_LICENSE_LEN];
enum bpf_prog_type type;
__u32 ifindex;
@@ -1899,12 +1902,25 @@ static int bpf_fetch_strtab(struct bpf_elf_ctx *ctx, int section,
return 0;
}
+static int bpf_fetch_text(struct bpf_elf_ctx *ctx, int section,
+ struct bpf_elf_sec_data *data)
+{
+ ctx->sec_text = section;
+ ctx->sec_done[section] = true;
+ return 0;
+}
+
static bool bpf_has_map_data(const struct bpf_elf_ctx *ctx)
{
return ctx->sym_tab && ctx->str_tab && ctx->sec_maps;
}
-static int bpf_fetch_ancillary(struct bpf_elf_ctx *ctx)
+static bool bpf_has_call_data(const struct bpf_elf_ctx *ctx)
+{
+ return ctx->sec_text;
+}
+
+static int bpf_fetch_ancillary(struct bpf_elf_ctx *ctx, bool check_text_sec)
{
struct bpf_elf_sec_data data;
int i, ret = -1;
@@ -1920,6 +1936,11 @@ static int bpf_fetch_ancillary(struct bpf_elf_ctx *ctx)
else if (data.sec_hdr.sh_type == SHT_PROGBITS &&
!strcmp(data.sec_name, ELF_SECTION_LICENSE))
ret = bpf_fetch_license(ctx, i, &data);
+ else if (data.sec_hdr.sh_type == SHT_PROGBITS &&
+ (data.sec_hdr.sh_flags & SHF_EXECINSTR) &&
+ !strcmp(data.sec_name, ".text") &&
+ check_text_sec)
+ ret = bpf_fetch_text(ctx, i, &data);
else if (data.sec_hdr.sh_type == SHT_SYMTAB &&
!strcmp(data.sec_name, ".symtab"))
ret = bpf_fetch_symtab(ctx, i, &data);
@@ -1964,17 +1985,18 @@ static int bpf_fetch_prog(struct bpf_elf_ctx *ctx, const char *section,
ret = bpf_fill_section_data(ctx, i, &data);
if (ret < 0 ||
!(data.sec_hdr.sh_type == SHT_PROGBITS &&
- data.sec_hdr.sh_flags & SHF_EXECINSTR &&
+ (data.sec_hdr.sh_flags & SHF_EXECINSTR) &&
!strcmp(data.sec_name, section)))
continue;
*sseen = true;
memset(&prog, 0, sizeof(prog));
- prog.type = ctx->type;
- prog.insns = data.sec_data->d_buf;
- prog.size = data.sec_data->d_size;
- prog.license = ctx->license;
+ prog.type = ctx->type;
+ prog.license = ctx->license;
+ prog.size = data.sec_data->d_size;
+ prog.insns_num = prog.size / sizeof(struct bpf_insn);
+ prog.insns = data.sec_data->d_buf;
fd = bpf_prog_attach(section, &prog, ctx);
if (fd < 0)
@@ -1987,84 +2009,120 @@ static int bpf_fetch_prog(struct bpf_elf_ctx *ctx, const char *section,
return fd;
}
-struct bpf_tail_call_props {
- unsigned int total;
- unsigned int jited;
+struct bpf_relo_props {
+ struct bpf_tail_call {
+ unsigned int total;
+ unsigned int jited;
+ } tc;
+ int main_num;
};
+static int bpf_apply_relo_map(struct bpf_elf_ctx *ctx, struct bpf_elf_prog *prog,
+ GElf_Rel *relo, GElf_Sym *sym,
+ struct bpf_relo_props *props)
+{
+ unsigned int insn_off = relo->r_offset / sizeof(struct bpf_insn);
+ unsigned int map_idx = sym->st_value / ctx->map_len;
+
+ if (insn_off >= prog->insns_num)
+ return -EINVAL;
+ if (prog->insns[insn_off].code != (BPF_LD | BPF_IMM | BPF_DW)) {
+ fprintf(stderr, "ELF contains relo data for non ld64 instruction at offset %u! Compiler bug?!\n",
+ insn_off);
+ return -EINVAL;
+ }
+
+ if (map_idx >= ARRAY_SIZE(ctx->map_fds))
+ return -EINVAL;
+ if (!ctx->map_fds[map_idx])
+ return -EINVAL;
+ if (ctx->maps[map_idx].type == BPF_MAP_TYPE_PROG_ARRAY) {
+ props->tc.total++;
+ if (ctx->maps_ext[map_idx].owner.jited ||
+ (ctx->maps_ext[map_idx].owner.type == 0 &&
+ ctx->cfg.jit_enabled))
+ props->tc.jited++;
+ }
+
+ prog->insns[insn_off].src_reg = BPF_PSEUDO_MAP_FD;
+ prog->insns[insn_off].imm = ctx->map_fds[map_idx];
+ return 0;
+}
+
+static int bpf_apply_relo_call(struct bpf_elf_ctx *ctx, struct bpf_elf_prog *prog,
+ GElf_Rel *relo, GElf_Sym *sym,
+ struct bpf_relo_props *props)
+{
+ unsigned int insn_off = relo->r_offset / sizeof(struct bpf_insn);
+ struct bpf_elf_prog *prog_text = &ctx->prog_text;
+
+ if (insn_off >= prog->insns_num)
+ return -EINVAL;
+ if (prog->insns[insn_off].code != (BPF_JMP | BPF_CALL) &&
+ prog->insns[insn_off].src_reg != BPF_PSEUDO_CALL) {
+ fprintf(stderr, "ELF contains relo data for non call instruction at offset %u! Compiler bug?!\n",
+ insn_off);
+ return -EINVAL;
+ }
+
+ if (!props->main_num) {
+ struct bpf_insn *insns = realloc(prog->insns,
+ prog->size + prog_text->size);
+ if (!insns)
+ return -ENOMEM;
+
+ memcpy(insns + prog->insns_num, prog_text->insns,
+ prog_text->size);
+ props->main_num = prog->insns_num;
+ prog->insns = insns;
+ prog->insns_num += prog_text->insns_num;
+ prog->size += prog_text->size;
+ }
+
+ prog->insns[insn_off].imm += props->main_num - insn_off;
+ return 0;
+}
+
static int bpf_apply_relo_data(struct bpf_elf_ctx *ctx,
struct bpf_elf_sec_data *data_relo,
- struct bpf_elf_sec_data *data_insn,
- struct bpf_tail_call_props *props)
+ struct bpf_elf_prog *prog,
+ struct bpf_relo_props *props)
{
- Elf_Data *idata = data_insn->sec_data;
GElf_Shdr *rhdr = &data_relo->sec_hdr;
int relo_ent, relo_num = rhdr->sh_size / rhdr->sh_entsize;
- struct bpf_insn *insns = idata->d_buf;
- unsigned int num_insns = idata->d_size / sizeof(*insns);
for (relo_ent = 0; relo_ent < relo_num; relo_ent++) {
- unsigned int ioff, rmap;
GElf_Rel relo;
GElf_Sym sym;
+ int ret = -EIO;
if (gelf_getrel(data_relo->sec_data, relo_ent, &relo) != &relo)
return -EIO;
-
- ioff = relo.r_offset / sizeof(struct bpf_insn);
- if (ioff >= num_insns ||
- insns[ioff].code != (BPF_LD | BPF_IMM | BPF_DW)) {
- fprintf(stderr, "ELF contains relo data for non ld64 instruction at offset %u! Compiler bug?!\n",
- ioff);
- fprintf(stderr, " - Current section: %s\n", data_relo->sec_name);
- if (ioff < num_insns &&
- insns[ioff].code == (BPF_JMP | BPF_CALL))
- fprintf(stderr, " - Try to annotate functions with always_inline attribute!\n");
- return -EINVAL;
- }
-
if (gelf_getsym(ctx->sym_tab, GELF_R_SYM(relo.r_info), &sym) != &sym)
return -EIO;
- if (sym.st_shndx != ctx->sec_maps) {
- fprintf(stderr, "ELF contains non-map related relo data in entry %u pointing to section %u! Compiler bug?!\n",
- relo_ent, sym.st_shndx);
- return -EIO;
- }
- rmap = sym.st_value / ctx->map_len;
- if (rmap >= ARRAY_SIZE(ctx->map_fds))
- return -EINVAL;
- if (!ctx->map_fds[rmap])
- return -EINVAL;
- if (ctx->maps[rmap].type == BPF_MAP_TYPE_PROG_ARRAY) {
- props->total++;
- if (ctx->maps_ext[rmap].owner.jited ||
- (ctx->maps_ext[rmap].owner.type == 0 &&
- ctx->cfg.jit_enabled))
- props->jited++;
- }
-
- if (ctx->verbose)
- fprintf(stderr, "Map \'%s\' (%d) injected into prog section \'%s\' at offset %u!\n",
- bpf_str_tab_name(ctx, &sym), ctx->map_fds[rmap],
- data_insn->sec_name, ioff);
-
- insns[ioff].src_reg = BPF_PSEUDO_MAP_FD;
- insns[ioff].imm = ctx->map_fds[rmap];
+ if (sym.st_shndx == ctx->sec_maps)
+ ret = bpf_apply_relo_map(ctx, prog, &relo, &sym, props);
+ else if (sym.st_shndx == ctx->sec_text)
+ ret = bpf_apply_relo_call(ctx, prog, &relo, &sym, props);
+ else
+ fprintf(stderr, "ELF contains non-{map,call} related relo data in entry %u pointing to section %u! Compiler bug?!\n",
+ relo_ent, sym.st_shndx);
+ if (ret < 0)
+ return ret;
}
return 0;
}
static int bpf_fetch_prog_relo(struct bpf_elf_ctx *ctx, const char *section,
- bool *lderr, bool *sseen)
+ bool *lderr, bool *sseen, struct bpf_elf_prog *prog)
{
struct bpf_elf_sec_data data_relo, data_insn;
- struct bpf_elf_prog prog;
int ret, idx, i, fd = -1;
for (i = 1; i < ctx->elf_hdr.e_shnum; i++) {
- struct bpf_tail_call_props props = {};
+ struct bpf_relo_props props = {};
ret = bpf_fill_section_data(ctx, i, &data_relo);
if (ret < 0 || data_relo.sec_hdr.sh_type != SHT_REL)
@@ -2075,40 +2133,54 @@ static int bpf_fetch_prog_relo(struct bpf_elf_ctx *ctx, const char *section,
ret = bpf_fill_section_data(ctx, idx, &data_insn);
if (ret < 0 ||
!(data_insn.sec_hdr.sh_type == SHT_PROGBITS &&
- data_insn.sec_hdr.sh_flags & SHF_EXECINSTR &&
+ (data_insn.sec_hdr.sh_flags & SHF_EXECINSTR) &&
!strcmp(data_insn.sec_name, section)))
continue;
+ if (sseen)
+ *sseen = true;
+
+ memset(prog, 0, sizeof(*prog));
+ prog->type = ctx->type;
+ prog->license = ctx->license;
+ prog->size = data_insn.sec_data->d_size;
+ prog->insns_num = prog->size / sizeof(struct bpf_insn);
+ prog->insns = malloc(prog->size);
+ if (!prog->insns) {
+ *lderr = true;
+ return -ENOMEM;
+ }
- *sseen = true;
+ memcpy(prog->insns, data_insn.sec_data->d_buf, prog->size);
- ret = bpf_apply_relo_data(ctx, &data_relo, &data_insn, &props);
+ ret = bpf_apply_relo_data(ctx, &data_relo, prog, &props);
if (ret < 0) {
*lderr = true;
+ if (ctx->sec_text != idx)
+ free(prog->insns);
return ret;
}
+ if (ctx->sec_text == idx) {
+ fd = 0;
+ goto out;
+ }
- memset(&prog, 0, sizeof(prog));
- prog.type = ctx->type;
- prog.insns = data_insn.sec_data->d_buf;
- prog.size = data_insn.sec_data->d_size;
- prog.license = ctx->license;
-
- fd = bpf_prog_attach(section, &prog, ctx);
+ fd = bpf_prog_attach(section, prog, ctx);
+ free(prog->insns);
if (fd < 0) {
*lderr = true;
- if (props.total) {
+ if (props.tc.total) {
if (ctx->cfg.jit_enabled &&
- props.total != props.jited)
+ props.tc.total != props.tc.jited)
fprintf(stderr, "JIT enabled, but only %u/%u tail call maps in the program have JITed owner!\n",
- props.jited, props.total);
+ props.tc.jited, props.tc.total);
if (!ctx->cfg.jit_enabled &&
- props.jited)
+ props.tc.jited)
fprintf(stderr, "JIT disabled, but %u/%u tail call maps in the program have JITed owner!\n",
- props.jited, props.total);
+ props.tc.jited, props.tc.total);
}
return fd;
}
-
+out:
ctx->sec_done[i] = true;
ctx->sec_done[idx] = true;
break;
@@ -2120,10 +2192,18 @@ static int bpf_fetch_prog_relo(struct bpf_elf_ctx *ctx, const char *section,
static int bpf_fetch_prog_sec(struct bpf_elf_ctx *ctx, const char *section)
{
bool lderr = false, sseen = false;
+ struct bpf_elf_prog prog;
int ret = -1;
- if (bpf_has_map_data(ctx))
- ret = bpf_fetch_prog_relo(ctx, section, &lderr, &sseen);
+ if (bpf_has_call_data(ctx)) {
+ ret = bpf_fetch_prog_relo(ctx, ".text", &lderr, NULL,
+ &ctx->prog_text);
+ if (ret < 0)
+ return ret;
+ }
+
+ if (bpf_has_map_data(ctx) || bpf_has_call_data(ctx))
+ ret = bpf_fetch_prog_relo(ctx, section, &lderr, &sseen, &prog);
if (ret < 0 && !lderr)
ret = bpf_fetch_prog(ctx, section, &sseen);
if (ret < 0 && !sseen)
@@ -2520,6 +2600,7 @@ static void bpf_elf_ctx_destroy(struct bpf_elf_ctx *ctx, bool failure)
bpf_hash_destroy(ctx);
+ free(ctx->prog_text.insns);
free(ctx->sec_done);
free(ctx->log);
@@ -2541,7 +2622,7 @@ static int bpf_obj_open(const char *pathname, enum bpf_prog_type type,
return ret;
}
- ret = bpf_fetch_ancillary(ctx);
+ ret = bpf_fetch_ancillary(ctx, strcmp(section, ".text"));
if (ret < 0) {
fprintf(stderr, "Error fetching ELF ancillary data!\n");
goto out;
--
2.9.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
2018-07-17 23:31 [PATCH iproute2 0/5] Various BPF improvements Daniel Borkmann
` (3 preceding siblings ...)
2018-07-17 23:31 ` [PATCH iproute2 4/5] bpf: implement bpf to bpf calls support Daniel Borkmann
@ 2018-07-17 23:31 ` Daniel Borkmann
2018-07-18 0:27 ` Jakub Kicinski
2018-07-18 2:43 ` [PATCH iproute2 0/5] Various BPF improvements David Ahern
5 siblings, 1 reply; 17+ messages in thread
From: Daniel Borkmann @ 2018-07-17 23:31 UTC (permalink / raw)
To: dsahern; +Cc: jakub.kicinski, alexei.starovoitov, netdev, Daniel Borkmann
Implement loading of .BTF section from object file and build up
internal table for retrieving key/value id related to maps in
the BPF program. Latter is done by setting up struct btf_type
table.
One of the issues is that there's a disconnect between the data
types used in the map and struct bpf_elf_map, meaning the underlying
types are unknown from the map description. One way to overcome
this is to add a annotation such that the loader will recognize
the relation to both. BPF_ANNOTATE_KV_PAIR(map_foo, struct key,
struct val); has been added to the API that programs can use.
The loader will then pick the corresponding key/value type ids and
attach it to the maps for creation. This can later on be dumped via
bpftool for introspection.
Example with test_xdp_noinline.o from kernel selftests:
[...]
struct ctl_value {
union {
__u64 value;
__u32 ifindex;
__u8 mac[6];
};
};
struct bpf_map_def __attribute__ ((section("maps"), used)) ctl_array = {
.type = BPF_MAP_TYPE_ARRAY,
.key_size = sizeof(__u32),
.value_size = sizeof(struct ctl_value),
.max_entries = 16,
.map_flags = 0,
};
BPF_ANNOTATE_KV_PAIR(ctl_array, __u32, struct ctl_value);
[...]
Above could also further be wrapped in a macro. Compiling through LLVM and
converting to BTF:
# llc --version
LLVM (http://llvm.org/):
LLVM version 7.0.0svn
Optimized build.
Default target: x86_64-unknown-linux-gnu
Host CPU: skylake
Registered Targets:
bpf - BPF (host endian)
bpfeb - BPF (big endian)
bpfel - BPF (little endian)
[...]
# clang [...] -O2 -target bpf -g -emit-llvm -c test_xdp_noinline.c -o - |
llc -march=bpf -mcpu=probe -mattr=dwarfris -filetype=obj -o test_xdp_noinline.o
# pahole -J test_xdp_noinline.o
Checking pahole dump of BPF object file:
# file test_xdp_noinline.o
test_xdp_noinline.o: ELF 64-bit LSB relocatable, *unknown arch 0xf7* version 1 (SYSV), with debug_info, not stripped
# pahole test_xdp_noinline.o
[...]
struct ctl_value {
union {
__u64 value; /* 0 8 */
__u32 ifindex; /* 0 4 */
__u8 mac[0]; /* 0 0 */
}; /* 0 8 */
/* size: 8, cachelines: 1, members: 1 */
/* last cacheline: 8 bytes */
};
Now loading into kernel and dumping the map via bpftool:
# ip -force link set dev lo xdp obj test_xdp_noinline.o sec xdp-test
# ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 xdpgeneric/id:227 qdisc noqueue state UNKNOWN group default qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
inet 127.0.0.1/8 scope host lo
valid_lft forever preferred_lft forever
inet6 ::1/128 scope host
valid_lft forever preferred_lft forever
[...]
# bpftool prog show id 227
227: xdp tag a85e060c275c5616 gpl
loaded_at 2018-07-17T14:41:29+0000 uid 0
xlated 8152B not jited memlock 12288B map_ids 381,385,386,382,384,383
# bpftool map dump id 386
[{
"key": 0,
"value": {
"": {
"value": 0,
"ifindex": 0,
"mac": []
}
}
},{
"key": 1,
"value": {
"": {
"value": 0,
"ifindex": 0,
"mac": []
}
}
},{
[...]
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/bpf_elf.h | 9 ++
include/bpf_util.h | 1 +
lib/bpf.c | 332 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 338 insertions(+), 4 deletions(-)
diff --git a/include/bpf_elf.h b/include/bpf_elf.h
index a8e360f..84e8ae0 100644
--- a/include/bpf_elf.h
+++ b/include/bpf_elf.h
@@ -41,4 +41,13 @@ struct bpf_elf_map {
__u32 inner_idx;
};
+#define BPF_ANNOTATE_KV_PAIR(name, type_key, type_val) \
+ struct ____btf_map_##name { \
+ type_key key; \
+ type_val value; \
+ }; \
+ struct ____btf_map_##name \
+ __attribute__ ((section(".maps." #name), used)) \
+ ____btf_map_##name = { }
+
#endif /* __BPF_ELF__ */
diff --git a/include/bpf_util.h b/include/bpf_util.h
index 219beb4..63837a0 100644
--- a/include/bpf_util.h
+++ b/include/bpf_util.h
@@ -14,6 +14,7 @@
#define __BPF_UTIL__
#include <linux/bpf.h>
+#include <linux/btf.h>
#include <linux/filter.h>
#include <linux/magic.h>
#include <linux/elf-em.h>
diff --git a/lib/bpf.c b/lib/bpf.c
index cea1273..794ad19 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -388,6 +388,8 @@ struct bpf_prog_data {
struct bpf_map_ext {
struct bpf_prog_data owner;
+ unsigned int btf_id_key;
+ unsigned int btf_id_val;
};
static int bpf_derive_elf_map_from_fdinfo(int fd, struct bpf_elf_map *map,
@@ -1120,24 +1122,36 @@ struct bpf_config {
unsigned int jit_enabled;
};
+struct bpf_btf {
+ const struct btf_header *hdr;
+ const void *raw;
+ const char *strings;
+ const struct btf_type **types;
+ int types_num;
+};
+
struct bpf_elf_ctx {
struct bpf_config cfg;
Elf *elf_fd;
GElf_Ehdr elf_hdr;
Elf_Data *sym_tab;
Elf_Data *str_tab;
+ Elf_Data *btf_data;
char obj_uid[64];
int obj_fd;
+ int btf_fd;
int map_fds[ELF_MAX_MAPS];
struct bpf_elf_map maps[ELF_MAX_MAPS];
struct bpf_map_ext maps_ext[ELF_MAX_MAPS];
struct bpf_elf_prog prog_text;
+ struct bpf_btf btf;
int sym_num;
int map_num;
int map_len;
bool *sec_done;
int sec_maps;
int sec_text;
+ int sec_btf;
char license[ELF_MAX_LICENSE_LEN];
enum bpf_prog_type type;
__u32 ifindex;
@@ -1162,6 +1176,11 @@ struct bpf_map_data {
struct bpf_elf_map *ent;
};
+static bool bpf_log_has_data(struct bpf_elf_ctx *ctx)
+{
+ return ctx->log && ctx->log[0];
+}
+
static __check_format_string(2, 3) void
bpf_dump_error(struct bpf_elf_ctx *ctx, const char *format, ...)
{
@@ -1171,7 +1190,7 @@ bpf_dump_error(struct bpf_elf_ctx *ctx, const char *format, ...)
vfprintf(stderr, format, vl);
va_end(vl);
- if (ctx->log && ctx->log[0]) {
+ if (bpf_log_has_data(ctx)) {
if (ctx->verbose) {
fprintf(stderr, "%s\n", ctx->log);
} else {
@@ -1218,7 +1237,9 @@ static int bpf_log_realloc(struct bpf_elf_ctx *ctx)
static int bpf_map_create(enum bpf_map_type type, uint32_t size_key,
uint32_t size_value, uint32_t max_elem,
- uint32_t flags, int inner_fd, uint32_t ifindex)
+ uint32_t flags, int inner_fd, int btf_fd,
+ uint32_t ifindex, uint32_t btf_id_key,
+ uint32_t btf_id_val)
{
union bpf_attr attr = {};
@@ -1229,10 +1250,30 @@ static int bpf_map_create(enum bpf_map_type type, uint32_t size_key,
attr.map_flags = flags;
attr.inner_map_fd = inner_fd;
attr.map_ifindex = ifindex;
+ attr.btf_fd = btf_fd;
+ attr.btf_key_type_id = btf_id_key;
+ attr.btf_value_type_id = btf_id_val;
return bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
}
+static int bpf_btf_load(void *btf, size_t size_btf,
+ char *log, size_t size_log)
+{
+ union bpf_attr attr = {};
+
+ attr.btf = bpf_ptr_to_u64(btf);
+ attr.btf_size = size_btf;
+
+ if (size_log > 0) {
+ attr.btf_log_buf = bpf_ptr_to_u64(log);
+ attr.btf_log_size = size_log;
+ attr.btf_log_level = 1;
+ }
+
+ return bpf(BPF_BTF_LOAD, &attr, sizeof(attr));
+}
+
static int bpf_obj_pin(int fd, const char *pathname)
{
union bpf_attr attr = {};
@@ -1608,7 +1649,8 @@ static int bpf_map_attach(const char *name, struct bpf_elf_ctx *ctx,
ifindex = bpf_map_offload_neutral(map->type) ? 0 : ctx->ifindex;
errno = 0;
fd = bpf_map_create(map->type, map->size_key, map->size_value,
- map->max_elem, map->flags, map_inner_fd, ifindex);
+ map->max_elem, map->flags, map_inner_fd, ctx->btf_fd,
+ ifindex, ext->btf_id_key, ext->btf_id_val);
if (fd < 0 || ctx->verbose) {
bpf_map_report(fd, name, map, ctx, map_inner_fd);
@@ -1633,8 +1675,80 @@ static const char *bpf_str_tab_name(const struct bpf_elf_ctx *ctx,
return ctx->str_tab->d_buf + sym->st_name;
}
+static int bpf_btf_find(struct bpf_elf_ctx *ctx, const char *name)
+{
+ const struct btf_type *type;
+ const char *res;
+ int id;
+
+ for (id = 1; id < ctx->btf.types_num; id++) {
+ type = ctx->btf.types[id];
+ if (type->name_off >= ctx->btf.hdr->str_len)
+ continue;
+ res = &ctx->btf.strings[type->name_off];
+ if (!strcmp(res, name))
+ return id;
+ }
+
+ return -ENOENT;
+}
+
+static int bpf_btf_find_kv(struct bpf_elf_ctx *ctx, const struct bpf_elf_map *map,
+ const char *name, uint32_t *id_key, uint32_t *id_val)
+{
+ const struct btf_member *key, *val;
+ const struct btf_type *type;
+ char btf_name[512];
+ const char *res;
+ int id;
+
+ snprintf(btf_name, sizeof(btf_name), "____btf_map_%s", name);
+ id = bpf_btf_find(ctx, btf_name);
+ if (id < 0)
+ return id;
+
+ type = ctx->btf.types[id];
+ if (BTF_INFO_KIND(type->info) != BTF_KIND_STRUCT)
+ return -EINVAL;
+ if (BTF_INFO_VLEN(type->info) != 2)
+ return -EINVAL;
+
+ key = ((void *) type) + sizeof(*type);
+ val = key + 1;
+ if (!key->type || key->type >= ctx->btf.types_num ||
+ !val->type || val->type >= ctx->btf.types_num)
+ return -EINVAL;
+
+ if (key->name_off >= ctx->btf.hdr->str_len ||
+ val->name_off >= ctx->btf.hdr->str_len)
+ return -EINVAL;
+
+ res = &ctx->btf.strings[key->name_off];
+ if (strcmp(res, "key"))
+ return -EINVAL;
+
+ res = &ctx->btf.strings[val->name_off];
+ if (strcmp(res, "value"))
+ return -EINVAL;
+
+ *id_key = key->type;
+ *id_val = val->type;
+ return 0;
+}
+
+static void bpf_btf_annotate(struct bpf_elf_ctx *ctx, int which, const char *name)
+{
+ uint32_t id_key = 0, id_val = 0;
+
+ if (!bpf_btf_find_kv(ctx, &ctx->maps[which], name, &id_key, &id_val)) {
+ ctx->maps_ext[which].btf_id_key = id_key;
+ ctx->maps_ext[which].btf_id_val = id_val;
+ }
+}
+
static const char *bpf_map_fetch_name(struct bpf_elf_ctx *ctx, int which)
{
+ const char *name;
GElf_Sym sym;
int i;
@@ -1648,7 +1762,9 @@ static const char *bpf_map_fetch_name(struct bpf_elf_ctx *ctx, int which)
sym.st_value / ctx->map_len != which)
continue;
- return bpf_str_tab_name(ctx, &sym);
+ name = bpf_str_tab_name(ctx, &sym);
+ bpf_btf_annotate(ctx, which, name);
+ return name;
}
return NULL;
@@ -1910,11 +2026,210 @@ static int bpf_fetch_text(struct bpf_elf_ctx *ctx, int section,
return 0;
}
+static void bpf_btf_report(int fd, struct bpf_elf_ctx *ctx)
+{
+ fprintf(stderr, "\nBTF debug data section \'.BTF\' %s%s (%d)!\n",
+ fd < 0 ? "rejected: " : "loaded",
+ fd < 0 ? strerror(errno) : "",
+ fd < 0 ? errno : fd);
+
+ fprintf(stderr, " - Length: %zu\n", ctx->btf_data->d_size);
+
+ bpf_dump_error(ctx, "Verifier analysis:\n\n");
+}
+
+static int bpf_btf_attach(struct bpf_elf_ctx *ctx)
+{
+ int tries = 0, fd;
+retry:
+ errno = 0;
+ fd = bpf_btf_load(ctx->btf_data->d_buf, ctx->btf_data->d_size,
+ ctx->log, ctx->log_size);
+ if (fd < 0 || ctx->verbose) {
+ if (fd < 0 && (errno == ENOSPC || !ctx->log_size)) {
+ if (tries++ < 10 && !bpf_log_realloc(ctx))
+ goto retry;
+
+ fprintf(stderr, "Log buffer too small to dump verifier log %zu bytes (%d tries)!\n",
+ ctx->log_size, tries);
+ return fd;
+ }
+
+ if (bpf_log_has_data(ctx))
+ bpf_btf_report(fd, ctx);
+ }
+
+ return fd;
+}
+
+static int bpf_fetch_btf_begin(struct bpf_elf_ctx *ctx, int section,
+ struct bpf_elf_sec_data *data)
+{
+ ctx->btf_data = data->sec_data;
+ ctx->sec_btf = section;
+ ctx->sec_done[section] = true;
+ return 0;
+}
+
+static int bpf_btf_check_header(struct bpf_elf_ctx *ctx)
+{
+ const struct btf_header *hdr = ctx->btf_data->d_buf;
+ const char *str_start, *str_end;
+ unsigned int data_len;
+
+ if (hdr->magic != BTF_MAGIC) {
+ fprintf(stderr, "Object has wrong BTF magic: %x, expected: %x!\n",
+ hdr->magic, BTF_MAGIC);
+ return -EINVAL;
+ }
+
+ if (hdr->version != BTF_VERSION) {
+ fprintf(stderr, "Object has wrong BTF version: %u, expected: %u!\n",
+ hdr->version, BTF_VERSION);
+ return -EINVAL;
+ }
+
+ if (hdr->flags) {
+ fprintf(stderr, "Object has unsupported BTF flags %x!\n",
+ hdr->flags);
+ return -EINVAL;
+ }
+
+ data_len = ctx->btf_data->d_size - sizeof(*hdr);
+ if (data_len < hdr->type_off ||
+ data_len < hdr->str_off ||
+ data_len < hdr->type_len + hdr->str_len ||
+ hdr->type_off >= hdr->str_off ||
+ hdr->type_off + hdr->type_len != hdr->str_off ||
+ hdr->str_off + hdr->str_len != data_len ||
+ (hdr->type_off & (sizeof(uint32_t) - 1))) {
+ fprintf(stderr, "Object has malformed BTF data!\n");
+ return -EINVAL;
+ }
+
+ ctx->btf.hdr = hdr;
+ ctx->btf.raw = hdr + 1;
+
+ str_start = ctx->btf.raw + hdr->str_off;
+ str_end = str_start + hdr->str_len;
+ if (!hdr->str_len ||
+ hdr->str_len - 1 > BTF_MAX_NAME_OFFSET ||
+ str_start[0] || str_end[-1]) {
+ fprintf(stderr, "Object has malformed BTF string data!\n");
+ return -EINVAL;
+ }
+
+ ctx->btf.strings = str_start;
+ return 0;
+}
+
+static int bpf_btf_register_type(struct bpf_elf_ctx *ctx,
+ const struct btf_type *type)
+{
+ int cur = ctx->btf.types_num, num = cur + 1;
+ const struct btf_type **types;
+
+ types = realloc(ctx->btf.types, num * sizeof(type));
+ if (!types) {
+ free(ctx->btf.types);
+ ctx->btf.types = NULL;
+ ctx->btf.types_num = 0;
+ return -ENOMEM;
+ }
+
+ ctx->btf.types = types;
+ ctx->btf.types[cur] = type;
+ ctx->btf.types_num = num;
+ return 0;
+}
+
+static struct btf_type btf_type_void;
+
+static int bpf_btf_prep_type_data(struct bpf_elf_ctx *ctx)
+{
+ const void *type_cur = ctx->btf.raw + ctx->btf.hdr->type_off;
+ const void *type_end = ctx->btf.raw + ctx->btf.hdr->str_off;
+ const struct btf_type *type;
+ uint16_t var_len;
+ int ret, kind;
+
+ ret = bpf_btf_register_type(ctx, &btf_type_void);
+ if (ret < 0)
+ return ret;
+
+ while (type_cur < type_end) {
+ type = type_cur;
+ type_cur += sizeof(*type);
+
+ var_len = BTF_INFO_VLEN(type->info);
+ kind = BTF_INFO_KIND(type->info);
+
+ switch (kind) {
+ case BTF_KIND_INT:
+ type_cur += sizeof(int);
+ break;
+ case BTF_KIND_ARRAY:
+ type_cur += sizeof(struct btf_array);
+ break;
+ case BTF_KIND_STRUCT:
+ case BTF_KIND_UNION:
+ type_cur += var_len * sizeof(struct btf_member);
+ break;
+ case BTF_KIND_ENUM:
+ type_cur += var_len * sizeof(struct btf_enum);
+ break;
+ case BTF_KIND_TYPEDEF:
+ case BTF_KIND_PTR:
+ case BTF_KIND_FWD:
+ case BTF_KIND_VOLATILE:
+ case BTF_KIND_CONST:
+ case BTF_KIND_RESTRICT:
+ break;
+ default:
+ fprintf(stderr, "Object has unknown BTF type: %u!\n", kind);
+ return -EINVAL;
+ }
+
+ ret = bpf_btf_register_type(ctx, type);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int bpf_btf_prep_data(struct bpf_elf_ctx *ctx)
+{
+ int ret = bpf_btf_check_header(ctx);
+
+ if (!ret)
+ return bpf_btf_prep_type_data(ctx);
+ return ret;
+}
+
+static void bpf_fetch_btf_end(struct bpf_elf_ctx *ctx)
+{
+ int fd = bpf_btf_attach(ctx);
+
+ if (fd < 0)
+ return;
+ ctx->btf_fd = fd;
+ if (bpf_btf_prep_data(ctx) < 0) {
+ close(ctx->btf_fd);
+ ctx->btf_fd = 0;
+ }
+}
+
static bool bpf_has_map_data(const struct bpf_elf_ctx *ctx)
{
return ctx->sym_tab && ctx->str_tab && ctx->sec_maps;
}
+static bool bpf_has_btf_data(const struct bpf_elf_ctx *ctx)
+{
+ return ctx->sec_btf;
+}
+
static bool bpf_has_call_data(const struct bpf_elf_ctx *ctx)
{
return ctx->sec_text;
@@ -1947,6 +2262,9 @@ static int bpf_fetch_ancillary(struct bpf_elf_ctx *ctx, bool check_text_sec)
else if (data.sec_hdr.sh_type == SHT_STRTAB &&
!strcmp(data.sec_name, ".strtab"))
ret = bpf_fetch_strtab(ctx, i, &data);
+ else if (data.sec_hdr.sh_type == SHT_PROGBITS &&
+ !strcmp(data.sec_name, ".BTF"))
+ ret = bpf_fetch_btf_begin(ctx, i, &data);
if (ret < 0) {
fprintf(stderr, "Error parsing section %d! Perhaps check with readelf -a?\n",
i);
@@ -1954,6 +2272,8 @@ static int bpf_fetch_ancillary(struct bpf_elf_ctx *ctx, bool check_text_sec)
}
}
+ if (bpf_has_btf_data(ctx))
+ bpf_fetch_btf_end(ctx);
if (bpf_has_map_data(ctx)) {
ret = bpf_fetch_maps_end(ctx);
if (ret < 0) {
@@ -2591,6 +2911,10 @@ static void bpf_maps_teardown(struct bpf_elf_ctx *ctx)
if (ctx->map_fds[i])
close(ctx->map_fds[i]);
}
+
+ if (ctx->btf_fd)
+ close(ctx->btf_fd);
+ free(ctx->btf.types);
}
static void bpf_elf_ctx_destroy(struct bpf_elf_ctx *ctx, bool failure)
--
2.9.5
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 2/5] bpf: move bpf_elf_map fixup notification under verbose
2018-07-17 23:31 ` [PATCH iproute2 2/5] bpf: move bpf_elf_map fixup notification under verbose Daniel Borkmann
@ 2018-07-18 0:24 ` Jakub Kicinski
0 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2018-07-18 0:24 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: dsahern, alexei.starovoitov, netdev
On Wed, 18 Jul 2018 01:31:19 +0200, Daniel Borkmann wrote:
> No need to spam the user with this if it can be fixed gracefully
> anyway. Therefore, move it under verbose option.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> lib/bpf.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/bpf.c b/lib/bpf.c
> index 4e26c0d..42093db 100644
> --- a/lib/bpf.c
> +++ b/lib/bpf.c
> @@ -1893,9 +1893,9 @@ static int bpf_fetch_maps_end(struct bpf_elf_ctx *ctx)
> }
>
> memcpy(ctx->maps, fixup, sizeof(fixup));
> -
> - printf("Note: %zu bytes struct bpf_elf_map fixup performed due to size mismatch!\n",
> - sizeof(struct bpf_elf_map) - ctx->map_len);
> + if (ctx->verbose)
> + printf("%zu bytes struct bpf_elf_map fixup performed due to size mismatch!\n",
> + sizeof(struct bpf_elf_map) - ctx->map_len);
Glad to see that :) FWIW you seem to use fprintf(stderr, ) in the btf
patch for information messages, I think that's a good approach.
Separating "results" from "info/error" messages. Helps JSON too.
Probably doesn't matter much for this message.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
2018-07-17 23:31 ` [PATCH iproute2 5/5] bpf: implement btf handling and map annotation Daniel Borkmann
@ 2018-07-18 0:27 ` Jakub Kicinski
2018-07-18 8:42 ` Daniel Borkmann
2018-07-18 23:52 ` Martin KaFai Lau
0 siblings, 2 replies; 17+ messages in thread
From: Jakub Kicinski @ 2018-07-18 0:27 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: dsahern, alexei.starovoitov, netdev
On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:
> # bpftool map dump id 386
> [{
> "key": 0,
> "value": {
> "": {
> "value": 0,
> "ifindex": 0,
> "mac": []
> }
> }
> },{
> "key": 1,
> "value": {
> "": {
> "value": 0,
> "ifindex": 0,
> "mac": []
> }
> }
> },{
> [...]
Ugh, the empty keys ("") look worrying, we should probably improve
handling of anonymous structs in bpftool :S
FWIW all the patches look nice to me! Thanks for keeping the support
for loading programs from the ".text" section :)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 0/5] Various BPF improvements
2018-07-17 23:31 [PATCH iproute2 0/5] Various BPF improvements Daniel Borkmann
` (4 preceding siblings ...)
2018-07-17 23:31 ` [PATCH iproute2 5/5] bpf: implement btf handling and map annotation Daniel Borkmann
@ 2018-07-18 2:43 ` David Ahern
5 siblings, 0 replies; 17+ messages in thread
From: David Ahern @ 2018-07-18 2:43 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: jakub.kicinski, alexei.starovoitov, netdev
On 7/17/18 5:31 PM, Daniel Borkmann wrote:
> Main part of this set is to: i) avoid strict af_alg kernel dependency,
> ii) add loader support for bpf to bpf calls and iii) add btf loader
> support with an option to annotate maps. For details please see the
> individual patches. Thanks!
>
> Daniel Borkmann (5):
> bpf: import btf uapi kernel header
> bpf: move bpf_elf_map fixup notification under verbose
> bpf: remove strict dependency on af_alg
> bpf: implement bpf to bpf calls support
> bpf: implement btf handling and map annotation
>
> include/bpf_elf.h | 9 +
> include/bpf_util.h | 1 +
> include/uapi/linux/btf.h | 113 +++++++++
> lib/bpf.c | 645 +++++++++++++++++++++++++++++++++++++----------
> 4 files changed, 639 insertions(+), 129 deletions(-)
> create mode 100644 include/uapi/linux/btf.h
>
Applied 2-5 to iproute2-next. Pulled btf.h from the last header sync
point for consistency.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
2018-07-18 0:27 ` Jakub Kicinski
@ 2018-07-18 8:42 ` Daniel Borkmann
2018-07-18 9:33 ` Daniel Borkmann
2018-07-18 23:52 ` Martin KaFai Lau
1 sibling, 1 reply; 17+ messages in thread
From: Daniel Borkmann @ 2018-07-18 8:42 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: dsahern, alexei.starovoitov, netdev
On 07/18/2018 02:27 AM, Jakub Kicinski wrote:
> On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:
>> # bpftool map dump id 386
>> [{
>> "key": 0,
>> "value": {
>> "": {
>> "value": 0,
>> "ifindex": 0,
>> "mac": []
>> }
>> }
>> },{
>> "key": 1,
>> "value": {
>> "": {
>> "value": 0,
>> "ifindex": 0,
>> "mac": []
>> }
>> }
>> },{
>> [...]
>
> Ugh, the empty keys ("") look worrying, we should probably improve
> handling of anonymous structs in bpftool :S
Yeah agree, I think it would be nice to see a more pahole style dump
where we have types and member names along with the value as otherwise
it might be a bit confusing.
> FWIW all the patches look nice to me! Thanks for keeping the support
> for loading programs from the ".text" section :)
The test_offload.py from BPF kselftests helped a lot in that! :)
Thanks,
Daniel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
2018-07-18 8:42 ` Daniel Borkmann
@ 2018-07-18 9:33 ` Daniel Borkmann
2018-07-18 18:13 ` Jakub Kicinski
0 siblings, 1 reply; 17+ messages in thread
From: Daniel Borkmann @ 2018-07-18 9:33 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: dsahern, alexei.starovoitov, netdev
On 07/18/2018 10:42 AM, Daniel Borkmann wrote:
> On 07/18/2018 02:27 AM, Jakub Kicinski wrote:
>> On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:
>>> # bpftool map dump id 386
>>> [{
>>> "key": 0,
>>> "value": {
>>> "": {
>>> "value": 0,
>>> "ifindex": 0,
>>> "mac": []
>>> }
>>> }
>>> },{
>>> "key": 1,
>>> "value": {
>>> "": {
>>> "value": 0,
>>> "ifindex": 0,
>>> "mac": []
>>> }
>>> }
>>> },{
>>> [...]
>>
>> Ugh, the empty keys ("") look worrying, we should probably improve
>> handling of anonymous structs in bpftool :S
>
> Yeah agree, I think it would be nice to see a more pahole style dump
> where we have types and member names along with the value as otherwise
> it might be a bit confusing.
Another feature that would be super useful imho would be in the /single/
map view e.g. 'bpftool map show id 123' to have a detailed BTF key+value
type dump, so in addition to the basic map info we show pahole like info
of the structs with length/offsets.
>> FWIW all the patches look nice to me! Thanks for keeping the support
>> for loading programs from the ".text" section :)
>
> The test_offload.py from BPF kselftests helped a lot in that! :)
>
> Thanks,
> Daniel
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
2018-07-18 9:33 ` Daniel Borkmann
@ 2018-07-18 18:13 ` Jakub Kicinski
2018-07-19 0:11 ` Martin KaFai Lau
0 siblings, 1 reply; 17+ messages in thread
From: Jakub Kicinski @ 2018-07-18 18:13 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: dsahern, alexei.starovoitov, netdev
On Wed, 18 Jul 2018 11:33:22 +0200, Daniel Borkmann wrote:
> On 07/18/2018 10:42 AM, Daniel Borkmann wrote:
> > On 07/18/2018 02:27 AM, Jakub Kicinski wrote:
> >> On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:
> >>> # bpftool map dump id 386
> >>> [{
> >>> "key": 0,
> >>> "value": {
> >>> "": {
> >>> "value": 0,
> >>> "ifindex": 0,
> >>> "mac": []
> >>> }
> >>> }
> >>> },{
> >>> "key": 1,
> >>> "value": {
> >>> "": {
> >>> "value": 0,
> >>> "ifindex": 0,
> >>> "mac": []
> >>> }
> >>> }
> >>> },{
> >>> [...]
> >>
> >> Ugh, the empty keys ("") look worrying, we should probably improve
> >> handling of anonymous structs in bpftool :S
> >
> > Yeah agree, I think it would be nice to see a more pahole style dump
> > where we have types and member names along with the value as otherwise
> > it might be a bit confusing.
>
> Another feature that would be super useful imho would be in the /single/
> map view e.g. 'bpftool map show id 123' to have a detailed BTF key+value
> type dump, so in addition to the basic map info we show pahole like info
> of the structs with length/offsets.
That sounds good! We could also consider adding a btf object and
commands to interrogate BTF types in the kernel in general.. Perhaps
then we could add something like bpftool btf describe map id 123.
Having the single map view show more information seems interesting, but
I wonder if it could be surprising. Is there precedent for such
behaviour?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
2018-07-18 0:27 ` Jakub Kicinski
2018-07-18 8:42 ` Daniel Borkmann
@ 2018-07-18 23:52 ` Martin KaFai Lau
2018-07-19 1:01 ` Jakub Kicinski
1 sibling, 1 reply; 17+ messages in thread
From: Martin KaFai Lau @ 2018-07-18 23:52 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Daniel Borkmann, dsahern, alexei.starovoitov, netdev
On Tue, Jul 17, 2018 at 05:27:43PM -0700, Jakub Kicinski wrote:
> On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:
> > # bpftool map dump id 386
> > [{
> > "key": 0,
> > "value": {
> > "": {
> > "value": 0,
> > "ifindex": 0,
> > "mac": []
> > }
> > }
> > },{
> > "key": 1,
> > "value": {
> > "": {
> > "value": 0,
> > "ifindex": 0,
> > "mac": []
> > }
> > }
> > },{
> > [...]
>
> Ugh, the empty keys ("") look worrying, we should probably improve
> handling of anonymous structs in bpftool :S
Note that the kernel's btf_verifier_log is using "(anon)" in this case.
Not sure if it is a good idea for json.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
2018-07-18 18:13 ` Jakub Kicinski
@ 2018-07-19 0:11 ` Martin KaFai Lau
2018-07-19 15:43 ` Daniel Borkmann
0 siblings, 1 reply; 17+ messages in thread
From: Martin KaFai Lau @ 2018-07-19 0:11 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Daniel Borkmann, dsahern, alexei.starovoitov, netdev
On Wed, Jul 18, 2018 at 11:13:37AM -0700, Jakub Kicinski wrote:
> On Wed, 18 Jul 2018 11:33:22 +0200, Daniel Borkmann wrote:
> > On 07/18/2018 10:42 AM, Daniel Borkmann wrote:
> > > On 07/18/2018 02:27 AM, Jakub Kicinski wrote:
> > >> On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:
> > >>> # bpftool map dump id 386
> > >>> [{
> > >>> "key": 0,
> > >>> "value": {
> > >>> "": {
> > >>> "value": 0,
> > >>> "ifindex": 0,
> > >>> "mac": []
> > >>> }
> > >>> }
> > >>> },{
> > >>> "key": 1,
> > >>> "value": {
> > >>> "": {
> > >>> "value": 0,
> > >>> "ifindex": 0,
> > >>> "mac": []
> > >>> }
> > >>> }
> > >>> },{
> > >>> [...]
> > >>
> > >> Ugh, the empty keys ("") look worrying, we should probably improve
> > >> handling of anonymous structs in bpftool :S
> > >
> > > Yeah agree, I think it would be nice to see a more pahole style dump
> > > where we have types and member names along with the value as otherwise
> > > it might be a bit confusing.
> >
> > Another feature that would be super useful imho would be in the /single/
> > map view e.g. 'bpftool map show id 123' to have a detailed BTF key+value
> > type dump, so in addition to the basic map info we show pahole like info
> > of the structs with length/offsets.
>
> That sounds good! We could also consider adding a btf object and
> commands to interrogate BTF types in the kernel in general.. Perhaps
> then we could add something like bpftool btf describe map id 123.
+1 on the btf subcommand.
>
> Having the single map view show more information seems interesting, but
> I wonder if it could be surprising. Is there precedent for such
> behaviour?
Having everything in one page (map show id 123) could be interesting.
One thing is the pahole-like output may be quite long?
e.g. the member of a struct could itself be another struct.
Not sure how the pahole-like output may look like in json though.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
2018-07-18 23:52 ` Martin KaFai Lau
@ 2018-07-19 1:01 ` Jakub Kicinski
0 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2018-07-19 1:01 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: Daniel Borkmann, dsahern, alexei.starovoitov, netdev
On Wed, 18 Jul 2018 16:52:34 -0700, Martin KaFai Lau wrote:
> On Tue, Jul 17, 2018 at 05:27:43PM -0700, Jakub Kicinski wrote:
> > On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:
> > > # bpftool map dump id 386
> > > [{
> > > "key": 0,
> > > "value": {
> > > "": {
> > > "value": 0,
> > > "ifindex": 0,
> > > "mac": []
> > > }
> > > }
> > > },{
> > > "key": 1,
> > > "value": {
> > > "": {
> > > "value": 0,
> > > "ifindex": 0,
> > > "mac": []
> > > }
> > > }
> > > },{
> > > [...]
> >
> > Ugh, the empty keys ("") look worrying, we should probably improve
> > handling of anonymous structs in bpftool :S
> Note that the kernel's btf_verifier_log is using "(anon)" in this case.
> Not sure if it is a good idea for json.
"(anon)" sounds good, we should probably add some id to it in JSON in
case there are multiple.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
2018-07-19 0:11 ` Martin KaFai Lau
@ 2018-07-19 15:43 ` Daniel Borkmann
2018-07-19 16:37 ` Martin KaFai Lau
0 siblings, 1 reply; 17+ messages in thread
From: Daniel Borkmann @ 2018-07-19 15:43 UTC (permalink / raw)
To: Martin KaFai Lau, Jakub Kicinski; +Cc: dsahern, alexei.starovoitov, netdev
On 07/19/2018 02:11 AM, Martin KaFai Lau wrote:
> On Wed, Jul 18, 2018 at 11:13:37AM -0700, Jakub Kicinski wrote:
>> On Wed, 18 Jul 2018 11:33:22 +0200, Daniel Borkmann wrote:
>>> On 07/18/2018 10:42 AM, Daniel Borkmann wrote:
>>>> On 07/18/2018 02:27 AM, Jakub Kicinski wrote:
>>>>> On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:
>>>>>> # bpftool map dump id 386
>>>>>> [{
>>>>>> "key": 0,
>>>>>> "value": {
>>>>>> "": {
>>>>>> "value": 0,
>>>>>> "ifindex": 0,
>>>>>> "mac": []
>>>>>> }
>>>>>> }
>>>>>> },{
>>>>>> "key": 1,
>>>>>> "value": {
>>>>>> "": {
>>>>>> "value": 0,
>>>>>> "ifindex": 0,
>>>>>> "mac": []
>>>>>> }
>>>>>> }
>>>>>> },{
>>>>>> [...]
>>>>>
>>>>> Ugh, the empty keys ("") look worrying, we should probably improve
>>>>> handling of anonymous structs in bpftool :S
>>>>
>>>> Yeah agree, I think it would be nice to see a more pahole style dump
>>>> where we have types and member names along with the value as otherwise
>>>> it might be a bit confusing.
>>>
>>> Another feature that would be super useful imho would be in the /single/
>>> map view e.g. 'bpftool map show id 123' to have a detailed BTF key+value
>>> type dump, so in addition to the basic map info we show pahole like info
>>> of the structs with length/offsets.
>>
>> That sounds good! We could also consider adding a btf object and
>> commands to interrogate BTF types in the kernel in general.. Perhaps
>> then we could add something like bpftool btf describe map id 123.
> +1 on the btf subcommand.
That would also work, I think both might be useful to have. Former would
all sit under a single command to show map details. With 'bpftool btf' you
would also allow for a full BTF dump when a specific BTF obj id is provided?
>> Having the single map view show more information seems interesting, but
>> I wonder if it could be surprising. Is there precedent for such
>> behaviour?
> Having everything in one page (map show id 123) could be interesting.
> One thing is the pahole-like output may be quite long?
> e.g. the member of a struct could itself be another struct.
Right, though probably fine when you want to see all information specific
to one map. Of course the 'bpftool map' list view would need to hide this
information.
> Not sure how the pahole-like output may look like in json though.
Would the existing map k/v dump have more or less the same 'issue'?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
2018-07-19 15:43 ` Daniel Borkmann
@ 2018-07-19 16:37 ` Martin KaFai Lau
0 siblings, 0 replies; 17+ messages in thread
From: Martin KaFai Lau @ 2018-07-19 16:37 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Jakub Kicinski, dsahern, alexei.starovoitov, netdev
On Thu, Jul 19, 2018 at 05:43:11PM +0200, Daniel Borkmann wrote:
> On 07/19/2018 02:11 AM, Martin KaFai Lau wrote:
> > On Wed, Jul 18, 2018 at 11:13:37AM -0700, Jakub Kicinski wrote:
> >> On Wed, 18 Jul 2018 11:33:22 +0200, Daniel Borkmann wrote:
> >>> On 07/18/2018 10:42 AM, Daniel Borkmann wrote:
> >>>> On 07/18/2018 02:27 AM, Jakub Kicinski wrote:
> >>>>> On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:
> >>>>>> # bpftool map dump id 386
> >>>>>> [{
> >>>>>> "key": 0,
> >>>>>> "value": {
> >>>>>> "": {
> >>>>>> "value": 0,
> >>>>>> "ifindex": 0,
> >>>>>> "mac": []
> >>>>>> }
> >>>>>> }
> >>>>>> },{
> >>>>>> "key": 1,
> >>>>>> "value": {
> >>>>>> "": {
> >>>>>> "value": 0,
> >>>>>> "ifindex": 0,
> >>>>>> "mac": []
> >>>>>> }
> >>>>>> }
> >>>>>> },{
> >>>>>> [...]
> >>>>>
> >>>>> Ugh, the empty keys ("") look worrying, we should probably improve
> >>>>> handling of anonymous structs in bpftool :S
> >>>>
> >>>> Yeah agree, I think it would be nice to see a more pahole style dump
> >>>> where we have types and member names along with the value as otherwise
> >>>> it might be a bit confusing.
> >>>
> >>> Another feature that would be super useful imho would be in the /single/
> >>> map view e.g. 'bpftool map show id 123' to have a detailed BTF key+value
> >>> type dump, so in addition to the basic map info we show pahole like info
> >>> of the structs with length/offsets.
> >>
> >> That sounds good! We could also consider adding a btf object and
> >> commands to interrogate BTF types in the kernel in general.. Perhaps
> >> then we could add something like bpftool btf describe map id 123.
> > +1 on the btf subcommand.
>
> That would also work, I think both might be useful to have. Former would
> all sit under a single command to show map details.
Agree that both would be useful. btf command could address the whole BTF
object which could include many maps/types while the map command is
focusing on its own map info.
> With 'bpftool btf' you
> would also allow for a full BTF dump when a specific BTF obj id is provided?
Right, I think the BTF obj id (or file) is needed for the btf command. and then
it should allow to do full dump or only show a particular map/type id.
A little forward thinking, map here is a C type. Hence, I think using the
name "type" like "btftool btf id 1 show _type_ id 123" may be better when
we later expand BTF usage beyond BPF program.
>
> >> Having the single map view show more information seems interesting, but
> >> I wonder if it could be surprising. Is there precedent for such
> >> behaviour?
> > Having everything in one page (map show id 123) could be interesting.
> > One thing is the pahole-like output may be quite long?
> > e.g. the member of a struct could itself be another struct.
>
> Right, though probably fine when you want to see all information specific
> to one map. Of course the 'bpftool map' list view would need to hide this
> information.
>
> > Not sure how the pahole-like output may look like in json though.
>
> Would the existing map k/v dump have more or less the same 'issue'?
True, the existing map k/v dump of the map data is reusing the
json {}/[]/""/number convention. I think that is ok and actually a
pretty condensed way since people are used to this convention when
reading "data" output.
For printing out C type, I think it is more natural to have it as
close to C syntax as possible in order to have it parsable by human
eyes. However, yes, we could reuse a similar fashion to print type
in json as we do in printing data. Just curious, the json type output
is more for script or mostly for people that can read everything from one
json output.
For plaintext, we can just print like pahole.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-07-19 17:23 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-17 23:31 [PATCH iproute2 0/5] Various BPF improvements Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 1/5] bpf: import btf uapi kernel header Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 2/5] bpf: move bpf_elf_map fixup notification under verbose Daniel Borkmann
2018-07-18 0:24 ` Jakub Kicinski
2018-07-17 23:31 ` [PATCH iproute2 3/5] bpf: remove strict dependency on af_alg Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 4/5] bpf: implement bpf to bpf calls support Daniel Borkmann
2018-07-17 23:31 ` [PATCH iproute2 5/5] bpf: implement btf handling and map annotation Daniel Borkmann
2018-07-18 0:27 ` Jakub Kicinski
2018-07-18 8:42 ` Daniel Borkmann
2018-07-18 9:33 ` Daniel Borkmann
2018-07-18 18:13 ` Jakub Kicinski
2018-07-19 0:11 ` Martin KaFai Lau
2018-07-19 15:43 ` Daniel Borkmann
2018-07-19 16:37 ` Martin KaFai Lau
2018-07-18 23:52 ` Martin KaFai Lau
2018-07-19 1:01 ` Jakub Kicinski
2018-07-18 2:43 ` [PATCH iproute2 0/5] Various BPF improvements David Ahern
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.