[PATCH bpf-next v4 0/8] libbpf: Implement BTFGen

[PATCH bpf-next v4 0/8] libbpf: Implement BTFGen

[Mauricio Vásquez]

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:
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/

Mauricio Vásquez (8):
  libbpf: split bpf_core_apply_relo()
  libbpf: Implement changes needed for BTFGen in bpftool
  bpftool: Add gen btf command
  bpftool: Implement btf_save_raw()
  bpftool: Add struct definitions and helpers for BTFGen
  bpftool: Implement btfgen()
  bpftool: Implement relocations recording for BTFGen
  bpftool: Implement btfgen_get_btf()

 kernel/bpf/btf.c                |  13 +-
 tools/bpf/bpftool/gen.c         | 849 ++++++++++++++++++++++++++++++++
 tools/lib/bpf/Makefile          |   2 +-
 tools/lib/bpf/libbpf.c          | 127 +++--
 tools/lib/bpf/libbpf_internal.h |  12 +
 tools/lib/bpf/relo_core.c       |  79 +--
 tools/lib/bpf/relo_core.h       |  42 +-
 7 files changed, 1012 insertions(+), 112 deletions(-)

-- 
2.25.1

[PATCH bpf-next v4 1/8] libbpf: split bpf_core_apply_relo()

[Mauricio Vásquez]

BTFGen needs to run the core relocation logic in order to understand
what are the types in the target BTF that 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_res() 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_res().

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    | 84 ++++++++++++++++++++++++---------------
 tools/lib/bpf/relo_core.c | 79 +++++++++++-------------------------
 tools/lib/bpf/relo_core.h | 42 +++++++++++++++++---
 4 files changed, 122 insertions(+), 96 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 33bb8ae4a804..9e86830a4316 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6788,6 +6788,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;
 
@@ -6827,13 +6828,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 fdb3536afa7d..4959c03a46f4 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5523,11 +5523,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);
@@ -5536,20 +5537,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)
@@ -5559,15 +5547,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);
@@ -5584,19 +5563,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;
 
@@ -5661,12 +5642,53 @@ 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);
+			if (prog->obj->gen_loader) {
+				const struct btf_type *local_type;
+				const char *local_name, *spec_str;
+
+				spec_str = btf__name_by_offset(obj->btf, rec->access_str_off);
+				if (!spec_str)
+					return -EINVAL;
+
+				local_type = btf__type_by_id(obj->btf, rec->type_id);
+				if (!local_type)
+					return -EINVAL;
+
+				local_name = btf__name_by_offset(obj->btf, local_type->name_off);
+				if (!local_name)
+					return -EINVAL;
+
+				pr_debug("record_relo_core: prog %td insn[%d] %s %s %s final insn_idx %d\n",
+					prog - prog->obj->programs, insn_idx,
+					btf_kind_str(local_type), local_name, spec_str, insn_idx);
+				return record_relo_core(prog, rec, insn_idx);
+			}
+
+			if (rec->insn_off % BPF_INSN_SZ)
+				return -EINVAL;
+			insn_idx = rec->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];
+
+			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

[PATCH bpf-next v4 2/8] libbpf: Implement changes needed for BTFGen in bpftool

[Mauricio Vásquez]

This commit extends libbpf with the features that are needed to
implement BTFGen:

- Implement bpf_core_create_cand_cache() and bpf_core_free_cand_cache()
to handle candidates cache.
- Expose bpf_core_add_cands() and bpf_core_free_cands to handle
candidates list.
- Expose bpf_core_calc_relo_insn() to bpftool.

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/Makefile          |  2 +-
 tools/lib/bpf/libbpf.c          | 43 +++++++++++++++++++++------------
 tools/lib/bpf/libbpf_internal.h | 12 +++++++++
 3 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index f947b61b2107..dba019ee2832 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -239,7 +239,7 @@ install_lib: all_cmd
 
 SRC_HDRS := bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h	     \
 	    bpf_helpers.h bpf_tracing.h bpf_endian.h bpf_core_read.h	     \
-	    skel_internal.h libbpf_version.h
+	    skel_internal.h libbpf_version.h relo_core.h libbpf_internal.h
 GEN_HDRS := $(BPF_GENERATED)
 
 INSTALL_PFX := $(DESTDIR)$(prefix)/include/bpf
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 4959c03a46f4..344b8b8e8a50 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -5185,18 +5185,18 @@ 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)
 {
 	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;
@@ -5567,6 +5567,24 @@ static int bpf_core_resolve_relo(struct bpf_program *prog,
 				       targ_res);
 }
 
+struct hashmap *bpf_core_create_cand_cache(void)
+{
+	return hashmap__new(bpf_core_hash_fn, bpf_core_equal_fn, NULL);
+}
+
+void bpf_core_free_cand_cache(struct hashmap *cand_cache)
+{
+	struct hashmap_entry *entry;
+	int i;
+
+	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);
+	}
+}
+
 static int
 bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 {
@@ -5574,7 +5592,6 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 	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;
@@ -5593,7 +5610,7 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 		}
 	}
 
-	cand_cache = hashmap__new(bpf_core_hash_fn, bpf_core_equal_fn, NULL);
+	cand_cache = bpf_core_create_cand_cache();
 	if (IS_ERR(cand_cache)) {
 		err = PTR_ERR(cand_cache);
 		goto out;
@@ -5697,12 +5714,8 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 	btf__free(obj->btf_vmlinux_override);
 	obj->btf_vmlinux_override = NULL;
 
-	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);
-	}
+	bpf_core_free_cand_cache(cand_cache);
+
 	return err;
 }
 
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 1565679eb432..0c258c252919 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -526,4 +526,16 @@ static inline int ensure_good_fd(int fd)
 	return fd;
 }
 
+struct hashmap;
+
+struct hashmap *bpf_core_create_cand_cache(void);
+void bpf_core_free_cand_cache(struct hashmap *cand_cache);
+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

[PATCH bpf-next v4 3/8] bpftool: Add gen btf command

[Mauricio Vásquez]

This command is implemented under the "gen" command in bpftool and the
syntax is the following:

$ bpftool gen btf INPUT OUTPUT OBJECT(S)

INPUT can be either a single BTF file or a folder containing BTF files,
when it's a folder, a BTF file is generated for each BTF file contained
in this folder. OUTPUT is the file (or folder) where generated files are
stored and OBJECT(S) is the list of bpf objects we want to generate the
BTF file(s) for (each generated BTF file contains all the types needed
by all 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/gen.c | 117 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 117 insertions(+)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 43e3f8700ecc..cdeb1047d79d 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -5,6 +5,7 @@
 #define _GNU_SOURCE
 #endif
 #include <ctype.h>
+#include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <linux/err.h>
@@ -1084,6 +1085,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 btf INPUT OUTPUT OBJECT(S)\n"
 		"       %1$s %2$s help\n"
 		"\n"
 		"       " HELP_SPEC_OPTIONS " |\n"
@@ -1094,9 +1096,124 @@ static int do_help(int argc, char **argv)
 	return 0;
 }
 
+/* Create BTF file for a set of BPF objects */
+static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
+{
+	return -EOPNOTSUPP;
+}
+
+static int is_file(const char *path)
+{
+	struct stat st;
+
+	if (stat(path, &st) < 0)
+		return -1;
+
+	switch (st.st_mode & S_IFMT) {
+	case S_IFDIR:
+		return 0;
+	case S_IFREG:
+		return 1;
+	default:
+		return -1;
+	}
+}
+
+static int do_gen_btf(int argc, char **argv)
+{
+	char src_btf_path[PATH_MAX], dst_btf_path[PATH_MAX];
+	bool input_is_file, output_is_file = false;
+	const char *input, *output;
+	const char **objs = NULL;
+	struct dirent *dir;
+	DIR *d = NULL;
+	int i, err;
+
+	if (!REQ_ARGS(3)) {
+		usage();
+		return -1;
+	}
+
+	input = GET_ARG();
+	err = is_file(input);
+	if (err < 0) {
+		p_err("failed to stat %s: %s", input, strerror(errno));
+		return err;
+	}
+	input_is_file = err;
+
+	output = GET_ARG();
+	err = is_file(output);
+	if (err != 0)
+		output_is_file = true;
+
+	objs = (const char **) malloc((argc + 1) * sizeof(*objs));
+	if (!objs)
+		return -ENOMEM;
+
+	i = 0;
+	while (argc > 0)
+		objs[i++] = GET_ARG();
+
+	objs[i] = NULL;
+
+	/* single BTF file */
+	if (input_is_file) {
+		printf("SBTF: %s\n", input);
+
+		if (output_is_file) {
+			err = btfgen(input, output, objs);
+			goto out;
+		}
+		snprintf(dst_btf_path, sizeof(dst_btf_path), "%s/%s", output,
+			 basename(input));
+		err = btfgen(input, dst_btf_path, objs);
+		goto out;
+	}
+
+	if (output_is_file) {
+		p_err("can't have just one file as output");
+		err = -EINVAL;
+		goto out;
+	}
+
+	/* directory with BTF files */
+	d = opendir(input);
+	if (!d) {
+		p_err("error opening input dir: %s", strerror(errno));
+		err = -errno;
+		goto out;
+	}
+
+	while ((dir = readdir(d)) != NULL) {
+		if (dir->d_type != DT_REG)
+			continue;
+
+		if (strncmp(dir->d_name + strlen(dir->d_name) - 4, ".btf", 4))
+			continue;
+
+		snprintf(src_btf_path, sizeof(src_btf_path), "%s/%s", input, dir->d_name);
+		snprintf(dst_btf_path, sizeof(dst_btf_path), "%s/%s", output, dir->d_name);
+
+		printf("SBTF: %s\n", src_btf_path);
+
+		err = btfgen(src_btf_path, dst_btf_path, objs);
+		if (err)
+			goto out;
+	}
+
+out:
+	if (!err)
+		printf("STAT: done!\n");
+	free(objs);
+	closedir(d);
+	return err;
+}
+
 static const struct cmd cmds[] = {
 	{ "object",	do_object },
 	{ "skeleton",	do_skeleton },
+	{ "btf",	do_gen_btf},
 	{ "help",	do_help },
 	{ 0 }
 };
