* [PATCH bpf-next v6 0/7] libbpf: Implement BTFGen
@ 2022-02-09 22:26 Mauricio Vásquez
2022-02-09 22:26 ` [PATCH bpf-next v6 1/7] libbpf: split bpf_core_apply_relo() Mauricio Vásquez
` (6 more replies)
0 siblings, 7 replies; 21+ messages in thread
From: Mauricio Vásquez @ 2022-02-09 22:26 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
CO-RE requires to have BTF information describing the kernel types in
order to perform the relocations. This is usually provided by the kernel
itself when it's configured with CONFIG_DEBUG_INFO_BTF. However, this
configuration is not enabled in all the distributions and it's not
available on kernels before 5.12.
It's possible to use CO-RE in kernels without CONFIG_DEBUG_INFO_BTF
support by providing the BTF information from an external source.
BTFHub[0] contains BTF files to each released kernel not supporting BTF,
for the most popular distributions.
Providing this BTF file for a given kernel has some challenges:
1. Each BTF file is a few MBs big, then it's not possible to ship the
eBPF program with all the BTF files needed to run in different kernels.
(The BTF files will be in the order of GBs if you want to support a high
number of kernels)
2. Downloading the BTF file for the current kernel at runtime delays the
start of the program and it's not always possible to reach an external
host to download such a file.
Providing the BTF file with the information about all the data types of
the kernel for running an eBPF program is an overkill in many of the
cases. Usually the eBPF programs access only some kernel fields.
This series implements BTFGen support in bpftool. This idea was
discussed during the "Towards truly portable eBPF"[1] presentation at
Linux Plumbers 2021.
There is a good example[2] on how to use BTFGen and BTFHub together
to generate multiple BTF files, to each existing/supported kernel,
tailored to one application. For example: a complex bpf object might
support nearly 400 kernels by having BTF files summing only 1.5 MB.
[0]: https://github.com/aquasecurity/btfhub/
[1]: https://www.youtube.com/watch?v=igJLKyP1lFk&t=2418s
[2]: https://github.com/aquasecurity/btfhub/tree/main/tools
Changelog:
v5 > v6:
- use BTF structure to store used member/types instead of hashmaps
- remove support for input/output folders
- remove bpf_core_{created,free}_cand_cache()
- reorganize commits to avoid having unused static functions
- remove usage of libbpf_get_error()
- fix some errno propagation issues
- do not record full types for type-based relocations
- add support for BTF_KIND_FUNC_PROTO
- implement tests based on core_reloc ones
v4 > v5:
- move some checks before invoking prog->obj->gen_loader
- use p_info() instead of printf()
- improve command output
- fix issue with record_relo_core()
- implement bash completion
- write man page
- implement some tests
v3 > v4:
- parse BTF and BTF.ext sections in bpftool and use
bpf_core_calc_relo_insn() directly
- expose less internal details from libbpf to bpftool
- implement support for enum-based relocations
- split commits in a more granular way
v2 > v3:
- expose internal libbpf APIs to bpftool instead
- implement btfgen in bpftool
- drop btf__raw_data() from libbpf
v1 > v2:
- introduce bpf_object__prepare() and ‘record_core_relos’ to expose
CO-RE relocations instead of bpf_object__reloc_info_gen()
- rename btf__save_to_file() to btf__raw_data()
v1: https://lore.kernel.org/bpf/20211027203727.208847-1-mauricio@kinvolk.io/
v2: https://lore.kernel.org/bpf/20211116164208.164245-1-mauricio@kinvolk.io/
v3: https://lore.kernel.org/bpf/20211217185654.311609-1-mauricio@kinvolk.io/
v4: https://lore.kernel.org/bpf/20220112142709.102423-1-mauricio@kinvolk.io/
v5: https://lore.kernel.org/bpf/20220128223312.1253169-1-mauricio@kinvolk.io/
Mauricio Vásquez (6):
libbpf: split bpf_core_apply_relo()
libbpf: Expose bpf_core_{add,free}_cands() to bpftool
bpftool: Add gen min_core_btf command
bpftool: Implement minimize_btf() and relocations recording for BTFGen
bpftool: Implement btfgen_get_btf()
selftests/bpf: Test "bpftool gen min_core_btf"
Rafael David Tinoco (1):
bpftool: gen min_core_btf explanation and examples
kernel/bpf/btf.c | 13 +-
.../bpf/bpftool/Documentation/bpftool-gen.rst | 93 +++
tools/bpf/bpftool/Makefile | 8 +-
tools/bpf/bpftool/bash-completion/bpftool | 6 +-
tools/bpf/bpftool/gen.c | 624 +++++++++++++++++-
tools/lib/bpf/libbpf.c | 88 +--
tools/lib/bpf/libbpf_internal.h | 9 +
tools/lib/bpf/relo_core.c | 79 +--
tools/lib/bpf/relo_core.h | 42 +-
.../selftests/bpf/prog_tests/core_reloc.c | 46 +-
10 files changed, 896 insertions(+), 112 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH bpf-next v6 1/7] libbpf: split bpf_core_apply_relo()
2022-02-09 22:26 [PATCH bpf-next v6 0/7] libbpf: Implement BTFGen Mauricio Vásquez
@ 2022-02-09 22:26 ` Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 2/7] libbpf: Expose bpf_core_{add,free}_cands() to bpftool Mauricio Vásquez
` (5 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Mauricio Vásquez @ 2022-02-09 22:26 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
BTFGen needs to run the core relocation logic in order to understand
what are the types involved in a given relocation.
Currently bpf_core_apply_relo() calculates and **applies** a relocation
to an instruction. Having both operations in the same function makes it
difficult to only calculate the relocation without patching the
instruction. This commit splits that logic in two different phases: (1)
calculate the relocation and (2) patch the instruction.
For the first phase bpf_core_apply_relo() is renamed to
bpf_core_calc_relo_insn() who is now only on charge of calculating the
relocation, the second phase uses the already existing
bpf_core_patch_insn(). bpf_object__relocate_core() uses both of them and
the BTFGen will use only bpf_core_calc_relo_insn().
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
kernel/bpf/btf.c | 13 +++++--
tools/lib/bpf/libbpf.c | 71 ++++++++++++++++++++---------------
tools/lib/bpf/relo_core.c | 79 ++++++++++++---------------------------
tools/lib/bpf/relo_core.h | 42 ++++++++++++++++++---
4 files changed, 109 insertions(+), 96 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 11740b300de9..f1d3d2a2f5f6 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7225,6 +7225,7 @@ int bpf_core_apply(struct bpf_core_ctx *ctx, const struct bpf_core_relo *relo,
{
bool need_cands = relo->kind != BPF_CORE_TYPE_ID_LOCAL;
struct bpf_core_cand_list cands = {};
+ struct bpf_core_relo_res targ_res;
struct bpf_core_spec *specs;
int err;
@@ -7264,13 +7265,19 @@ int bpf_core_apply(struct bpf_core_ctx *ctx, const struct bpf_core_relo *relo,
cands.len = cc->cnt;
/* cand_cache_mutex needs to span the cache lookup and
* copy of btf pointer into bpf_core_cand_list,
- * since module can be unloaded while bpf_core_apply_relo_insn
+ * since module can be unloaded while bpf_core_calc_relo_insn
* is working with module's btf.
*/
}
- err = bpf_core_apply_relo_insn((void *)ctx->log, insn, relo->insn_off / 8,
- relo, relo_idx, ctx->btf, &cands, specs);
+ err = bpf_core_calc_relo_insn((void *)ctx->log, relo, relo_idx, ctx->btf, &cands, specs,
+ &targ_res);
+ if (err)
+ goto out;
+
+ err = bpf_core_patch_insn((void *)ctx->log, insn, relo->insn_off / 8, relo, relo_idx,
+ &targ_res);
+
out:
kfree(specs);
if (need_cands) {
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2262bcdfee92..d3c457fb045e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5530,11 +5530,12 @@ static int record_relo_core(struct bpf_program *prog,
return 0;
}
-static int bpf_core_apply_relo(struct bpf_program *prog,
- const struct bpf_core_relo *relo,
- int relo_idx,
- const struct btf *local_btf,
- struct hashmap *cand_cache)
+static int bpf_core_resolve_relo(struct bpf_program *prog,
+ const struct bpf_core_relo *relo,
+ int relo_idx,
+ const struct btf *local_btf,
+ struct hashmap *cand_cache,
+ struct bpf_core_relo_res *targ_res)
{
struct bpf_core_spec specs_scratch[3] = {};
const void *type_key = u32_as_hash_key(relo->type_id);
@@ -5543,20 +5544,7 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
const struct btf_type *local_type;
const char *local_name;
__u32 local_id = relo->type_id;
- struct bpf_insn *insn;
- int insn_idx, err;
-
- if (relo->insn_off % BPF_INSN_SZ)
- return -EINVAL;
- insn_idx = relo->insn_off / BPF_INSN_SZ;
- /* adjust insn_idx from section frame of reference to the local
- * program's frame of reference; (sub-)program code is not yet
- * relocated, so it's enough to just subtract in-section offset
- */
- insn_idx = insn_idx - prog->sec_insn_off;
- if (insn_idx >= prog->insns_cnt)
- return -EINVAL;
- insn = &prog->insns[insn_idx];
+ int err;
local_type = btf__type_by_id(local_btf, local_id);
if (!local_type)
@@ -5566,15 +5554,6 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
if (!local_name)
return -EINVAL;
- if (prog->obj->gen_loader) {
- const char *spec_str = btf__name_by_offset(local_btf, relo->access_str_off);
-
- pr_debug("record_relo_core: prog %td insn[%d] %s %s %s final insn_idx %d\n",
- prog - prog->obj->programs, relo->insn_off / 8,
- btf_kind_str(local_type), local_name, spec_str, insn_idx);
- return record_relo_core(prog, relo, insn_idx);
- }
-
if (relo->kind != BPF_CORE_TYPE_ID_LOCAL &&
!hashmap__find(cand_cache, type_key, (void **)&cands)) {
cands = bpf_core_find_cands(prog->obj, local_btf, local_id);
@@ -5591,19 +5570,21 @@ static int bpf_core_apply_relo(struct bpf_program *prog,
}
}
- return bpf_core_apply_relo_insn(prog_name, insn, insn_idx, relo,
- relo_idx, local_btf, cands, specs_scratch);
+ return bpf_core_calc_relo_insn(prog_name, relo, relo_idx, local_btf, cands, specs_scratch,
+ targ_res);
}
static int
bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
{
const struct btf_ext_info_sec *sec;
+ struct bpf_core_relo_res targ_res;
const struct bpf_core_relo *rec;
const struct btf_ext_info *seg;
struct hashmap_entry *entry;
struct hashmap *cand_cache = NULL;
struct bpf_program *prog;
+ struct bpf_insn *insn;
const char *sec_name;
int i, err = 0, insn_idx, sec_idx;
@@ -5654,6 +5635,8 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
sec_name, sec->num_info);
for_each_btf_ext_rec(seg, sec, i, rec) {
+ if (rec->insn_off % BPF_INSN_SZ)
+ return -EINVAL;
insn_idx = rec->insn_off / BPF_INSN_SZ;
prog = find_prog_by_sec_insn(obj, sec_idx, insn_idx);
if (!prog) {
@@ -5668,12 +5651,38 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
if (!prog->load)
continue;
- err = bpf_core_apply_relo(prog, rec, i, obj->btf, cand_cache);
+ /* adjust insn_idx from section frame of reference to the local
+ * program's frame of reference; (sub-)program code is not yet
+ * relocated, so it's enough to just subtract in-section offset
+ */
+ insn_idx = insn_idx - prog->sec_insn_off;
+ if (insn_idx >= prog->insns_cnt)
+ return -EINVAL;
+ insn = &prog->insns[insn_idx];
+
+ if (prog->obj->gen_loader) {
+ err = record_relo_core(prog, rec, insn_idx);
+ if (err) {
+ pr_warn("prog '%s': relo #%d: failed to record relocation: %d\n",
+ prog->name, i, err);
+ goto out;
+ }
+ continue;
+ }
+
+ err = bpf_core_resolve_relo(prog, rec, i, obj->btf, cand_cache, &targ_res);
if (err) {
pr_warn("prog '%s': relo #%d: failed to relocate: %d\n",
prog->name, i, err);
goto out;
}
+
+ err = bpf_core_patch_insn(prog->name, insn, insn_idx, rec, i, &targ_res);
+ if (err) {
+ pr_warn("prog '%s': relo #%d: failed to patch insn #%u: %d\n",
+ prog->name, i, insn_idx, err);
+ goto out;
+ }
}
}
diff --git a/tools/lib/bpf/relo_core.c b/tools/lib/bpf/relo_core.c
index 910865e29edc..f946f23eab20 100644
--- a/tools/lib/bpf/relo_core.c
+++ b/tools/lib/bpf/relo_core.c
@@ -775,31 +775,6 @@ static int bpf_core_calc_enumval_relo(const struct bpf_core_relo *relo,
return 0;
}
-struct bpf_core_relo_res
-{
- /* expected value in the instruction, unless validate == false */
- __u32 orig_val;
- /* new value that needs to be patched up to */
- __u32 new_val;
- /* relocation unsuccessful, poison instruction, but don't fail load */
- bool poison;
- /* some relocations can't be validated against orig_val */
- bool validate;
- /* for field byte offset relocations or the forms:
- * *(T *)(rX + <off>) = rY
- * rX = *(T *)(rY + <off>),
- * we remember original and resolved field size to adjust direct
- * memory loads of pointers and integers; this is necessary for 32-bit
- * host kernel architectures, but also allows to automatically
- * relocate fields that were resized from, e.g., u32 to u64, etc.
- */
- bool fail_memsz_adjust;
- __u32 orig_sz;
- __u32 orig_type_id;
- __u32 new_sz;
- __u32 new_type_id;
-};
-
/* Calculate original and target relocation values, given local and target
* specs and relocation kind. These values are calculated for each candidate.
* If there are multiple candidates, resulting values should all be consistent
@@ -951,9 +926,9 @@ static int insn_bytes_to_bpf_size(__u32 sz)
* 5. *(T *)(rX + <off>) = rY, where T is one of {u8, u16, u32, u64};
* 6. *(T *)(rX + <off>) = <imm>, where T is one of {u8, u16, u32, u64}.
*/
-static int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
- int insn_idx, const struct bpf_core_relo *relo,
- int relo_idx, const struct bpf_core_relo_res *res)
+int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
+ int insn_idx, const struct bpf_core_relo *relo,
+ int relo_idx, const struct bpf_core_relo_res *res)
{
__u32 orig_val, new_val;
__u8 class;
@@ -1128,7 +1103,7 @@ static void bpf_core_dump_spec(const char *prog_name, int level, const struct bp
}
/*
- * CO-RE relocate single instruction.
+ * Calculate CO-RE relocation target result.
*
* The outline and important points of the algorithm:
* 1. For given local type, find corresponding candidate target types.
@@ -1177,18 +1152,18 @@ static void bpf_core_dump_spec(const char *prog_name, int level, const struct bp
* between multiple relocations for the same type ID and is updated as some
* of the candidates are pruned due to structural incompatibility.
*/
-int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,
- int insn_idx,
- const struct bpf_core_relo *relo,
- int relo_idx,
- const struct btf *local_btf,
- struct bpf_core_cand_list *cands,
- struct bpf_core_spec *specs_scratch)
+int bpf_core_calc_relo_insn(const char *prog_name,
+ const struct bpf_core_relo *relo,
+ int relo_idx,
+ const struct btf *local_btf,
+ struct bpf_core_cand_list *cands,
+ struct bpf_core_spec *specs_scratch,
+ struct bpf_core_relo_res *targ_res)
{
struct bpf_core_spec *local_spec = &specs_scratch[0];
struct bpf_core_spec *cand_spec = &specs_scratch[1];
struct bpf_core_spec *targ_spec = &specs_scratch[2];
- struct bpf_core_relo_res cand_res, targ_res;
+ struct bpf_core_relo_res cand_res;
const struct btf_type *local_type;
const char *local_name;
__u32 local_id;
@@ -1223,12 +1198,12 @@ int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,
/* TYPE_ID_LOCAL relo is special and doesn't need candidate search */
if (relo->kind == BPF_CORE_TYPE_ID_LOCAL) {
/* bpf_insn's imm value could get out of sync during linking */
- memset(&targ_res, 0, sizeof(targ_res));
- targ_res.validate = false;
- targ_res.poison = false;
- targ_res.orig_val = local_spec->root_type_id;
- targ_res.new_val = local_spec->root_type_id;
- goto patch_insn;
+ memset(targ_res, 0, sizeof(*targ_res));
+ targ_res->validate = false;
+ targ_res->poison = false;
+ targ_res->orig_val = local_spec->root_type_id;
+ targ_res->new_val = local_spec->root_type_id;
+ return 0;
}
/* libbpf doesn't support candidate search for anonymous types */
@@ -1262,7 +1237,7 @@ int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,
return err;
if (j == 0) {
- targ_res = cand_res;
+ *targ_res = cand_res;
*targ_spec = *cand_spec;
} else if (cand_spec->bit_offset != targ_spec->bit_offset) {
/* if there are many field relo candidates, they
@@ -1272,7 +1247,8 @@ int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,
prog_name, relo_idx, cand_spec->bit_offset,
targ_spec->bit_offset);
return -EINVAL;
- } else if (cand_res.poison != targ_res.poison || cand_res.new_val != targ_res.new_val) {
+ } else if (cand_res.poison != targ_res->poison ||
+ cand_res.new_val != targ_res->new_val) {
/* all candidates should result in the same relocation
* decision and value, otherwise it's dangerous to
* proceed due to ambiguity
@@ -1280,7 +1256,7 @@ int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,
pr_warn("prog '%s': relo #%d: relocation decision ambiguity: %s %u != %s %u\n",
prog_name, relo_idx,
cand_res.poison ? "failure" : "success", cand_res.new_val,
- targ_res.poison ? "failure" : "success", targ_res.new_val);
+ targ_res->poison ? "failure" : "success", targ_res->new_val);
return -EINVAL;
}
@@ -1314,19 +1290,10 @@ int bpf_core_apply_relo_insn(const char *prog_name, struct bpf_insn *insn,
prog_name, relo_idx);
/* calculate single target relo result explicitly */
- err = bpf_core_calc_relo(prog_name, relo, relo_idx, local_spec, NULL, &targ_res);
+ err = bpf_core_calc_relo(prog_name, relo, relo_idx, local_spec, NULL, targ_res);
if (err)
return err;
}
-patch_insn:
- /* bpf_core_patch_insn() should know how to handle missing targ_spec */
- err = bpf_core_patch_insn(prog_name, insn, insn_idx, relo, relo_idx, &targ_res);
- if (err) {
- pr_warn("prog '%s': relo #%d: failed to patch insn #%u: %d\n",
- prog_name, relo_idx, relo->insn_off / 8, err);
- return -EINVAL;
- }
-
return 0;
}
diff --git a/tools/lib/bpf/relo_core.h b/tools/lib/bpf/relo_core.h
index 17799819ad7c..a28bf3711ce2 100644
--- a/tools/lib/bpf/relo_core.h
+++ b/tools/lib/bpf/relo_core.h
@@ -44,14 +44,44 @@ struct bpf_core_spec {
__u32 bit_offset;
};
-int bpf_core_apply_relo_insn(const char *prog_name,
- struct bpf_insn *insn, int insn_idx,
- const struct bpf_core_relo *relo, int relo_idx,
- const struct btf *local_btf,
- struct bpf_core_cand_list *cands,
- struct bpf_core_spec *specs_scratch);
+struct bpf_core_relo_res {
+ /* expected value in the instruction, unless validate == false */
+ __u32 orig_val;
+ /* new value that needs to be patched up to */
+ __u32 new_val;
+ /* relocation unsuccessful, poison instruction, but don't fail load */
+ bool poison;
+ /* some relocations can't be validated against orig_val */
+ bool validate;
+ /* for field byte offset relocations or the forms:
+ * *(T *)(rX + <off>) = rY
+ * rX = *(T *)(rY + <off>),
+ * we remember original and resolved field size to adjust direct
+ * memory loads of pointers and integers; this is necessary for 32-bit
+ * host kernel architectures, but also allows to automatically
+ * relocate fields that were resized from, e.g., u32 to u64, etc.
+ */
+ bool fail_memsz_adjust;
+ __u32 orig_sz;
+ __u32 orig_type_id;
+ __u32 new_sz;
+ __u32 new_type_id;
+};
+
int bpf_core_types_are_compat(const struct btf *local_btf, __u32 local_id,
const struct btf *targ_btf, __u32 targ_id);
size_t bpf_core_essential_name_len(const char *name);
+
+int bpf_core_calc_relo_insn(const char *prog_name,
+ const struct bpf_core_relo *relo, int relo_idx,
+ const struct btf *local_btf,
+ struct bpf_core_cand_list *cands,
+ struct bpf_core_spec *specs_scratch,
+ struct bpf_core_relo_res *targ_res);
+
+int bpf_core_patch_insn(const char *prog_name, struct bpf_insn *insn,
+ int insn_idx, const struct bpf_core_relo *relo,
+ int relo_idx, const struct bpf_core_relo_res *res);
+
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH bpf-next v6 2/7] libbpf: Expose bpf_core_{add,free}_cands() to bpftool
2022-02-09 22:26 [PATCH bpf-next v6 0/7] libbpf: Implement BTFGen Mauricio Vásquez
2022-02-09 22:26 ` [PATCH bpf-next v6 1/7] libbpf: split bpf_core_apply_relo() Mauricio Vásquez
@ 2022-02-09 22:26 ` Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 3/7] bpftool: Add gen min_core_btf command Mauricio Vásquez
` (4 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Mauricio Vásquez @ 2022-02-09 22:26 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
Expose bpf_core_add_cands() and bpf_core_free_cands() to handle
candidates list.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
tools/lib/bpf/libbpf.c | 17 ++++++++++-------
tools/lib/bpf/libbpf_internal.h | 9 +++++++++
2 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d3c457fb045e..ad43b6ce825e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5192,18 +5192,21 @@ size_t bpf_core_essential_name_len(const char *name)
return n;
}
-static void bpf_core_free_cands(struct bpf_core_cand_list *cands)
+void bpf_core_free_cands(struct bpf_core_cand_list *cands)
{
+ if (!cands)
+ return;
+
free(cands->cands);
free(cands);
}
-static int bpf_core_add_cands(struct bpf_core_cand *local_cand,
- size_t local_essent_len,
- const struct btf *targ_btf,
- const char *targ_btf_name,
- int targ_start_id,
- struct bpf_core_cand_list *cands)
+int bpf_core_add_cands(struct bpf_core_cand *local_cand,
+ size_t local_essent_len,
+ const struct btf *targ_btf,
+ const char *targ_btf_name,
+ int targ_start_id,
+ struct bpf_core_cand_list *cands)
{
struct bpf_core_cand *new_cands, *cand;
const struct btf_type *t, *local_t;
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index bc86b82e90d1..4fda8bdf0a0d 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -529,4 +529,13 @@ static inline int ensure_good_fd(int fd)
return fd;
}
+/* The following two functions are exposed to bpftool */
+int bpf_core_add_cands(struct bpf_core_cand *local_cand,
+ size_t local_essent_len,
+ const struct btf *targ_btf,
+ const char *targ_btf_name,
+ int targ_start_id,
+ struct bpf_core_cand_list *cands);
+void bpf_core_free_cands(struct bpf_core_cand_list *cands);
+
#endif /* __LIBBPF_LIBBPF_INTERNAL_H */
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH bpf-next v6 3/7] bpftool: Add gen min_core_btf command
2022-02-09 22:26 [PATCH bpf-next v6 0/7] libbpf: Implement BTFGen Mauricio Vásquez
2022-02-09 22:26 ` [PATCH bpf-next v6 1/7] libbpf: split bpf_core_apply_relo() Mauricio Vásquez
2022-02-09 22:26 ` [PATCH bpf-next v6 2/7] libbpf: Expose bpf_core_{add,free}_cands() to bpftool Mauricio Vásquez
@ 2022-02-09 22:26 ` Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 4/7] bpftool: Implement minimize_btf() and relocations recording for BTFGen Mauricio Vásquez
` (3 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Mauricio Vásquez @ 2022-02-09 22:26 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
This command is implemented under the "gen" command in bpftool and the
syntax is the following:
$ bpftool gen min_core_btf INPUT OUTPUT OBJECT [OBJECT...]
INPUT is the file that contains all the BTF types for a kernel and
OUTPUT is the path of the minimize BTF file that will be created with
only the types needed by the objects.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
tools/bpf/bpftool/bash-completion/bpftool | 6 +++-
tools/bpf/bpftool/gen.c | 42 +++++++++++++++++++++--
2 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool
index 493753a4962e..958e1fd71b5c 100644
--- a/tools/bpf/bpftool/bash-completion/bpftool
+++ b/tools/bpf/bpftool/bash-completion/bpftool
@@ -1003,9 +1003,13 @@ _bpftool()
;;
esac
;;
+ min_core_btf)
+ _filedir
+ return 0
+ ;;
*)
[[ $prev == $object ]] && \
- COMPREPLY=( $( compgen -W 'object skeleton help' -- "$cur" ) )
+ COMPREPLY=( $( compgen -W 'object skeleton help min_core_btf' -- "$cur" ) )
;;
esac
;;
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index eacfc6a2060d..582c20602639 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1087,6 +1087,7 @@ static int do_help(int argc, char **argv)
fprintf(stderr,
"Usage: %1$s %2$s object OUTPUT_FILE INPUT_FILE [INPUT_FILE...]\n"
" %1$s %2$s skeleton FILE [name OBJECT_NAME]\n"
+ " %1$s %2$s min_core_btf INPUT OUTPUT OBJECT [OBJECT...]\n"
" %1$s %2$s help\n"
"\n"
" " HELP_SPEC_OPTIONS " |\n"
@@ -1097,10 +1098,45 @@ static int do_help(int argc, char **argv)
return 0;
}
+/* Create minimized BTF file for a set of BPF objects */
+static int minimize_btf(const char *src_btf, const char *dst_btf, const char *objspaths[])
+{
+ return -EOPNOTSUPP;
+}
+
+static int do_min_core_btf(int argc, char **argv)
+{
+ const char *input, *output, **objs;
+ int i, err;
+
+ if (!REQ_ARGS(3)) {
+ usage();
+ return -1;
+ }
+
+ input = GET_ARG();
+ output = GET_ARG();
+
+ objs = (const char **) calloc(argc + 1, sizeof(*objs));
+ if (!objs) {
+ p_err("failed to allocate array for object names");
+ return -ENOMEM;
+ }
+
+ i = 0;
+ while (argc)
+ objs[i++] = GET_ARG();
+
+ err = minimize_btf(input, output, objs);
+ free(objs);
+ return err;
+}
+
static const struct cmd cmds[] = {
- { "object", do_object },
- { "skeleton", do_skeleton },
- { "help", do_help },
+ { "object", do_object },
+ { "skeleton", do_skeleton },
+ { "min_core_btf", do_min_core_btf},
+ { "help", do_help },
{ 0 }
};
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH bpf-next v6 4/7] bpftool: Implement minimize_btf() and relocations recording for BTFGen
2022-02-09 22:26 [PATCH bpf-next v6 0/7] libbpf: Implement BTFGen Mauricio Vásquez
` (2 preceding siblings ...)
2022-02-09 22:26 ` [PATCH bpf-next v6 3/7] bpftool: Add gen min_core_btf command Mauricio Vásquez
@ 2022-02-09 22:26 ` Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 5/7] bpftool: Implement btfgen_get_btf() Mauricio Vásquez
` (2 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Mauricio Vásquez @ 2022-02-09 22:26 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
minimize_btf() receives the path of a source and destination BTF files
and a list of BPF objects. This function records the relocations for
all objects and then generates the BTF file by calling btfgen_get_btf()
(implemented in the following commit).
btfgen_record_obj() loads the BTF and BTF.ext sections of the BPF
objects and loops through all CO-RE relocations. It uses
bpf_core_calc_relo_insn() from libbpf and passes the target spec to
btfgen_record_reloc(), that calls one of the following functions
depending on the relocation kind.
btfgen_record_field_relo() uses the target specification to mark all the
types that are involved in a field-based CO-RE relocation. In this case
types resolved and marked recursively using btfgen_mark_type().
Only the struct and union members (and their types) involved in the
relocation are marked to optimize the size of the generated BTF file.
btfgen_record_type_relo() marks the types involved in a type-based
CO-RE relocation. In this case no members for the struct and union
types are marked as libbpf doesn't use them while performing this kind
of relocation. Pointed types are marked as they are used by libbpf in
this case.
btfgen_record_enumval_relo() marks the whole enum type for enum-based
relocations.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
tools/bpf/bpftool/Makefile | 8 +-
tools/bpf/bpftool/gen.c | 452 ++++++++++++++++++++++++++++++++++++-
2 files changed, 454 insertions(+), 6 deletions(-)
diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 83369f55df61..97d447135536 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -34,10 +34,10 @@ LIBBPF_BOOTSTRAP_INCLUDE := $(LIBBPF_BOOTSTRAP_DESTDIR)/include
LIBBPF_BOOTSTRAP_HDRS_DIR := $(LIBBPF_BOOTSTRAP_INCLUDE)/bpf
LIBBPF_BOOTSTRAP := $(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a
-# We need to copy hashmap.h and nlattr.h which is not otherwise exported by
-# libbpf, but still required by bpftool.
-LIBBPF_INTERNAL_HDRS := $(addprefix $(LIBBPF_HDRS_DIR)/,hashmap.h nlattr.h)
-LIBBPF_BOOTSTRAP_INTERNAL_HDRS := $(addprefix $(LIBBPF_BOOTSTRAP_HDRS_DIR)/,hashmap.h)
+# We need to copy hashmap.h, nlattr.h, relo_core.h and libbpf_internal.h
+# which are not otherwise exported by libbpf, but still required by bpftool.
+LIBBPF_INTERNAL_HDRS := $(addprefix $(LIBBPF_HDRS_DIR)/,hashmap.h nlattr.h relo_core.h libbpf_internal.h)
+LIBBPF_BOOTSTRAP_INTERNAL_HDRS := $(addprefix $(LIBBPF_BOOTSTRAP_HDRS_DIR)/,hashmap.h relo_core.h libbpf_internal.h)
ifeq ($(BPFTOOL_VERSION),)
BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 582c20602639..c3e34db2ec8a 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -14,6 +14,7 @@
#include <unistd.h>
#include <bpf/bpf.h>
#include <bpf/libbpf.h>
+#include <bpf/libbpf_internal.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/mman.h>
@@ -1098,10 +1099,457 @@ static int do_help(int argc, char **argv)
return 0;
}
-/* Create minimized BTF file for a set of BPF objects */
+static int btf_save_raw(const struct btf *btf, const char *path)
+{
+ const void *data;
+ FILE *f = NULL;
+ __u32 data_sz;
+ int err = 0;
+
+ data = btf__raw_data(btf, &data_sz);
+ if (!data)
+ return -ENOMEM;
+
+ f = fopen(path, "wb");
+ if (!f)
+ return -errno;
+
+ if (fwrite(data, 1, data_sz, f) != data_sz)
+ err = -errno;
+
+ fclose(f);
+ return err;
+}
+
+struct btfgen_info {
+ struct btf *src_btf;
+ struct btf *marked_btf; // btf structure used to mark used types
+};
+
+static size_t btfgen_hash_fn(const void *key, void *ctx)
+{
+ return (size_t)key;
+}
+
+static bool btfgen_equal_fn(const void *k1, const void *k2, void *ctx)
+{
+ return k1 == k2;
+}
+
+static void *uint_as_hash_key(int x)
+{
+ return (void *)(uintptr_t)x;
+}
+
+static void *u32_as_hash_key(__u32 x)
+{
+ return (void *)(uintptr_t)x;
+}
+
+static void btfgen_free_info(struct btfgen_info *info)
+{
+ if (!info)
+ return;
+
+ btf__free(info->src_btf);
+ btf__free(info->marked_btf);
+
+ free(info);
+}
+
+static struct btfgen_info *
+btfgen_new_info(const char *targ_btf_path)
+{
+ struct btfgen_info *info;
+ int err;
+
+ info = calloc(1, sizeof(*info));
+ if (!info)
+ return NULL;
+
+ info->src_btf = btf__parse(targ_btf_path, NULL);
+ if (!info->src_btf) {
+ p_err("failed parsing '%s' BTF file: %s", targ_btf_path, strerror(errno));
+ err = -errno;
+ goto err_out;
+ }
+
+ info->marked_btf = btf__parse(targ_btf_path, NULL);
+ if (!info->marked_btf) {
+ p_err("failed parsing '%s' BTF file: %s", targ_btf_path, strerror(errno));
+ err = -errno;
+ goto err_out;
+ }
+
+ return info;
+
+err_out:
+ btfgen_free_info(info);
+ errno = -err;
+ return NULL;
+}
+
+#define MARKED UINT32_MAX
+
+static void btfgen_mark_member(struct btfgen_info *info, int type_id, int idx)
+{
+ const struct btf_type *t = btf__type_by_id(info->marked_btf, type_id);
+ struct btf_member *m = btf_members(t) + idx;
+
+ m->name_off = MARKED;
+}
+
+static int
+btfgen_mark_type(struct btfgen_info *info, unsigned int id, bool follow_pointers)
+{
+ const struct btf_type *btf_type = btf__type_by_id(info->src_btf, id);
+ struct btf_type *cloned_type;
+ struct btf_param *param;
+ struct btf_array *array;
+ int err, i;
+
+ if (id == 0)
+ return 0;
+
+ /* mark type on cloned BTF as used */
+ cloned_type = (struct btf_type *) btf__type_by_id(info->marked_btf, id);
+ cloned_type->name_off = MARKED;
+
+ /* recursively mark other types needed by it */
+ switch (btf_kind(btf_type)) {
+ case BTF_KIND_UNKN:
+ case BTF_KIND_INT:
+ case BTF_KIND_FLOAT:
+ case BTF_KIND_ENUM:
+ case BTF_KIND_STRUCT:
+ case BTF_KIND_UNION:
+ break;
+ case BTF_KIND_PTR:
+ if (follow_pointers) {
+ err = btfgen_mark_type(info, btf_type->type, follow_pointers);
+ if (err)
+ return err;
+ }
+ break;
+ case BTF_KIND_CONST:
+ case BTF_KIND_VOLATILE:
+ case BTF_KIND_TYPEDEF:
+ err = btfgen_mark_type(info, btf_type->type, follow_pointers);
+ if (err)
+ return err;
+ break;
+ case BTF_KIND_ARRAY:
+ array = btf_array(btf_type);
+
+ /* mark array type */
+ err = btfgen_mark_type(info, array->type, follow_pointers);
+ /* mark array's index type */
+ err = err ? : btfgen_mark_type(info, array->index_type, follow_pointers);
+ if (err)
+ return err;
+ break;
+ case BTF_KIND_FUNC_PROTO:
+ /* mark ret type */
+ err = btfgen_mark_type(info, btf_type->type, follow_pointers);
+ if (err)
+ return err;
+
+ /* mark parameters types */
+ param = btf_params(btf_type);
+ for (i = 0; i < btf_vlen(btf_type); i++) {
+ err = btfgen_mark_type(info, param->type, follow_pointers);
+ if (err)
+ return err;
+ param++;
+ }
+ break;
+ /* tells if some other type needs to be handled */
+ default:
+ p_err("unsupported kind: %s (%d)", btf_kind_str(btf_type), id);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int btfgen_record_field_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
+{
+ struct btf *btf = (struct btf *) info->src_btf;
+ const struct btf_type *btf_type;
+ struct btf_member *btf_member;
+ struct btf_array *array;
+ unsigned int id = targ_spec->root_type_id;
+ int idx, err;
+
+ /* mark root type */
+ btf_type = btf__type_by_id(btf, id);
+ err = btfgen_mark_type(info, id, false);
+ if (err)
+ return err;
+
+ /* mark types for complex types (arrays, unions, structures) */
+ for (int i = 1; i < targ_spec->raw_len; i++) {
+ /* skip typedefs and mods */
+ while (btf_is_mod(btf_type) || btf_is_typedef(btf_type)) {
+ id = btf_type->type;
+ btf_type = btf__type_by_id(btf, id);
+ }
+
+ switch (btf_kind(btf_type)) {
+ case BTF_KIND_STRUCT:
+ case BTF_KIND_UNION:
+ idx = targ_spec->raw_spec[i];
+ btf_member = btf_members(btf_type) + idx;
+
+ /* mark member */
+ btfgen_mark_member(info, id, idx);
+
+ /* mark member's type */
+ id = btf_member->type;
+ btf_type = btf__type_by_id(btf, id);
+ err = btfgen_mark_type(info, id, false);
+ if (err)
+ return err;
+ break;
+ case BTF_KIND_ARRAY:
+ array = btf_array(btf_type);
+ id = array->type;
+ btf_type = btf__type_by_id(btf, id);
+ break;
+ default:
+ p_err("unsupported kind: %s (%d)",
+ btf_kind_str(btf_type), btf_type->type);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+static int btfgen_record_type_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
+{
+ return btfgen_mark_type(info, targ_spec->root_type_id, true);
+}
+
+static int btfgen_record_enumval_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
+{
+ return btfgen_mark_type(info, targ_spec->root_type_id, false);
+}
+
+static int btfgen_record_reloc(struct btfgen_info *info, struct bpf_core_spec *res)
+{
+ switch (res->relo_kind) {
+ case BPF_CORE_FIELD_BYTE_OFFSET:
+ case BPF_CORE_FIELD_BYTE_SIZE:
+ case BPF_CORE_FIELD_EXISTS:
+ case BPF_CORE_FIELD_SIGNED:
+ case BPF_CORE_FIELD_LSHIFT_U64:
+ case BPF_CORE_FIELD_RSHIFT_U64:
+ return btfgen_record_field_relo(info, res);
+ case BPF_CORE_TYPE_ID_LOCAL: /* BPF_CORE_TYPE_ID_LOCAL doesn't require kernel BTF */
+ return 0;
+ case BPF_CORE_TYPE_ID_TARGET:
+ case BPF_CORE_TYPE_EXISTS:
+ case BPF_CORE_TYPE_SIZE:
+ return btfgen_record_type_relo(info, res);
+ case BPF_CORE_ENUMVAL_EXISTS:
+ case BPF_CORE_ENUMVAL_VALUE:
+ return btfgen_record_enumval_relo(info, res);
+ default:
+ return -EINVAL;
+ }
+}
+
+static struct bpf_core_cand_list *
+btfgen_find_cands(const struct btf *local_btf, const struct btf *targ_btf, __u32 local_id)
+{
+ const struct btf_type *local_type;
+ struct bpf_core_cand_list *cands = NULL;
+ struct bpf_core_cand local_cand = {};
+ size_t local_essent_len;
+ const char *local_name;
+ int err;
+
+ local_cand.btf = local_btf;
+ local_cand.id = local_id;
+
+ local_type = btf__type_by_id(local_btf, local_id);
+ if (!local_type) {
+ err = -EINVAL;
+ goto err_out;
+ }
+
+ local_name = btf__name_by_offset(local_btf, local_type->name_off);
+ if (!local_name) {
+ err = -EINVAL;
+ goto err_out;
+ }
+ local_essent_len = bpf_core_essential_name_len(local_name);
+
+ cands = calloc(1, sizeof(*cands));
+ if (!cands)
+ return NULL;
+
+ err = bpf_core_add_cands(&local_cand, local_essent_len, targ_btf, "vmlinux", 1, cands);
+ if (err)
+ goto err_out;
+
+ return cands;
+
+err_out:
+ bpf_core_free_cands(cands);
+ errno = -err;
+ return NULL;
+}
+
+/* Record relocation information for a single BPF object*/
+static int btfgen_record_obj(struct btfgen_info *info, const char *obj_path)
+{
+ const struct btf_ext_info_sec *sec;
+ const struct bpf_core_relo *relo;
+ const struct btf_ext_info *seg;
+ struct hashmap_entry *entry;
+ struct hashmap *cand_cache = NULL;
+ struct btf_ext *btf_ext = NULL;
+ unsigned int relo_idx;
+ struct btf *btf = NULL;
+ size_t i;
+ int err;
+
+ btf = btf__parse(obj_path, &btf_ext);
+ if (!btf) {
+ p_err("failed to parse BPF object '%s': %s", obj_path, strerror(errno));
+ return -errno;
+ }
+
+ if (btf_ext->core_relo_info.len == 0) {
+ err = 0;
+ goto out;
+ }
+
+ cand_cache = hashmap__new(btfgen_hash_fn, btfgen_equal_fn, NULL);
+ if (IS_ERR(cand_cache)) {
+ err = PTR_ERR(cand_cache);
+ goto out;
+ }
+
+ seg = &btf_ext->core_relo_info;
+ for_each_btf_ext_sec(seg, sec) {
+ for_each_btf_ext_rec(seg, sec, relo_idx, relo) {
+ struct bpf_core_spec specs_scratch[3] = {};
+ struct bpf_core_relo_res targ_res = {};
+ struct bpf_core_cand_list *cands = NULL;
+ const void *type_key = u32_as_hash_key(relo->type_id);
+ const char *sec_name = btf__name_by_offset(btf, sec->sec_name_off);
+
+ if (relo->kind != BPF_CORE_TYPE_ID_LOCAL &&
+ !hashmap__find(cand_cache, type_key, (void **)&cands)) {
+ cands = btfgen_find_cands(btf, info->src_btf, relo->type_id);
+ if (!cands) {
+ err = -errno;
+ goto out;
+ }
+
+ err = hashmap__set(cand_cache, type_key, cands, NULL, NULL);
+ if (err)
+ goto out;
+ }
+
+ err = bpf_core_calc_relo_insn(sec_name, relo, relo_idx, btf, cands,
+ specs_scratch, &targ_res);
+ if (err)
+ goto out;
+
+ /* specs_scratch[2] is the target spec */
+ err = btfgen_record_reloc(info, &specs_scratch[2]);
+ if (err)
+ goto out;
+ }
+ }
+
+out:
+ btf__free(btf);
+ btf_ext__free(btf_ext);
+
+ if (!IS_ERR_OR_NULL(cand_cache)) {
+ hashmap__for_each_entry(cand_cache, entry, i) {
+ bpf_core_free_cands(entry->value);
+ }
+ hashmap__free(cand_cache);
+ }
+
+ return err;
+}
+
+/* Generate BTF from relocation information previously recorded */
+static struct btf *btfgen_get_btf(struct btfgen_info *info)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
+/* Create minimized BTF file for a set of BPF objects.
+ *
+ * The BTFGen algorithm is divided in two main parts: (1) collect the
+ * BTF types that are involved in relocations and (2) generate the BTF
+ * object using the collected types.
+ *
+ * In order to collect the types involved in the relocations, we parse
+ * the BTF and BTF.ext sections of the BPF objects and use
+ * bpf_core_calc_relo_insn() to get the target specification, this
+ * indicates how the types and fields are used in a relocation.
+ *
+ * Types are recorded in different ways according to the kind of the
+ * relocation. For field-based relocations only the members that are
+ * actually used are saved in order to reduce the size of the generated
+ * BTF file. For type-based relocations empty struct / unions are
+ * generated and for enum-based relocations the whole type is saved.
+ *
+ * The second part of the algorithm generates the BTF object. It creates
+ * an empty BTF object and fills it with the types recorded in the
+ * previous step. This function takes care of only adding the structure
+ * and union members that were marked as used and it also fixes up the
+ * type IDs on the generated BTF object.
+ */
static int minimize_btf(const char *src_btf, const char *dst_btf, const char *objspaths[])
{
- return -EOPNOTSUPP;
+ struct btfgen_info *info;
+ struct btf *btf_new = NULL;
+ int err, i;
+
+ info = btfgen_new_info(src_btf);
+ if (!info) {
+ p_err("failed to allocate info structure: %s", strerror(errno));
+ err = -errno;
+ goto out;
+ }
+
+ for (i = 0; objspaths[i] != NULL; i++) {
+ err = btfgen_record_obj(info, objspaths[i]);
+ if (err) {
+ p_err("error recording relocations for %s: %s", objspaths[i],
+ strerror(errno));
+ goto out;
+ }
+ }
+
+ btf_new = btfgen_get_btf(info);
+ if (!btf_new) {
+ err = -errno;
+ p_err("error generating BTF: %s", strerror(errno));
+ goto out;
+ }
+
+ err = btf_save_raw(btf_new, dst_btf);
+ if (err) {
+ p_err("error saving btf file: %s", strerror(errno));
+ goto out;
+ }
+
+out:
+ btf__free(btf_new);
+ btfgen_free_info(info);
+
+ return err;
}
static int do_min_core_btf(int argc, char **argv)
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH bpf-next v6 5/7] bpftool: Implement btfgen_get_btf()
2022-02-09 22:26 [PATCH bpf-next v6 0/7] libbpf: Implement BTFGen Mauricio Vásquez
` (3 preceding siblings ...)
2022-02-09 22:26 ` [PATCH bpf-next v6 4/7] bpftool: Implement minimize_btf() and relocations recording for BTFGen Mauricio Vásquez
@ 2022-02-09 22:26 ` Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 6/7] bpftool: gen min_core_btf explanation and examples Mauricio Vásquez
2022-02-09 22:26 ` [PATCH bpf-next v6 7/7] selftests/bpf: Test "bpftool gen min_core_btf" Mauricio Vásquez
6 siblings, 1 reply; 21+ messages in thread
From: Mauricio Vásquez @ 2022-02-09 22:26 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
The last part of the BTFGen algorithm is to create a new BTF object with
all the types that were recorded in the previous steps.
This function performs two different steps:
1. Add the types to the new BTF object by using btf__add_type(). Some
special logic around struct and unions is implemented to only add the
members that are really used in the field-based relocations. The type
ID on the new and old BTF objects is stored on a map.
2. Fix all the type IDs on the new BTF object by using the IDs saved in
the previous step.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
tools/bpf/bpftool/gen.c | 136 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 135 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index c3e34db2ec8a..1efc7f3c64b2 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1481,10 +1481,144 @@ static int btfgen_record_obj(struct btfgen_info *info, const char *obj_path)
return err;
}
+static unsigned int btfgen_get_id(struct hashmap *ids, unsigned int old)
+{
+ uintptr_t new;
+
+ if (!hashmap__find(ids, uint_as_hash_key(old), (void **)&new))
+ /* return id for BTF_KIND_VOID as it's possible that the
+ * ID we're looking for is the type of a pointer that
+ * we're not adding.
+ */
+ return 0;
+
+ return (unsigned int)(uintptr_t)new;
+}
+
+static int btfgen_add_id(struct hashmap *ids, unsigned int old, unsigned int new)
+{
+ return hashmap__add(ids, uint_as_hash_key(old), uint_as_hash_key(new));
+}
+
+static int btfgen_remap_id(__u32 *type_id, void *ctx)
+{
+ struct hashmap *ids = ctx;
+
+ *type_id = btfgen_get_id(ids, *type_id);
+
+ return 0;
+}
+
/* Generate BTF from relocation information previously recorded */
static struct btf *btfgen_get_btf(struct btfgen_info *info)
{
- return ERR_PTR(-EOPNOTSUPP);
+ struct btf *btf_new = NULL;
+ struct hashmap *ids = NULL;
+ unsigned int i;
+ int err = 0;
+
+ btf_new = btf__new_empty();
+ if (!btf_new) {
+ err = -errno;
+ goto err_out;
+ }
+
+ ids = hashmap__new(btfgen_hash_fn, btfgen_equal_fn, NULL);
+ if (IS_ERR(ids)) {
+ err = PTR_ERR(ids);
+ goto err_out;
+ }
+
+ /* first pass: add all marked types to btf_new and add their new ids to the ids map */
+ for (i = 1; i < btf__type_cnt(info->marked_btf); i++) {
+ const struct btf_type *cloned_type, *btf_type;
+ int new_id;
+
+ cloned_type = btf__type_by_id(info->marked_btf, i);
+
+ if (cloned_type->name_off != MARKED)
+ continue;
+
+ btf_type = btf__type_by_id(info->src_btf, i);
+
+ /* add members for struct and union */
+ if (btf_is_struct(btf_type) || btf_is_union(btf_type)) {
+ struct btf_type *btf_type_cpy;
+ int nmembers = 0, idx_dst, idx_src;
+ size_t new_size;
+
+ /* calculate nmembers */
+ for (idx_src = 0; idx_src < btf_vlen(cloned_type); idx_src++) {
+ struct btf_member *cloned_m = btf_members(cloned_type) + idx_src;
+
+ if (cloned_m->name_off == MARKED)
+ nmembers++;
+ }
+
+ new_size = sizeof(struct btf_type) + nmembers * sizeof(struct btf_member);
+
+ btf_type_cpy = malloc(new_size);
+ if (!btf_type_cpy)
+ goto err_out;
+
+ /* copy btf type */
+ *btf_type_cpy = *btf_type;
+
+ idx_dst = 0;
+ for (idx_src = 0; idx_src < btf_vlen(cloned_type); idx_src++) {
+ struct btf_member *btf_member_src, *btf_member_dst;
+ struct btf_member *cloned_m = btf_members(cloned_type) + idx_src;
+
+ /* copy only members that are marked as used */
+ if (cloned_m->name_off != MARKED)
+ continue;
+
+ btf_member_src = btf_members(btf_type) + idx_src;
+ btf_member_dst = btf_members(btf_type_cpy) + idx_dst;
+
+ *btf_member_dst = *btf_member_src;
+
+ idx_dst++;
+ }
+
+ /* set new vlen */
+ btf_type_cpy->info = btf_type_info(btf_kind(btf_type_cpy), nmembers,
+ btf_kflag(btf_type_cpy));
+
+ err = btf__add_type(btf_new, info->src_btf, btf_type_cpy);
+ free(btf_type_cpy);
+ } else {
+ err = btf__add_type(btf_new, info->src_btf, btf_type);
+ }
+
+ if (err < 0)
+ goto err_out;
+
+ new_id = err;
+
+ /* add ID mapping */
+ err = btfgen_add_id(ids, i, new_id);
+ if (err)
+ goto err_out;
+ }
+
+ /* second pass: fix up type ids */
+ for (i = 1; i < btf__type_cnt(btf_new); i++) {
+ struct btf_type *btf_type = (struct btf_type *) btf__type_by_id(btf_new, i);
+
+ err = btf_type_visit_type_ids(btf_type, btfgen_remap_id, ids);
+ if (err)
+ goto err_out;
+ }
+
+ hashmap__free(ids);
+ return btf_new;
+
+err_out:
+ btf__free(btf_new);
+ hashmap__free(ids);
+ errno = -err;
+ return NULL;
}
/* Create minimized BTF file for a set of BPF objects.
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH bpf-next v6 6/7] bpftool: gen min_core_btf explanation and examples
2022-02-09 22:26 [PATCH bpf-next v6 0/7] libbpf: Implement BTFGen Mauricio Vásquez
` (4 preceding siblings ...)
2022-02-09 22:26 ` [PATCH bpf-next v6 5/7] bpftool: Implement btfgen_get_btf() Mauricio Vásquez
@ 2022-02-09 22:26 ` Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 7/7] selftests/bpf: Test "bpftool gen min_core_btf" Mauricio Vásquez
6 siblings, 1 reply; 21+ messages in thread
From: Mauricio Vásquez @ 2022-02-09 22:26 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
From: Rafael David Tinoco <rafaeldtinoco@gmail.com>
Add "min_core_btf" feature explanation and one example of how to use it
to bpftool-gen man page.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
.../bpf/bpftool/Documentation/bpftool-gen.rst | 93 +++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-gen.rst b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
index bc276388f432..4e654b16dfd0 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-gen.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-gen.rst
@@ -25,6 +25,7 @@ GEN COMMANDS
| **bpftool** **gen object** *OUTPUT_FILE* *INPUT_FILE* [*INPUT_FILE*...]
| **bpftool** **gen skeleton** *FILE* [**name** *OBJECT_NAME*]
+| **bpftool** **gen min_core_btf** *INPUT* *OUTPUT* *OBJECT* [*OBJECT*...]
| **bpftool** **gen help**
DESCRIPTION
@@ -149,6 +150,26 @@ DESCRIPTION
(non-read-only) data from userspace, with same simplicity
as for BPF side.
+ **bpftool** **gen min_core_btf** *INPUT* *OUTPUT* *OBJECT* [*OBJECT*...]
+ Generate a minimum BTF file as *OUTPUT*, derived from a given
+ *INPUT* BTF file, containing all needed BTF types so one, or
+ more, given eBPF objects CO-RE relocations may be satisfied.
+
+ When kernels aren't compiled with CONFIG_DEBUG_INFO_BTF,
+ libbpf, when loading an eBPF object, has to rely in external
+ BTF files to be able to calculate CO-RE relocations.
+
+ Usually, an external BTF file is built from existing kernel
+ DWARF data using pahole. It contains all the types used by
+ its respective kernel image and, because of that, is big.
+
+ The min_core_btf feature builds smaller BTF files, customized
+ to one or multiple eBPF objects, so they can be distributed
+ together with an eBPF CO-RE based application, turning the
+ application portable to different kernel versions.
+
+ Check examples bellow for more information how to use it.
+
**bpftool gen help**
Print short help message.
@@ -215,7 +236,9 @@ This is example BPF application with two BPF programs and a mix of BPF maps
and global variables. Source code is split across two source code files.
**$ clang -target bpf -g example1.bpf.c -o example1.bpf.o**
+
**$ clang -target bpf -g example2.bpf.c -o example2.bpf.o**
+
**$ bpftool gen object example.bpf.o example1.bpf.o example2.bpf.o**
This set of commands compiles *example1.bpf.c* and *example2.bpf.c*
@@ -329,3 +352,73 @@ BPF ELF object file *example.bpf.o*.
my_static_var: 7
This is a stripped-out version of skeleton generated for above example code.
+
+min_core_btf
+------------
+
+**$ bpftool btf dump file ./5.4.0-example.btf format raw**
+
+::
+
+ [1] INT 'long unsigned int' size=8 bits_offset=0 nr_bits=64 encoding=(none)
+ [2] CONST '(anon)' type_id=1
+ [3] VOLATILE '(anon)' type_id=1
+ [4] ARRAY '(anon)' type_id=1 index_type_id=21 nr_elems=2
+ [5] PTR '(anon)' type_id=8
+ [6] CONST '(anon)' type_id=5
+ [7] INT 'char' size=1 bits_offset=0 nr_bits=8 encoding=(none)
+ [8] CONST '(anon)' type_id=7
+ [9] INT 'unsigned int' size=4 bits_offset=0 nr_bits=32 encoding=(none)
+ <long output>
+
+**$ bpftool btf dump file ./one.bpf.o format raw**
+
+::
+
+ [1] PTR '(anon)' type_id=2
+ [2] STRUCT 'trace_event_raw_sys_enter' size=64 vlen=4
+ 'ent' type_id=3 bits_offset=0
+ 'id' type_id=7 bits_offset=64
+ 'args' type_id=9 bits_offset=128
+ '__data' type_id=12 bits_offset=512
+ [3] STRUCT 'trace_entry' size=8 vlen=4
+ 'type' type_id=4 bits_offset=0
+ 'flags' type_id=5 bits_offset=16
+ 'preempt_count' type_id=5 bits_offset=24
+ <long output>
+
+**$ bpftool gen min_core_btf ./5.4.0-example.btf ./5.4.0-smaller.btf ./one.bpf.o**
+
+**$ bpftool btf dump file ./5.4.0-smaller.btf format raw**
+
+::
+
+ [1] TYPEDEF 'pid_t' type_id=6
+ [2] STRUCT 'trace_event_raw_sys_enter' size=64 vlen=1
+ 'args' type_id=4 bits_offset=128
+ [3] STRUCT 'task_struct' size=9216 vlen=2
+ 'pid' type_id=1 bits_offset=17920
+ 'real_parent' type_id=7 bits_offset=18048
+ [4] ARRAY '(anon)' type_id=5 index_type_id=8 nr_elems=6
+ [5] INT 'long unsigned int' size=8 bits_offset=0 nr_bits=64 encoding=(none)
+ [6] TYPEDEF '__kernel_pid_t' type_id=8
+ [7] PTR '(anon)' type_id=3
+ [8] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
+ <end>
+
+Now, the "5.4.0-smaller.btf" file may be used by libbpf as an external BTF file
+when loading the "one.bpf.o" object into the "5.4.0-example" kernel. Note that
+the generated BTF file won't allow other eBPF objects to be loaded, just the
+ones given to min_core_btf.
+
+::
+
+ struct bpf_object *obj = NULL;
+ struct bpf_object_open_opts openopts = {};
+
+ openopts.sz = sizeof(struct bpf_object_open_opts);
+ openopts.btf_custom_path = "./5.4.0-smaller.btf";
+
+ obj = bpf_object__open_file("./one.bpf.o", &openopts);
+
+ ...
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH bpf-next v6 7/7] selftests/bpf: Test "bpftool gen min_core_btf"
2022-02-09 22:26 [PATCH bpf-next v6 0/7] libbpf: Implement BTFGen Mauricio Vásquez
` (5 preceding siblings ...)
2022-02-09 22:26 ` [PATCH bpf-next v6 6/7] bpftool: gen min_core_btf explanation and examples Mauricio Vásquez
@ 2022-02-09 22:26 ` Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
6 siblings, 1 reply; 21+ messages in thread
From: Mauricio Vásquez @ 2022-02-09 22:26 UTC (permalink / raw)
To: netdev, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Rafael David Tinoco, Lorenzo Fontana,
Leonardo Di Donato
This commit reuses the core_reloc test to check if the BTF files
generated with "bpftool gen min_core_btf" are correct. This introduces
test_core_btfgen() that runs all the core_reloc tests, but this time
the source BTF files are generated by using "bpftool gen min_core_btf".
The goal of this test is to check that the generated files are usable,
and not to check if the algorithm is creating an optimized BTF file.
Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
---
.../selftests/bpf/prog_tests/core_reloc.c | 46 ++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index b8bdd1c3efca..10a1d5fb788e 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -2,6 +2,7 @@
#include <test_progs.h>
#include "progs/core_reloc_types.h"
#include "bpf_testmod/bpf_testmod.h"
+#include <linux/limits.h>
#include <sys/mman.h>
#include <sys/syscall.h>
#include <bpf/btf.h>
@@ -354,6 +355,8 @@ static int duration = 0;
.fails = true, \
}
+#define BTFGEN_BTF_PATH "/tmp/btfgen.btf"
+
struct core_reloc_test_case;
typedef int (*setup_test_fn)(struct core_reloc_test_case *test);
@@ -836,7 +839,21 @@ static size_t roundup_page(size_t sz)
return (sz + page_size - 1) / page_size * page_size;
}
-void test_core_reloc(void)
+static int run_btfgen(const char *src_btf, const char *dst_btf, const char *objpath)
+{
+ char command[4096];
+ int n;
+
+ n = snprintf(command, sizeof(command),
+ "./tools/build/bpftool/bpftool gen min_core_btf %s %s %s",
+ src_btf, dst_btf, objpath);
+ if (n < 0 || n >= sizeof(command))
+ return -1;
+
+ return system(command);
+}
+
+static void _test_core_reloc(bool btfgen)
{
const size_t mmap_sz = roundup_page(sizeof(struct data));
DECLARE_LIBBPF_OPTS(bpf_object_open_opts, open_opts);
@@ -863,6 +880,22 @@ void test_core_reloc(void)
continue;
}
+ /* generate a "minimal" BTF file and use it as source */
+ if (btfgen) {
+ if (!test_case->btf_src_file || test_case->fails) {
+ test__skip();
+ continue;
+ }
+
+ unlink(BTFGEN_BTF_PATH);
+ err = run_btfgen(test_case->btf_src_file, BTFGEN_BTF_PATH,
+ test_case->bpf_obj_file);
+ if (!ASSERT_OK(err, "run_btfgen"))
+ goto cleanup;
+
+ test_case->btf_src_file = BTFGEN_BTF_PATH;
+ }
+
if (test_case->setup) {
err = test_case->setup(test_case);
if (CHECK(err, "test_setup", "test #%d setup failed: %d\n", i, err))
@@ -954,8 +987,19 @@ void test_core_reloc(void)
CHECK_FAIL(munmap(mmap_data, mmap_sz));
mmap_data = NULL;
}
+ unlink(BTFGEN_BTF_PATH);
bpf_link__destroy(link);
link = NULL;
bpf_object__close(obj);
}
}
+
+void test_core_reloc(void)
+{
+ _test_core_reloc(false);
+}
+
+void test_core_btfgen(void)
+{
+ _test_core_reloc(true);
+}
--
2.25.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 1/7] libbpf: split bpf_core_apply_relo()
2022-02-09 22:26 ` [PATCH bpf-next v6 1/7] libbpf: split bpf_core_apply_relo() Mauricio Vásquez
@ 2022-02-12 0:42 ` Andrii Nakryiko
0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2022-02-12 0:42 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> BTFGen needs to run the core relocation logic in order to understand
> what are the types involved in a given relocation.
>
> Currently bpf_core_apply_relo() calculates and **applies** a relocation
> to an instruction. Having both operations in the same function makes it
> difficult to only calculate the relocation without patching the
> instruction. This commit splits that logic in two different phases: (1)
> calculate the relocation and (2) patch the instruction.
>
> For the first phase bpf_core_apply_relo() is renamed to
> bpf_core_calc_relo_insn() who is now only on charge of calculating the
> relocation, the second phase uses the already existing
> bpf_core_patch_insn(). bpf_object__relocate_core() uses both of them and
> the BTFGen will use only bpf_core_calc_relo_insn().
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
LGTM.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> kernel/bpf/btf.c | 13 +++++--
> tools/lib/bpf/libbpf.c | 71 ++++++++++++++++++++---------------
> tools/lib/bpf/relo_core.c | 79 ++++++++++++---------------------------
> tools/lib/bpf/relo_core.h | 42 ++++++++++++++++++---
> 4 files changed, 109 insertions(+), 96 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 2/7] libbpf: Expose bpf_core_{add,free}_cands() to bpftool
2022-02-09 22:26 ` [PATCH bpf-next v6 2/7] libbpf: Expose bpf_core_{add,free}_cands() to bpftool Mauricio Vásquez
@ 2022-02-12 0:42 ` Andrii Nakryiko
0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2022-02-12 0:42 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> Expose bpf_core_add_cands() and bpf_core_free_cands() to handle
> candidates list.
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> tools/lib/bpf/libbpf.c | 17 ++++++++++-------
> tools/lib/bpf/libbpf_internal.h | 9 +++++++++
> 2 files changed, 19 insertions(+), 7 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 3/7] bpftool: Add gen min_core_btf command
2022-02-09 22:26 ` [PATCH bpf-next v6 3/7] bpftool: Add gen min_core_btf command Mauricio Vásquez
@ 2022-02-12 0:42 ` Andrii Nakryiko
0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2022-02-12 0:42 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> This command is implemented under the "gen" command in bpftool and the
> syntax is the following:
>
> $ bpftool gen min_core_btf INPUT OUTPUT OBJECT [OBJECT...]
>
> INPUT is the file that contains all the BTF types for a kernel and
> OUTPUT is the path of the minimize BTF file that will be created with
> only the types needed by the objects.
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
LGTM. So much simpler interface.
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> tools/bpf/bpftool/bash-completion/bpftool | 6 +++-
> tools/bpf/bpftool/gen.c | 42 +++++++++++++++++++++--
> 2 files changed, 44 insertions(+), 4 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 4/7] bpftool: Implement minimize_btf() and relocations recording for BTFGen
2022-02-09 22:26 ` [PATCH bpf-next v6 4/7] bpftool: Implement minimize_btf() and relocations recording for BTFGen Mauricio Vásquez
@ 2022-02-12 0:42 ` Andrii Nakryiko
2022-02-15 22:56 ` Mauricio Vásquez Bernal
0 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2022-02-12 0:42 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
It would be good to shorten the subject line, it's very long.
> minimize_btf() receives the path of a source and destination BTF files
> and a list of BPF objects. This function records the relocations for
> all objects and then generates the BTF file by calling btfgen_get_btf()
> (implemented in the following commit).
>
> btfgen_record_obj() loads the BTF and BTF.ext sections of the BPF
> objects and loops through all CO-RE relocations. It uses
> bpf_core_calc_relo_insn() from libbpf and passes the target spec to
> btfgen_record_reloc(), that calls one of the following functions
> depending on the relocation kind.
>
> btfgen_record_field_relo() uses the target specification to mark all the
> types that are involved in a field-based CO-RE relocation. In this case
> types resolved and marked recursively using btfgen_mark_type().
> Only the struct and union members (and their types) involved in the
> relocation are marked to optimize the size of the generated BTF file.
>
> btfgen_record_type_relo() marks the types involved in a type-based
> CO-RE relocation. In this case no members for the struct and union
> types are marked as libbpf doesn't use them while performing this kind
> of relocation. Pointed types are marked as they are used by libbpf in
> this case.
>
> btfgen_record_enumval_relo() marks the whole enum type for enum-based
> relocations.
It should be enough to leave only used enumerators, but I suppose it
doesn't take much space to record all. We can adjust that later, if
necessary.
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
> tools/bpf/bpftool/Makefile | 8 +-
> tools/bpf/bpftool/gen.c | 452 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 454 insertions(+), 6 deletions(-)
>
Looks good, few nits and concerns, but it feels like it's really close
to being ready.
[...]
> +}
> +
> +struct btfgen_info {
> + struct btf *src_btf;
> + struct btf *marked_btf; // btf structure used to mark used types
C++ comment, please use /* */
> +};
> +
> +static size_t btfgen_hash_fn(const void *key, void *ctx)
> +{
> + return (size_t)key;
> +}
> +
> +static bool btfgen_equal_fn(const void *k1, const void *k2, void *ctx)
> +{
> + return k1 == k2;
> +}
> +
> +static void *uint_as_hash_key(int x)
> +{
> + return (void *)(uintptr_t)x;
> +}
> +
> +static void *u32_as_hash_key(__u32 x)
> +{
> + return (void *)(uintptr_t)x;
> +}
> +
> +static void btfgen_free_info(struct btfgen_info *info)
> +{
> + if (!info)
> + return;
> +
> + btf__free(info->src_btf);
> + btf__free(info->marked_btf);
> +
> + free(info);
> +}
> +
> +static struct btfgen_info *
> +btfgen_new_info(const char *targ_btf_path)
> +{
> + struct btfgen_info *info;
> + int err;
> +
> + info = calloc(1, sizeof(*info));
> + if (!info)
> + return NULL;
> +
> + info->src_btf = btf__parse(targ_btf_path, NULL);
> + if (!info->src_btf) {
> + p_err("failed parsing '%s' BTF file: %s", targ_btf_path, strerror(errno));
> + err = -errno;
save errno before p_err, it can clobber errno otherwise
> + goto err_out;
> + }
> +
> + info->marked_btf = btf__parse(targ_btf_path, NULL);
> + if (!info->marked_btf) {
> + p_err("failed parsing '%s' BTF file: %s", targ_btf_path, strerror(errno));
> + err = -errno;
same, always save errno first before any non-trivial function/macro call
> + goto err_out;
> + }
> +
> + return info;
> +
> +err_out:
> + btfgen_free_info(info);
> + errno = -err;
> + return NULL;
> +}
> +
> +#define MARKED UINT32_MAX
> +
> +static void btfgen_mark_member(struct btfgen_info *info, int type_id, int idx)
> +{
> + const struct btf_type *t = btf__type_by_id(info->marked_btf, type_id);
> + struct btf_member *m = btf_members(t) + idx;
> +
> + m->name_off = MARKED;
> +}
> +
> +static int
> +btfgen_mark_type(struct btfgen_info *info, unsigned int id, bool follow_pointers)
id is type_id or could be some other id? It's best to be consistent in
naming to avoid second guessing like in this case.
> +{
> + const struct btf_type *btf_type = btf__type_by_id(info->src_btf, id);
> + struct btf_type *cloned_type;
> + struct btf_param *param;
> + struct btf_array *array;
> + int err, i;
[...]
> + /* tells if some other type needs to be handled */
> + default:
> + p_err("unsupported kind: %s (%d)", btf_kind_str(btf_type), id);
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int btfgen_record_field_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
> +{
> + struct btf *btf = (struct btf *) info->src_btf;
why the cast?
> + const struct btf_type *btf_type;
> + struct btf_member *btf_member;
> + struct btf_array *array;
> + unsigned int id = targ_spec->root_type_id;
> + int idx, err;
> +
> + /* mark root type */
> + btf_type = btf__type_by_id(btf, id);
> + err = btfgen_mark_type(info, id, false);
> + if (err)
> + return err;
> +
> + /* mark types for complex types (arrays, unions, structures) */
> + for (int i = 1; i < targ_spec->raw_len; i++) {
> + /* skip typedefs and mods */
> + while (btf_is_mod(btf_type) || btf_is_typedef(btf_type)) {
> + id = btf_type->type;
> + btf_type = btf__type_by_id(btf, id);
> + }
> +
> + switch (btf_kind(btf_type)) {
> + case BTF_KIND_STRUCT:
> + case BTF_KIND_UNION:
> + idx = targ_spec->raw_spec[i];
> + btf_member = btf_members(btf_type) + idx;
> +
> + /* mark member */
> + btfgen_mark_member(info, id, idx);
> +
> + /* mark member's type */
> + id = btf_member->type;
> + btf_type = btf__type_by_id(btf, id);
> + err = btfgen_mark_type(info, id, false);
why would it not follow the pointer? E.g., if I have a field defined as
struct blah ***my_field;
You at the very least would need either an empty struct blah or FWD
for struct blah, no?
> + if (err)
> + return err;
> + break;
> + case BTF_KIND_ARRAY:
> + array = btf_array(btf_type);
> + id = array->type;
> + btf_type = btf__type_by_id(btf, id);
> + break;
[...]
> +err_out:
> + bpf_core_free_cands(cands);
> + errno = -err;
> + return NULL;
> +}
> +
> +/* Record relocation information for a single BPF object*/
nit: missing space before */
> +static int btfgen_record_obj(struct btfgen_info *info, const char *obj_path)
> +{
> + const struct btf_ext_info_sec *sec;
> + const struct bpf_core_relo *relo;
> + const struct btf_ext_info *seg;
> + struct hashmap_entry *entry;
> + struct hashmap *cand_cache = NULL;
> + struct btf_ext *btf_ext = NULL;
> + unsigned int relo_idx;
> + struct btf *btf = NULL;
> + size_t i;
> + int err;
> +
> + btf = btf__parse(obj_path, &btf_ext);
> + if (!btf) {
> + p_err("failed to parse BPF object '%s': %s", obj_path, strerror(errno));
> + return -errno;
> + }
check that btf_ext is not NULL?
> +
> + if (btf_ext->core_relo_info.len == 0) {
> + err = 0;
> + goto out;
> + }
> +
[...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 5/7] bpftool: Implement btfgen_get_btf()
2022-02-09 22:26 ` [PATCH bpf-next v6 5/7] bpftool: Implement btfgen_get_btf() Mauricio Vásquez
@ 2022-02-12 0:42 ` Andrii Nakryiko
2022-02-15 22:56 ` Mauricio Vásquez Bernal
0 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2022-02-12 0:42 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> The last part of the BTFGen algorithm is to create a new BTF object with
> all the types that were recorded in the previous steps.
>
> This function performs two different steps:
> 1. Add the types to the new BTF object by using btf__add_type(). Some
> special logic around struct and unions is implemented to only add the
> members that are really used in the field-based relocations. The type
> ID on the new and old BTF objects is stored on a map.
> 2. Fix all the type IDs on the new BTF object by using the IDs saved in
> the previous step.
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
> tools/bpf/bpftool/gen.c | 136 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 135 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index c3e34db2ec8a..1efc7f3c64b2 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -1481,10 +1481,144 @@ static int btfgen_record_obj(struct btfgen_info *info, const char *obj_path)
> return err;
> }
>
> +static unsigned int btfgen_get_id(struct hashmap *ids, unsigned int old)
> +{
> + uintptr_t new;
> +
> + if (!hashmap__find(ids, uint_as_hash_key(old), (void **)&new))
> + /* return id for BTF_KIND_VOID as it's possible that the
> + * ID we're looking for is the type of a pointer that
> + * we're not adding.
> + */
> + return 0;
> +
> + return (unsigned int)(uintptr_t)new;
> +}
> +
> +static int btfgen_add_id(struct hashmap *ids, unsigned int old, unsigned int new)
> +{
> + return hashmap__add(ids, uint_as_hash_key(old), uint_as_hash_key(new));
> +}
> +
> +static int btfgen_remap_id(__u32 *type_id, void *ctx)
> +{
> + struct hashmap *ids = ctx;
> +
> + *type_id = btfgen_get_id(ids, *type_id);
> +
> + return 0;
> +}
> +
> /* Generate BTF from relocation information previously recorded */
> static struct btf *btfgen_get_btf(struct btfgen_info *info)
> {
> - return ERR_PTR(-EOPNOTSUPP);
> + struct btf *btf_new = NULL;
> + struct hashmap *ids = NULL;
> + unsigned int i;
> + int err = 0;
> +
> + btf_new = btf__new_empty();
> + if (!btf_new) {
> + err = -errno;
> + goto err_out;
> + }
> +
> + ids = hashmap__new(btfgen_hash_fn, btfgen_equal_fn, NULL);
> + if (IS_ERR(ids)) {
> + err = PTR_ERR(ids);
> + goto err_out;
> + }
> +
> + /* first pass: add all marked types to btf_new and add their new ids to the ids map */
> + for (i = 1; i < btf__type_cnt(info->marked_btf); i++) {
small nit: why keep calling btf__type_cnt() on each iteration? store
it as n = btf__type_cnt(...) and do i < n ?
> + const struct btf_type *cloned_type, *btf_type;
> + int new_id;
> +
> + cloned_type = btf__type_by_id(info->marked_btf, i);
> +
> + if (cloned_type->name_off != MARKED)
> + continue;
see, if you did
#define MARKED (1<<31)
and did
t->name_off |= MARKED
everywhere, then you wouldn't need src_btf anymore, as you'd just
restore original name_off right here with t->name_off &= ~MARKED.
But it's fine, just wanted to point out why I wanted to use one bit,
so that original values are still available.
> +
> + btf_type = btf__type_by_id(info->src_btf, i);
> +
> + /* add members for struct and union */
> + if (btf_is_struct(btf_type) || btf_is_union(btf_type)) {
btf_is_composite(btf_type)
> + struct btf_type *btf_type_cpy;
> + int nmembers = 0, idx_dst, idx_src;
> + size_t new_size;
> +
> + /* calculate nmembers */
> + for (idx_src = 0; idx_src < btf_vlen(cloned_type); idx_src++) {
> + struct btf_member *cloned_m = btf_members(cloned_type) + idx_src;
a bit nicer pattern is:
struct btf_member *m = btf_members(cloned_type);
int vlen = btf_vlen(cloned_type)
for (i = 0; i < vlen; i++, m++) {
}
That way you don't have to re-calculate member
> +
> + if (cloned_m->name_off == MARKED)
> + nmembers++;
> + }
> +
> + new_size = sizeof(struct btf_type) + nmembers * sizeof(struct btf_member);
> +
> + btf_type_cpy = malloc(new_size);
> + if (!btf_type_cpy)
> + goto err_out;
> +
> + /* copy btf type */
> + *btf_type_cpy = *btf_type;
> +
> + idx_dst = 0;
> + for (idx_src = 0; idx_src < btf_vlen(cloned_type); idx_src++) {
> + struct btf_member *btf_member_src, *btf_member_dst;
> + struct btf_member *cloned_m = btf_members(cloned_type) + idx_src;
> +
> + /* copy only members that are marked as used */
> + if (cloned_m->name_off != MARKED)
> + continue;
> +
> + btf_member_src = btf_members(btf_type) + idx_src;
> + btf_member_dst = btf_members(btf_type_cpy) + idx_dst;
> +
> + *btf_member_dst = *btf_member_src;
> +
> + idx_dst++;
> + }
> +
> + /* set new vlen */
> + btf_type_cpy->info = btf_type_info(btf_kind(btf_type_cpy), nmembers,
> + btf_kflag(btf_type_cpy));
> +
> + err = btf__add_type(btf_new, info->src_btf, btf_type_cpy);
> + free(btf_type_cpy);
hmm.. this malloc and the rest still feels clunky... why not do it
explicitly with btf__add_struct()/btf__add_union() and then
btf__add_field() for each marked field? You also won't need to
pre-calculate the number of members (libbpf will adjust number of
members automatically, it's pretty nice API, try it).
You can also use err = err ?: btf__add_xxx() pattern to minimize error
handling conditionals
> + } else {
> + err = btf__add_type(btf_new, info->src_btf, btf_type);
> + }
> +
> + if (err < 0)
> + goto err_out;
> +
> + new_id = err;
> +
> + /* add ID mapping */
> + err = btfgen_add_id(ids, i, new_id);
Why using clunky hashmap API if we are talking about mapping
sequential integers? Just allocate an array of btf__type_cnt()
integers and use that as a straightforward map?
> + if (err)
> + goto err_out;
> + }
> +
> + /* second pass: fix up type ids */
> + for (i = 1; i < btf__type_cnt(btf_new); i++) {
> + struct btf_type *btf_type = (struct btf_type *) btf__type_by_id(btf_new, i);
> +
> + err = btf_type_visit_type_ids(btf_type, btfgen_remap_id, ids);
> + if (err)
> + goto err_out;
> + }
> +
> + hashmap__free(ids);
> + return btf_new;
> +
> +err_out:
> + btf__free(btf_new);
> + hashmap__free(ids);
> + errno = -err;
> + return NULL;
> }
>
> /* Create minimized BTF file for a set of BPF objects.
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 6/7] bpftool: gen min_core_btf explanation and examples
2022-02-09 22:26 ` [PATCH bpf-next v6 6/7] bpftool: gen min_core_btf explanation and examples Mauricio Vásquez
@ 2022-02-12 0:42 ` Andrii Nakryiko
2022-02-15 22:56 ` Mauricio Vásquez Bernal
0 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2022-02-12 0:42 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> From: Rafael David Tinoco <rafaeldtinoco@gmail.com>
>
> Add "min_core_btf" feature explanation and one example of how to use it
> to bpftool-gen man page.
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
> .../bpf/bpftool/Documentation/bpftool-gen.rst | 93 +++++++++++++++++++
> 1 file changed, 93 insertions(+)
>
[...]
> +Now, the "5.4.0-smaller.btf" file may be used by libbpf as an external BTF file
> +when loading the "one.bpf.o" object into the "5.4.0-example" kernel. Note that
> +the generated BTF file won't allow other eBPF objects to be loaded, just the
> +ones given to min_core_btf.
> +
> +::
> +
> + struct bpf_object *obj = NULL;
> + struct bpf_object_open_opts openopts = {};
> +
> + openopts.sz = sizeof(struct bpf_object_open_opts);
> + openopts.btf_custom_path = "./5.4.0-smaller.btf";
> +
> + obj = bpf_object__open_file("./one.bpf.o", &openopts);
Can you please use LIBBPF_OPTS() macro in the example, that's how
users are normally expected to use OPTS-based APIs anyways. Also there
is no need for "./" when specifying file location. This is a different
case than running a binary in the shell, where binary is searched in
PATH. This is never done when opening files.
So all this should be:
LIBBPF_OPTS(bpf_object_open_opts, opts, .btf_custom_path = "5.4.0-smaller.btf");
struct bpf_object *obj;
obj = bpf_object__open_file("one.bpf.o", &opts);
That's all.
> +
> + ...
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 7/7] selftests/bpf: Test "bpftool gen min_core_btf"
2022-02-09 22:26 ` [PATCH bpf-next v6 7/7] selftests/bpf: Test "bpftool gen min_core_btf" Mauricio Vásquez
@ 2022-02-12 0:42 ` Andrii Nakryiko
2022-02-15 22:56 ` Mauricio Vásquez Bernal
0 siblings, 1 reply; 21+ messages in thread
From: Andrii Nakryiko @ 2022-02-12 0:42 UTC (permalink / raw)
To: Mauricio Vásquez
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> This commit reuses the core_reloc test to check if the BTF files
> generated with "bpftool gen min_core_btf" are correct. This introduces
> test_core_btfgen() that runs all the core_reloc tests, but this time
> the source BTF files are generated by using "bpftool gen min_core_btf".
>
> The goal of this test is to check that the generated files are usable,
> and not to check if the algorithm is creating an optimized BTF file.
>
> Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> ---
> .../selftests/bpf/prog_tests/core_reloc.c | 46 ++++++++++++++++++-
> 1 file changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> index b8bdd1c3efca..10a1d5fb788e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> @@ -2,6 +2,7 @@
> #include <test_progs.h>
> #include "progs/core_reloc_types.h"
> #include "bpf_testmod/bpf_testmod.h"
> +#include <linux/limits.h>
> #include <sys/mman.h>
> #include <sys/syscall.h>
> #include <bpf/btf.h>
> @@ -354,6 +355,8 @@ static int duration = 0;
> .fails = true, \
> }
>
> +#define BTFGEN_BTF_PATH "/tmp/btfgen.btf"
let's not use fixed file path, better use mkstemp() to create
temporary file (see core_autosize.c for an example)
> +
> struct core_reloc_test_case;
>
> typedef int (*setup_test_fn)(struct core_reloc_test_case *test);
> @@ -836,7 +839,21 @@ static size_t roundup_page(size_t sz)
> return (sz + page_size - 1) / page_size * page_size;
> }
>
> -void test_core_reloc(void)
> +static int run_btfgen(const char *src_btf, const char *dst_btf, const char *objpath)
> +{
> + char command[4096];
> + int n;
> +
> + n = snprintf(command, sizeof(command),
> + "./tools/build/bpftool/bpftool gen min_core_btf %s %s %s",
> + src_btf, dst_btf, objpath);
> + if (n < 0 || n >= sizeof(command))
> + return -1;
> +
> + return system(command);
> +}
> +
> +static void _test_core_reloc(bool btfgen)
ugh, please don't do leading underscore. Call it
"run_core_reloc_tests" or whatever, it's easy.
another naming nit (I did get confused for a second because of this):
use_btfgen, otherwise below in the code my first guess was that
"btfgen" is actually a path to bpftool or something like that
> {
> const size_t mmap_sz = roundup_page(sizeof(struct data));
> DECLARE_LIBBPF_OPTS(bpf_object_open_opts, open_opts);
> @@ -863,6 +880,22 @@ void test_core_reloc(void)
> continue;
> }
>
> + /* generate a "minimal" BTF file and use it as source */
> + if (btfgen) {
> + if (!test_case->btf_src_file || test_case->fails) {
> + test__skip();
> + continue;
> + }
> +
> + unlink(BTFGEN_BTF_PATH);
> + err = run_btfgen(test_case->btf_src_file, BTFGEN_BTF_PATH,
> + test_case->bpf_obj_file);
> + if (!ASSERT_OK(err, "run_btfgen"))
> + goto cleanup;
> +
> + test_case->btf_src_file = BTFGEN_BTF_PATH;
> + }
> +
> if (test_case->setup) {
> err = test_case->setup(test_case);
> if (CHECK(err, "test_setup", "test #%d setup failed: %d\n", i, err))
> @@ -954,8 +987,19 @@ void test_core_reloc(void)
> CHECK_FAIL(munmap(mmap_data, mmap_sz));
> mmap_data = NULL;
> }
> + unlink(BTFGEN_BTF_PATH);
> bpf_link__destroy(link);
> link = NULL;
> bpf_object__close(obj);
> }
> }
> +
> +void test_core_reloc(void)
> +{
> + _test_core_reloc(false);
> +}
> +
> +void test_core_btfgen(void)
> +{
> + _test_core_reloc(true);
> +}
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 4/7] bpftool: Implement minimize_btf() and relocations recording for BTFGen
2022-02-12 0:42 ` Andrii Nakryiko
@ 2022-02-15 22:56 ` Mauricio Vásquez Bernal
2022-02-16 1:23 ` Andrii Nakryiko
0 siblings, 1 reply; 21+ messages in thread
From: Mauricio Vásquez Bernal @ 2022-02-15 22:56 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Fri, Feb 11, 2022 at 7:42 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
>
> It would be good to shorten the subject line, it's very long.
>
Will do.
> > minimize_btf() receives the path of a source and destination BTF files
> > and a list of BPF objects. This function records the relocations for
> > all objects and then generates the BTF file by calling btfgen_get_btf()
> > (implemented in the following commit).
> >
> > btfgen_record_obj() loads the BTF and BTF.ext sections of the BPF
> > objects and loops through all CO-RE relocations. It uses
> > bpf_core_calc_relo_insn() from libbpf and passes the target spec to
> > btfgen_record_reloc(), that calls one of the following functions
> > depending on the relocation kind.
> >
> > btfgen_record_field_relo() uses the target specification to mark all the
> > types that are involved in a field-based CO-RE relocation. In this case
> > types resolved and marked recursively using btfgen_mark_type().
> > Only the struct and union members (and their types) involved in the
> > relocation are marked to optimize the size of the generated BTF file.
> >
> > btfgen_record_type_relo() marks the types involved in a type-based
> > CO-RE relocation. In this case no members for the struct and union
> > types are marked as libbpf doesn't use them while performing this kind
> > of relocation. Pointed types are marked as they are used by libbpf in
> > this case.
> >
> > btfgen_record_enumval_relo() marks the whole enum type for enum-based
> > relocations.
>
> It should be enough to leave only used enumerators, but I suppose it
> doesn't take much space to record all. We can adjust that later, if
> necessary.
>
I think the overhead is really minimal and we can improve later on if we want.
> >
> > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > ---
> > tools/bpf/bpftool/Makefile | 8 +-
> > tools/bpf/bpftool/gen.c | 452 ++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 454 insertions(+), 6 deletions(-)
> >
>
> Looks good, few nits and concerns, but it feels like it's really close
> to being ready.
>
> [...]
>
> > +}
> > +
> > +struct btfgen_info {
> > + struct btf *src_btf;
> > + struct btf *marked_btf; // btf structure used to mark used types
>
> C++ comment, please use /* */
>
> > +};
> > +
> > +static size_t btfgen_hash_fn(const void *key, void *ctx)
> > +{
> > + return (size_t)key;
> > +}
> > +
> > +static bool btfgen_equal_fn(const void *k1, const void *k2, void *ctx)
> > +{
> > + return k1 == k2;
> > +}
> > +
> > +static void *uint_as_hash_key(int x)
> > +{
> > + return (void *)(uintptr_t)x;
> > +}
> > +
> > +static void *u32_as_hash_key(__u32 x)
> > +{
> > + return (void *)(uintptr_t)x;
> > +}
> > +
> > +static void btfgen_free_info(struct btfgen_info *info)
> > +{
> > + if (!info)
> > + return;
> > +
> > + btf__free(info->src_btf);
> > + btf__free(info->marked_btf);
> > +
> > + free(info);
> > +}
> > +
> > +static struct btfgen_info *
> > +btfgen_new_info(const char *targ_btf_path)
> > +{
> > + struct btfgen_info *info;
> > + int err;
> > +
> > + info = calloc(1, sizeof(*info));
> > + if (!info)
> > + return NULL;
> > +
> > + info->src_btf = btf__parse(targ_btf_path, NULL);
> > + if (!info->src_btf) {
> > + p_err("failed parsing '%s' BTF file: %s", targ_btf_path, strerror(errno));
> > + err = -errno;
>
> save errno before p_err, it can clobber errno otherwise
>
> > + goto err_out;
> > + }
> > +
> > + info->marked_btf = btf__parse(targ_btf_path, NULL);
> > + if (!info->marked_btf) {
> > + p_err("failed parsing '%s' BTF file: %s", targ_btf_path, strerror(errno));
> > + err = -errno;
>
> same, always save errno first before any non-trivial function/macro call
>
oh right, thanks!
>
> > + goto err_out;
> > + }
> > +
> > + return info;
> > +
> > +err_out:
> > + btfgen_free_info(info);
> > + errno = -err;
> > + return NULL;
> > +}
> > +
> > +#define MARKED UINT32_MAX
> > +
> > +static void btfgen_mark_member(struct btfgen_info *info, int type_id, int idx)
> > +{
> > + const struct btf_type *t = btf__type_by_id(info->marked_btf, type_id);
> > + struct btf_member *m = btf_members(t) + idx;
> > +
> > + m->name_off = MARKED;
> > +}
> > +
> > +static int
> > +btfgen_mark_type(struct btfgen_info *info, unsigned int id, bool follow_pointers)
>
> id is type_id or could be some other id? It's best to be consistent in
> naming to avoid second guessing like in this case.
It's always type_id. Renamed it.
>
> > +{
> > + const struct btf_type *btf_type = btf__type_by_id(info->src_btf, id);
> > + struct btf_type *cloned_type;
> > + struct btf_param *param;
> > + struct btf_array *array;
> > + int err, i;
>
> [...]
>
> > + /* tells if some other type needs to be handled */
> > + default:
> > + p_err("unsupported kind: %s (%d)", btf_kind_str(btf_type), id);
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int btfgen_record_field_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
> > +{
> > + struct btf *btf = (struct btf *) info->src_btf;
>
> why the cast?
>
No reason. Will remove it.
> > + const struct btf_type *btf_type;
> > + struct btf_member *btf_member;
> > + struct btf_array *array;
> > + unsigned int id = targ_spec->root_type_id;
> > + int idx, err;
> > +
> > + /* mark root type */
> > + btf_type = btf__type_by_id(btf, id);
> > + err = btfgen_mark_type(info, id, false);
> > + if (err)
> > + return err;
> > +
> > + /* mark types for complex types (arrays, unions, structures) */
> > + for (int i = 1; i < targ_spec->raw_len; i++) {
> > + /* skip typedefs and mods */
> > + while (btf_is_mod(btf_type) || btf_is_typedef(btf_type)) {
> > + id = btf_type->type;
> > + btf_type = btf__type_by_id(btf, id);
> > + }
> > +
> > + switch (btf_kind(btf_type)) {
> > + case BTF_KIND_STRUCT:
> > + case BTF_KIND_UNION:
> > + idx = targ_spec->raw_spec[i];
> > + btf_member = btf_members(btf_type) + idx;
> > +
> > + /* mark member */
> > + btfgen_mark_member(info, id, idx);
> > +
> > + /* mark member's type */
> > + id = btf_member->type;
> > + btf_type = btf__type_by_id(btf, id);
> > + err = btfgen_mark_type(info, id, false);
>
> why would it not follow the pointer? E.g., if I have a field defined as
>
> struct blah ***my_field;
>
> You at the very least would need either an empty struct blah or FWD
> for struct blah, no?
>
It's an optimization we do, we don't follow the pointer here because
it is possible that the definition of the pointed type is not needed.
For instance, a relocation like:
BPF_CORE_READ(task, nsproxy);
will generate this:
[1] STRUCT 'task_struct' size=9472 vlen=1
'nsproxy' type_id=2 bits_offset=23040
[2] PTR '(anon)' type_id=0
struct nsproxy is not really accessed, so we don't need it's
definition. On the other hand, something like
BPF_CORE_READ(task, nsproxy, count);
has two relocations, and nsproxy is actually accessed, so in this case
the generated BTF includes a nsproxy struct:
[1] STRUCT '(anon)' size=4 vlen=0
[2] TYPEDEF 'atomic_t' type_id=1
[3] STRUCT 'task_struct' size=9472 vlen=1
'nsproxy' type_id=4 bits_offset=23040
[4] PTR '(anon)' type_id=5
[5] STRUCT 'nsproxy' size=72 vlen=1
'count' type_id=2 bits_offset=0
> > + if (err)
> > + return err;
> > + break;
> > + case BTF_KIND_ARRAY:
> > + array = btf_array(btf_type);
> > + id = array->type;
> > + btf_type = btf__type_by_id(btf, id);
> > + break;
>
> [...]
>
> > +err_out:
> > + bpf_core_free_cands(cands);
> > + errno = -err;
> > + return NULL;
> > +}
> > +
> > +/* Record relocation information for a single BPF object*/
>
> nit: missing space before */
>
> > +static int btfgen_record_obj(struct btfgen_info *info, const char *obj_path)
> > +{
> > + const struct btf_ext_info_sec *sec;
> > + const struct bpf_core_relo *relo;
> > + const struct btf_ext_info *seg;
> > + struct hashmap_entry *entry;
> > + struct hashmap *cand_cache = NULL;
> > + struct btf_ext *btf_ext = NULL;
> > + unsigned int relo_idx;
> > + struct btf *btf = NULL;
> > + size_t i;
> > + int err;
> > +
> > + btf = btf__parse(obj_path, &btf_ext);
> > + if (!btf) {
> > + p_err("failed to parse BPF object '%s': %s", obj_path, strerror(errno));
> > + return -errno;
> > + }
>
> check that btf_ext is not NULL?
>
Done.
> > +
> > + if (btf_ext->core_relo_info.len == 0) {
> > + err = 0;
> > + goto out;
> > + }
> > +
>
> [...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 5/7] bpftool: Implement btfgen_get_btf()
2022-02-12 0:42 ` Andrii Nakryiko
@ 2022-02-15 22:56 ` Mauricio Vásquez Bernal
0 siblings, 0 replies; 21+ messages in thread
From: Mauricio Vásquez Bernal @ 2022-02-15 22:56 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Fri, Feb 11, 2022 at 7:42 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
> > The last part of the BTFGen algorithm is to create a new BTF object with
> > all the types that were recorded in the previous steps.
> >
> > This function performs two different steps:
> > 1. Add the types to the new BTF object by using btf__add_type(). Some
> > special logic around struct and unions is implemented to only add the
> > members that are really used in the field-based relocations. The type
> > ID on the new and old BTF objects is stored on a map.
> > 2. Fix all the type IDs on the new BTF object by using the IDs saved in
> > the previous step.
> >
> > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > ---
> > tools/bpf/bpftool/gen.c | 136 +++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 135 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> > index c3e34db2ec8a..1efc7f3c64b2 100644
> > --- a/tools/bpf/bpftool/gen.c
> > +++ b/tools/bpf/bpftool/gen.c
> > @@ -1481,10 +1481,144 @@ static int btfgen_record_obj(struct btfgen_info *info, const char *obj_path)
> > return err;
> > }
> >
> > +static unsigned int btfgen_get_id(struct hashmap *ids, unsigned int old)
> > +{
> > + uintptr_t new;
> > +
> > + if (!hashmap__find(ids, uint_as_hash_key(old), (void **)&new))
> > + /* return id for BTF_KIND_VOID as it's possible that the
> > + * ID we're looking for is the type of a pointer that
> > + * we're not adding.
> > + */
> > + return 0;
> > +
> > + return (unsigned int)(uintptr_t)new;
> > +}
> > +
> > +static int btfgen_add_id(struct hashmap *ids, unsigned int old, unsigned int new)
> > +{
> > + return hashmap__add(ids, uint_as_hash_key(old), uint_as_hash_key(new));
> > +}
> > +
> > +static int btfgen_remap_id(__u32 *type_id, void *ctx)
> > +{
> > + struct hashmap *ids = ctx;
> > +
> > + *type_id = btfgen_get_id(ids, *type_id);
> > +
> > + return 0;
> > +}
> > +
> > /* Generate BTF from relocation information previously recorded */
> > static struct btf *btfgen_get_btf(struct btfgen_info *info)
> > {
> > - return ERR_PTR(-EOPNOTSUPP);
> > + struct btf *btf_new = NULL;
> > + struct hashmap *ids = NULL;
> > + unsigned int i;
> > + int err = 0;
> > +
> > + btf_new = btf__new_empty();
> > + if (!btf_new) {
> > + err = -errno;
> > + goto err_out;
> > + }
> > +
> > + ids = hashmap__new(btfgen_hash_fn, btfgen_equal_fn, NULL);
> > + if (IS_ERR(ids)) {
> > + err = PTR_ERR(ids);
> > + goto err_out;
> > + }
> > +
> > + /* first pass: add all marked types to btf_new and add their new ids to the ids map */
> > + for (i = 1; i < btf__type_cnt(info->marked_btf); i++) {
>
> small nit: why keep calling btf__type_cnt() on each iteration? store
> it as n = btf__type_cnt(...) and do i < n ?
Fixed
>
> > + const struct btf_type *cloned_type, *btf_type;
> > + int new_id;
> > +
> > + cloned_type = btf__type_by_id(info->marked_btf, i);
> > +
> > + if (cloned_type->name_off != MARKED)
> > + continue;
>
> see, if you did
>
> #define MARKED (1<<31)
>
> and did
>
> t->name_off |= MARKED
>
> everywhere, then you wouldn't need src_btf anymore, as you'd just
> restore original name_off right here with t->name_off &= ~MARKED.
>
> But it's fine, just wanted to point out why I wanted to use one bit,
> so that original values are still available.
I see, thanks for the explanation. In both cases a BTF copy to pass to
libbpf is needed, hence I'd say there's not that much difference.
>
> > +
> > + btf_type = btf__type_by_id(info->src_btf, i);
> > +
> > + /* add members for struct and union */
> > + if (btf_is_struct(btf_type) || btf_is_union(btf_type)) {
>
> btf_is_composite(btf_type)
>
> > + struct btf_type *btf_type_cpy;
> > + int nmembers = 0, idx_dst, idx_src;
> > + size_t new_size;
> > +
> > + /* calculate nmembers */
> > + for (idx_src = 0; idx_src < btf_vlen(cloned_type); idx_src++) {
> > + struct btf_member *cloned_m = btf_members(cloned_type) + idx_src;
>
> a bit nicer pattern is:
>
>
> struct btf_member *m = btf_members(cloned_type);
> int vlen = btf_vlen(cloned_type)
>
> for (i = 0; i < vlen; i++, m++) {
> }
>
> That way you don't have to re-calculate member
>
Reworked the code with the other suggestions below.
> > +
> > + if (cloned_m->name_off == MARKED)
> > + nmembers++;
> > + }
> > +
> > + new_size = sizeof(struct btf_type) + nmembers * sizeof(struct btf_member);
> > +
> > + btf_type_cpy = malloc(new_size);
> > + if (!btf_type_cpy)
> > + goto err_out;
> > +
> > + /* copy btf type */
> > + *btf_type_cpy = *btf_type;
> > +
> > + idx_dst = 0;
> > + for (idx_src = 0; idx_src < btf_vlen(cloned_type); idx_src++) {
> > + struct btf_member *btf_member_src, *btf_member_dst;
> > + struct btf_member *cloned_m = btf_members(cloned_type) + idx_src;
> > +
> > + /* copy only members that are marked as used */
> > + if (cloned_m->name_off != MARKED)
> > + continue;
> > +
> > + btf_member_src = btf_members(btf_type) + idx_src;
> > + btf_member_dst = btf_members(btf_type_cpy) + idx_dst;
> > +
> > + *btf_member_dst = *btf_member_src;
> > +
> > + idx_dst++;
> > + }
> > +
> > + /* set new vlen */
> > + btf_type_cpy->info = btf_type_info(btf_kind(btf_type_cpy), nmembers,
> > + btf_kflag(btf_type_cpy));
> > +
> > + err = btf__add_type(btf_new, info->src_btf, btf_type_cpy);
> > + free(btf_type_cpy);
>
> hmm.. this malloc and the rest still feels clunky... why not do it
> explicitly with btf__add_struct()/btf__add_union() and then
> btf__add_field() for each marked field? You also won't need to
> pre-calculate the number of members (libbpf will adjust number of
> members automatically, it's pretty nice API, try it).
>
You're right. Code looks better with this API.
> You can also use err = err ?: btf__add_xxx() pattern to minimize error
> handling conditionals
>
mmm, I didn't find a place where it could improve the code in this case.
>
>
> > + } else {
> > + err = btf__add_type(btf_new, info->src_btf, btf_type);
> > + }
> > +
> > + if (err < 0)
> > + goto err_out;
> > +
> > + new_id = err;
> > +
> > + /* add ID mapping */
> > + err = btfgen_add_id(ids, i, new_id);
>
> Why using clunky hashmap API if we are talking about mapping
> sequential integers? Just allocate an array of btf__type_cnt()
> integers and use that as a straightforward map?
>
Makes sense. Probably a hashmap will use a bit less memory but I think
the readability improvement is worth it.
>
> > + if (err)
> > + goto err_out;
> > + }
> > +
> > + /* second pass: fix up type ids */
> > + for (i = 1; i < btf__type_cnt(btf_new); i++) {
> > + struct btf_type *btf_type = (struct btf_type *) btf__type_by_id(btf_new, i);
> > +
> > + err = btf_type_visit_type_ids(btf_type, btfgen_remap_id, ids);
> > + if (err)
> > + goto err_out;
> > + }
> > +
> > + hashmap__free(ids);
> > + return btf_new;
> > +
> > +err_out:
> > + btf__free(btf_new);
> > + hashmap__free(ids);
> > + errno = -err;
> > + return NULL;
> > }
> >
> > /* Create minimized BTF file for a set of BPF objects.
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 6/7] bpftool: gen min_core_btf explanation and examples
2022-02-12 0:42 ` Andrii Nakryiko
@ 2022-02-15 22:56 ` Mauricio Vásquez Bernal
2022-02-16 1:26 ` Andrii Nakryiko
0 siblings, 1 reply; 21+ messages in thread
From: Mauricio Vásquez Bernal @ 2022-02-15 22:56 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Fri, Feb 11, 2022 at 7:42 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
> > From: Rafael David Tinoco <rafaeldtinoco@gmail.com>
> >
> > Add "min_core_btf" feature explanation and one example of how to use it
> > to bpftool-gen man page.
> >
> > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > ---
> > .../bpf/bpftool/Documentation/bpftool-gen.rst | 93 +++++++++++++++++++
> > 1 file changed, 93 insertions(+)
> >
>
> [...]
>
> > +Now, the "5.4.0-smaller.btf" file may be used by libbpf as an external BTF file
> > +when loading the "one.bpf.o" object into the "5.4.0-example" kernel. Note that
> > +the generated BTF file won't allow other eBPF objects to be loaded, just the
> > +ones given to min_core_btf.
> > +
> > +::
> > +
> > + struct bpf_object *obj = NULL;
> > + struct bpf_object_open_opts openopts = {};
> > +
> > + openopts.sz = sizeof(struct bpf_object_open_opts);
> > + openopts.btf_custom_path = "./5.4.0-smaller.btf";
> > +
> > + obj = bpf_object__open_file("./one.bpf.o", &openopts);
>
> Can you please use LIBBPF_OPTS() macro in the example, that's how
> users are normally expected to use OPTS-based APIs anyways. Also there
> is no need for "./" when specifying file location. This is a different
> case than running a binary in the shell, where binary is searched in
> PATH. This is never done when opening files.
>
> So all this should be:
>
> LIBBPF_OPTS(bpf_object_open_opts, opts, .btf_custom_path = "5.4.0-smaller.btf");
> struct bpf_object *obj;
>
I suppose you meant DECLARE_LIBBPF_OPTS(...)
> obj = bpf_object__open_file("one.bpf.o", &opts);
>
> That's all.
>
>
> > +
> > + ...
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 7/7] selftests/bpf: Test "bpftool gen min_core_btf"
2022-02-12 0:42 ` Andrii Nakryiko
@ 2022-02-15 22:56 ` Mauricio Vásquez Bernal
0 siblings, 0 replies; 21+ messages in thread
From: Mauricio Vásquez Bernal @ 2022-02-15 22:56 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Fri, Feb 11, 2022 at 7:42 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
> > This commit reuses the core_reloc test to check if the BTF files
> > generated with "bpftool gen min_core_btf" are correct. This introduces
> > test_core_btfgen() that runs all the core_reloc tests, but this time
> > the source BTF files are generated by using "bpftool gen min_core_btf".
> >
> > The goal of this test is to check that the generated files are usable,
> > and not to check if the algorithm is creating an optimized BTF file.
> >
> > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > ---
> > .../selftests/bpf/prog_tests/core_reloc.c | 46 ++++++++++++++++++-
> > 1 file changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> > index b8bdd1c3efca..10a1d5fb788e 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> > @@ -2,6 +2,7 @@
> > #include <test_progs.h>
> > #include "progs/core_reloc_types.h"
> > #include "bpf_testmod/bpf_testmod.h"
> > +#include <linux/limits.h>
> > #include <sys/mman.h>
> > #include <sys/syscall.h>
> > #include <bpf/btf.h>
> > @@ -354,6 +355,8 @@ static int duration = 0;
> > .fails = true, \
> > }
> >
> > +#define BTFGEN_BTF_PATH "/tmp/btfgen.btf"
>
> let's not use fixed file path, better use mkstemp() to create
> temporary file (see core_autosize.c for an example)
>
You're right.
> > +
> > struct core_reloc_test_case;
> >
> > typedef int (*setup_test_fn)(struct core_reloc_test_case *test);
> > @@ -836,7 +839,21 @@ static size_t roundup_page(size_t sz)
> > return (sz + page_size - 1) / page_size * page_size;
> > }
> >
> > -void test_core_reloc(void)
> > +static int run_btfgen(const char *src_btf, const char *dst_btf, const char *objpath)
> > +{
> > + char command[4096];
> > + int n;
> > +
> > + n = snprintf(command, sizeof(command),
> > + "./tools/build/bpftool/bpftool gen min_core_btf %s %s %s",
> > + src_btf, dst_btf, objpath);
> > + if (n < 0 || n >= sizeof(command))
> > + return -1;
> > +
> > + return system(command);
> > +}
> > +
> > +static void _test_core_reloc(bool btfgen)
>
> ugh, please don't do leading underscore. Call it
> "run_core_reloc_tests" or whatever, it's easy.
>
> another naming nit (I did get confused for a second because of this):
> use_btfgen, otherwise below in the code my first guess was that
> "btfgen" is actually a path to bpftool or something like that
>
>
> > {
> > const size_t mmap_sz = roundup_page(sizeof(struct data));
> > DECLARE_LIBBPF_OPTS(bpf_object_open_opts, open_opts);
> > @@ -863,6 +880,22 @@ void test_core_reloc(void)
> > continue;
> > }
> >
> > + /* generate a "minimal" BTF file and use it as source */
> > + if (btfgen) {
> > + if (!test_case->btf_src_file || test_case->fails) {
> > + test__skip();
> > + continue;
> > + }
> > +
> > + unlink(BTFGEN_BTF_PATH);
> > + err = run_btfgen(test_case->btf_src_file, BTFGEN_BTF_PATH,
> > + test_case->bpf_obj_file);
> > + if (!ASSERT_OK(err, "run_btfgen"))
> > + goto cleanup;
> > +
> > + test_case->btf_src_file = BTFGEN_BTF_PATH;
> > + }
> > +
> > if (test_case->setup) {
> > err = test_case->setup(test_case);
> > if (CHECK(err, "test_setup", "test #%d setup failed: %d\n", i, err))
> > @@ -954,8 +987,19 @@ void test_core_reloc(void)
> > CHECK_FAIL(munmap(mmap_data, mmap_sz));
> > mmap_data = NULL;
> > }
> > + unlink(BTFGEN_BTF_PATH);
> > bpf_link__destroy(link);
> > link = NULL;
> > bpf_object__close(obj);
> > }
> > }
> > +
> > +void test_core_reloc(void)
> > +{
> > + _test_core_reloc(false);
> > +}
> > +
> > +void test_core_btfgen(void)
> > +{
> > + _test_core_reloc(true);
> > +}
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 4/7] bpftool: Implement minimize_btf() and relocations recording for BTFGen
2022-02-15 22:56 ` Mauricio Vásquez Bernal
@ 2022-02-16 1:23 ` Andrii Nakryiko
0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2022-02-16 1:23 UTC (permalink / raw)
To: Mauricio Vásquez Bernal
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Tue, Feb 15, 2022 at 2:56 PM Mauricio Vásquez Bernal
<mauricio@kinvolk.io> wrote:
>
> On Fri, Feb 11, 2022 at 7:42 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> > >
> >
> > It would be good to shorten the subject line, it's very long.
> >
>
> Will do.
>
> > > minimize_btf() receives the path of a source and destination BTF files
> > > and a list of BPF objects. This function records the relocations for
> > > all objects and then generates the BTF file by calling btfgen_get_btf()
> > > (implemented in the following commit).
> > >
> > > btfgen_record_obj() loads the BTF and BTF.ext sections of the BPF
> > > objects and loops through all CO-RE relocations. It uses
> > > bpf_core_calc_relo_insn() from libbpf and passes the target spec to
> > > btfgen_record_reloc(), that calls one of the following functions
> > > depending on the relocation kind.
> > >
> > > btfgen_record_field_relo() uses the target specification to mark all the
> > > types that are involved in a field-based CO-RE relocation. In this case
> > > types resolved and marked recursively using btfgen_mark_type().
> > > Only the struct and union members (and their types) involved in the
> > > relocation are marked to optimize the size of the generated BTF file.
> > >
> > > btfgen_record_type_relo() marks the types involved in a type-based
> > > CO-RE relocation. In this case no members for the struct and union
> > > types are marked as libbpf doesn't use them while performing this kind
> > > of relocation. Pointed types are marked as they are used by libbpf in
> > > this case.
> > >
> > > btfgen_record_enumval_relo() marks the whole enum type for enum-based
> > > relocations.
> >
> > It should be enough to leave only used enumerators, but I suppose it
> > doesn't take much space to record all. We can adjust that later, if
> > necessary.
> >
>
> I think the overhead is really minimal and we can improve later on if we want.
>
> > >
> > > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > > ---
> > > tools/bpf/bpftool/Makefile | 8 +-
> > > tools/bpf/bpftool/gen.c | 452 ++++++++++++++++++++++++++++++++++++-
> > > 2 files changed, 454 insertions(+), 6 deletions(-)
> > >
> >
> > Looks good, few nits and concerns, but it feels like it's really close
> > to being ready.
> >
> > [...]
> >
> > > +}
> > > +
> > > +struct btfgen_info {
> > > + struct btf *src_btf;
> > > + struct btf *marked_btf; // btf structure used to mark used types
> >
> > C++ comment, please use /* */
> >
> > > +};
> > > +
> > > +static size_t btfgen_hash_fn(const void *key, void *ctx)
> > > +{
> > > + return (size_t)key;
> > > +}
> > > +
> > > +static bool btfgen_equal_fn(const void *k1, const void *k2, void *ctx)
> > > +{
> > > + return k1 == k2;
> > > +}
> > > +
> > > +static void *uint_as_hash_key(int x)
> > > +{
> > > + return (void *)(uintptr_t)x;
> > > +}
> > > +
> > > +static void *u32_as_hash_key(__u32 x)
> > > +{
> > > + return (void *)(uintptr_t)x;
> > > +}
> > > +
> > > +static void btfgen_free_info(struct btfgen_info *info)
> > > +{
> > > + if (!info)
> > > + return;
> > > +
> > > + btf__free(info->src_btf);
> > > + btf__free(info->marked_btf);
> > > +
> > > + free(info);
> > > +}
> > > +
> > > +static struct btfgen_info *
> > > +btfgen_new_info(const char *targ_btf_path)
> > > +{
> > > + struct btfgen_info *info;
> > > + int err;
> > > +
> > > + info = calloc(1, sizeof(*info));
> > > + if (!info)
> > > + return NULL;
> > > +
> > > + info->src_btf = btf__parse(targ_btf_path, NULL);
> > > + if (!info->src_btf) {
> > > + p_err("failed parsing '%s' BTF file: %s", targ_btf_path, strerror(errno));
> > > + err = -errno;
> >
> > save errno before p_err, it can clobber errno otherwise
> >
> > > + goto err_out;
> > > + }
> > > +
> > > + info->marked_btf = btf__parse(targ_btf_path, NULL);
> > > + if (!info->marked_btf) {
> > > + p_err("failed parsing '%s' BTF file: %s", targ_btf_path, strerror(errno));
> > > + err = -errno;
> >
> > same, always save errno first before any non-trivial function/macro call
> >
>
> oh right, thanks!
>
> >
> > > + goto err_out;
> > > + }
> > > +
> > > + return info;
> > > +
> > > +err_out:
> > > + btfgen_free_info(info);
> > > + errno = -err;
> > > + return NULL;
> > > +}
> > > +
> > > +#define MARKED UINT32_MAX
> > > +
> > > +static void btfgen_mark_member(struct btfgen_info *info, int type_id, int idx)
> > > +{
> > > + const struct btf_type *t = btf__type_by_id(info->marked_btf, type_id);
> > > + struct btf_member *m = btf_members(t) + idx;
> > > +
> > > + m->name_off = MARKED;
> > > +}
> > > +
> > > +static int
> > > +btfgen_mark_type(struct btfgen_info *info, unsigned int id, bool follow_pointers)
> >
> > id is type_id or could be some other id? It's best to be consistent in
> > naming to avoid second guessing like in this case.
>
> It's always type_id. Renamed it.
>
> >
> > > +{
> > > + const struct btf_type *btf_type = btf__type_by_id(info->src_btf, id);
> > > + struct btf_type *cloned_type;
> > > + struct btf_param *param;
> > > + struct btf_array *array;
> > > + int err, i;
> >
> > [...]
> >
> > > + /* tells if some other type needs to be handled */
> > > + default:
> > > + p_err("unsupported kind: %s (%d)", btf_kind_str(btf_type), id);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int btfgen_record_field_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
> > > +{
> > > + struct btf *btf = (struct btf *) info->src_btf;
> >
> > why the cast?
> >
>
> No reason. Will remove it.
>
> > > + const struct btf_type *btf_type;
> > > + struct btf_member *btf_member;
> > > + struct btf_array *array;
> > > + unsigned int id = targ_spec->root_type_id;
> > > + int idx, err;
> > > +
> > > + /* mark root type */
> > > + btf_type = btf__type_by_id(btf, id);
> > > + err = btfgen_mark_type(info, id, false);
> > > + if (err)
> > > + return err;
> > > +
> > > + /* mark types for complex types (arrays, unions, structures) */
> > > + for (int i = 1; i < targ_spec->raw_len; i++) {
> > > + /* skip typedefs and mods */
> > > + while (btf_is_mod(btf_type) || btf_is_typedef(btf_type)) {
> > > + id = btf_type->type;
> > > + btf_type = btf__type_by_id(btf, id);
> > > + }
> > > +
> > > + switch (btf_kind(btf_type)) {
> > > + case BTF_KIND_STRUCT:
> > > + case BTF_KIND_UNION:
> > > + idx = targ_spec->raw_spec[i];
> > > + btf_member = btf_members(btf_type) + idx;
> > > +
> > > + /* mark member */
> > > + btfgen_mark_member(info, id, idx);
> > > +
> > > + /* mark member's type */
> > > + id = btf_member->type;
> > > + btf_type = btf__type_by_id(btf, id);
> > > + err = btfgen_mark_type(info, id, false);
> >
> > why would it not follow the pointer? E.g., if I have a field defined as
> >
> > struct blah ***my_field;
> >
> > You at the very least would need either an empty struct blah or FWD
> > for struct blah, no?
> >
>
> It's an optimization we do, we don't follow the pointer here because
> it is possible that the definition of the pointed type is not needed.
> For instance, a relocation like:
>
> BPF_CORE_READ(task, nsproxy);
>
> will generate this:
>
> [1] STRUCT 'task_struct' size=9472 vlen=1
> 'nsproxy' type_id=2 bits_offset=23040
> [2] PTR '(anon)' type_id=0
>
> struct nsproxy is not really accessed, so we don't need it's
> definition. On the other hand, something like
>
> BPF_CORE_READ(task, nsproxy, count);
>
> has two relocations, and nsproxy is actually accessed, so in this case
> the generated BTF includes a nsproxy struct:
>
> [1] STRUCT '(anon)' size=4 vlen=0
> [2] TYPEDEF 'atomic_t' type_id=1
> [3] STRUCT 'task_struct' size=9472 vlen=1
> 'nsproxy' type_id=4 bits_offset=23040
> [4] PTR '(anon)' type_id=5
> [5] STRUCT 'nsproxy' size=72 vlen=1
> 'count' type_id=2 bits_offset=0
Ok, so you are just replacing what would be a pointer to forward
declaration with void *. Ok, I guess that works as well.
>
> > > + if (err)
> > > + return err;
> > > + break;
> > > + case BTF_KIND_ARRAY:
> > > + array = btf_array(btf_type);
> > > + id = array->type;
> > > + btf_type = btf__type_by_id(btf, id);
> > > + break;
> >
> > [...]
> >
> > > +err_out:
> > > + bpf_core_free_cands(cands);
> > > + errno = -err;
> > > + return NULL;
> > > +}
> > > +
> > > +/* Record relocation information for a single BPF object*/
> >
> > nit: missing space before */
> >
> > > +static int btfgen_record_obj(struct btfgen_info *info, const char *obj_path)
> > > +{
> > > + const struct btf_ext_info_sec *sec;
> > > + const struct bpf_core_relo *relo;
> > > + const struct btf_ext_info *seg;
> > > + struct hashmap_entry *entry;
> > > + struct hashmap *cand_cache = NULL;
> > > + struct btf_ext *btf_ext = NULL;
> > > + unsigned int relo_idx;
> > > + struct btf *btf = NULL;
> > > + size_t i;
> > > + int err;
> > > +
> > > + btf = btf__parse(obj_path, &btf_ext);
> > > + if (!btf) {
> > > + p_err("failed to parse BPF object '%s': %s", obj_path, strerror(errno));
> > > + return -errno;
> > > + }
> >
> > check that btf_ext is not NULL?
> >
>
> Done.
>
>
> > > +
> > > + if (btf_ext->core_relo_info.len == 0) {
> > > + err = 0;
> > > + goto out;
> > > + }
> > > +
> >
> > [...]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH bpf-next v6 6/7] bpftool: gen min_core_btf explanation and examples
2022-02-15 22:56 ` Mauricio Vásquez Bernal
@ 2022-02-16 1:26 ` Andrii Nakryiko
0 siblings, 0 replies; 21+ messages in thread
From: Andrii Nakryiko @ 2022-02-16 1:26 UTC (permalink / raw)
To: Mauricio Vásquez Bernal
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Quentin Monnet, Rafael David Tinoco,
Lorenzo Fontana, Leonardo Di Donato
On Tue, Feb 15, 2022 at 2:56 PM Mauricio Vásquez Bernal
<mauricio@kinvolk.io> wrote:
>
> On Fri, Feb 11, 2022 at 7:42 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Feb 9, 2022 at 2:27 PM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> > >
> > > From: Rafael David Tinoco <rafaeldtinoco@gmail.com>
> > >
> > > Add "min_core_btf" feature explanation and one example of how to use it
> > > to bpftool-gen man page.
> > >
> > > Signed-off-by: Mauricio Vásquez <mauricio@kinvolk.io>
> > > Signed-off-by: Rafael David Tinoco <rafael.tinoco@aquasec.com>
> > > Signed-off-by: Lorenzo Fontana <lorenzo.fontana@elastic.co>
> > > Signed-off-by: Leonardo Di Donato <leonardo.didonato@elastic.co>
> > > ---
> > > .../bpf/bpftool/Documentation/bpftool-gen.rst | 93 +++++++++++++++++++
> > > 1 file changed, 93 insertions(+)
> > >
> >
> > [...]
> >
> > > +Now, the "5.4.0-smaller.btf" file may be used by libbpf as an external BTF file
> > > +when loading the "one.bpf.o" object into the "5.4.0-example" kernel. Note that
> > > +the generated BTF file won't allow other eBPF objects to be loaded, just the
> > > +ones given to min_core_btf.
> > > +
> > > +::
> > > +
> > > + struct bpf_object *obj = NULL;
> > > + struct bpf_object_open_opts openopts = {};
> > > +
> > > + openopts.sz = sizeof(struct bpf_object_open_opts);
> > > + openopts.btf_custom_path = "./5.4.0-smaller.btf";
> > > +
> > > + obj = bpf_object__open_file("./one.bpf.o", &openopts);
> >
> > Can you please use LIBBPF_OPTS() macro in the example, that's how
> > users are normally expected to use OPTS-based APIs anyways. Also there
> > is no need for "./" when specifying file location. This is a different
> > case than running a binary in the shell, where binary is searched in
> > PATH. This is never done when opening files.
> >
> > So all this should be:
> >
> > LIBBPF_OPTS(bpf_object_open_opts, opts, .btf_custom_path = "5.4.0-smaller.btf");
> > struct bpf_object *obj;
> >
>
> I suppose you meant DECLARE_LIBBPF_OPTS(...)
No, actually, we do have LIBBPF_OPTS and DECLARE_LIBBPF_OPTS is just
an (logically) deprecated alias to LIBBPF_OPTS. Minor difference, but
shorter LIBBPF_OPTS is easier to remember.
>
> > obj = bpf_object__open_file("one.bpf.o", &opts);
> >
> > That's all.
> >
> >
> > > +
> > > + ...
> > > --
> > > 2.25.1
> > >
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2022-02-16 1:26 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-09 22:26 [PATCH bpf-next v6 0/7] libbpf: Implement BTFGen Mauricio Vásquez
2022-02-09 22:26 ` [PATCH bpf-next v6 1/7] libbpf: split bpf_core_apply_relo() Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 2/7] libbpf: Expose bpf_core_{add,free}_cands() to bpftool Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 3/7] bpftool: Add gen min_core_btf command Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 4/7] bpftool: Implement minimize_btf() and relocations recording for BTFGen Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-15 22:56 ` Mauricio Vásquez Bernal
2022-02-16 1:23 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 5/7] bpftool: Implement btfgen_get_btf() Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-15 22:56 ` Mauricio Vásquez Bernal
2022-02-09 22:26 ` [PATCH bpf-next v6 6/7] bpftool: gen min_core_btf explanation and examples Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-15 22:56 ` Mauricio Vásquez Bernal
2022-02-16 1:26 ` Andrii Nakryiko
2022-02-09 22:26 ` [PATCH bpf-next v6 7/7] selftests/bpf: Test "bpftool gen min_core_btf" Mauricio Vásquez
2022-02-12 0:42 ` Andrii Nakryiko
2022-02-15 22:56 ` Mauricio Vásquez Bernal
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).