-- 
2.25.1

[PATCH bpf-next v4 4/8] bpftool: Implement btf_save_raw()

[Mauricio Vásquez]

Helper function to save a BTF object to a 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>
---
 tools/bpf/bpftool/gen.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index cdeb1047d79d..5a74fb68dc84 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1096,6 +1096,36 @@ static int do_help(int argc, char **argv)
 	return 0;
 }
 
+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) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	f = fopen(path, "wb");
+	if (!f) {
+		err = -errno;
+		goto out;
+	}
+
+	if (fwrite(data, 1, data_sz, f) != data_sz) {
+		err = -errno;
+		goto out;
+	}
+
+out:
+	if (f)
+		fclose(f);
+	return err;
+}
+
 /* Create BTF file for a set of BPF objects */
 static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
 {
-- 
2.25.1

[PATCH bpf-next v4 5/8] bpftool: Add struct definitions and helpers for BTFGen

[Mauricio Vásquez]

Add some structs and helpers that will be used by BTFGen in the next
commits.

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 | 75 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 5a74fb68dc84..905ab0ee6542 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1126,6 +1126,81 @@ static int btf_save_raw(const struct btf *btf, const char *path)
 	return err;
 }
 
+struct btfgen_type {
+	struct btf_type *type;
+	unsigned int id;
+};
+
+struct btfgen_info {
+	struct hashmap *types;
+	struct btf *src_btf;
+};
+
+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 btfgen_free_type(struct btfgen_type *type)
+{
+	free(type);
+}
+
+static void btfgen_free_info(struct btfgen_info *info)
+{
+	struct hashmap_entry *entry;
+	size_t bkt;
+
+	if (!info)
+		return;
+
+	if (!IS_ERR_OR_NULL(info->types)) {
+		hashmap__for_each_entry(info->types, entry, bkt) {
+			btfgen_free_type(entry->value);
+		}
+		hashmap__free(info->types);
+	}
+
+	btf__free(info->src_btf);
+
+	free(info);
+}
+
+static struct btfgen_info *
+btfgen_new_info(const char *targ_btf_path)
+{
+	struct btfgen_info *info;
+
+	info = calloc(1, sizeof(*info));
+	if (!info)
+		return NULL;
+
+	info->src_btf = btf__parse(targ_btf_path, NULL);
+	if (libbpf_get_error(info->src_btf)) {
+		btfgen_free_info(info);
+		return NULL;
+	}
+
+	info->types = hashmap__new(btfgen_hash_fn, btfgen_equal_fn, NULL);
+	if (IS_ERR(info->types)) {
+		errno = -PTR_ERR(info->types);
+		btfgen_free_info(info);
+		return NULL;
+	}
+
+	return info;
+}
+
 /* Create BTF file for a set of BPF objects */
 static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
 {
-- 
2.25.1

[PATCH bpf-next v4 6/8] bpftool: Implement btfgen()

[Mauricio Vásquez]

btfgen() 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 commits).

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 saves the types involved in such relocation.

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 | 221 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 219 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 905ab0ee6542..cef0ea99d4d9 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -15,6 +15,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>
@@ -1151,6 +1152,11 @@ 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_type(struct btfgen_type *type)
 {
 	free(type);
@@ -1201,12 +1207,223 @@ btfgen_new_info(const char *targ_btf_path)
 	return info;
 }
 
-/* Create BTF file for a set of BPF objects */
-static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
+static int btfgen_record_field_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
+{
+	return -EOPNOTSUPP;
+}
+
+static int btfgen_record_type_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
+{
+	return -EOPNOTSUPP;
+}
+
+static int btfgen_record_enumval_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
 {
 	return -EOPNOTSUPP;
 }
 
+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:
+	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:
+	if (cands)
+		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 *cand_cache;
+	struct btf_ext *btf_ext;
+	unsigned int relo_idx;
+	struct btf *btf;
+	int err;
+
+	btf = btf__parse(obj_path, &btf_ext);
+	err = libbpf_get_error(btf);
+	if (err) {
+		p_err("failed to parse bpf object '%s': %s", obj_path, strerror(errno));
+		return err;
+	}
+
+	if (btf_ext->core_relo_info.len == 0)
+		return 0;
+
+	cand_cache = bpf_core_create_cand_cache();
+	if (IS_ERR(cand_cache))
+		return PTR_ERR(cand_cache);
+
+	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;
+
+			err = btfgen_record_reloc(info, &specs_scratch[2]);
+			if (err)
+				goto out;
+		}
+	}
+
+out:
+	bpf_core_free_cand_cache(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 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 and 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 btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
+{
+	struct btfgen_info *info;
+	struct btf *btf_new = NULL;
+	int err;
+
+	info = btfgen_new_info(src_btf);
+	if (!info) {
+		p_err("failed to allocate info structure: %s", strerror(errno));
+		err = -errno;
+		goto out;
+	}
+
+	for (int i = 0; objspaths[i] != NULL; i++) {
+		printf("OBJ : %s\n", objspaths[i]);
+
+		err = btfgen_record_obj(info, objspaths[i]);
+		if (err)
+			goto out;
+	}
+
+	btf_new = btfgen_get_btf(info);
+	if (!btf_new) {
+		err = -errno;
+		p_err("error generating btf: %s", strerror(errno));
+		goto out;
+	}
+
+	printf("DBTF: %s\n", dst_btf);
+	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 is_file(const char *path)
 {
 	struct stat st;
-- 
2.25.1

[PATCH bpf-next v4 7/8] bpftool: Implement relocations recording for BTFGen

[Mauricio Vásquez]

This commit implements the logic to record the relocation information
for the different kind of relocations.

btfgen_record_field_relo() uses the target specification to save all the
types that are involved in a field-based CO-RE relocation. In this case
types resolved and added recursively (using btfgen_put_type()).
Only the struct and union members and their types) involved in the
relocation are added to optimize the size of the generated BTF file.

On the other hand, btfgen_record_type_relo() saves the types involved in
a type-based CO-RE relocation. In this case all the members for the
struct and union types are added. This is not strictly required since
libbpf doesn't use them while performing this kind of relocation,
however that logic could change on the future. Additionally, we expect
that the number of this kind of relocations in an BPF object to be very
low, hence the impact on the size of the generated BTF should be
negligible.

Finally, btfgen_record_enumval_relo() saves 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/gen.c | 260 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 257 insertions(+), 3 deletions(-)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index cef0ea99d4d9..8c13dde0b74d 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1127,9 +1127,17 @@ static int btf_save_raw(const struct btf *btf, const char *path)
 	return err;
 }
 
+struct btfgen_member {
+	struct btf_member *member;
+	int idx;
+};
+
 struct btfgen_type {
 	struct btf_type *type;
 	unsigned int id;
+	bool all_members;
+
+	struct hashmap *members;
 };
 
 struct btfgen_info {
@@ -1159,6 +1167,19 @@ static void *u32_as_hash_key(__u32 x)
 
 static void btfgen_free_type(struct btfgen_type *type)
 {
+	struct hashmap_entry *entry;
+	size_t bkt;
+
+	if (!type)
+		return;
+
+	if (!IS_ERR_OR_NULL(type->members)) {
+		hashmap__for_each_entry(type->members, entry, bkt) {
+			free(entry->value);
+		}
+		hashmap__free(type->members);
+	}
+
 	free(type);
 }
 
@@ -1207,19 +1228,252 @@ btfgen_new_info(const char *targ_btf_path)
 	return info;
 }
 
+static int btfgen_add_member(struct btfgen_type *btfgen_type,
+			     struct btf_member *btf_member, int idx)
+{
+	struct btfgen_member *btfgen_member;
+	int err;
+
+	/* create new members hashmap for this btfgen type if needed */
+	if (!btfgen_type->members) {
+		btfgen_type->members = hashmap__new(btfgen_hash_fn, btfgen_equal_fn, NULL);
+		if (IS_ERR(btfgen_type->members))
+			return PTR_ERR(btfgen_type->members);
+	}
+
+	btfgen_member = calloc(1, sizeof(*btfgen_member));
+	if (!btfgen_member)
+		return -ENOMEM;
+	btfgen_member->member = btf_member;
+	btfgen_member->idx = idx;
+	/* add btf_member as member to given btfgen_type */
+	err = hashmap__add(btfgen_type->members, uint_as_hash_key(btfgen_member->idx),
+			   btfgen_member);
+	if (err) {
+		free(btfgen_member);
+		if (err != -EEXIST)
+			return err;
+	}
+
+	return 0;
+}
+
+static struct btfgen_type *btfgen_get_type(struct btfgen_info *info, int id)
+{
+	struct btfgen_type *type = NULL;
+
+	hashmap__find(info->types, uint_as_hash_key(id), (void **)&type);
+
+	return type;
+}
+
+static struct btfgen_type *
+_btfgen_put_type(struct btf *btf, struct btfgen_info *info, struct btf_type *btf_type,
+		 unsigned int id, bool all_members)
+{
+	struct btfgen_type *btfgen_type, *tmp;
+	struct btf_array *array;
+	unsigned int child_id;
+	struct btf_member *m;
+	int err, i, n;
+
+	/* check if we already have this type */
+	if (hashmap__find(info->types, uint_as_hash_key(id), (void **) &btfgen_type)) {
+		if (!all_members || btfgen_type->all_members)
+			return btfgen_type;
+	} else {
+		btfgen_type = calloc(1, sizeof(*btfgen_type));
+		if (!btfgen_type)
+			return NULL;
+
+		btfgen_type->type = btf_type;
+		btfgen_type->id = id;
+
+		/* append this type to the types list before anything else */
+		err = hashmap__add(info->types, uint_as_hash_key(btfgen_type->id), btfgen_type);
+		if (err) {
+			free(btfgen_type);
+			return NULL;
+		}
+	}
+
+	/* avoid infinite recursion and yet be able to add all
+	 * fields/members for types also managed by this function
+	 */
+	btfgen_type->all_members = all_members;
+
+
+	/* recursively add other types needed by it */
+	switch (btf_kind(btfgen_type->type)) {
+	case BTF_KIND_UNKN:
+	case BTF_KIND_INT:
+	case BTF_KIND_FLOAT:
+	case BTF_KIND_ENUM:
+		break;
+	case BTF_KIND_STRUCT:
+	case BTF_KIND_UNION:
+		/* doesn't need resolution if not adding all members */
+		if (!all_members)
+			break;
+
+		n = btf_vlen(btf_type);
+		m = btf_members(btf_type);
+		for (i = 0; i < n; i++, m++) {
+			btf_type = (struct btf_type *) btf__type_by_id(btf, m->type);
+
+			/* add all member types */
+			tmp = _btfgen_put_type(btf, info, btf_type, m->type, all_members);
+			if (!tmp)
+				return NULL;
+
+			/* add all members */
+			err = btfgen_add_member(btfgen_type, m, i);
+			if (err)
+				return NULL;
+		}
+		break;
+	case BTF_KIND_PTR:
+		if (!all_members)
+			break;
+	/* fall through */
+	/* Also add the type it's pointing to when adding all members */
+	case BTF_KIND_CONST:
+	case BTF_KIND_VOLATILE:
+	case BTF_KIND_TYPEDEF:
+		child_id = btf_type->type;
+		btf_type = (struct btf_type *) btf__type_by_id(btf, child_id);
+
+		tmp = _btfgen_put_type(btf, info, btf_type, child_id, all_members);
+		if (!tmp)
+			return NULL;
+		break;
+	case BTF_KIND_ARRAY:
+		array = btf_array(btfgen_type->type);
+
+		/* add type for array type */
+		btf_type = (struct btf_type *) btf__type_by_id(btf, array->type);
+		tmp = _btfgen_put_type(btf, info, btf_type, array->type, all_members);
+		if (!tmp)
+			return NULL;
+
+		/* add type for array's index type */
+		btf_type = (struct btf_type *) btf__type_by_id(btf, array->index_type);
+		tmp = _btfgen_put_type(btf, info, btf_type, array->index_type, all_members);
+		if (!tmp)
+			return NULL;
+		break;
+	/* tells if some other type needs to be handled */
+	default:
+		p_err("unsupported kind: %s (%d)",
+		      btf_kind_str(btfgen_type->type), btfgen_type->id);
+		errno = EINVAL;
+		return NULL;
+	}
+
+	return btfgen_type;
+}
+
+/* Put type in the list. If the type already exists it's returned, otherwise a
+ * new one is created and added to the list. This is called recursively adding
+ * all the types that are needed for the current one.
+ */
+static struct btfgen_type *
+btfgen_put_type(struct btf *btf, struct btfgen_info *info, struct btf_type *btf_type,
+		unsigned int id)
+{
+	return _btfgen_put_type(btf, info, btf_type, id, false);
+}
+
+/* Same as btfgen_put_type, but adding all members, from given complex type, recursively */
+static struct btfgen_type *
+btfgen_put_type_all(struct btf *btf, struct btfgen_info *info,
+		    struct btf_type *btf_type, unsigned int id)
+{
+	return _btfgen_put_type(btf, info, btf_type, id, true);
+}
+
 static int btfgen_record_field_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
 {
-	return -EOPNOTSUPP;
+	struct btf *btf = (struct btf *) info->src_btf;
+	struct btfgen_type *btfgen_type;
+	struct btf_member *btf_member;
+	struct btf_type *btf_type;
+	struct btf_array *array;
+	unsigned int id;
+	int idx, err;
+
+	btf_type = (struct btf_type *) btf__type_by_id(btf, targ_spec->root_type_id);
+
+	/* create btfgen_type for root type */
+	btfgen_type = btfgen_put_type(btf, info, btf_type, targ_spec->root_type_id);
+	if (!btfgen_type)
+		return -errno;
+
+	/* add 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;
+			btfgen_type = btfgen_get_type(info, id);
+			if (!btfgen_type)
+				return -ENOENT;
+			btf_type = (struct 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;
+			btf_type = (struct btf_type *) btf__type_by_id(btf, btf_member->type);
+
+			/* add member to relocation type */
+			err = btfgen_add_member(btfgen_type, btf_member, idx);
+			if (err)
+				return err;
+			/* put btfgen type */
+			btfgen_type = btfgen_put_type(btf, info, btf_type, btf_member->type);
+			if (!btfgen_type)
+				return -errno;
+			break;
+		case BTF_KIND_ARRAY:
+			array = btf_array(btf_type);
+			btfgen_type = btfgen_get_type(info, array->type);
+			if (!btfgen_type)
+				return -ENOENT;
+			btf_type = (struct btf_type *) btf__type_by_id(btf, array->type);
+			break;
+		default:
+			p_err("spec type wasn't handled");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
 }
 
 static int btfgen_record_type_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
 {
-	return -EOPNOTSUPP;
+	struct btf *btf = (struct btf *) info->src_btf;
+	struct btfgen_type *btfgen_type;
+	struct btf_type *btf_type;
+
+	btf_type = (struct btf_type *) btf__type_by_id(btf, targ_spec->root_type_id);
+
+	btfgen_type = btfgen_put_type_all(btf, info, btf_type, targ_spec->root_type_id);
+	return btfgen_type ?  0 : -errno;
 }
 
 static int btfgen_record_enumval_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
 {
-	return -EOPNOTSUPP;
+	struct btf *btf = (struct btf *) info->src_btf;
+	struct btfgen_type *btfgen_type;
+	struct btf_type *btf_type;
+
+	btf_type = (struct btf_type *) btf__type_by_id(btf, targ_spec->root_type_id);
+
+	btfgen_type = btfgen_put_type_all(btf, info, btf_type, targ_spec->root_type_id);
+	return btfgen_type ?  0 : -errno;
 }
 
 static int btfgen_record_reloc(struct btfgen_info *info, struct bpf_core_spec *res)
-- 
2.25.1

[PATCH bpf-next v4 8/8] bpftool: Implement btfgen_get_btf()

[Mauricio Vásquez]

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 | 158 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 157 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 8c13dde0b74d..696a66bded32 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -1607,10 +1607,166 @@ 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 = 0;
+
+	/* deal with BTF_KIND_VOID */
+	if (old == 0)
+		return 0;
+
+	if (!hashmap__find(ids, uint_as_hash_key(old), (void **)&new)) {
+		/* return id for 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));
+}
+
 /* Generate BTF from relocation information previously recorded */
 static struct btf *btfgen_get_btf(struct btfgen_info *info)
 {
-	return ERR_PTR(-EOPNOTSUPP);
+	struct hashmap_entry *entry;
+	struct btf *btf_new = NULL;
+	struct hashmap *ids = NULL;
+	size_t bkt;
+	int err = 0;
+
+	btf_new = btf__new_empty();
+	if (libbpf_get_error(btf_new))
+		goto err_out;
+
+	ids = hashmap__new(btfgen_hash_fn, btfgen_equal_fn, NULL);
+	if (IS_ERR(ids)) {
+		errno = -PTR_ERR(ids);
+		goto err_out;
+	}
+
+	/* first pass: add all types and add their new ids to the ids map */
+	hashmap__for_each_entry(info->types, entry, bkt) {
+		struct btfgen_type *btfgen_type = entry->value;
+		struct btf_type *btf_type = btfgen_type->type;
+		int new_id;
+
+		/* we're adding BTF_KIND_VOID to the list but it can't be added to
+		 * the generated BTF object, hence we skip it here.
+		 */
+		if (btfgen_type->id == 0)
+			continue;
+
+		/* add members for struct and union */
+		if (btf_is_struct(btf_type) || btf_is_union(btf_type)) {
+			struct hashmap_entry *member_entry;
+			struct btf_type *btf_type_cpy;
+			int nmembers, index;
+			size_t new_size;
+
+			nmembers = btfgen_type->members ? hashmap__size(btfgen_type->members) : 0;
+			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 header */
+			memcpy(btf_type_cpy, btf_type, sizeof(*btf_type_cpy));
+
+			/* copy only members that are needed */
+			index = 0;
+			if (nmembers > 0) {
+				size_t bkt2;
+
+				hashmap__for_each_entry(btfgen_type->members, member_entry, bkt2) {
+					struct btfgen_member *btfgen_member;
+					struct btf_member *btf_member;
+
+					btfgen_member = member_entry->value;
+					btf_member = btf_members(btf_type) + btfgen_member->idx;
+
+					memcpy(btf_members(btf_type_cpy) + index, btf_member,
+					       sizeof(struct btf_member));
+
+					index++;
+				}
+			}
+
+			/* 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, btfgen_type->id, new_id);
+		if (err)
+			goto err_out;
+	}
+
+	/* second pass: fix up type ids */
+	for (unsigned int i = 0; i < btf__type_cnt(btf_new); i++) {
+		struct btf_member *btf_member;
+		struct btf_type *btf_type;
+		struct btf_param *params;
+		struct btf_array *array;
+
+		btf_type = (struct btf_type *) btf__type_by_id(btf_new, i);
+
+		switch (btf_kind(btf_type)) {
+		case BTF_KIND_STRUCT:
+		case BTF_KIND_UNION:
+			for (unsigned short j = 0; j < btf_vlen(btf_type); j++) {
+				btf_member = btf_members(btf_type) + j;
+				btf_member->type = btfgen_get_id(ids, btf_member->type);
+			}
+			break;
+		case BTF_KIND_PTR:
+		case BTF_KIND_TYPEDEF:
+		case BTF_KIND_VOLATILE:
+		case BTF_KIND_CONST:
+		case BTF_KIND_RESTRICT:
+		case BTF_KIND_FUNC:
+		case BTF_KIND_VAR:
+			btf_type->type = btfgen_get_id(ids, btf_type->type);
+			break;
+		case BTF_KIND_ARRAY:
+			array = btf_array(btf_type);
+			array->index_type = btfgen_get_id(ids, array->index_type);
+			array->type = btfgen_get_id(ids, array->type);
+			break;
+		case BTF_KIND_FUNC_PROTO:
+			btf_type->type = btfgen_get_id(ids, btf_type->type);
+			params = btf_params(btf_type);
+			for (unsigned short j = 0; j < btf_vlen(btf_type); j++)
+				params[j].type = btfgen_get_id(ids, params[j].type);
+			break;
+		default:
+			break;
+		}
+	}
+
+	hashmap__free(ids);
+	return btf_new;
+
+err_out:
+	btf__free(btf_new);
+	hashmap__free(ids);
+	return NULL;
 }
 
 /* Create BTF file for a set of BPF objects.
-- 
2.25.1

Re: [PATCH bpf-next v4 2/8] libbpf: Implement changes needed for BTFGen in bpftool

[Quentin Monnet]

2022-01-12 09:27 UTC-0500 ~ Mauricio Vásquez <mauricio@kinvolk.io>
> This commit extends libbpf with the features that are needed to
> implement BTFGen:
> 
> - Implement bpf_core_create_cand_cache() and bpf_core_free_cand_cache()
> to handle candidates cache.
> - Expose bpf_core_add_cands() and bpf_core_free_cands to handle
> candidates list.
> - Expose bpf_core_calc_relo_insn() to bpftool.
> 
> 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/Makefile          |  2 +-
>  tools/lib/bpf/libbpf.c          | 43 +++++++++++++++++++++------------
>  tools/lib/bpf/libbpf_internal.h | 12 +++++++++
>  3 files changed, 41 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index f947b61b2107..dba019ee2832 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -239,7 +239,7 @@ install_lib: all_cmd
>  
>  SRC_HDRS := bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h	     \
>  	    bpf_helpers.h bpf_tracing.h bpf_endian.h bpf_core_read.h	     \
> -	    skel_internal.h libbpf_version.h
> +	    skel_internal.h libbpf_version.h relo_core.h libbpf_internal.h
>  GEN_HDRS := $(BPF_GENERATED)

I don't think these headers should be added to libbpf's SRC_HDRS. If we
must make them available to bpftool, we probably want to copy them
explicitly through LIBBPF_INTERNAL_HDRS in bpftool's Makefile.

Re: [PATCH bpf-next v4 3/8] bpftool: Add gen btf command

[Quentin Monnet]

2022-01-12 09:27 UTC-0500 ~ Mauricio Vásquez <mauricio@kinvolk.io>
> This command is implemented under the "gen" command in bpftool and the
> syntax is the following:
> 
> $ bpftool gen btf INPUT OUTPUT OBJECT(S)

Thanks a lot for this work!

Please update the relevant manual page under Documentation, to let users
know how to use the feature. You may also consider adding an example at
the end of that document.

The bash completion script should also be updated with the new "btf"
subcommand for "gen". Given that all the arguments are directories and
files, it should not be hard.

Have you considered adding tests for the feature? There are a few
bpftool-related selftests under tools/testing/selftests/bpf/.

> 
> INPUT can be either a single BTF file or a folder containing BTF files,
> when it's a folder, a BTF file is generated for each BTF file contained
> in this folder. OUTPUT is the file (or folder) where generated files are
> stored and OBJECT(S) is the list of bpf objects we want to generate the
> BTF file(s) for (each generated BTF file contains all the types needed
> by all 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/gen.c | 117 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index 43e3f8700ecc..cdeb1047d79d 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -5,6 +5,7 @@
>  #define _GNU_SOURCE
>  #endif
>  #include <ctype.h>
> +#include <dirent.h>
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <linux/err.h>
> @@ -1084,6 +1085,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 btf INPUT OUTPUT OBJECT(S)\n"
>  		"       %1$s %2$s help\n"
>  		"\n"
>  		"       " HELP_SPEC_OPTIONS " |\n"
> @@ -1094,9 +1096,124 @@ static int do_help(int argc, char **argv)
>  	return 0;
>  }
>  
> +/* Create BTF file for a set of BPF objects */
> +static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int is_file(const char *path)
> +{
> +	struct stat st;
> +
> +	if (stat(path, &st) < 0)
> +		return -1;
> +
> +	switch (st.st_mode & S_IFMT) {
> +	case S_IFDIR:
> +		return 0;
> +	case S_IFREG:
> +		return 1;
> +	default:
> +		return -1;
> +	}
> +}
> +
> +static int do_gen_btf(int argc, char **argv)
> +{
> +	char src_btf_path[PATH_MAX], dst_btf_path[PATH_MAX];
> +	bool input_is_file, output_is_file = false;
> +	const char *input, *output;
> +	const char **objs = NULL;
> +	struct dirent *dir;
> +	DIR *d = NULL;
> +	int i, err;
> +
> +	if (!REQ_ARGS(3)) {
> +		usage();
> +		return -1;
> +	}
> +
> +	input = GET_ARG();
> +	err = is_file(input);
> +	if (err < 0) {
> +		p_err("failed to stat %s: %s", input, strerror(errno));
> +		return err;
> +	}
> +	input_is_file = err;
> +
> +	output = GET_ARG();
> +	err = is_file(output);
> +	if (err != 0)
> +		output_is_file = true;

Why not return if err < 0? This will set output_is_file to true and will
fail later, I think?

> +
> +	objs = (const char **) malloc((argc + 1) * sizeof(*objs));
> +	if (!objs)
> +		return -ENOMEM;

Let's p_err() a message.

> +
> +	i = 0;
> +	while (argc > 0)
> +		objs[i++] = GET_ARG();
> +
> +	objs[i] = NULL;
> +
> +	/* single BTF file */
> +	if (input_is_file) {
> +		printf("SBTF: %s\n", input);

We can use "p_info()" instead of "printf()". In particular, this avoids
printing the message when the JSON output is required.

> +
> +		if (output_is_file) {
> +			err = btfgen(input, output, objs);
> +			goto out;
> +		}
> +		snprintf(dst_btf_path, sizeof(dst_btf_path), "%s/%s", output,
> +			 basename(input));

Am I right that the output file should be just a file name, and not a
path, and that it will be created under the same directory as the input
file? And that providing a relative or absolute path instead will cause
issues here? If so, please document it. It would be nice to be able to
support paths too, but I'm not sure how much work that represents.

> +		err = btfgen(input, dst_btf_path, objs);
> +		goto out;
> +	}
> +
> +	if (output_is_file) {
> +		p_err("can't have just one file as output");

See comment above, this message is misleading if stat() returned with an
error.

> +		err = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* directory with BTF files */
> +	d = opendir(input);
> +	if (!d) {
> +		p_err("error opening input dir: %s", strerror(errno));
> +		err = -errno;
> +		goto out;
> +	}
> +
> +	while ((dir = readdir(d)) != NULL) {
> +		if (dir->d_type != DT_REG)
> +			continue;
> +
> +		if (strncmp(dir->d_name + strlen(dir->d_name) - 4, ".btf", 4))
> +			continue;
> +
> +		snprintf(src_btf_path, sizeof(src_btf_path), "%s/%s", input, dir->d_name);
> +		snprintf(dst_btf_path, sizeof(dst_btf_path), "%s/%s", output, dir->d_name);
> +
> +		printf("SBTF: %s\n", src_btf_path);
> +
> +		err = btfgen(src_btf_path, dst_btf_path, objs);
> +		if (err)
> +			goto out;
> +	}
> +
> +out:
> +	if (!err)
> +		printf("STAT: done!\n");

p_info()

> +	free(objs);
> +	closedir(d);

If input is a single file, "d" will not be a valid directory stream
descriptor, and I expect closedir() to fail and set errno even if
everything else went fine. The return code does not matter, but "errno
!= 0" may cause bpftool's batch mode to error out. Can you please run
closedir() only "if (d)"?

> +	return err;
> +}
> +
>  static const struct cmd cmds[] = {
>  	{ "object",	do_object },
>  	{ "skeleton",	do_skeleton },
> +	{ "btf",	do_gen_btf},
>  	{ "help",	do_help },
>  	{ 0 }
>  };

Re: [PATCH bpf-next v4 6/8] bpftool: Implement btfgen()

[Quentin Monnet]

2022-01-12 09:27 UTC-0500 ~ Mauricio Vásquez <mauricio@kinvolk.io>
> btfgen() 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 commits).
> 
> 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 saves the types involved in such relocation.
> 
> 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 | 221 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 219 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index 905ab0ee6542..cef0ea99d4d9 100644

> +static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
> +{
> +	struct btfgen_info *info;
> +	struct btf *btf_new = NULL;
> +	int err;
> +
> +	info = btfgen_new_info(src_btf);
> +	if (!info) {
> +		p_err("failed to allocate info structure: %s", strerror(errno));
> +		err = -errno;
> +		goto out;
> +	}
> +
> +	for (int i = 0; objspaths[i] != NULL; i++) {
> +		printf("OBJ : %s\n", objspaths[i]);

p_info()

> +
> +		err = btfgen_record_obj(info, objspaths[i]);
> +		if (err)
> +			goto out;
> +	}
> +
> +	btf_new = btfgen_get_btf(info);
> +	if (!btf_new) {
> +		err = -errno;
> +		p_err("error generating btf: %s", strerror(errno));
> +		goto out;
> +	}
> +
> +	printf("DBTF: %s\n", dst_btf);

p_info()

Re: [PATCH bpf-next v4 7/8] bpftool: Implement relocations recording for BTFGen

[Quentin Monnet]

2022-01-12 09:27 UTC-0500 ~ Mauricio Vásquez <mauricio@kinvolk.io>
> This commit implements the logic to record the relocation information
> for the different kind of relocations.
> 
> btfgen_record_field_relo() uses the target specification to save all the
> types that are involved in a field-based CO-RE relocation. In this case
> types resolved and added recursively (using btfgen_put_type()).
> Only the struct and union members and their types) involved in the
> relocation are added to optimize the size of the generated BTF file.
> 
> On the other hand, btfgen_record_type_relo() saves the types involved in
> a type-based CO-RE relocation. In this case all the members for the
> struct and union types are added. This is not strictly required since
> libbpf doesn't use them while performing this kind of relocation,
> however that logic could change on the future. Additionally, we expect
> that the number of this kind of relocations in an BPF object to be very
> low, hence the impact on the size of the generated BTF should be
> negligible.
> 
> Finally, btfgen_record_enumval_relo() saves 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/gen.c | 260 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 257 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index cef0ea99d4d9..8c13dde0b74d 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -1207,19 +1228,252 @@ btfgen_new_info(const char *targ_btf_path)

> +static struct btfgen_type *
> +_btfgen_put_type(struct btf *btf, struct btfgen_info *info, struct btf_type *btf_type,
> +		 unsigned int id, bool all_members)
> +{
> +	struct btfgen_type *btfgen_type, *tmp;
> +	struct btf_array *array;
> +	unsigned int child_id;
> +	struct btf_member *m;
> +	int err, i, n;
> +
> +	/* check if we already have this type */
> +	if (hashmap__find(info->types, uint_as_hash_key(id), (void **) &btfgen_type)) {
> +		if (!all_members || btfgen_type->all_members)
> +			return btfgen_type;
> +	} else {
> +		btfgen_type = calloc(1, sizeof(*btfgen_type));
> +		if (!btfgen_type)
> +			return NULL;
> +
> +		btfgen_type->type = btf_type;
> +		btfgen_type->id = id;
> +
> +		/* append this type to the types list before anything else */
> +		err = hashmap__add(info->types, uint_as_hash_key(btfgen_type->id), btfgen_type);
> +		if (err) {
> +			free(btfgen_type);
> +			return NULL;
> +		}
> +	}
> +
> +	/* avoid infinite recursion and yet be able to add all
> +	 * fields/members for types also managed by this function
> +	 */
> +	btfgen_type->all_members = all_members;
> +
> +

Nit: double blank line.

> +	/* recursively add other types needed by it */
> +	switch (btf_kind(btfgen_type->type)) {
> +	case BTF_KIND_UNKN:
> +	case BTF_KIND_INT:

>  static int btfgen_record_field_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
>  {
> -	return -EOPNOTSUPP;
> +	struct btf *btf = (struct btf *) info->src_btf;
> +	struct btfgen_type *btfgen_type;
> +	struct btf_member *btf_member;
> +	struct btf_type *btf_type;
> +	struct btf_array *array;
> +	unsigned int id;
> +	int idx, err;
> +
> +	btf_type = (struct btf_type *) btf__type_by_id(btf, targ_spec->root_type_id);
> +
> +	/* create btfgen_type for root type */
> +	btfgen_type = btfgen_put_type(btf, info, btf_type, targ_spec->root_type_id);
> +	if (!btfgen_type)
> +		return -errno;
> +
> +	/* add 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;
> +			btfgen_type = btfgen_get_type(info, id);
> +			if (!btfgen_type)
> +				return -ENOENT;
> +			btf_type = (struct 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;
> +			btf_type = (struct btf_type *) btf__type_by_id(btf, btf_member->type);
> +
> +			/* add member to relocation type */
> +			err = btfgen_add_member(btfgen_type, btf_member, idx);
> +			if (err)
> +				return err;
> +			/* put btfgen type */
> +			btfgen_type = btfgen_put_type(btf, info, btf_type, btf_member->type);
> +			if (!btfgen_type)
> +				return -errno;
> +			break;
> +		case BTF_KIND_ARRAY:
> +			array = btf_array(btf_type);
> +			btfgen_type = btfgen_get_type(info, array->type);
> +			if (!btfgen_type)
> +				return -ENOENT;
> +			btf_type = (struct btf_type *) btf__type_by_id(btf, array->type);
> +			break;
> +		default:
> +			p_err("spec type wasn't handled");

This message might benefit from a bit more context (step, type number?).

> +			return -EINVAL;
> +		}
> +	}
> +
> +	return 0;
>  }
>  
>  static int btfgen_record_type_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
>  {
> -	return -EOPNOTSUPP;
> +	struct btf *btf = (struct btf *) info->src_btf;
> +	struct btfgen_type *btfgen_type;
> +	struct btf_type *btf_type;
> +
> +	btf_type = (struct btf_type *) btf__type_by_id(btf, targ_spec->root_type_id);
> +
> +	btfgen_type = btfgen_put_type_all(btf, info, btf_type, targ_spec->root_type_id);
> +	return btfgen_type ?  0 : -errno;

Nit: double space in "?  0".

>  }
>  
>  static int btfgen_record_enumval_relo(struct btfgen_info *info, struct bpf_core_spec *targ_spec)
>  {
> -	return -EOPNOTSUPP;
> +	struct btf *btf = (struct btf *) info->src_btf;
> +	struct btfgen_type *btfgen_type;
> +	struct btf_type *btf_type;
> +
> +	btf_type = (struct btf_type *) btf__type_by_id(btf, targ_spec->root_type_id);
> +
> +	btfgen_type = btfgen_put_type_all(btf, info, btf_type, targ_spec->root_type_id);
> +	return btfgen_type ?  0 : -errno;

Nit: double space in "?  0".

>  }
>  
>  static int btfgen_record_reloc(struct btfgen_info *info, struct bpf_core_spec *res)

Re: [PATCH bpf-next v4 1/8] libbpf: split bpf_core_apply_relo()

[Andrii Nakryiko]

On Wed, Jan 12, 2022 at 6:27 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> BTFGen needs to run the core relocation logic in order to understand
> what are the types in the target BTF that 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_res() who is now only on charge of calculating the

outdated name?

> 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_res().

same?


BTW, this patch set breaks CI ([0]), please investigate

  [0] https://github.com/kernel-patches/bpf/runs/4797721812?check_suite_focus=true

>
> 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    | 84 ++++++++++++++++++++++++---------------
>  tools/lib/bpf/relo_core.c | 79 +++++++++++-------------------------
>  tools/lib/bpf/relo_core.h | 42 +++++++++++++++++---
>  4 files changed, 122 insertions(+), 96 deletions(-)
>

[...]

> @@ -5661,12 +5642,53 @@ 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);
> +                       if (prog->obj->gen_loader) {
> +                               const struct btf_type *local_type;
> +                               const char *local_name, *spec_str;
> +
> +                               spec_str = btf__name_by_offset(obj->btf, rec->access_str_off);
> +                               if (!spec_str)
> +                                       return -EINVAL;
> +
> +                               local_type = btf__type_by_id(obj->btf, rec->type_id);
> +                               if (!local_type)
> +                                       return -EINVAL;
> +
> +                               local_name = btf__name_by_offset(obj->btf, local_type->name_off);
> +                               if (!local_name)
> +                                       return -EINVAL;
> +
> +                               pr_debug("record_relo_core: prog %td insn[%d] %s %s %s final insn_idx %d\n",
> +                                       prog - prog->obj->programs, insn_idx,
> +                                       btf_kind_str(local_type), local_name, spec_str, insn_idx);

hmm, maybe let's just drop this pr_debug instead? that's a lot of code
and checks just to emit this debug info.

> +                               return record_relo_core(prog, rec, insn_idx);
> +                       }
> +
> +                       if (rec->insn_off % BPF_INSN_SZ)
> +                               return -EINVAL;
> +                       insn_idx = rec->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];
> +

This validation probably is better to do before prog->obj->gen_loader
check so that we don't silently do something bad in record_relo_core()
if insn_idx is wrong? It doesn't change the rest of the logic, right?
So there shouldn't be any harm or change of behavior.

> +                       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;
> +                       }
>                 }
>         }
>

[...]

Re: [PATCH bpf-next v4 2/8] libbpf: Implement changes needed for BTFGen in bpftool

[Andrii Nakryiko]

On Wed, Jan 12, 2022 at 6:27 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> This commit extends libbpf with the features that are needed to
> implement BTFGen:
>
> - Implement bpf_core_create_cand_cache() and bpf_core_free_cand_cache()
> to handle candidates cache.
> - Expose bpf_core_add_cands() and bpf_core_free_cands to handle
> candidates list.
> - Expose bpf_core_calc_relo_insn() to bpftool.
>
> 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/Makefile          |  2 +-
>  tools/lib/bpf/libbpf.c          | 43 +++++++++++++++++++++------------
>  tools/lib/bpf/libbpf_internal.h | 12 +++++++++
>  3 files changed, 41 insertions(+), 16 deletions(-)
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index f947b61b2107..dba019ee2832 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -239,7 +239,7 @@ install_lib: all_cmd
>
>  SRC_HDRS := bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h      \
>             bpf_helpers.h bpf_tracing.h bpf_endian.h bpf_core_read.h         \
> -           skel_internal.h libbpf_version.h
> +           skel_internal.h libbpf_version.h relo_core.h libbpf_internal.h

this is the list of public API headers, this is not the right place,
as Quentin pointed out

>  GEN_HDRS := $(BPF_GENERATED)
>
>  INSTALL_PFX := $(DESTDIR)$(prefix)/include/bpf
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 4959c03a46f4..344b8b8e8a50 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -5185,18 +5185,18 @@ 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)
>  {
>         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;
> @@ -5567,6 +5567,24 @@ static int bpf_core_resolve_relo(struct bpf_program *prog,
>                                        targ_res);
>  }
>
> +struct hashmap *bpf_core_create_cand_cache(void)
> +{
> +       return hashmap__new(bpf_core_hash_fn, bpf_core_equal_fn, NULL);
> +}
> +
> +void bpf_core_free_cand_cache(struct hashmap *cand_cache)
> +{
> +       struct hashmap_entry *entry;
> +       int i;
> +
> +       if (!IS_ERR_OR_NULL(cand_cache)) {

with separate function for clean up, it's nicer to invert the if
condition and exit early to reduce nesting


> +               hashmap__for_each_entry(cand_cache, entry, i) {
> +                       bpf_core_free_cands(entry->value);
> +               }
> +               hashmap__free(cand_cache);
> +       }
> +}
> +

[...]

Re: [PATCH bpf-next v4 3/8] bpftool: Add gen btf command

[Andrii Nakryiko]

On Wed, Jan 12, 2022 at 6:27 AM 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 btf INPUT OUTPUT OBJECT(S)

"gen btf" doesn't really convey that it's a minimized BTF for CO-RE,
maybe let's do something more verbose but also more precise, it's not
like this is going to be used by everyone multiple times a day, so
verboseness is not a bad thing here. Naming is hard, but something
like `bpftool gen min_core_btf` probably would give a bit better
pointer as to what this command is doing (minimal CO-RE BTF, right?)

>
> INPUT can be either a single BTF file or a folder containing BTF files,
> when it's a folder, a BTF file is generated for each BTF file contained
> in this folder. OUTPUT is the file (or folder) where generated files are
> stored and OBJECT(S) is the list of bpf objects we want to generate the
> BTF file(s) for (each generated BTF file contains all the types needed
> by all 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/gen.c | 117 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 117 insertions(+)
>

[...]

> +
> +       while ((dir = readdir(d)) != NULL) {
> +               if (dir->d_type != DT_REG)
> +                       continue;
> +
> +               if (strncmp(dir->d_name + strlen(dir->d_name) - 4, ".btf", 4))
> +                       continue;
> +
> +               snprintf(src_btf_path, sizeof(src_btf_path), "%s/%s", input, dir->d_name);
> +               snprintf(dst_btf_path, sizeof(dst_btf_path), "%s/%s", output, dir->d_name);
> +
> +               printf("SBTF: %s\n", src_btf_path);

What's SBTF? Is this part of bpftool "protocol" now? It should be
something a bit more meaningful...

> +
> +               err = btfgen(src_btf_path, dst_btf_path, objs);
> +               if (err)
> +                       goto out;
> +       }
> +
> +out:
> +       if (!err)
> +               printf("STAT: done!\n");

similar, STAT? what's that? Do we need "done!" message in tool's output?

> +       free(objs);
> +       closedir(d);
> +       return err;
> +}
> +
>  static const struct cmd cmds[] = {
>         { "object",     do_object },
>         { "skeleton",   do_skeleton },
> +       { "btf",        do_gen_btf},
>         { "help",       do_help },
>         { 0 }
>  };
> --
> 2.25.1
>

Re: [PATCH bpf-next v4 4/8] bpftool: Implement btf_save_raw()

[Andrii Nakryiko]

On Wed, Jan 12, 2022 at 6:27 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> Helper function to save a BTF object to a 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>
> ---
>  tools/bpf/bpftool/gen.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>

See suggestions, but either way:

Acked-by: Andrii Nakryiko <andrii@kernel.org>


> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index cdeb1047d79d..5a74fb68dc84 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> @@ -1096,6 +1096,36 @@ static int do_help(int argc, char **argv)
>         return 0;
>  }
>
> +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) {
> +               err = -ENOMEM;
> +               goto out;
> +       }

can do just return -ENOMEM instead of goto

> +
> +       f = fopen(path, "wb");
> +       if (!f) {
> +               err = -errno;
> +               goto out;
> +       }
> +
> +       if (fwrite(data, 1, data_sz, f) != data_sz) {
> +               err = -errno;
> +               goto out;
> +       }
> +
> +out:
> +       if (f)

with early return above, no need for if (f) check

> +               fclose(f);
> +       return err;
> +}
> +
>  /* Create BTF file for a set of BPF objects */
>  static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
>  {
> --
> 2.25.1
>

Re: [PATCH bpf-next v4 6/8] bpftool: Implement btfgen()

[Andrii Nakryiko]

On Wed, Jan 12, 2022 at 6:27 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
>
> btfgen() 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 commits).
>
> 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 saves the types involved in such relocation.
>
> 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 | 221 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 219 insertions(+), 2 deletions(-)
>

[...]

> +       info = btfgen_new_info(src_btf);
> +       if (!info) {
> +               p_err("failed to allocate info structure: %s", strerror(errno));
> +               err = -errno;
> +               goto out;
> +       }
> +
> +       for (int i = 0; objspaths[i] != NULL; i++) {
> +               printf("OBJ : %s\n", objspaths[i]);

same as in previous patches, OBJ, DBTF, that's quite cryptic. Is this
used for some parsing of the output or it's just debugging leftovers?

> +
> +               err = btfgen_record_obj(info, objspaths[i]);
> +               if (err)
> +                       goto out;
> +       }
> +
> +       btf_new = btfgen_get_btf(info);
> +       if (!btf_new) {
> +               err = -errno;
> +               p_err("error generating btf: %s", strerror(errno));
> +               goto out;
> +       }
> +
> +       printf("DBTF: %s\n", dst_btf);
> +       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 is_file(const char *path)
>  {
>         struct stat st;
> --
> 2.25.1
>

Re: [PATCH bpf-next v4 3/8] bpftool: Add gen btf command

[Mauricio Vásquez Bernal]

On Wed, Jan 12, 2022 at 1:08 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2022-01-12 09:27 UTC-0500 ~ Mauricio Vásquez <mauricio@kinvolk.io>
> > This command is implemented under the "gen" command in bpftool and the
> > syntax is the following:
> >
> > $ bpftool gen btf INPUT OUTPUT OBJECT(S)
>
> Thanks a lot for this work!
>
> Please update the relevant manual page under Documentation, to let users
> know how to use the feature. You may also consider adding an example at
> the end of that document.
>

We're working on it, and will be part of the next spin.

> The bash completion script should also be updated with the new "btf"
> subcommand for "gen". Given that all the arguments are directories and
> files, it should not be hard.
>

Will do.

> Have you considered adding tests for the feature? There are a few
> bpftool-related selftests under tools/testing/selftests/bpf/.
>

Yes, we have but it seems not that trivial. One idea I have is to
include a couple of BTF source files from
https://github.com/aquasecurity/btfhub-archive/ and create a test
program that generates some field, type and enum relocations. Then we
could check if the generated BTF file has the expected types, fields
and so on by parsing it and using the BTF API from libbpf. One concern
about it is the size of those two source BTF files (~5MB each),
perhaps we should not include a full file but something that is
already "trimmed"? Another possibility is to use
"/sys/kernel/btf/vmlinux" but it'll limit the test to machines with
CONFIG_DEBUG_INFO_BTF.

Do you have any ideas / feedback on this one? Should the tests be
included in this series or can we push them later on?

> >
> > INPUT can be either a single BTF file or a folder containing BTF files,
> > when it's a folder, a BTF file is generated for each BTF file contained
> > in this folder. OUTPUT is the file (or folder) where generated files are
> > stored and OBJECT(S) is the list of bpf objects we want to generate the
> > BTF file(s) for (each generated BTF file contains all the types needed
> > by all 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/gen.c | 117 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 117 insertions(+)
> >
> > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> > index 43e3f8700ecc..cdeb1047d79d 100644
> > --- a/tools/bpf/bpftool/gen.c
> > +++ b/tools/bpf/bpftool/gen.c
> > @@ -5,6 +5,7 @@
> >  #define _GNU_SOURCE
> >  #endif
> >  #include <ctype.h>
> > +#include <dirent.h>
> >  #include <errno.h>
> >  #include <fcntl.h>
> >  #include <linux/err.h>
> > @@ -1084,6 +1085,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 btf INPUT OUTPUT OBJECT(S)\n"
> >               "       %1$s %2$s help\n"
> >               "\n"
> >               "       " HELP_SPEC_OPTIONS " |\n"
> > @@ -1094,9 +1096,124 @@ static int do_help(int argc, char **argv)
> >       return 0;
> >  }
> >
> > +/* Create BTF file for a set of BPF objects */
> > +static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
> > +{
> > +     return -EOPNOTSUPP;
> > +}
> > +
> > +static int is_file(const char *path)
> > +{
> > +     struct stat st;
> > +
> > +     if (stat(path, &st) < 0)
> > +             return -1;
> > +
> > +     switch (st.st_mode & S_IFMT) {
> > +     case S_IFDIR:
> > +             return 0;
> > +     case S_IFREG:
> > +             return 1;
> > +     default:
> > +             return -1;
> > +     }
> > +}
> > +
> > +static int do_gen_btf(int argc, char **argv)
> > +{
> > +     char src_btf_path[PATH_MAX], dst_btf_path[PATH_MAX];
> > +     bool input_is_file, output_is_file = false;
> > +     const char *input, *output;
> > +     const char **objs = NULL;
> > +     struct dirent *dir;
> > +     DIR *d = NULL;
> > +     int i, err;
> > +
> > +     if (!REQ_ARGS(3)) {
> > +             usage();
> > +             return -1;
> > +     }
> > +
> > +     input = GET_ARG();
> > +     err = is_file(input);
> > +     if (err < 0) {
> > +             p_err("failed to stat %s: %s", input, strerror(errno));
> > +             return err;
> > +     }
> > +     input_is_file = err;
> > +
> > +     output = GET_ARG();
> > +     err = is_file(output);
> > +     if (err != 0)
> > +             output_is_file = true;
>
> Why not return if err < 0? This will set output_is_file to true and will
> fail later, I think?
>

I knew it was going to be confusing. is_file() returns -1 when the
file is not found, for the output one it's not a big deal as we're
going to create it, so this logic was correct. I decided to drop this
is_file() function and using stat() directly in the code.

> > +
> > +     objs = (const char **) malloc((argc + 1) * sizeof(*objs));
> > +     if (!objs)
> > +             return -ENOMEM;
>
> Let's p_err() a message.
>
> > +
> > +     i = 0;
> > +     while (argc > 0)
> > +             objs[i++] = GET_ARG();
> > +
> > +     objs[i] = NULL;
> > +
> > +     /* single BTF file */
> > +     if (input_is_file) {
> > +             printf("SBTF: %s\n", input);
>
> We can use "p_info()" instead of "printf()". In particular, this avoids
> printing the message when the JSON output is required.
>
> > +
> > +             if (output_is_file) {
> > +                     err = btfgen(input, output, objs);
> > +                     goto out;
> > +             }
> > +             snprintf(dst_btf_path, sizeof(dst_btf_path), "%s/%s", output,
> > +                      basename(input));
>
> Am I right that the output file should be just a file name, and not a
> path, and that it will be created under the same directory as the input
> file?

No exactly. There are two ways we support (1) the source BTF file is a
single file and (2) it's a folder containing BTF files.
For the first case, we require the output path to be a file or a
folder and the second case requires the output to be a folder. The
reason we don't generate the files in the same input folder is that
the user can have a library with a lot of BTF files (like
https://github.com/aquasecurity/btfhub-archive/) and you don't want to
pollute it with the files you generate.

> And that providing a relative or absolute path instead will cause
> issues here? If so, please document it. It would be nice to be able to
> support paths too, but I'm not sure how much work that represents.
>

I tried with absolute and relative paths and it seems to work fine.

> > +             err = btfgen(input, dst_btf_path, objs);
> > +             goto out;
> > +     }
> > +
> > +     if (output_is_file) {
> > +             p_err("can't have just one file as output");
>
> See comment above, this message is misleading if stat() returned with an
> error.
>
> > +             err = -EINVAL;
> > +             goto out;
> > +     }
> > +
> > +     /* directory with BTF files */
> > +     d = opendir(input);
> > +     if (!d) {
> > +             p_err("error opening input dir: %s", strerror(errno));
> > +             err = -errno;
> > +             goto out;
> > +     }
> > +
> > +     while ((dir = readdir(d)) != NULL) {
> > +             if (dir->d_type != DT_REG)
> > +                     continue;
> > +
> > +             if (strncmp(dir->d_name + strlen(dir->d_name) - 4, ".btf", 4))
> > +                     continue;
> > +
> > +             snprintf(src_btf_path, sizeof(src_btf_path), "%s/%s", input, dir->d_name);
> > +             snprintf(dst_btf_path, sizeof(dst_btf_path), "%s/%s", output, dir->d_name);
> > +
> > +             printf("SBTF: %s\n", src_btf_path);
> > +
> > +             err = btfgen(src_btf_path, dst_btf_path, objs);
> > +             if (err)
> > +                     goto out;
> > +     }
> > +
> > +out:
> > +     if (!err)
> > +             printf("STAT: done!\n");
>
> p_info()
>
> > +     free(objs);
> > +     closedir(d);
>
> If input is a single file, "d" will not be a valid directory stream
> descriptor, and I expect closedir() to fail and set errno even if
> everything else went fine. The return code does not matter, but "errno
> != 0" may cause bpftool's batch mode to error out. Can you please run
> closedir() only "if (d)"?
>

Yes, you're totally right.





> > +     return err;
> > +}
> > +
> >  static const struct cmd cmds[] = {
> >       { "object",     do_object },
> >       { "skeleton",   do_skeleton },
> > +     { "btf",        do_gen_btf},
> >       { "help",       do_help },
> >       { 0 }
> >  };
>

Re: [PATCH bpf-next v4 1/8] libbpf: split bpf_core_apply_relo()

[Mauricio Vásquez Bernal]

On Fri, Jan 14, 2022 at 9:02 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Jan 12, 2022 at 6:27 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
> > BTFGen needs to run the core relocation logic in order to understand
> > what are the types in the target BTF that 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_res() who is now only on charge of calculating the
>
> outdated name?
>
> > 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_res().
>
> same?
>
>
> BTW, this patch set breaks CI ([0]), please investigate
>
>   [0] https://github.com/kernel-patches/bpf/runs/4797721812?check_suite_focus=true
>
> >
> > 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    | 84 ++++++++++++++++++++++++---------------
> >  tools/lib/bpf/relo_core.c | 79 +++++++++++-------------------------
> >  tools/lib/bpf/relo_core.h | 42 +++++++++++++++++---
> >  4 files changed, 122 insertions(+), 96 deletions(-)
> >
>
> [...]
>
> > @@ -5661,12 +5642,53 @@ 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);
> > +                       if (prog->obj->gen_loader) {
> > +                               const struct btf_type *local_type;
> > +                               const char *local_name, *spec_str;
> > +
> > +                               spec_str = btf__name_by_offset(obj->btf, rec->access_str_off);
> > +                               if (!spec_str)
> > +                                       return -EINVAL;
> > +
> > +                               local_type = btf__type_by_id(obj->btf, rec->type_id);
> > +                               if (!local_type)
> > +                                       return -EINVAL;
> > +
> > +                               local_name = btf__name_by_offset(obj->btf, local_type->name_off);
> > +                               if (!local_name)
> > +                                       return -EINVAL;
> > +
> > +                               pr_debug("record_relo_core: prog %td insn[%d] %s %s %s final insn_idx %d\n",
> > +                                       prog - prog->obj->programs, insn_idx,
> > +                                       btf_kind_str(local_type), local_name, spec_str, insn_idx);
>
> hmm, maybe let's just drop this pr_debug instead? that's a lot of code
> and checks just to emit this debug info.
>

Makes sense.

> > +                               return record_relo_core(prog, rec, insn_idx);

This was the reason why the CI was failing. This should not "return"
but "continue" here, otherwise only a single relocation will be
recorded.

> > +                       }
> > +
> > +                       if (rec->insn_off % BPF_INSN_SZ)
> > +                               return -EINVAL;
> > +                       insn_idx = rec->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];
> > +
>
> This validation probably is better to do before prog->obj->gen_loader
> check so that we don't silently do something bad in record_relo_core()
> if insn_idx is wrong? It doesn't change the rest of the logic, right?
> So there shouldn't be any harm or change of behavior.
>

I agree.


> > +                       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;
> > +                       }
> >                 }
> >         }
> >
>
> [...]

Re: [PATCH bpf-next v4 3/8] bpftool: Add gen btf command

[Mauricio Vásquez Bernal]

On Fri, Jan 14, 2022 at 9:09 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Jan 12, 2022 at 6:27 AM 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 btf INPUT OUTPUT OBJECT(S)
>
> "gen btf" doesn't really convey that it's a minimized BTF for CO-RE,
> maybe let's do something more verbose but also more precise, it's not
> like this is going to be used by everyone multiple times a day, so
> verboseness is not a bad thing here. Naming is hard, but something
> like `bpftool gen min_core_btf` probably would give a bit better
> pointer as to what this command is doing (minimal CO-RE BTF, right?)
>

I agree. `bpftool gen min_core_btf` seems just fine to us.

> >
> > INPUT can be either a single BTF file or a folder containing BTF files,
> > when it's a folder, a BTF file is generated for each BTF file contained
> > in this folder. OUTPUT is the file (or folder) where generated files are
> > stored and OBJECT(S) is the list of bpf objects we want to generate the
> > BTF file(s) for (each generated BTF file contains all the types needed
> > by all 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/gen.c | 117 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 117 insertions(+)
> >
>
> [...]
>
> > +
> > +       while ((dir = readdir(d)) != NULL) {
> > +               if (dir->d_type != DT_REG)
> > +                       continue;
> > +
> > +               if (strncmp(dir->d_name + strlen(dir->d_name) - 4, ".btf", 4))
> > +                       continue;
> > +
> > +               snprintf(src_btf_path, sizeof(src_btf_path), "%s/%s", input, dir->d_name);
> > +               snprintf(dst_btf_path, sizeof(dst_btf_path), "%s/%s", output, dir->d_name);
> > +
> > +               printf("SBTF: %s\n", src_btf_path);
>
> What's SBTF? Is this part of bpftool "protocol" now? It should be
> something a bit more meaningful...
>

These are almost debug leftovers. I updated them to be more meaningful.



> > +
> > +               err = btfgen(src_btf_path, dst_btf_path, objs);
> > +               if (err)
> > +                       goto out;
> > +       }
> > +
> > +out:
> > +       if (!err)
> > +               printf("STAT: done!\n");
>
> similar, STAT? what's that? Do we need "done!" message in tool's output?
>
> > +       free(objs);
> > +       closedir(d);
> > +       return err;
> > +}
> > +
> >  static const struct cmd cmds[] = {
> >         { "object",     do_object },
> >         { "skeleton",   do_skeleton },
> > +       { "btf",        do_gen_btf},
> >         { "help",       do_help },
> >         { 0 }
> >  };
> > --
> > 2.25.1
> >

Re: [PATCH bpf-next v4 2/8] libbpf: Implement changes needed for BTFGen in bpftool

[Mauricio Vásquez Bernal]

On Wed, Jan 12, 2022 at 1:08 PM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2022-01-12 09:27 UTC-0500 ~ Mauricio Vásquez <mauricio@kinvolk.io>
> > This commit extends libbpf with the features that are needed to
> > implement BTFGen:
> >
> > - Implement bpf_core_create_cand_cache() and bpf_core_free_cand_cache()
> > to handle candidates cache.
> > - Expose bpf_core_add_cands() and bpf_core_free_cands to handle
> > candidates list.
> > - Expose bpf_core_calc_relo_insn() to bpftool.
> >
> > 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/Makefile          |  2 +-
> >  tools/lib/bpf/libbpf.c          | 43 +++++++++++++++++++++------------
> >  tools/lib/bpf/libbpf_internal.h | 12 +++++++++
> >  3 files changed, 41 insertions(+), 16 deletions(-)
> >
> > diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> > index f947b61b2107..dba019ee2832 100644
> > --- a/tools/lib/bpf/Makefile
> > +++ b/tools/lib/bpf/Makefile
> > @@ -239,7 +239,7 @@ install_lib: all_cmd
> >
> >  SRC_HDRS := bpf.h libbpf.h btf.h libbpf_common.h libbpf_legacy.h xsk.h            \
> >           bpf_helpers.h bpf_tracing.h bpf_endian.h bpf_core_read.h         \
> > -         skel_internal.h libbpf_version.h
> > +         skel_internal.h libbpf_version.h relo_core.h libbpf_internal.h
> >  GEN_HDRS := $(BPF_GENERATED)
>
> I don't think these headers should be added to libbpf's SRC_HDRS. If we
> must make them available to bpftool, we probably want to copy them
> explicitly through LIBBPF_INTERNAL_HDRS in bpftool's Makefile.

I got confused, thanks for catching this up!

Re: [PATCH bpf-next v4 4/8] bpftool: Implement btf_save_raw()

[Mauricio Vásquez Bernal]

On Fri, Jan 14, 2022 at 9:10 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Jan 12, 2022 at 6:27 AM Mauricio Vásquez <mauricio@kinvolk.io> wrote:
> >
> > Helper function to save a BTF object to a 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>
> > ---
> >  tools/bpf/bpftool/gen.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
>
> See suggestions, but either way:
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
>
>
> > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> > index cdeb1047d79d..5a74fb68dc84 100644
> > --- a/tools/bpf/bpftool/gen.c
> > +++ b/tools/bpf/bpftool/gen.c
> > @@ -1096,6 +1096,36 @@ static int do_help(int argc, char **argv)
> >         return 0;
> >  }
> >
> > +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) {
> > +               err = -ENOMEM;
> > +               goto out;
> > +       }
>
> can do just return -ENOMEM instead of goto
>
> > +
> > +       f = fopen(path, "wb");
> > +       if (!f) {
> > +               err = -errno;
> > +               goto out;
> > +       }
> > +
> > +       if (fwrite(data, 1, data_sz, f) != data_sz) {
> > +               err = -errno;
> > +               goto out;
> > +       }
> > +
> > +out:
> > +       if (f)
>
> with early return above, no need for if (f) check
>

After those suggestions I decided to completely remove the out label.



> > +               fclose(f);
> > +       return err;
> > +}
> > +
> >  /* Create BTF file for a set of BPF objects */
> >  static int btfgen(const char *src_btf, const char *dst_btf, const char *objspaths[])
> >  {
> > --
> > 2.25.1
> >

Re: [PATCH bpf-next v4 3/8] bpftool: Add gen btf command

[Andrii Nakryiko]

On Fri, Jan 21, 2022 at 12:40 PM Mauricio Vásquez Bernal
<mauricio@kinvolk.io> wrote:
>
> On Wed, Jan 12, 2022 at 1:08 PM Quentin Monnet <quentin@isovalent.com> wrote:
> >
> > 2022-01-12 09:27 UTC-0500 ~ Mauricio Vásquez <mauricio@kinvolk.io>
> > > This command is implemented under the "gen" command in bpftool and the
> > > syntax is the following:
> > >
> > > $ bpftool gen btf INPUT OUTPUT OBJECT(S)
> >
> > Thanks a lot for this work!
> >
> > Please update the relevant manual page under Documentation, to let users
> > know how to use the feature. You may also consider adding an example at
> > the end of that document.
> >
>
> We're working on it, and will be part of the next spin.
>
> > The bash completion script should also be updated with the new "btf"
> > subcommand for "gen". Given that all the arguments are directories and
> > files, it should not be hard.
> >
>
> Will do.
>
> > Have you considered adding tests for the feature? There are a few
> > bpftool-related selftests under tools/testing/selftests/bpf/.
> >
>
> Yes, we have but it seems not that trivial. One idea I have is to
> include a couple of BTF source files from
> https://github.com/aquasecurity/btfhub-archive/ and create a test
> program that generates some field, type and enum relocations. Then we
> could check if the generated BTF file has the expected types, fields
> and so on by parsing it and using the BTF API from libbpf. One concern
> about it is the size of those two source BTF files (~5MB each),
> perhaps we should not include a full file but something that is
> already "trimmed"? Another possibility is to use
> "/sys/kernel/btf/vmlinux" but it'll limit the test to machines with
> CONFIG_DEBUG_INFO_BTF.
>
> Do you have any ideas / feedback on this one? Should the tests be
> included in this series or can we push them later on?

See how we test CO-RE relocations and how we use C files to get custom
BTFs. See progs/btf_* and prog_tests/core_reloc.c selftests. You don't
have to use real vmlinux BTF to test this functionality.


>
> > >
> > > INPUT can be either a single BTF file or a folder containing BTF files,
> > > when it's a folder, a BTF file is generated for each BTF file contained
> > > in this folder. OUTPUT is the file (or folder) where generated files are
> > > stored and OBJECT(S) is the list of bpf objects we want to generate the
> > > BTF file(s) for (each generated BTF file contains all the types needed
> > > by all 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/gen.c | 117 ++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 117 insertions(+)
> > >

[...]