All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 00/10] BPF static linking
@ 2021-03-10  4:04 Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 01/10] libbpf: expose btf_type_by_id() internally Andrii Nakryiko
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

This patch set adds new libbpf APIs and their bpftool integration that allows
to perform static linking of BPF object files. Currently no extern resolution
across object files is performed. This is going to be the focus of the follow
up patches. But, given amount of code and logic necessary to perform just
basic functionality of linking together mostly independent BPF object files,
it was decided to land basic BPF linker code and logic first and extend it
afterwards.

The motivation for BPF static linking is to provide the functionality that is
naturally assumed for user-space development process: ability to structure
application's code without artificial restrictions of having all the code and
data (variables and maps) inside a single source code file.

This enables better engineering practices of splitting code into
well-encapsulated parts. It provides ability to hide internal state from other
parts of the code base through static variables and maps. It is also a first
steps towards having generic reusable BPF libraries.

Please see individual patches (mostly #5 and #6) for more details. Patch #9
passes all test_progs' individual BPF .o files through BPF static linker,
which is supposed to be a no-op operation, so is essentially validating that
BPF static linker doesn't produce corrupted ELF object files. Patch #10 adds
Makefile infra to be able to specify multi-file BPF object files and adds the
first multi-file test to validate correctness.

Andrii Nakryiko (10):
  libbpf: expose btf_type_by_id() internally
  libbpf: add internal helper to get raw BTF strings section
  libbpf: generalize BTF and BTF.ext type ID and strings iteration
  libbpf: add generic BTF type shallow copy API
  libbpf: add BPF static linker APIs
  libbpf: add BPF static linker BTF and BTF.ext support
  bpftool: add `gen bpfo` command to perform BPF static linking
  selftests/bpf: re-generate vmlinux.h and BPF skeletons if bpftool
    changed
  selftests/bpf: pass all BPF .o's through BPF static linker
  selftests/bpf: add multi-file statically linked BPF object file test

 tools/bpf/bpftool/gen.c                       |   46 +-
 tools/lib/bpf/Build                           |    2 +-
 tools/lib/bpf/btf.c                           |  454 ++--
 tools/lib/bpf/btf.h                           |    2 +
 tools/lib/bpf/libbpf.c                        |   11 +-
 tools/lib/bpf/libbpf.h                        |   13 +
 tools/lib/bpf/libbpf.map                      |    5 +
 tools/lib/bpf/libbpf_internal.h               |   33 +
 tools/lib/bpf/linker.c                        | 1945 +++++++++++++++++
 tools/testing/selftests/bpf/.gitignore        |    1 +
 tools/testing/selftests/bpf/Makefile          |   21 +-
 .../selftests/bpf/prog_tests/static_linked.c  |   40 +
 .../selftests/bpf/progs/test_static_linked1.c |   30 +
 .../selftests/bpf/progs/test_static_linked2.c |   31 +
 14 files changed, 2444 insertions(+), 190 deletions(-)
 create mode 100644 tools/lib/bpf/linker.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/static_linked.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_static_linked1.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_static_linked2.c

-- 
2.24.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH bpf-next 01/10] libbpf: expose btf_type_by_id() internally
  2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
@ 2021-03-10  4:04 ` Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 02/10] libbpf: add internal helper to get raw BTF strings section Andrii Nakryiko
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

btf_type_by_id() is internal-only convenience API returning non-const pointer
to struct btf_type. Expose it outside of btf.c for re-use.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/btf.c             | 2 +-
 tools/lib/bpf/libbpf_internal.h | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 3aa58f2ac183..e0b0a78b04fe 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -435,7 +435,7 @@ const struct btf *btf__base_btf(const struct btf *btf)
 }
 
 /* internal helper returning non-const pointer to a type */
-static struct btf_type *btf_type_by_id(struct btf *btf, __u32 type_id)
+struct btf_type *btf_type_by_id(struct btf *btf, __u32 type_id)
 {
 	if (type_id == 0)
 		return &btf_void;
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 343f6eb05637..d09860e435c8 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -107,6 +107,11 @@ static inline void *libbpf_reallocarray(void *ptr, size_t nmemb, size_t size)
 	return realloc(ptr, total);
 }
 
+struct btf;
+struct btf_type;
+
+struct btf_type *btf_type_by_id(struct btf *btf, __u32 type_id);
+
 void *btf_add_mem(void **data, size_t *cap_cnt, size_t elem_sz,
 		  size_t cur_cnt, size_t max_cnt, size_t add_cnt);
 int btf_ensure_mem(void **data, size_t *cap_cnt, size_t elem_sz, size_t need_cnt);
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next 02/10] libbpf: add internal helper to get raw BTF strings section
  2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 01/10] libbpf: expose btf_type_by_id() internally Andrii Nakryiko
@ 2021-03-10  4:04 ` Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 03/10] libbpf: generalize BTF and BTF.ext type ID and strings iteration Andrii Nakryiko
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

struct btf is an efficient and convenient data structure to be used as a set
of deduplicated strings. This patch adds libbpf-internal btf_raw_str() helper
that gives access to strings section raw data (regardless of whether BTF
object is read-only or writeable) and its size in bytes. This is going to be
used by bpf_linker to implement ELF string table section.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/btf.c             | 11 +++++++++++
 tools/lib/bpf/libbpf_internal.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index e0b0a78b04fe..6ee82ffcf3ff 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1296,6 +1296,17 @@ static void *btf_get_raw_data(const struct btf *btf, __u32 *size, bool swap_endi
 	return NULL;
 }
 
+/*
+ * Internal helper to get the size and direct pointer to strings section.
+ * This is used in cases where struct btf is used as an efficient and
+ * convenient strings container (e.g., bpf_linker).
+ */
+const void *btf_raw_strs(const struct btf *btf, size_t *size)
+{
+	*size = btf->hdr->str_len;
+	return btf->strs_data;
+}
+
 const void *btf__get_raw_data(const struct btf *btf_ro, __u32 *size)
 {
 	struct btf *btf = (struct btf *)btf_ro;
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index d09860e435c8..069250e8e871 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -115,6 +115,7 @@ struct btf_type *btf_type_by_id(struct btf *btf, __u32 type_id);
 void *btf_add_mem(void **data, size_t *cap_cnt, size_t elem_sz,
 		  size_t cur_cnt, size_t max_cnt, size_t add_cnt);
 int btf_ensure_mem(void **data, size_t *cap_cnt, size_t elem_sz, size_t need_cnt);
+const void *btf_raw_strs(const struct btf *btf, size_t *size);
 
 static inline bool libbpf_validate_opts(const char *opts,
 					size_t opts_sz, size_t user_sz,
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next 03/10] libbpf: generalize BTF and BTF.ext type ID and strings iteration
  2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 01/10] libbpf: expose btf_type_by_id() internally Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 02/10] libbpf: add internal helper to get raw BTF strings section Andrii Nakryiko
@ 2021-03-10  4:04 ` Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 04/10] libbpf: add generic BTF type shallow copy API Andrii Nakryiko
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

Extract and generalize the logic to iterate BTF type ID and string offset
fields within BTF types and .BTF.ext data. Expose this internally in libbpf
for re-use by bpf_linker.

Additionally, complete strings deduplication handling for BTF.ext (e.g., CO-RE
access strings), which was previously missing. There previously was no
case of deduplicating .BTF.ext data, but bpf_linker is going to use it.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/btf.c             | 393 ++++++++++++++++++--------------
 tools/lib/bpf/libbpf_internal.h |   7 +
 2 files changed, 228 insertions(+), 172 deletions(-)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 6ee82ffcf3ff..6e7781d3c458 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -3166,95 +3166,28 @@ static struct btf_dedup *btf_dedup_new(struct btf *btf, struct btf_ext *btf_ext,
 	return d;
 }
 
-typedef int (*str_off_fn_t)(__u32 *str_off_ptr, void *ctx);
-
 /*
  * Iterate over all possible places in .BTF and .BTF.ext that can reference
  * string and pass pointer to it to a provided callback `fn`.
  */
-static int btf_for_each_str_off(struct btf_dedup *d, str_off_fn_t fn, void *ctx)
+static int btf_for_each_str_off(struct btf_dedup *d, str_off_visit_fn fn, void *ctx)
 {
-	void *line_data_cur, *line_data_end;
-	int i, j, r, rec_size;
-	struct btf_type *t;
+	int i, r;
 
 	for (i = 0; i < d->btf->nr_types; i++) {
-		t = btf_type_by_id(d->btf, d->btf->start_id + i);
-		r = fn(&t->name_off, ctx);
+		struct btf_type *t = btf_type_by_id(d->btf, d->btf->start_id + i);
+
+		r = btf_type_visit_str_offs(t, fn, ctx);
 		if (r)
 			return r;
-
-		switch (btf_kind(t)) {
-		case BTF_KIND_STRUCT:
-		case BTF_KIND_UNION: {
-			struct btf_member *m = btf_members(t);
-			__u16 vlen = btf_vlen(t);
-
-			for (j = 0; j < vlen; j++) {
-				r = fn(&m->name_off, ctx);
-				if (r)
-					return r;
-				m++;
-			}
-			break;
-		}
-		case BTF_KIND_ENUM: {
-			struct btf_enum *m = btf_enum(t);
-			__u16 vlen = btf_vlen(t);
-
-			for (j = 0; j < vlen; j++) {
-				r = fn(&m->name_off, ctx);
-				if (r)
-					return r;
-				m++;
-			}
-			break;
-		}
-		case BTF_KIND_FUNC_PROTO: {
-			struct btf_param *m = btf_params(t);
-			__u16 vlen = btf_vlen(t);
-
-			for (j = 0; j < vlen; j++) {
-				r = fn(&m->name_off, ctx);
-				if (r)
-					return r;
-				m++;
-			}
-			break;
-		}
-		default:
-			break;
-		}
 	}
 
 	if (!d->btf_ext)
 		return 0;
 
-	line_data_cur = d->btf_ext->line_info.info;
-	line_data_end = d->btf_ext->line_info.info + d->btf_ext->line_info.len;
-	rec_size = d->btf_ext->line_info.rec_size;
-
-	while (line_data_cur < line_data_end) {
-		struct btf_ext_info_sec *sec = line_data_cur;
-		struct bpf_line_info_min *line_info;
-		__u32 num_info = sec->num_info;
-
-		r = fn(&sec->sec_name_off, ctx);
-		if (r)
-			return r;
-
-		line_data_cur += sizeof(struct btf_ext_info_sec);
-		for (i = 0; i < num_info; i++) {
-			line_info = line_data_cur;
-			r = fn(&line_info->file_name_off, ctx);
-			if (r)
-				return r;
-			r = fn(&line_info->line_off, ctx);
-			if (r)
-				return r;
-			line_data_cur += rec_size;
-		}
-	}
+	r = btf_ext_visit_str_offs(d->btf_ext, fn, ctx);
+	if (r)
+		return r;
 
 	return 0;
 }
@@ -4509,15 +4442,18 @@ static int btf_dedup_compact_types(struct btf_dedup *d)
  * then mapping it to a deduplicated type ID, stored in btf_dedup->hypot_map,
  * which is populated during compaction phase.
  */
-static int btf_dedup_remap_type_id(struct btf_dedup *d, __u32 type_id)
+static int btf_dedup_remap_type_id(__u32 *type_id, void *ctx)
 {
+	struct btf_dedup *d = ctx;
 	__u32 resolved_type_id, new_type_id;
 
-	resolved_type_id = resolve_type_id(d, type_id);
+	resolved_type_id = resolve_type_id(d, *type_id);
 	new_type_id = d->hypot_map[resolved_type_id];
 	if (new_type_id > BTF_MAX_NR_TYPES)
 		return -EINVAL;
-	return new_type_id;
+
+	*type_id = new_type_id;
+	return 0;
 }
 
 /*
@@ -4530,109 +4466,25 @@ static int btf_dedup_remap_type_id(struct btf_dedup *d, __u32 type_id)
  * referenced from any BTF type (e.g., struct fields, func proto args, etc) to
  * their final deduped type IDs.
  */
-static int btf_dedup_remap_type(struct btf_dedup *d, __u32 type_id)
+static int btf_dedup_remap_types(struct btf_dedup *d)
 {
-	struct btf_type *t = btf_type_by_id(d->btf, type_id);
 	int i, r;
 
-	switch (btf_kind(t)) {
-	case BTF_KIND_INT:
-	case BTF_KIND_ENUM:
-	case BTF_KIND_FLOAT:
-		break;
-
-	case BTF_KIND_FWD:
-	case BTF_KIND_CONST:
-	case BTF_KIND_VOLATILE:
-	case BTF_KIND_RESTRICT:
-	case BTF_KIND_PTR:
-	case BTF_KIND_TYPEDEF:
-	case BTF_KIND_FUNC:
-	case BTF_KIND_VAR:
-		r = btf_dedup_remap_type_id(d, t->type);
-		if (r < 0)
-			return r;
-		t->type = r;
-		break;
-
-	case BTF_KIND_ARRAY: {
-		struct btf_array *arr_info = btf_array(t);
-
-		r = btf_dedup_remap_type_id(d, arr_info->type);
-		if (r < 0)
-			return r;
-		arr_info->type = r;
-		r = btf_dedup_remap_type_id(d, arr_info->index_type);
-		if (r < 0)
-			return r;
-		arr_info->index_type = r;
-		break;
-	}
-
-	case BTF_KIND_STRUCT:
-	case BTF_KIND_UNION: {
-		struct btf_member *member = btf_members(t);
-		__u16 vlen = btf_vlen(t);
-
-		for (i = 0; i < vlen; i++) {
-			r = btf_dedup_remap_type_id(d, member->type);
-			if (r < 0)
-				return r;
-			member->type = r;
-			member++;
-		}
-		break;
-	}
-
-	case BTF_KIND_FUNC_PROTO: {
-		struct btf_param *param = btf_params(t);
-		__u16 vlen = btf_vlen(t);
+	for (i = 0; i < d->btf->nr_types; i++) {
+		struct btf_type *t = btf_type_by_id(d->btf, d->btf->start_id + i);
 
-		r = btf_dedup_remap_type_id(d, t->type);
-		if (r < 0)
+		r = btf_type_visit_type_ids(t, btf_dedup_remap_type_id, d);
+		if (r)
 			return r;
-		t->type = r;
-
-		for (i = 0; i < vlen; i++) {
-			r = btf_dedup_remap_type_id(d, param->type);
-			if (r < 0)
-				return r;
-			param->type = r;
-			param++;
-		}
-		break;
-	}
-
-	case BTF_KIND_DATASEC: {
-		struct btf_var_secinfo *var = btf_var_secinfos(t);
-		__u16 vlen = btf_vlen(t);
-
-		for (i = 0; i < vlen; i++) {
-			r = btf_dedup_remap_type_id(d, var->type);
-			if (r < 0)
-				return r;
-			var->type = r;
-			var++;
-		}
-		break;
-	}
-
-	default:
-		return -EINVAL;
 	}
 
-	return 0;
-}
+	if (!d->btf_ext)
+		return 0;
 
-static int btf_dedup_remap_types(struct btf_dedup *d)
-{
-	int i, r;
+	r = btf_ext_visit_type_ids(d->btf_ext, btf_dedup_remap_type_id, d);
+	if (r)
+		return r;
 
-	for (i = 0; i < d->btf->nr_types; i++) {
-		r = btf_dedup_remap_type(d, d->btf->start_id + i);
-		if (r < 0)
-			return r;
-	}
 	return 0;
 }
 
@@ -4686,3 +4538,200 @@ struct btf *libbpf_find_kernel_btf(void)
 	pr_warn("failed to find valid kernel BTF\n");
 	return ERR_PTR(-ESRCH);
 }
+
+int btf_type_visit_type_ids(struct btf_type *t, type_id_visit_fn visit, void *ctx)
+{
+	int i, n, err;
+
+	switch (btf_kind(t)) {
+	case BTF_KIND_INT:
+	case BTF_KIND_FLOAT:
+	case BTF_KIND_ENUM:
+		return 0;
+
+	case BTF_KIND_FWD:
+	case BTF_KIND_CONST:
+	case BTF_KIND_VOLATILE:
+	case BTF_KIND_RESTRICT:
+	case BTF_KIND_PTR:
+	case BTF_KIND_TYPEDEF:
+	case BTF_KIND_FUNC:
+	case BTF_KIND_VAR:
+		return visit(&t->type, ctx);
+
+	case BTF_KIND_ARRAY: {
+		struct btf_array *a = btf_array(t);
+
+		err = visit(&a->type, ctx);
+		err = err ?: visit(&a->index_type, ctx);
+		return err;
+	}
+
+	case BTF_KIND_STRUCT:
+	case BTF_KIND_UNION: {
+		struct btf_member *m = btf_members(t);
+
+		for (i = 0, n = btf_vlen(t); i < n; i++, m++) {
+			err = visit(&m->type, ctx);
+			if (err)
+				return err;
+		}
+		return 0;
+	}
+
+	case BTF_KIND_FUNC_PROTO: {
+		struct btf_param *m = btf_params(t);
+
+		err = visit(&t->type, ctx);
+		if (err)
+			return err;
+		for (i = 0, n = btf_vlen(t); i < n; i++, m++) {
+			err = visit(&m->type, ctx);
+			if (err)
+				return err;
+		}
+		return 0;
+	}
+
+	case BTF_KIND_DATASEC: {
+		struct btf_var_secinfo *m = btf_var_secinfos(t);
+
+		for (i = 0, n = btf_vlen(t); i < n; i++, m++) {
+			err = visit(&m->type, ctx);
+			if (err)
+				return err;
+		}
+		return 0;
+	}
+
+	default:
+		return -EINVAL;
+	}
+}
+
+int btf_type_visit_str_offs(struct btf_type *t, str_off_visit_fn visit, void *ctx)
+{
+	int i, n, err;
+
+	err = visit(&t->name_off, ctx);
+	if (err)
+		return err;
+
+	switch (btf_kind(t)) {
+	case BTF_KIND_STRUCT:
+	case BTF_KIND_UNION: {
+		struct btf_member *m = btf_members(t);
+
+		for (i = 0, n = btf_vlen(t); i < n; i++, m++) {
+			err = visit(&m->name_off, ctx);
+			if (err)
+				return err;
+		}
+		break;
+	}
+	case BTF_KIND_ENUM: {
+		struct btf_enum *m = btf_enum(t);
+
+		for (i = 0, n = btf_vlen(t); i < n; i++, m++) {
+			err = visit(&m->name_off, ctx);
+			if (err)
+				return err;
+		}
+		break;
+	}
+	case BTF_KIND_FUNC_PROTO: {
+		struct btf_param *m = btf_params(t);
+
+		for (i = 0, n = btf_vlen(t); i < n; i++, m++) {
+			err = visit(&m->name_off, ctx);
+			if (err)
+				return err;
+		}
+		break;
+	}
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+int btf_ext_visit_type_ids(struct btf_ext *btf_ext, type_id_visit_fn visit, void *ctx)
+{
+	const struct btf_ext_info *seg;
+	struct btf_ext_info_sec *sec;
+	int i, err;
+
+	seg = &btf_ext->func_info;
+	for_each_btf_ext_sec(seg, sec) {
+		struct bpf_func_info_min *rec;
+
+		for_each_btf_ext_rec(seg, sec, i, rec) {
+			err = visit(&rec->type_id, ctx);
+			if (err < 0)
+				return err;
+		}
+	}
+
+	seg = &btf_ext->core_relo_info;
+	for_each_btf_ext_sec(seg, sec) {
+		struct bpf_core_relo *rec;
+
+		for_each_btf_ext_rec(seg, sec, i, rec) {
+			err = visit(&rec->type_id, ctx);
+			if (err < 0)
+				return err;
+		}
+	}
+
+	return 0;
+}
+
+int btf_ext_visit_str_offs(struct btf_ext *btf_ext, str_off_visit_fn visit, void *ctx)
+{
+	const struct btf_ext_info *seg;
+	struct btf_ext_info_sec *sec;
+	int i, err;
+
+	seg = &btf_ext->func_info;
+	for_each_btf_ext_sec(seg, sec) {
+		err = visit(&sec->sec_name_off, ctx);
+		if (err)
+			return err;
+	}
+
+	seg = &btf_ext->line_info;
+	for_each_btf_ext_sec(seg, sec) {
+		struct bpf_line_info_min *rec;
+
+		err = visit(&sec->sec_name_off, ctx);
+		if (err)
+			return err;
+
+		for_each_btf_ext_rec(seg, sec, i, rec) {
+			err = visit(&rec->file_name_off, ctx);
+			if (err)
+				return err;
+			err = visit(&rec->line_off, ctx);
+			if (err)
+				return err;
+		}
+	}
+
+	seg = &btf_ext->core_relo_info;
+	for_each_btf_ext_sec(seg, sec) {
+		struct bpf_core_relo *rec;
+
+		err = visit(&sec->sec_name_off, ctx);
+		if (err)
+			return err;
+
+		for_each_btf_ext_rec(seg, sec, i, rec) {
+			err = visit(&rec->access_str_off, ctx);
+			if (err)
+				return err;
+		}
+	}
+
+	return 0;
+}
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 069250e8e871..2753494ac8ab 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -357,4 +357,11 @@ struct bpf_core_relo {
 	enum bpf_core_relo_kind kind;
 };
 
+typedef int (*type_id_visit_fn)(__u32 *type_id, void *ctx);
+typedef int (*str_off_visit_fn)(__u32 *str_off, void *ctx);
+int btf_type_visit_type_ids(struct btf_type *t, type_id_visit_fn visit, void *ctx);
+int btf_type_visit_str_offs(struct btf_type *t, str_off_visit_fn visit, void *ctx);
+int btf_ext_visit_type_ids(struct btf_ext *btf_ext, type_id_visit_fn visit, void *ctx);
+int btf_ext_visit_str_offs(struct btf_ext *btf_ext, str_off_visit_fn visit, void *ctx);
+
 #endif /* __LIBBPF_LIBBPF_INTERNAL_H */
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next 04/10] libbpf: add generic BTF type shallow copy API
  2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
                   ` (2 preceding siblings ...)
  2021-03-10  4:04 ` [PATCH bpf-next 03/10] libbpf: generalize BTF and BTF.ext type ID and strings iteration Andrii Nakryiko
@ 2021-03-10  4:04 ` Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 05/10] libbpf: add BPF static linker APIs Andrii Nakryiko
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

Add btf__add_type() API that performs shallow copy of a given BTF type from
the source BTF into the destination BTF. All the information and type IDs are
preserved, but all the strings encountered are added into the destination BTF
and corresponding offsets are rewritten. BTF type IDs are assumed to be
correct or such that will be (somehow) modified afterwards.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/btf.c      | 48 ++++++++++++++++++++++++++++++++++++++++
 tools/lib/bpf/btf.h      |  2 ++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 51 insertions(+)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 6e7781d3c458..a51a3d6dd9a9 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1722,6 +1722,54 @@ static int btf_commit_type(struct btf *btf, int data_sz)
 	return btf->start_id + btf->nr_types - 1;
 }
 
+struct btf_pipe {
+	const struct btf *src;
+	struct btf *dst;
+};
+
+static int btf_rewrite_str(__u32 *str_off, void *ctx)
+{
+	struct btf_pipe *p = ctx;
+	int off;
+
+	if (!*str_off) /* nothing to do for empty strings */
+		return 0;
+
+	off = btf__add_str(p->dst, btf__str_by_offset(p->src, *str_off));
+	if (off < 0)
+		return off;
+
+	*str_off = off;
+	return 0;
+}
+
+int btf__add_type(struct btf *btf, const struct btf *src_btf, const struct btf_type *src_type)
+{
+	struct btf_pipe p = { .src = src_btf, .dst = btf };
+	struct btf_type *t;
+	int sz, err;
+
+	sz = btf_type_size(src_type);
+	if (sz < 0)
+		return sz;
+
+	/* deconstruct BTF, if necessary, and invalidate raw_data */
+	if (btf_ensure_modifiable(btf))
+		return -ENOMEM;
+
+	t = btf_add_type_mem(btf, sz);
+	if (!t)
+		return -ENOMEM;
+
+	memcpy(t, src_type, sz);
+
+	err = btf_type_visit_str_offs(t, btf_rewrite_str, &p);
+	if (err)
+		return err;
+
+	return btf_commit_type(btf, sz);
+}
+
 /*
  * Append new BTF_KIND_INT type with:
  *   - *name* - non-empty, non-NULL type name;
diff --git a/tools/lib/bpf/btf.h b/tools/lib/bpf/btf.h
index 029a9cfc8c2d..3b0b17ba94a1 100644
--- a/tools/lib/bpf/btf.h
+++ b/tools/lib/bpf/btf.h
@@ -93,6 +93,8 @@ LIBBPF_API struct btf *libbpf_find_kernel_btf(void);
 
 LIBBPF_API int btf__find_str(struct btf *btf, const char *s);
 LIBBPF_API int btf__add_str(struct btf *btf, const char *s);
+LIBBPF_API int btf__add_type(struct btf *btf, const struct btf *src_btf,
+			     const struct btf_type *src_type);
 
 LIBBPF_API int btf__add_int(struct btf *btf, const char *name, size_t byte_sz, int encoding);
 LIBBPF_API int btf__add_float(struct btf *btf, const char *name, size_t byte_sz);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index ec898f464ab9..d31d8c968097 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -354,4 +354,5 @@ LIBBPF_0.3.0 {
 LIBBPF_0.4.0 {
 	global:
 		btf__add_float;
+		btf__add_type;
 } LIBBPF_0.3.0;
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next 05/10] libbpf: add BPF static linker APIs
  2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
                   ` (3 preceding siblings ...)
  2021-03-10  4:04 ` [PATCH bpf-next 04/10] libbpf: add generic BTF type shallow copy API Andrii Nakryiko
@ 2021-03-10  4:04 ` Andrii Nakryiko
  2021-03-11  2:34   ` Alexei Starovoitov
  2021-03-10  4:04 ` [PATCH bpf-next 06/10] libbpf: add BPF static linker BTF and BTF.ext support Andrii Nakryiko
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

Introduce BPF static linker APIs to libbpf. BPF static linker allows to
perform static linking of multiple BPF object files into a single combined
resulting object file, preserving all the BPF programs, maps, global
variables, etc.

Data sections (.bss, .data, .rodata, .maps, maps, etc) with the same name are
concatenated together. Similarly, code sections are also concatenated. All the
symbols and ELF relocations are also concatenated in their respective ELF
sections and are adjusted accordingly to the new object file layout.

Static variables and functions are handled correctly as well, adjusting BPF
instructions offsets to reflect new variable/function offset within the
combined ELF section. Such relocations are referencing STT_SECTION symbols and
that stays intact.

Data sections in different files can have different alignment requirements, so
that is taken care of as well, adjusting sizes and offsets as necessary to
satisfy both old and new alignment requirements.

DWARF data sections are stripped out, currently. As well as LLLVM_ADDRSIG
section, which is ignored by libbpf in bpf_object__open() anyways. So, in
a way, BPF static linker is an analogue to `llvm-strip -g`, which is a pretty
nice property, especially if resulting .o file is then used to generate BPF
skeleton.

Original string sections are ignored and instead a dedicated `struct btf`
object is used to collect and deduplicate all the unique strings referenced
from ELF sections and symbols.

To reduce the size of the patch, all the .BTF and .BTF.ext processing was
moved into a separate patch.

The high-level API consists of just 4 functions:
  - bpf_linker__new() creates an instance of BPF static linker. It accepts
    output filename and (currently empty) options struct;
  - bpf_linker__add_file() takes input filename and appends it to the already
    processed ELF data; it can be called multiple times, one for each BPF
    ELF object file that needs to be linked in;
  - bpf_linker__finalize() needs to be called to dump final ELF contents into
    the output file, specified when bpf_linker was created; after
    bpf_linker__finalize() is called, no more bpf_linker__add_file() and
    bpf_linker__finalize() calls are allowed, they will return error;
  - regardless of whether bpf_linker__finalize() was called or not,
    bpf_linker__free() will free up all the used resources.

Currently, BPF static linker doesn't resolve cross-object file references
(extern variables and/or functions). This will be added in the follow up patch
set.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/Build             |    2 +-
 tools/lib/bpf/libbpf.c          |   11 +-
 tools/lib/bpf/libbpf.h          |   13 +
 tools/lib/bpf/libbpf.map        |    4 +
 tools/lib/bpf/libbpf_internal.h |   20 +
 tools/lib/bpf/linker.c          | 1176 +++++++++++++++++++++++++++++++
 6 files changed, 1215 insertions(+), 11 deletions(-)
 create mode 100644 tools/lib/bpf/linker.c

diff --git a/tools/lib/bpf/Build b/tools/lib/bpf/Build
index 190366d05588..ca158915d0f8 100644
--- a/tools/lib/bpf/Build
+++ b/tools/lib/bpf/Build
@@ -1,3 +1,3 @@
 libbpf-y := libbpf.o bpf.o nlattr.o btf.o libbpf_errno.o str_error.o \
 	    netlink.o bpf_prog_linfo.o libbpf_probes.o xsk.o hashmap.o \
-	    btf_dump.o ringbuf.o
+	    btf_dump.o ringbuf.o linker.o
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 2f351d3ad3e7..540899c83bab 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -55,10 +55,6 @@
 #include "libbpf_internal.h"
 #include "hashmap.h"
 
-#ifndef EM_BPF
-#define EM_BPF 247
-#endif
-
 #ifndef BPF_FS_MAGIC
 #define BPF_FS_MAGIC		0xcafe4a11
 #endif
@@ -1134,11 +1130,6 @@ static void bpf_object__elf_finish(struct bpf_object *obj)
 	obj->efile.obj_buf_sz = 0;
 }
 
-/* if libelf is old and doesn't support mmap(), fall back to read() */
-#ifndef ELF_C_READ_MMAP
-#define ELF_C_READ_MMAP ELF_C_READ
-#endif
-
 static int bpf_object__elf_init(struct bpf_object *obj)
 {
 	int err = 0;
@@ -2807,7 +2798,7 @@ static bool ignore_elf_section(GElf_Shdr *hdr, const char *name)
 		return true;
 
 	/* ignore .llvm_addrsig section as well */
-	if (hdr->sh_type == 0x6FFF4C03 /* SHT_LLVM_ADDRSIG */)
+	if (hdr->sh_type == SHT_LLVM_ADDRSIG)
 		return true;
 
 	/* no subprograms will lead to an empty .text section, ignore it */
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 3d690d4e785c..a1a424b9b8ff 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -760,6 +760,19 @@ enum libbpf_tristate {
 	TRI_MODULE = 2,
 };
 
+struct bpf_linker_opts {
+	/* size of this struct, for forward/backward compatiblity */
+	size_t sz;
+};
+#define bpf_linker_opts__last_field sz
+
+struct bpf_linker;
+
+LIBBPF_API struct bpf_linker *bpf_linker__new(const char *filename, struct bpf_linker_opts *opts);
+LIBBPF_API int bpf_linker__add_file(struct bpf_linker *linker, const char *filename);
+LIBBPF_API int bpf_linker__finalize(struct bpf_linker *linker);
+LIBBPF_API void bpf_linker__free(struct bpf_linker *linker);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index d31d8c968097..279ae861f568 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -355,4 +355,8 @@ LIBBPF_0.4.0 {
 	global:
 		btf__add_float;
 		btf__add_type;
+		bpf_linker__add_file;
+		bpf_linker__finalize;
+		bpf_linker__free;
+		bpf_linker__new;
 } LIBBPF_0.3.0;
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 2753494ac8ab..d3e96b563efc 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -20,6 +20,26 @@
 
 #include "libbpf.h"
 
+#ifndef EM_BPF
+#define EM_BPF 247
+#endif
+
+#ifndef R_BPF_64_64
+#define R_BPF_64_64 1
+#endif
+#ifndef R_BPF_64_32
+#define R_BPF_64_32 10
+#endif
+
+#ifndef SHT_LLVM_ADDRSIG
+#define SHT_LLVM_ADDRSIG 0x6FFF4C03
+#endif
+
+/* if libelf is old and doesn't support mmap(), fall back to read() */
+#ifndef ELF_C_READ_MMAP
+#define ELF_C_READ_MMAP ELF_C_READ
+#endif
+
 #define BTF_INFO_ENC(kind, kind_flag, vlen) \
 	((!!(kind_flag) << 31) | ((kind) << 24) | ((vlen) & BTF_MAX_VLEN))
 #define BTF_TYPE_ENC(name, info, size_or_type) (name), (info), (size_or_type)
diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
new file mode 100644
index 000000000000..15d4f8787a48
--- /dev/null
+++ b/tools/lib/bpf/linker.c
@@ -0,0 +1,1176 @@
+// SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
+/*
+ * BPF static linker
+ *
+ * Copyright (c) 2021 Facebook
+ */
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <errno.h>
+#include <linux/err.h>
+#include <linux/btf.h>
+#include <elf.h>
+#include <libelf.h>
+#include <gelf.h>
+#include <fcntl.h>
+#include "libbpf.h"
+#include "bpf.h"
+#include "btf.h"
+#include "str_error.h"
+#include "libbpf_internal.h"
+#include "hashmap.h"
+
+struct src_sec {
+	const char *sec_name;
+	/* positional (not necessarily ELF) index in an array of sections */
+	int id;
+	/* positional (not necessarily ELF) index of a matching section in a final object file */
+	int dst_id;
+	/* section data offset in a matching output section */
+	int dst_off;
+	/* whether section is omitted from the final ELF file */
+	bool skipped;
+
+	/* ELF info */
+	size_t sec_idx;
+	Elf_Scn *scn;
+	Elf64_Shdr *shdr;
+	Elf_Data *data;
+};
+
+struct src_obj {
+	const char *filename;
+	int fd;
+	Elf *elf;
+	/* Section header strings section index */
+	size_t shstrs_sec_idx;
+	/* SYMTAB section index */
+	size_t symtab_sec_idx;
+
+	/* List of sections. Slot zero is unused. */
+	struct src_sec *secs;
+	int sec_cnt;
+
+	/* mapping of symbol indices from src to dst ELF */
+	int *sym_map;
+};
+
+struct dst_sec {
+	char *sec_name;
+	/* positional (not necessarily ELF) index in an array of sections */
+	int id;
+
+	/* ELF info */
+	size_t sec_idx;
+	Elf_Scn *scn;
+	Elf64_Shdr *shdr;
+	Elf_Data *data;
+
+	/* final output section size */
+	int sec_sz;
+	/* final output contents of the section */
+	void *raw_data;
+
+	/* corresponding STT_SECTION symbol index in SYMTAB */
+	int sec_sym_idx;
+};
+
+struct bpf_linker {
+	char *filename;
+	int fd;
+	Elf *elf;
+	Elf64_Ehdr *elf_hdr;
+
+	/* Output sections metadata */
+	struct dst_sec *secs;
+	int sec_cnt;
+
+	struct btf *strtab_btf; /* we use struct btf to manage strings */
+	size_t strtab_sec_idx; /* STRTAB section index */
+	size_t symtab_sec_idx; /* SYMTAB section index */
+};
+
+#define pr_warn_elf(fmt, ...)									\
+do {												\
+	libbpf_print(LIBBPF_WARN, "libbpf: " fmt ": %s\n", ##__VA_ARGS__, elf_errmsg(-1));	\
+} while (0)
+
+static int init_output_elf(struct bpf_linker *linker, const char *file);
+
+static int linker_load_obj_file(struct bpf_linker *linker, const char *filename, struct src_obj *obj);
+static int linker_sanity_check_elf(struct src_obj *obj);
+static int linker_append_sec_data(struct bpf_linker *linker, struct src_obj *obj);
+static int linker_append_elf_syms(struct bpf_linker *linker, struct src_obj *obj);
+static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *obj);
+
+void bpf_linker__free(struct bpf_linker *linker)
+{
+	int i;
+
+	if (!linker)
+		return;
+
+	free(linker->filename);
+
+	if (linker->elf)
+		elf_end(linker->elf);
+
+	if (linker->fd >= 0)
+		close(linker->fd);
+
+	btf__free(linker->strtab_btf);
+
+	for (i = 1; i < linker->sec_cnt; i++) {
+		struct dst_sec *sec = &linker->secs[i];
+
+		free(sec->sec_name);
+		free(sec->raw_data);
+	}
+	free(linker->secs);
+
+	free(linker);
+}
+
+struct bpf_linker *bpf_linker__new(const char *filename, struct bpf_linker_opts *opts)
+{
+	struct bpf_linker *linker;
+	int err;
+
+	if (!OPTS_VALID(opts, bpf_linker_opts))
+		return NULL;
+
+	if (elf_version(EV_CURRENT) == EV_NONE) {
+		pr_warn_elf("libelf initialization failed");
+		return NULL;
+	}
+
+	linker = calloc(1, sizeof(*linker));
+	if (!linker)
+		return NULL;
+
+	linker->fd = -1;
+
+	err = init_output_elf(linker, filename);
+	if (err)
+		goto err_out;
+
+	return linker;
+
+err_out:
+	bpf_linker__free(linker);
+	return NULL;
+}
+
+static struct dst_sec *add_dst_sec(struct bpf_linker *linker, const char *sec_name)
+{
+	struct dst_sec *secs = linker->secs, *sec;
+	size_t new_cnt = linker->sec_cnt ? linker->sec_cnt + 1 : 2;
+
+	secs = libbpf_reallocarray(secs, new_cnt, sizeof(*secs));
+	if (!secs)
+		return NULL;
+
+	/* zero out newly allocated memory */
+	memset(secs + linker->sec_cnt, 0, (new_cnt - linker->sec_cnt) * sizeof(*secs));
+
+	linker->secs = secs;
+	linker->sec_cnt = new_cnt;
+
+	sec = &linker->secs[new_cnt - 1];
+	sec->id = new_cnt - 1;
+	sec->sec_name = strdup(sec_name);
+	if (!sec->sec_name)
+		return NULL;
+
+	return sec;
+}
+
+static Elf64_Sym *add_new_sym(struct bpf_linker *linker, size_t *sym_idx)
+{
+	struct dst_sec *symtab = &linker->secs[linker->symtab_sec_idx];
+	Elf64_Sym *sym;
+	void *tmp;
+
+	tmp = realloc(symtab->raw_data, symtab->sec_sz + sizeof(*sym));
+	if (!tmp)
+		return NULL;
+
+	if (sym_idx)
+		*sym_idx = symtab->sec_sz / sizeof(*sym);
+
+	sym = tmp + symtab->sec_sz;
+	memset(sym, 0, sizeof(*sym));
+
+	symtab->raw_data = tmp;
+	symtab->sec_sz += sizeof(*sym);
+	symtab->shdr->sh_size += sizeof(*sym);
+	symtab->data->d_size += sizeof(*sym);
+
+	return sym;
+}
+
+static int init_output_elf(struct bpf_linker *linker, const char *file)
+{
+	int err, str_off;
+	Elf64_Sym *init_sym;
+	struct dst_sec *sec;
+
+	linker->filename = strdup(file);
+	if (!linker->filename)
+		return -ENOMEM;
+
+	linker->fd = open(file, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+	if (linker->fd < 0) {
+		err = -errno;
+		pr_warn("failed to create '%s': %d\n", file, err);
+		return err;
+	}
+
+	linker->elf = elf_begin(linker->fd, ELF_C_WRITE, NULL);
+	if (!linker->elf) {
+		pr_warn_elf("failed to create ELF object");
+		return -EINVAL;
+	}
+
+	/* ELF header */
+	linker->elf_hdr = elf64_newehdr(linker->elf);
+	if (!linker->elf_hdr){
+		pr_warn_elf("failed to create ELF header");
+		return -EINVAL;
+	}
+
+	linker->elf_hdr->e_machine = EM_BPF;
+	linker->elf_hdr->e_type = ET_REL;
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+	linker->elf_hdr->e_ident[EI_DATA] = ELFDATA2LSB;
+#elif __BYTE_ORDER == __BIG_ENDIAN
+	linker->elf_hdr->e_ident[EI_DATA] = ELFDATA2MSB;
+#else
+#error "Unknown __BYTE_ORDER"
+#endif
+
+	/* STRTAB */
+	linker->strtab_btf = btf__new_empty();
+	if (libbpf_get_error(linker->strtab_btf))
+		return libbpf_get_error(linker->strtab_btf);
+
+	sec = add_dst_sec(linker, ".strtab");
+	if (!sec)
+		return -ENOMEM;
+
+	sec->scn = elf_newscn(linker->elf);
+	if (!sec->scn) {
+		pr_warn_elf("failed to create STRTAB section");
+		return -EINVAL;
+	}
+
+	sec->shdr = elf64_getshdr(sec->scn);
+	if (!sec->shdr)
+		return -EINVAL;
+
+	sec->data = elf_newdata(sec->scn);
+	if (!sec->data) {
+		pr_warn_elf("failed to create STRTAB data");
+		return -EINVAL;
+	}
+
+	str_off = btf__add_str(linker->strtab_btf, sec->sec_name);
+	if (str_off < 0)
+		return str_off;
+
+	sec->sec_idx = elf_ndxscn(sec->scn);
+	linker->elf_hdr->e_shstrndx = sec->sec_idx;
+	linker->strtab_sec_idx = sec->sec_idx;
+
+	sec->shdr->sh_name = str_off;
+	sec->shdr->sh_type = SHT_STRTAB;
+	sec->shdr->sh_flags = SHF_STRINGS;
+	sec->shdr->sh_offset = 0;
+	sec->shdr->sh_link = 0;
+	sec->shdr->sh_info = 0;
+	sec->shdr->sh_addralign = 1;
+	sec->shdr->sh_size = sec->sec_sz = 0;
+	sec->shdr->sh_entsize = 0;
+
+	/* SYMTAB */
+	sec = add_dst_sec(linker, ".symtab");
+	if (!sec)
+		return -ENOMEM;
+
+	sec->scn = elf_newscn(linker->elf);
+	if (!sec->scn) {
+		pr_warn_elf("failed to create SYMTAB section");
+		return -EINVAL;
+	}
+
+	sec->shdr = elf64_getshdr(sec->scn);
+	if (!sec->shdr)
+		return -EINVAL;
+
+	sec->data = elf_newdata(sec->scn);
+	if (!sec->data) {
+		pr_warn_elf("failed to create SYMTAB data");
+		return -EINVAL;
+	}
+
+	str_off = btf__add_str(linker->strtab_btf, sec->sec_name);
+	if (str_off < 0)
+		return str_off;
+
+	sec->sec_idx = elf_ndxscn(sec->scn);
+	linker->symtab_sec_idx = sec->sec_idx;
+
+	sec->shdr->sh_name = str_off;
+	sec->shdr->sh_type = SHT_SYMTAB;
+	sec->shdr->sh_flags = 0;
+	sec->shdr->sh_offset = 0;
+	sec->shdr->sh_link = linker->strtab_sec_idx;
+	/* sh_info should be one greater than the index of the last local
+	 * symbol (i.e., binding is STB_LOCAL). But why and who cares?
+	 */
+	sec->shdr->sh_info = 0;
+	sec->shdr->sh_addralign = 8;
+	sec->shdr->sh_entsize = sizeof(Elf64_Sym);
+
+	/* add the special all-zero symbol */
+	init_sym = add_new_sym(linker, NULL);
+	if (!init_sym)
+		return -EINVAL;
+
+	init_sym->st_name = 0;
+	init_sym->st_info = 0;
+	init_sym->st_other = 0;
+	init_sym->st_shndx = SHN_UNDEF;
+	init_sym->st_value = 0;
+	init_sym->st_size = 0;
+
+	return 0;
+}
+
+int bpf_linker__add_file(struct bpf_linker *linker, const char *filename)
+{
+	struct src_obj obj = {};
+	int err = 0;
+
+	if (!linker->elf)
+		return -EINVAL;
+
+	err = err ?: linker_load_obj_file(linker, filename, &obj);
+	err = err ?: linker_append_sec_data(linker, &obj);
+	err = err ?: linker_append_elf_syms(linker, &obj);
+	err = err ?: linker_append_elf_relos(linker, &obj);
+
+	/* free up src_obj resources */
+	free(obj.secs);
+	free(obj.sym_map);
+	if (obj.elf)
+		elf_end(obj.elf);
+	if (obj.fd >= 0)
+		close(obj.fd);
+
+	return err;
+}
+
+static bool is_dwarf_sec_name(const char *name)
+{
+	/* approximation, but the actual list is too long */
+	return strncmp(name, ".debug_", sizeof(".debug_") - 1) == 0;
+}
+
+static bool is_ignored_sec(struct src_sec *sec)
+{
+	Elf64_Shdr *shdr = sec->shdr;
+	const char *name = sec->sec_name;
+
+	/* no special handling of .strtab */
+	if (shdr->sh_type == SHT_STRTAB)
+		return true;
+
+	/* ignore .llvm_addrsig section as well */
+	if (shdr->sh_type == SHT_LLVM_ADDRSIG)
+		return true;
+
+	/* no subprograms will lead to an empty .text section, ignore it */
+	if (shdr->sh_type == SHT_PROGBITS && shdr->sh_size == 0 &&
+	    strcmp(sec->sec_name, ".text") == 0)
+		return true;
+
+	/* DWARF sections */
+	if (is_dwarf_sec_name(sec->sec_name))
+		return true;
+
+	if (strncmp(name, ".rel", sizeof(".rel") - 1) == 0) {
+		name += sizeof(".rel") - 1;
+		/* DWARF section relocations */
+		if (is_dwarf_sec_name(name))
+			return true;
+
+		/* .BTF and .BTF.ext don't need relocations */
+		if (strcmp(name, BTF_ELF_SEC) == 0 ||
+		    strcmp(name, BTF_EXT_ELF_SEC) == 0)
+			return true;
+	}
+
+	return false;
+}
+
+static struct src_sec *add_src_sec(struct src_obj *obj, const char *sec_name)
+{
+	struct src_sec *secs = obj->secs, *sec;
+	size_t new_cnt = obj->sec_cnt ? obj->sec_cnt + 1 : 2;
+
+	secs = libbpf_reallocarray(secs, new_cnt, sizeof(*secs));
+	if (!secs)
+		return NULL;
+
+	/* zero out newly allocated memory */
+	memset(secs + obj->sec_cnt, 0, (new_cnt - obj->sec_cnt) * sizeof(*secs));
+
+	obj->secs = secs;
+	obj->sec_cnt = new_cnt;
+
+	sec = &obj->secs[new_cnt - 1];
+	sec->id = new_cnt - 1;
+	sec->sec_name = sec_name;
+
+	return sec;
+}
+
+static int linker_load_obj_file(struct bpf_linker *linker, const char *filename, struct src_obj *obj)
+{
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+	const int host_endianness = ELFDATA2LSB;
+#elif __BYTE_ORDER == __BIG_ENDIAN
+	const int host_endianness = ELFDATA2MSB;
+#else
+#error "Unknown __BYTE_ORDER"
+#endif
+	int err = 0;
+	Elf_Scn *scn;
+	Elf_Data *data;
+	Elf64_Ehdr *ehdr;
+	Elf64_Shdr *shdr;
+	struct src_sec *sec;
+
+	pr_debug("linker: adding object file '%s'...\n", filename);
+
+	obj->filename = filename;
+
+	obj->fd = open(filename, O_RDONLY);
+	if (obj->fd < 0) {
+		err = -errno;
+		pr_warn("failed to open file '%s': %d\n", filename, err);
+		return err;
+	}
+	obj->elf = elf_begin(obj->fd, ELF_C_READ_MMAP, NULL);
+	if (!obj->elf) {
+		err = -errno;
+		pr_warn_elf("failed to parse ELF file '%s'", filename);
+		return err;
+	}
+
+	/* Sanity check ELF file high-level properties */
+	ehdr = elf64_getehdr(obj->elf);
+	if (!ehdr) {
+		err = -errno;
+		pr_warn_elf("failed to get ELF header for %s", filename);
+		return err;
+	}
+	if (ehdr->e_ident[EI_DATA] != host_endianness) {
+		err = -EOPNOTSUPP;
+		pr_warn_elf("unsupported byte order of ELF file %s", filename);
+		return err;
+	}
+	if (ehdr->e_type != ET_REL
+	    || ehdr->e_machine != EM_BPF
+	    || ehdr->e_ident[EI_CLASS] != ELFCLASS64) {
+		err = -EOPNOTSUPP;
+		pr_warn_elf("unsupported kind of ELF file %s", filename);
+		return err;
+	}
+
+	if (elf_getshdrstrndx(obj->elf, &obj->shstrs_sec_idx)) {
+		err = -errno;
+		pr_warn_elf("failed to get SHSTRTAB section index for %s", filename);
+		return err;
+	}
+
+	scn = NULL;
+	while ((scn = elf_nextscn(obj->elf, scn)) != NULL) {
+		size_t sec_idx = elf_ndxscn(scn);
+		const char *sec_name;
+
+		shdr = elf64_getshdr(scn);
+		if (!shdr) {
+			err = -errno;
+			pr_warn_elf("failed to get section #%zu header for %s",
+				    sec_idx, filename);
+			return err;
+		}
+
+		sec_name = elf_strptr(obj->elf, obj->shstrs_sec_idx, shdr->sh_name);
+		if (!sec_name) {
+			err = -errno;
+			pr_warn_elf("failed to get section #%zu name for %s",
+				    sec_idx, filename);
+			return err;
+		}
+
+		data = elf_getdata(scn, 0);
+		if (!data) {
+			err = -errno;
+			pr_warn_elf("failed to get section #%zu (%s) data from %s",
+				    sec_idx, sec_name, filename);
+			return err;
+		}
+
+		sec = add_src_sec(obj, sec_name);
+		if (!sec)
+			return -ENOMEM;
+
+		sec->scn = scn;
+		sec->shdr = shdr;
+		sec->data = data;
+		sec->sec_idx = elf_ndxscn(scn);
+
+		if (is_ignored_sec(sec)) {
+			sec->skipped = true;
+			continue;
+		}
+
+		switch (shdr->sh_type) {
+		case SHT_SYMTAB:
+			if (obj->symtab_sec_idx) {
+				err = -EOPNOTSUPP;
+				pr_warn("multiple SYMTAB sections found, not supported\n");
+				return err;
+			}
+			obj->symtab_sec_idx = sec_idx;
+			break;
+		case SHT_STRTAB:
+			/* we'll construct our own string table */
+			break;
+		case SHT_PROGBITS:
+			if (strcmp(sec_name, BTF_ELF_SEC) == 0) {
+				sec->skipped = true;
+				continue;
+			}
+			if (strcmp(sec_name, BTF_EXT_ELF_SEC) == 0) {
+				sec->skipped = true;
+				continue;
+			}
+
+			/* data & code */
+			break;
+		case SHT_NOBITS:
+			/* BSS */
+			break;
+		case SHT_REL:
+			/* relocations */
+			break;
+		default:
+			pr_warn("unrecognized section #%zu (%s) in %s\n",
+				sec_idx, sec_name, filename);
+			err = -EINVAL;
+			return err;
+		}
+	}
+
+	err = err ?: linker_sanity_check_elf(obj);
+
+	return err;
+}
+
+static bool is_pow_of_2(size_t x)
+{
+	return x && (x & (x - 1)) == 0;
+}
+
+static int linker_sanity_check_elf(struct src_obj *obj)
+{
+	struct src_sec *sec, *link_sec;
+	int i, j, n;
+
+	if (!obj->symtab_sec_idx) {
+		pr_warn("ELF is missing SYMTAB section in %s\n", obj->filename);
+		return -EINVAL;
+	}
+	if (!obj->shstrs_sec_idx) {
+		pr_warn("ELF is missing section headers STRTAB section in %s\n", obj->filename);
+		return -EINVAL;
+	}
+
+	for (i = 1; i < obj->sec_cnt; i++) {
+		sec = &obj->secs[i];
+
+		if (sec->sec_name[0] == '\0') {
+			pr_warn("ELF section #%zu has empty name in %s\n", sec->sec_idx, obj->filename);
+			return -EINVAL;
+		}
+
+		if (sec->shdr->sh_addralign && !is_pow_of_2(sec->shdr->sh_addralign))
+			return -EINVAL;
+		if (sec->shdr->sh_addralign != sec->data->d_align)
+			return -EINVAL;
+
+		if (sec->shdr->sh_size != sec->data->d_size)
+			return -EINVAL;
+
+		switch (sec->shdr->sh_type) {
+		case SHT_SYMTAB: {
+			Elf64_Sym *sym;
+
+			if (sec->shdr->sh_entsize != sizeof(Elf64_Sym))
+				return -EINVAL;
+			if (sec->shdr->sh_size % sec->shdr->sh_entsize != 0)
+				return -EINVAL;
+
+			if (!sec->shdr->sh_link || sec->shdr->sh_link >= obj->sec_cnt) {
+				pr_warn("ELF SYMTAB section #%zu points to missing STRTAB section #%zu in %s\n",
+					sec->sec_idx, (size_t)sec->shdr->sh_link, obj->filename);
+				return -EINVAL;
+			}
+			link_sec = &obj->secs[sec->shdr->sh_link];
+			if (link_sec->shdr->sh_type != SHT_STRTAB) {
+				pr_warn("ELF SYMTAB section #%zu points to invalid STRTAB section #%zu in %s\n",
+					sec->sec_idx, (size_t)sec->shdr->sh_link, obj->filename);
+				return -EINVAL;
+			}
+
+			n = sec->shdr->sh_size / sec->shdr->sh_entsize;
+			sym = sec->data->d_buf;
+			for (j = 0; j < n; j++, sym++) {
+				if (sym->st_shndx
+				    && sym->st_shndx < SHN_LORESERVE
+				    && sym->st_shndx >= obj->sec_cnt) {
+					pr_warn("ELF sym #%d in section #%zu points to missing section #%zu in %s\n",
+						j, sec->sec_idx, (size_t)sym->st_shndx, obj->filename);
+					return -EINVAL;
+				}
+				if (ELF64_ST_TYPE(sym->st_info) == STT_SECTION) {
+					if (sym->st_value != 0)
+						return -EINVAL;
+				}
+			}
+			break;
+		}
+		case SHT_STRTAB:
+			break;
+		case SHT_PROGBITS:
+			if (sec->shdr->sh_flags & SHF_EXECINSTR) {
+				if (sec->shdr->sh_size % sizeof(struct bpf_insn) != 0)
+					return -EINVAL;
+			}
+			break;
+		case SHT_NOBITS:
+			break;
+		case SHT_REL: {
+			Elf64_Rel *relo;
+			struct src_sec *sym_sec;
+
+			if (sec->shdr->sh_entsize != sizeof(Elf64_Rel))
+				return -EINVAL;
+			if (sec->shdr->sh_size % sec->shdr->sh_entsize != 0)
+				return -EINVAL;
+
+			/* SHT_REL's sh_link should point to SYMTAB */
+			if (sec->shdr->sh_link != obj->symtab_sec_idx) {
+				pr_warn("ELF relo section #%zu points to invalid SYMTAB section #%zu in %s\n",
+					sec->sec_idx, (size_t)sec->shdr->sh_link, obj->filename);
+				return -EINVAL;
+			}
+
+			/* SHT_REL's sh_info points to relocated section */
+			if (!sec->shdr->sh_info || sec->shdr->sh_info >= obj->sec_cnt) {
+				pr_warn("ELF relo section #%zu points to missing section #%zu in %s\n",
+					sec->sec_idx, (size_t)sec->shdr->sh_info, obj->filename);
+				return -EINVAL;
+			}
+			link_sec = &obj->secs[sec->shdr->sh_info];
+
+			/* .rel<secname> -> <secname> pattern is followed */
+			if (strncmp(sec->sec_name, ".rel", sizeof(".rel") - 1) != 0
+			    || strcmp(sec->sec_name + sizeof(".rel") - 1, link_sec->sec_name) != 0) {
+				pr_warn("ELF relo section #%zu name has invalid name in %s\n",
+					sec->sec_idx, obj->filename);
+				return -EINVAL;
+			}
+
+			/* don't further validate relocations for ignored sections */
+			if (link_sec->skipped)
+				break;
+
+			/* relocatable section is data or instructions */
+			if (link_sec->shdr->sh_type != SHT_PROGBITS
+			    && link_sec->shdr->sh_type != SHT_NOBITS) {
+				pr_warn("ELF relo section #%zu points to invalid section #%zu in %s\n",
+					sec->sec_idx, (size_t)sec->shdr->sh_info, obj->filename);
+				return -EINVAL;
+			}
+
+			/* check sanity of each relocation */
+			n = sec->shdr->sh_size / sec->shdr->sh_entsize;
+			relo = sec->data->d_buf;
+			sym_sec = &obj->secs[obj->symtab_sec_idx];
+			for (j = 0; j < n; j++, relo++) {
+				size_t sym_idx = ELF64_R_SYM(relo->r_info);
+				size_t sym_type = ELF64_R_TYPE(relo->r_info);
+
+				if (sym_type != R_BPF_64_64 && sym_type != R_BPF_64_32) {
+					pr_warn("ELF relo #%d in section #%zu has unexpected type %zu in %s\n",
+						j, sec->sec_idx, sym_type, obj->filename);
+					return -EINVAL;
+				}
+
+				if (!sym_idx || sym_idx * sizeof(Elf64_Sym) >= sym_sec->shdr->sh_size) {
+					pr_warn("ELF relo #%d in section #%zu points to invalid symbol #%zu in %s\n",
+						j, sec->sec_idx, sym_idx, obj->filename);
+					return -EINVAL;
+				}
+
+				if (link_sec->shdr->sh_flags & SHF_EXECINSTR) {
+					if (relo->r_offset % sizeof(struct bpf_insn) != 0) {
+						pr_warn("ELF relo #%d in section #%zu points to missing symbol #%zu in %s\n",
+							j, sec->sec_idx, sym_idx, obj->filename);
+						return -EINVAL;
+					}
+				}
+			}
+			break;
+		}
+		case SHT_LLVM_ADDRSIG:
+			break;
+		default:
+			pr_warn("ELF section #%zu (%s) has unrecognized type %zu in %s\n",
+				sec->sec_idx, sec->sec_name, (size_t)sec->shdr->sh_type, obj->filename);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int init_sec(struct bpf_linker *linker, struct dst_sec *dst_sec, struct src_sec *src_sec)
+{
+	Elf_Scn *scn;
+	Elf_Data *data;
+	Elf64_Shdr *shdr;
+	int name_off;
+
+	dst_sec->sec_sz = 0;
+	dst_sec->sec_idx = 0;
+
+	scn = elf_newscn(linker->elf);
+	if (!scn)
+		return -1;
+	data = elf_newdata(scn);
+	if (!data)
+		return -1;
+	shdr = elf64_getshdr(scn);
+	if (!shdr)
+		return -1;
+
+	dst_sec->scn = scn;
+	dst_sec->shdr = shdr;
+	dst_sec->data = data;
+	dst_sec->sec_idx = elf_ndxscn(scn);
+
+	name_off = btf__add_str(linker->strtab_btf, src_sec->sec_name);
+	if (name_off < 0)
+		return name_off;
+
+	shdr->sh_name = name_off;
+	shdr->sh_type = src_sec->shdr->sh_type;
+	shdr->sh_flags = src_sec->shdr->sh_flags;
+	shdr->sh_size = 0;
+	/* sh_link and sh_info have different meaning for different types of
+	 * sections, so we leave it up to the caller code to fill them in, if
+	 * necessary
+	 */
+	shdr->sh_link = 0;
+	shdr->sh_info = 0;
+	shdr->sh_addralign = src_sec->shdr->sh_addralign;
+	shdr->sh_entsize = src_sec->shdr->sh_entsize;
+
+	data->d_type = src_sec->data->d_type;
+	data->d_size = 0;
+	data->d_buf = NULL;
+	data->d_align = src_sec->data->d_align;
+	data->d_off = 0;
+
+	return 0;
+}
+
+static struct dst_sec *find_dst_sec_by_name(struct bpf_linker *linker, const char *sec_name)
+{
+	struct dst_sec *sec;
+	int i;
+
+	for (i = 1; i < linker->sec_cnt; i++) {
+		sec = &linker->secs[i];
+
+		if (strcmp(sec->sec_name, sec_name) == 0)
+			return sec;
+	}
+
+	return NULL;
+}
+
+static bool secs_match(struct dst_sec *dst, struct src_sec *src)
+{
+	if (dst->shdr->sh_type != src->shdr->sh_type) {
+		pr_warn("sec %s types mismatch\n", dst->sec_name);
+		return false;
+	}
+	if (dst->shdr->sh_flags != src->shdr->sh_flags) {
+		pr_warn("sec %s flags mismatch\n", dst->sec_name);
+		return false;
+	}
+	if (dst->shdr->sh_entsize != src->shdr->sh_entsize) {
+		pr_warn("sec %s entsize mismatch\n", dst->sec_name);
+		return false;
+	}
+
+	return true;
+}
+
+static bool sec_content_is_same(struct dst_sec *dst_sec, struct src_sec *src_sec)
+{
+	if (dst_sec->sec_sz != src_sec->shdr->sh_size)
+		return false;
+	if (memcmp(dst_sec->raw_data, src_sec->data->d_buf, dst_sec->sec_sz) != 0)
+		return false;
+	return true;
+}
+
+static int extend_sec(struct dst_sec *dst, struct src_sec *src)
+{
+	void *tmp;
+	size_t dst_align = dst->shdr->sh_addralign;
+	size_t src_align = src->shdr->sh_addralign;
+	size_t dst_align_sz, dst_final_sz;
+
+	if (dst_align == 0)
+		dst_align = 1;
+	if (dst_align < src_align)
+		dst_align = src_align;
+
+	dst_align_sz = (dst->sec_sz + dst_align - 1) / dst_align * dst_align;
+
+	/* no need to re-align final size */
+	dst_final_sz = dst_align_sz + src->shdr->sh_size;
+
+	if (src->shdr->sh_type != SHT_NOBITS) {
+		tmp = realloc(dst->raw_data, dst_final_sz);
+		if (!tmp)
+			return -ENOMEM;
+		dst->raw_data = tmp;
+
+		/* pad dst section, if it's alignment forced size increase */
+		memset(dst->raw_data + dst->sec_sz, 0, dst_align_sz - dst->sec_sz);
+		/* now copy src data at a properly aligned offset */
+		memcpy(dst->raw_data + dst_align_sz, src->data->d_buf, src->shdr->sh_size);
+	}
+
+	dst->sec_sz = dst_final_sz;
+	dst->shdr->sh_size = dst_final_sz;
+	dst->data->d_size = dst_final_sz;
+
+	dst->shdr->sh_addralign = dst_align;
+	dst->data->d_align = dst_align;
+
+	src->dst_off = dst_align_sz;
+
+	return 0;
+}
+
+static bool is_data_sec(struct src_sec *sec)
+{
+	if (!sec || sec->skipped)
+		return false;
+	return sec->shdr->sh_type == SHT_PROGBITS || sec->shdr->sh_type == SHT_NOBITS;
+}
+
+static bool is_relo_sec(struct src_sec *sec)
+{
+	if (!sec || sec->skipped)
+		return false;
+	return sec->shdr->sh_type == SHT_REL;
+}
+
+static int linker_append_sec_data(struct bpf_linker *linker, struct src_obj *obj)
+{
+	int i, err;
+
+	for (i = 1; i < obj->sec_cnt; i++) {
+		struct src_sec *src_sec;
+		struct dst_sec *dst_sec;
+
+		src_sec = &obj->secs[i];
+		if (!is_data_sec(src_sec))
+			continue;
+
+		dst_sec = find_dst_sec_by_name(linker, src_sec->sec_name);
+		if (!dst_sec) {
+			dst_sec = add_dst_sec(linker, src_sec->sec_name);
+			if (!dst_sec)
+				return -ENOMEM;
+			err = init_sec(linker, dst_sec, src_sec);
+			if (err) {
+				pr_warn("failed to init section '%s'\n", src_sec->sec_name);
+				return err;
+			}
+		} else {
+			if (!secs_match(dst_sec, src_sec)) {
+				pr_warn("ELF sections %s are incompatible\n", src_sec->sec_name);
+				return -1;
+			}
+
+			/* "license" and "version" sections are deduped */
+			if (strcmp(src_sec->sec_name, "license") == 0
+			    || strcmp(src_sec->sec_name, "version") == 0) {
+				if (!sec_content_is_same(dst_sec, src_sec)) {
+					pr_warn("non-identical contents of section '%s' are not supported\n", src_sec->sec_name);
+					return -EINVAL;
+				}
+				src_sec->skipped = true;
+				src_sec->dst_id = dst_sec->id;
+				continue;
+			}
+		}
+
+		/* record mapped section index */
+		src_sec->dst_id = dst_sec->id;
+
+		err = extend_sec(dst_sec, src_sec);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int linker_append_elf_syms(struct bpf_linker *linker, struct src_obj *obj)
+{
+	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
+	Elf64_Sym *sym = symtab->data->d_buf, *dst_sym;
+	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
+	int str_sec_idx = symtab->shdr->sh_link;
+
+	obj->sym_map = calloc(n + 1, sizeof(*obj->sym_map));
+	if (!obj->sym_map)
+		return -ENOMEM;
+
+	for (i = 0; i < n; i++, sym++) {
+		struct src_sec *src_sec = NULL;
+		struct dst_sec *dst_sec = NULL;
+		const char *sym_name;
+		size_t dst_sym_idx;
+		int name_off;
+
+		/* we already have all-zero initial symbol */
+		if (sym->st_name == 0 && sym->st_info == 0 &&
+		    sym->st_other == 0 && sym->st_shndx == SHN_UNDEF &&
+		    sym->st_value == 0 && sym->st_size ==0)
+			continue;
+
+		sym_name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
+		if (!sym_name) {
+			pr_warn("can't fetch symbol name for symbol #%d in '%s'\n", i, obj->filename);
+			return -1;
+		}
+
+		if (sym->st_shndx && sym->st_shndx < SHN_LORESERVE) {
+			src_sec = &obj->secs[sym->st_shndx];
+			if (src_sec->skipped)
+				continue;
+			dst_sec = &linker->secs[src_sec->dst_id];
+
+			/* allow only one STT_SECTION symbol per section */
+			if (ELF64_ST_TYPE(sym->st_info) == STT_SECTION && dst_sec->sec_sym_idx) {
+				obj->sym_map[i] = dst_sec->sec_sym_idx;
+				continue;
+			}
+		}
+
+		name_off = btf__add_str(linker->strtab_btf, sym_name);
+		if (name_off < 0)
+			return name_off;
+
+		dst_sym = add_new_sym(linker, &dst_sym_idx);
+		if (!dst_sym)
+			return -ENOMEM;
+
+		dst_sym->st_name = name_off;
+		dst_sym->st_info = sym->st_info;
+		dst_sym->st_other = sym->st_other;
+		dst_sym->st_shndx = src_sec ? dst_sec->sec_idx : sym->st_shndx;
+		dst_sym->st_value = (src_sec ? src_sec->dst_off : 0) + sym->st_value;
+		dst_sym->st_size = sym->st_size;
+
+		obj->sym_map[i] = dst_sym_idx;
+
+		if (ELF64_ST_TYPE(sym->st_info) == STT_SECTION && dst_sym) {
+			dst_sec->sec_sym_idx = dst_sym_idx;
+			dst_sym->st_value = 0;
+		}
+
+	}
+
+	return 0;
+}
+
+static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *obj)
+{
+	struct src_sec *src_symtab = &obj->secs[obj->symtab_sec_idx];
+	struct dst_sec *dst_symtab = &linker->secs[linker->symtab_sec_idx];
+	int i, err;
+
+	for (i = 1; i < obj->sec_cnt; i++) {
+		struct src_sec *src_sec, *src_linked_sec;
+		struct dst_sec *dst_sec, *dst_linked_sec;
+		Elf64_Rel *src_rel, *dst_rel;
+		int j, n;
+
+		src_sec = &obj->secs[i];
+		if (!is_relo_sec(src_sec))
+			continue;
+
+		/* shdr->sh_info points to relocatable section */
+		src_linked_sec = &obj->secs[src_sec->shdr->sh_info];
+		if (src_linked_sec->skipped)
+			continue;
+
+		dst_sec = find_dst_sec_by_name(linker, src_sec->sec_name);
+		if (!dst_sec) {
+			dst_sec = add_dst_sec(linker, src_sec->sec_name);
+			if (!dst_sec)
+				return -ENOMEM;
+			err = init_sec(linker, dst_sec, src_sec);
+			if (err) {
+				pr_warn("failed to init section '%s'\n", src_sec->sec_name);
+				return err;
+			}
+		} else if (!secs_match(dst_sec, src_sec)) {
+			pr_warn("Secs %s are not compatible\n", src_sec->sec_name);
+			return -1;
+		}
+
+		/* shdr->sh_link points to SYMTAB */
+		dst_sec->shdr->sh_link = linker->symtab_sec_idx;
+
+		/* shdr->sh_info points to relocated section */
+		dst_linked_sec = &linker->secs[src_linked_sec->dst_id];
+		dst_sec->shdr->sh_info = dst_linked_sec->sec_idx;
+
+		src_sec->dst_id = dst_sec->id;
+		err = extend_sec(dst_sec, src_sec);
+		if (err)
+			return err;
+
+		src_rel = src_sec->data->d_buf;
+		dst_rel = dst_sec->raw_data + src_sec->dst_off;
+		n = src_sec->shdr->sh_size / src_sec->shdr->sh_entsize;
+		for (j = 0; j < n; j++, src_rel++, dst_rel++) {
+			size_t src_sym_idx = ELF64_R_SYM(src_rel->r_info);
+			size_t sym_type = ELF64_R_TYPE(src_rel->r_info);
+			Elf64_Sym *src_sym, *dst_sym;
+			size_t dst_sym_idx;
+
+			src_sym_idx = ELF64_R_SYM(src_rel->r_info);
+			src_sym = src_symtab->data->d_buf + sizeof(*src_sym) * src_sym_idx;
+
+			dst_sym_idx = obj->sym_map[src_sym_idx];
+			dst_sym = dst_symtab->raw_data + sizeof(*dst_sym) * dst_sym_idx;
+			dst_rel->r_offset += src_linked_sec->dst_off;
+			sym_type = ELF64_R_TYPE(src_rel->r_info);
+			dst_rel->r_info = ELF64_R_INFO(dst_sym_idx, sym_type);
+
+			if (ELF64_ST_TYPE(src_sym->st_info) == STT_SECTION) {
+				struct src_sec *sec = &obj->secs[src_sym->st_shndx];
+				struct bpf_insn *insn;
+
+				if (src_linked_sec->shdr->sh_flags & SHF_EXECINSTR) {
+					/* calls to the very first static function inside
+					 * .text section at offset 0 will
+					 * reference section symbol, not the
+					 * function symbol. Fix that up,
+					 * otherwise it won't be possible to
+					 * relocate calls to two different
+					 * static functions with the same name
+					 * (rom two different object files)
+					 */
+					insn = dst_linked_sec->raw_data + dst_rel->r_offset;
+					if (insn->code == (BPF_JMP | BPF_CALL))
+						insn->imm += sec->dst_off / sizeof(struct bpf_insn);
+					else
+						insn->imm += sec->dst_off;
+				} else {
+					pr_warn("relocation against STT_SECTION in non-exec section is not supported!\n");
+					return -EINVAL;
+				}
+			}
+
+		}
+	}
+
+	return 0;
+}
+
+int bpf_linker__finalize(struct bpf_linker *linker)
+{
+	struct dst_sec *sec;
+	size_t strs_sz;
+	const void *strs;
+	int err, i;
+
+	if (!linker->elf)
+		return -EINVAL;
+
+	/* Finalize strings */
+	strs = btf_raw_strs(linker->strtab_btf, &strs_sz);
+
+	sec = &linker->secs[linker->strtab_sec_idx];
+	sec->data->d_align = 1;
+	sec->data->d_off = 0LL;
+	sec->data->d_buf = (void *)strs;
+	sec->data->d_type = ELF_T_BYTE;
+	sec->data->d_size = strs_sz;
+	sec->shdr->sh_size = strs_sz;
+
+	for (i = 1; i < linker->sec_cnt; i++) {
+		sec = &linker->secs[i];
+
+		/* STRTAB is handled specially above */
+		if (sec->sec_idx == linker->strtab_sec_idx)
+			continue;
+
+		sec->data->d_buf = sec->raw_data;
+	}
+
+	/* Finalize ELF layout */
+	if (elf_update(linker->elf, ELF_C_NULL) < 0) {
+		err = -errno;
+		pr_warn_elf("failed to finalize ELF layout");
+		return err;
+	}
+
+	/* Write out final ELF contents */
+	if (elf_update(linker->elf, ELF_C_WRITE) < 0) {
+		err = -errno;
+		pr_warn_elf("failed to write ELF contents");
+		return err;
+	}
+
+	elf_end(linker->elf);
+	close(linker->fd);
+
+	linker->elf = NULL;
+	linker->fd = -1;
+
+	return 0;
+}
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next 06/10] libbpf: add BPF static linker BTF and BTF.ext support
  2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
                   ` (4 preceding siblings ...)
  2021-03-10  4:04 ` [PATCH bpf-next 05/10] libbpf: add BPF static linker APIs Andrii Nakryiko
@ 2021-03-10  4:04 ` Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking Andrii Nakryiko
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

Add .BTF and .BTF.ext static linking logic.

When multiple BPF object files are linked together, their respective .BTF and
.BTF.ext sections are merged together. BTF types are not just concatenated,
but also deduplicated. .BTF.ext data is grouped by type (func info, line info,
core_relos) and target section names, and then all the records are
concatenated together, preserving their relative order. All the BTF type ID
references and string offsets are updated as necessary, to take into account
possibly deduplicated strings and types.

BTF DATASEC types are handled specially. Their respective var_secinfos are
accumulated separately in special per-section data and then final DATASEC
types are emitted at the very end during bpf_linker__finalize() operation,
just before emitting final ELF output file.

BTF data can also provide "section annotations" for some extern variables.
Such concept is missing in ELF, but BTF will have DATASEC types for such
special extern datasections (e.g., .kconfig, .ksyms). Such sections are called
"ephemeral" internally. Internally linker will keep metadata for each such
section, collecting variables information, but those sections won't be emitted
into the final ELF file.

Also, given LLVM/Clang during compilation emits BTF DATASECS that are
incomplete, missing section size and variable offsets for static variables,
BPF static linker will initially fix up such DATASECs, using ELF symbols data.
The final DATASECs will preserve section sizes and all variable offsets. This
is handled correctly by libbpf already, so won't cause any new issues. On the
other hand, it's actually a nice property to have a complete BTF data without
runtime adjustments done during bpf_object__open() by libbpf. In that sense,
BPF static linker is also a BTF normalizer.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/linker.c | 773 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 771 insertions(+), 2 deletions(-)

diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index 15d4f8787a48..98f6e45a0736 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -34,12 +34,17 @@ struct src_sec {
 	int dst_off;
 	/* whether section is omitted from the final ELF file */
 	bool skipped;
+	/* whether section is an ephemeral section, not mapped to an ELF section */
+	bool ephemeral;
 
 	/* ELF info */
 	size_t sec_idx;
 	Elf_Scn *scn;
 	Elf64_Shdr *shdr;
 	Elf_Data *data;
+
+	/* corresponding BTF DATASEC type ID */
+	int sec_type_id;
 };
 
 struct src_obj {
@@ -51,12 +56,24 @@ struct src_obj {
 	/* SYMTAB section index */
 	size_t symtab_sec_idx;
 
-	/* List of sections. Slot zero is unused. */
+	struct btf *btf;
+	struct btf_ext *btf_ext;
+
+	/* List of sections (including ephemeral). Slot zero is unused. */
 	struct src_sec *secs;
 	int sec_cnt;
 
 	/* mapping of symbol indices from src to dst ELF */
 	int *sym_map;
+	/* mapping from the src BTF type IDs to dst ones */
+	int *btf_type_map;
+};
+
+/* single .BTF.ext data section */
+struct btf_ext_sec_data {
+	size_t rec_cnt;
+	__u32 rec_sz;
+	void *recs;
 };
 
 struct dst_sec {
@@ -77,6 +94,15 @@ struct dst_sec {
 
 	/* corresponding STT_SECTION symbol index in SYMTAB */
 	int sec_sym_idx;
+
+	/* section's DATASEC variable info, emitted on BTF finalization */
+	int sec_var_cnt;
+	struct btf_var_secinfo *sec_vars;
+
+	/* section's .BTF.ext data */
+	struct btf_ext_sec_data func_info;
+	struct btf_ext_sec_data line_info;
+	struct btf_ext_sec_data core_relo_info;
 };
 
 struct bpf_linker {
@@ -92,6 +118,9 @@ struct bpf_linker {
 	struct btf *strtab_btf; /* we use struct btf to manage strings */
 	size_t strtab_sec_idx; /* STRTAB section index */
 	size_t symtab_sec_idx; /* SYMTAB section index */
+
+	struct btf *btf;
+	struct btf_ext *btf_ext;
 };
 
 #define pr_warn_elf(fmt, ...)									\
@@ -103,9 +132,17 @@ static int init_output_elf(struct bpf_linker *linker, const char *file);
 
 static int linker_load_obj_file(struct bpf_linker *linker, const char *filename, struct src_obj *obj);
 static int linker_sanity_check_elf(struct src_obj *obj);
+static int linker_sanity_check_btf(struct src_obj *obj);
+static int linker_sanity_check_btf_ext(struct src_obj *obj);
+static int linker_fixup_btf(struct src_obj *obj);
 static int linker_append_sec_data(struct bpf_linker *linker, struct src_obj *obj);
 static int linker_append_elf_syms(struct bpf_linker *linker, struct src_obj *obj);
 static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *obj);
+static int linker_append_btf(struct bpf_linker *linker, struct src_obj *obj);
+static int linker_append_btf_ext(struct bpf_linker *linker, struct src_obj *obj);
+
+static int finalize_btf(struct bpf_linker *linker);
+static int finalize_btf_ext(struct bpf_linker *linker);
 
 void bpf_linker__free(struct bpf_linker *linker)
 {
@@ -123,12 +160,20 @@ void bpf_linker__free(struct bpf_linker *linker)
 		close(linker->fd);
 
 	btf__free(linker->strtab_btf);
+	btf__free(linker->btf);
+
+	btf_ext__free(linker->btf_ext);
 
 	for (i = 1; i < linker->sec_cnt; i++) {
 		struct dst_sec *sec = &linker->secs[i];
 
 		free(sec->sec_name);
 		free(sec->raw_data);
+		free(sec->sec_vars);
+
+		free(sec->func_info.recs);
+		free(sec->line_info.recs);
+		free(sec->core_relo_info.recs);
 	}
 	free(linker->secs);
 
@@ -336,6 +381,12 @@ static int init_output_elf(struct bpf_linker *linker, const char *file)
 	sec->shdr->sh_addralign = 8;
 	sec->shdr->sh_entsize = sizeof(Elf64_Sym);
 
+	/* .BTF */
+	linker->btf = btf__new_empty();
+	err = libbpf_get_error(linker->btf);
+	if (err)
+		return err;
+
 	/* add the special all-zero symbol */
 	init_sym = add_new_sym(linker, NULL);
 	if (!init_sym)
@@ -363,8 +414,13 @@ int bpf_linker__add_file(struct bpf_linker *linker, const char *filename)
 	err = err ?: linker_append_sec_data(linker, &obj);
 	err = err ?: linker_append_elf_syms(linker, &obj);
 	err = err ?: linker_append_elf_relos(linker, &obj);
+	err = err ?: linker_append_btf(linker, &obj);
+	err = err ?: linker_append_btf_ext(linker, &obj);
 
 	/* free up src_obj resources */
+	free(obj.btf_type_map);
+	btf__free(obj.btf);
+	btf_ext__free(obj.btf_ext);
 	free(obj.secs);
 	free(obj.sym_map);
 	if (obj.elf)
@@ -556,10 +612,22 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
 			break;
 		case SHT_PROGBITS:
 			if (strcmp(sec_name, BTF_ELF_SEC) == 0) {
+				obj->btf = btf__new(data->d_buf, shdr->sh_size);
+				err = libbpf_get_error(obj->btf);
+				if (err) {
+					pr_warn("failed to parse .BTF from %s: %d\n", filename, err);
+					return err;
+				}
 				sec->skipped = true;
 				continue;
 			}
 			if (strcmp(sec_name, BTF_EXT_ELF_SEC) == 0) {
+				obj->btf_ext = btf_ext__new(data->d_buf, shdr->sh_size);
+				err = libbpf_get_error(obj->btf_ext);
+				if (err) {
+					pr_warn("failed to parse .BTF.ext from '%s': %d\n", filename, err);
+					return err;
+				}
 				sec->skipped = true;
 				continue;
 			}
@@ -581,6 +649,9 @@ static int linker_load_obj_file(struct bpf_linker *linker, const char *filename,
 	}
 
 	err = err ?: linker_sanity_check_elf(obj);
+	err = err ?: linker_sanity_check_btf(obj);
+	err = err ?: linker_sanity_check_btf_ext(obj);
+	err = err ?: linker_fixup_btf(obj);
 
 	return err;
 }
@@ -754,6 +825,69 @@ static int linker_sanity_check_elf(struct src_obj *obj)
 	return 0;
 }
 
+static int check_btf_type_id(__u32 *type_id, void *ctx)
+{
+	struct btf *btf = ctx;
+
+	if (*type_id > btf__get_nr_types(btf))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int check_btf_str_off(__u32 *str_off, void *ctx)
+{
+	struct btf *btf = ctx;
+	const char *s;
+
+	s = btf__str_by_offset(btf, *str_off);
+
+	if (!s)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int linker_sanity_check_btf(struct src_obj *obj)
+{
+	struct btf_type *t;
+	int i, n, err = 0;
+
+	if (!obj->btf)
+		return 0;
+
+	n = btf__get_nr_types(obj->btf);
+	for (i = 1; i <= n; i++) {
+		t = btf_type_by_id(obj->btf, i);
+
+		err = err ?: btf_type_visit_type_ids(t, check_btf_type_id, obj->btf);
+		err = err ?: btf_type_visit_str_offs(t, check_btf_str_off, obj->btf);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int linker_sanity_check_btf_ext(struct src_obj *obj)
+{
+	int err = 0;
+
+	if (!obj->btf_ext)
+		return 0;
+
+	/* can't use .BTF.ext without .BTF */
+	if (!obj->btf)
+		return -EINVAL;
+
+	err = err ?: btf_ext_visit_type_ids(obj->btf_ext, check_btf_type_id, obj->btf);
+	err = err ?: btf_ext_visit_str_offs(obj->btf_ext, check_btf_str_off, obj->btf);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 static int init_sec(struct bpf_linker *linker, struct dst_sec *dst_sec, struct src_sec *src_sec)
 {
 	Elf_Scn *scn;
@@ -764,6 +898,10 @@ static int init_sec(struct bpf_linker *linker, struct dst_sec *dst_sec, struct s
 	dst_sec->sec_sz = 0;
 	dst_sec->sec_idx = 0;
 
+	/* ephemeral sections are just thin section shells lacking most parts */
+	if (src_sec->ephemeral)
+		return 0;
+
 	scn = elf_newscn(linker->elf);
 	if (!scn)
 		return -1;
@@ -892,12 +1030,15 @@ static bool is_data_sec(struct src_sec *sec)
 {
 	if (!sec || sec->skipped)
 		return false;
+	/* ephemeral sections are data sections, e.g., .kconfig, .ksyms */
+	if (sec->ephemeral)
+		return true;
 	return sec->shdr->sh_type == SHT_PROGBITS || sec->shdr->sh_type == SHT_NOBITS;
 }
 
 static bool is_relo_sec(struct src_sec *sec)
 {
-	if (!sec || sec->skipped)
+	if (!sec || sec->skipped || sec->ephemeral)
 		return false;
 	return sec->shdr->sh_type == SHT_REL;
 }
@@ -946,6 +1087,9 @@ static int linker_append_sec_data(struct bpf_linker *linker, struct src_obj *obj
 		/* record mapped section index */
 		src_sec->dst_id = dst_sec->id;
 
+		if (src_sec->ephemeral)
+			continue;
+
 		err = extend_sec(dst_sec, src_sec);
 		if (err)
 			return err;
@@ -1121,6 +1265,340 @@ static int linker_append_elf_relos(struct bpf_linker *linker, struct src_obj *ob
 	return 0;
 }
 
+static struct src_sec *find_src_sec_by_name(struct src_obj *obj, const char *sec_name)
+{
+	struct src_sec *sec;
+	int i;
+
+	for (i = 1; i < obj->sec_cnt; i++) {
+		sec = &obj->secs[i];
+
+		if (strcmp(sec->sec_name, sec_name) == 0)
+			return sec;
+	}
+
+	return NULL;
+}
+
+static Elf64_Sym *find_sym_by_name(struct src_obj *obj, size_t sec_idx,
+				   int sym_type, const char *sym_name)
+{
+	struct src_sec *symtab = &obj->secs[obj->symtab_sec_idx];
+	Elf64_Sym *sym = symtab->data->d_buf;
+	int i, n = symtab->shdr->sh_size / symtab->shdr->sh_entsize;
+	int str_sec_idx = symtab->shdr->sh_link;
+	const char *name;
+
+	for (i = 0; i < n; i++, sym++) {
+		if (sym->st_shndx != sec_idx)
+			continue;
+		if (ELF64_ST_TYPE(sym->st_info) != sym_type)
+			continue;
+
+		name = elf_strptr(obj->elf, str_sec_idx, sym->st_name);
+		if (!name)
+			return NULL;
+
+		if (strcmp(sym_name, name) != 0)
+			continue;
+
+		return sym;
+	}
+
+	return NULL;
+}
+
+static int linker_fixup_btf(struct src_obj *obj)
+{
+	const char *sec_name;
+	struct src_sec *sec;
+	int i, j, n, m;
+
+	n = btf__get_nr_types(obj->btf);
+	for (i = 1; i <= n; i++) {
+		struct btf_var_secinfo *vi;
+		struct btf_type *t;
+
+		t = btf_type_by_id(obj->btf, i);
+		if (btf_kind(t) != BTF_KIND_DATASEC)
+			continue;
+
+		sec_name = btf__str_by_offset(obj->btf, t->name_off);
+		sec = find_src_sec_by_name(obj, sec_name);
+		if (sec) {
+			/* record actual section size, unless ephemeral */
+			if (sec->shdr)
+				t->size = sec->shdr->sh_size;
+		} else {
+			/* BTF can have some sections that are not represented
+			 * in ELF, e.g., .kconfig and .ksyms, which are used
+			 * for special extern variables.  Here we'll
+			 * pre-create "section shells" for them to be able to
+			 * keep track of extra per-section metadata later
+			 * (e.g., BTF variables).
+			 */
+			sec = add_src_sec(obj, sec_name);
+			if (!sec)
+				return -ENOMEM;
+
+			sec->ephemeral = true;
+			sec->sec_idx = 0; /* will match UNDEF shndx in ELF */
+		}
+
+		/* remember ELF section and its BTF type ID match */
+		sec->sec_type_id = i;
+
+		/* fix up variable offsets */
+		vi = btf_var_secinfos(t);
+		for (j = 0, m = btf_vlen(t); j < m; j++, vi++) {
+			const struct btf_type *vt = btf__type_by_id(obj->btf, vi->type);
+			const char *var_name = btf__str_by_offset(obj->btf, vt->name_off);
+			int var_linkage = btf_var(vt)->linkage;
+			Elf64_Sym *sym;
+
+			/* no need to patch up static or extern vars */
+			if (var_linkage != BTF_VAR_GLOBAL_ALLOCATED)
+				continue;
+
+			sym = find_sym_by_name(obj, sec->sec_idx, STT_OBJECT, var_name);
+			if (!sym) {
+				pr_warn("failed to find symbol for variable '%s' in section '%s'\n", var_name, sec_name);
+				return -ENOENT;
+			}
+
+			vi->offset = sym->st_value;
+		}
+	}
+
+	return 0;
+}
+
+static int remap_type_id(__u32 *type_id, void *ctx)
+{
+	int *id_map = ctx;
+
+	*type_id = id_map[*type_id];
+
+	return 0;
+}
+
+static int linker_append_btf(struct bpf_linker *linker, struct src_obj *obj)
+{
+	const struct btf_type *t;
+	int i, j, n, start_id, id;
+
+	if (!obj->btf)
+		return 0;
+
+	start_id = btf__get_nr_types(linker->btf) + 1;
+	n = btf__get_nr_types(obj->btf);
+
+	obj->btf_type_map = calloc(n + 1, sizeof(int));
+	if (!obj->btf_type_map)
+		return -ENOMEM;
+
+	for (i = 1; i <= n; i++) {
+		t = btf__type_by_id(obj->btf, i);
+
+		/* DATASECs are handled specially below */
+		if (btf_kind(t) == BTF_KIND_DATASEC)
+			continue;
+
+		id = btf__add_type(linker->btf, obj->btf, t);
+		if (id < 0) {
+			pr_warn("failed to append BTF type #%d from file '%s'\n", i, obj->filename);
+			return id;
+		}
+
+		obj->btf_type_map[i] = id;
+	}
+
+	/* remap all the types except DATASECs */
+	n = btf__get_nr_types(linker->btf);
+	for (i = start_id; i <= n; i++) {
+		struct btf_type *dst_t = btf_type_by_id(linker->btf, i);
+
+		if (btf_type_visit_type_ids(dst_t, remap_type_id, obj->btf_type_map))
+			return -EINVAL;
+	}
+
+	/* append DATASEC info */
+	for (i = 1; i < obj->sec_cnt; i++) {
+		struct src_sec *src_sec;
+		struct dst_sec *dst_sec;
+		const struct btf_var_secinfo *src_var;
+		struct btf_var_secinfo *dst_var;
+
+		src_sec = &obj->secs[i];
+		if (!src_sec->sec_type_id)
+			continue;
+		dst_sec = &linker->secs[src_sec->dst_id];
+
+		t = btf__type_by_id(obj->btf, src_sec->sec_type_id);
+		src_var = btf_var_secinfos(t);
+		n = btf_vlen(t);
+		for (j = 0; j < n; j++, src_var++) {
+			void *sec_vars = dst_sec->sec_vars;
+
+			sec_vars = libbpf_reallocarray(sec_vars,
+						       dst_sec->sec_var_cnt + 1,
+						       sizeof(*dst_sec->sec_vars));
+			if (!sec_vars)
+				return -ENOMEM;
+
+			dst_sec->sec_vars = sec_vars;
+			dst_sec->sec_var_cnt++;
+
+			dst_var = &dst_sec->sec_vars[dst_sec->sec_var_cnt - 1];
+			dst_var->type = obj->btf_type_map[src_var->type];
+			dst_var->size = src_var->size;
+			dst_var->offset = src_sec->dst_off + src_var->offset;
+		}
+	}
+
+	return 0;
+}
+
+static void *add_btf_ext_rec(struct btf_ext_sec_data *ext_data, const void *src_rec)
+{
+	size_t new_sz = (ext_data->rec_cnt + 1) * ext_data->rec_sz;
+	void *tmp;
+
+	tmp = realloc(ext_data->recs, new_sz);
+	if (!tmp)
+		return NULL;
+
+	ext_data->recs = tmp;
+	ext_data->rec_cnt++;
+
+	tmp += new_sz - ext_data->rec_sz;
+	memcpy(tmp, src_rec, ext_data->rec_sz);
+
+	return tmp;
+}
+
+static int linker_append_btf_ext(struct bpf_linker *linker, struct src_obj *obj)
+{
+	const struct btf_ext_info_sec *ext_sec;
+	const char *sec_name, *s;
+	struct src_sec *src_sec;
+	struct dst_sec *dst_sec;
+	int rec_sz, str_off, i;
+
+	if (!obj->btf_ext)
+		return 0;
+
+	rec_sz = obj->btf_ext->func_info.rec_size;
+	for_each_btf_ext_sec(&obj->btf_ext->func_info, ext_sec) {
+		struct bpf_func_info_min *src_rec, *dst_rec;
+
+		sec_name = btf__name_by_offset(obj->btf, ext_sec->sec_name_off);
+		src_sec = find_src_sec_by_name(obj, sec_name);
+		if (!src_sec) {
+			pr_warn("can't find section '%s' referenced from .BTF.ext\n", sec_name);
+			return -EINVAL;
+		}
+		dst_sec = &linker->secs[src_sec->dst_id];
+
+		if (dst_sec->func_info.rec_sz == 0)
+			dst_sec->func_info.rec_sz = rec_sz;
+		if (dst_sec->func_info.rec_sz != rec_sz) {
+			pr_warn("incompatible .BTF.ext record sizes for section '%s'\n", sec_name);
+			return -EINVAL;
+		}
+
+		for_each_btf_ext_rec(&obj->btf_ext->func_info, ext_sec, i, src_rec) {
+			dst_rec = add_btf_ext_rec(&dst_sec->func_info, src_rec);
+			if (!dst_rec)
+				return -ENOMEM;
+
+			dst_rec->insn_off += src_sec->dst_off;
+			dst_rec->type_id = obj->btf_type_map[dst_rec->type_id];
+		}
+	}
+
+	rec_sz = obj->btf_ext->line_info.rec_size;
+	for_each_btf_ext_sec(&obj->btf_ext->line_info, ext_sec) {
+		struct bpf_line_info_min *src_rec, *dst_rec;
+
+		sec_name = btf__name_by_offset(obj->btf, ext_sec->sec_name_off);
+		src_sec = find_src_sec_by_name(obj, sec_name);
+		if (!src_sec) {
+			pr_warn("can't find section '%s' referenced from .BTF.ext\n", sec_name);
+			return -EINVAL;
+		}
+		dst_sec = &linker->secs[src_sec->dst_id];
+
+		if (dst_sec->line_info.rec_sz == 0)
+			dst_sec->line_info.rec_sz = rec_sz;
+		if (dst_sec->line_info.rec_sz != rec_sz) {
+			pr_warn("incompatible .BTF.ext record sizes for section '%s'\n", sec_name);
+			return -EINVAL;
+		}
+
+		for_each_btf_ext_rec(&obj->btf_ext->line_info, ext_sec, i, src_rec) {
+			dst_rec = add_btf_ext_rec(&dst_sec->line_info, src_rec);
+			if (!dst_rec)
+				return -ENOMEM;
+
+			dst_rec->insn_off += src_sec->dst_off;
+
+			s = btf__str_by_offset(obj->btf, src_rec->file_name_off);
+			str_off = btf__add_str(linker->btf, s);
+			if (str_off < 0)
+				return -ENOMEM;
+			dst_rec->file_name_off = str_off;
+
+			s = btf__str_by_offset(obj->btf, src_rec->line_off);
+			str_off = btf__add_str(linker->btf, s);
+			if (str_off < 0)
+				return -ENOMEM;
+			dst_rec->line_off = str_off;
+
+			/* dst_rec->line_col is fine */
+		}
+	}
+
+	rec_sz = obj->btf_ext->core_relo_info.rec_size;
+	for_each_btf_ext_sec(&obj->btf_ext->core_relo_info, ext_sec) {
+		struct bpf_core_relo *src_rec, *dst_rec;
+
+		sec_name = btf__name_by_offset(obj->btf, ext_sec->sec_name_off);
+		src_sec = find_src_sec_by_name(obj, sec_name);
+		if (!src_sec) {
+			pr_warn("can't find section '%s' referenced from .BTF.ext\n", sec_name);
+			return -EINVAL;
+		}
+		dst_sec = &linker->secs[src_sec->dst_id];
+
+		if (dst_sec->core_relo_info.rec_sz == 0)
+			dst_sec->core_relo_info.rec_sz = rec_sz;
+		if (dst_sec->core_relo_info.rec_sz != rec_sz) {
+			pr_warn("incompatible .BTF.ext record sizes for section '%s'\n", sec_name);
+			return -EINVAL;
+		}
+
+		for_each_btf_ext_rec(&obj->btf_ext->core_relo_info, ext_sec, i, src_rec) {
+			dst_rec = add_btf_ext_rec(&dst_sec->core_relo_info, src_rec);
+			if (!dst_rec)
+				return -ENOMEM;
+
+			dst_rec->insn_off += src_sec->dst_off;
+			dst_rec->type_id = obj->btf_type_map[dst_rec->type_id];
+
+			s = btf__str_by_offset(obj->btf, src_rec->access_str_off);
+			str_off = btf__add_str(linker->btf, s);
+			if (str_off < 0)
+				return -ENOMEM;
+			dst_rec->access_str_off = str_off;
+
+			/* dst_rec->kind is fine */
+		}
+	}
+
+	return 0;
+}
+
 int bpf_linker__finalize(struct bpf_linker *linker)
 {
 	struct dst_sec *sec;
@@ -1131,6 +1609,10 @@ int bpf_linker__finalize(struct bpf_linker *linker)
 	if (!linker->elf)
 		return -EINVAL;
 
+	err = finalize_btf(linker);
+	if (err)
+		return err;
+
 	/* Finalize strings */
 	strs = btf_raw_strs(linker->strtab_btf, &strs_sz);
 
@@ -1149,6 +1631,10 @@ int bpf_linker__finalize(struct bpf_linker *linker)
 		if (sec->sec_idx == linker->strtab_sec_idx)
 			continue;
 
+		/* special ephemeral sections (.ksyms, .kconfig, etc) */
+		if (!sec->scn)
+			continue;
+
 		sec->data->d_buf = sec->raw_data;
 	}
 
@@ -1174,3 +1660,286 @@ int bpf_linker__finalize(struct bpf_linker *linker)
 
 	return 0;
 }
+
+static int emit_elf_data_sec(struct bpf_linker *linker, const char *sec_name,
+			     size_t align, const void *raw_data, size_t raw_sz)
+{
+	Elf_Scn *scn;
+	Elf_Data *data;
+	Elf64_Shdr *shdr;
+	int name_off;
+
+	name_off = btf__add_str(linker->strtab_btf, sec_name);
+	if (name_off < 0)
+		return name_off;
+
+	scn = elf_newscn(linker->elf);
+	if (!scn)
+		return -ENOMEM;
+	data = elf_newdata(scn);
+	if (!data)
+		return -ENOMEM;
+	shdr = elf64_getshdr(scn);
+	if (!shdr)
+		return -EINVAL;
+
+	shdr->sh_name = name_off;
+	shdr->sh_type = SHT_PROGBITS;
+	shdr->sh_flags = 0;
+	shdr->sh_size = raw_sz;
+	shdr->sh_link = 0;
+	shdr->sh_info = 0;
+	shdr->sh_addralign = align;
+	shdr->sh_entsize = 0;
+
+	data->d_type = ELF_T_BYTE;
+	data->d_size = raw_sz;
+	data->d_buf = (void *)raw_data;
+	data->d_align = align;
+	data->d_off = 0;
+
+	return 0;
+}
+
+static int finalize_btf(struct bpf_linker *linker)
+{
+	struct btf *btf = linker->btf;
+	const void *raw_data;
+	int i, j, id, err;
+	__u32 raw_sz;
+
+	/* bail out if no BTF data was produced */
+	if (btf__get_nr_types(linker->btf) == 0)
+		return 0;
+
+	for (i = 1; i < linker->sec_cnt; i++) {
+		struct dst_sec *sec = &linker->secs[i];
+
+		if (!sec->sec_var_cnt)
+			continue;
+
+		id = btf__add_datasec(btf, sec->sec_name, sec->sec_sz);
+		if (id < 0) {
+			pr_warn("failed to add consolidated BTF type for datasec '%s': %d\n",
+				sec->sec_name, id);
+			return id;
+		}
+
+		for (j = 0; j < sec->sec_var_cnt; j++) {
+			struct btf_var_secinfo *vi = &sec->sec_vars[j];
+
+			if (btf__add_datasec_var_info(btf, vi->type, vi->offset, vi->size))
+				return -EINVAL;
+		}
+	}
+
+	err = finalize_btf_ext(linker);
+	if (err) {
+		pr_warn(".BTF.ext generation failed: %d\n", err);
+		return err;
+	}
+
+	err = btf__dedup(linker->btf, linker->btf_ext, NULL);
+	if (err) {
+		pr_warn("BTF dedup failed: %d\n", err);
+		return err;
+	}
+
+	/* Emit .BTF section */
+	raw_data = btf__get_raw_data(linker->btf, &raw_sz);
+	if (!raw_data)
+		return -ENOMEM;
+
+	err = emit_elf_data_sec(linker, BTF_ELF_SEC, 8, raw_data, raw_sz);
+	if (err) {
+		pr_warn("failed to write out .BTF ELF section: %d\n", err);
+		return err;
+	}
+
+	/* Emit .BTF.ext section */
+	if (linker->btf_ext) {
+		raw_data = btf_ext__get_raw_data(linker->btf_ext, &raw_sz);
+		if (!raw_data)
+			return -ENOMEM;
+
+		err = emit_elf_data_sec(linker, BTF_EXT_ELF_SEC, 8, raw_data, raw_sz);
+		if (err) {
+			pr_warn("failed to write out .BTF.ext ELF section: %d\n", err);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+static int finalize_btf_ext(struct bpf_linker *linker)
+{
+	size_t funcs_sz = 0, lines_sz = 0, core_relos_sz = 0, total_sz = 0;
+	size_t func_rec_sz = 0, line_rec_sz = 0, core_relo_rec_sz = 0;
+	struct btf_ext_header *hdr;
+	struct btf_ext_info_sec *sec_info;
+	void *data, *cur;
+	int i, err, str_off;
+
+	for (i = 1; i < linker->sec_cnt; i++) {
+		struct dst_sec *sec = &linker->secs[i];
+
+		if (sec->func_info.rec_cnt) {
+			if (func_rec_sz == 0)
+				func_rec_sz = sec->func_info.rec_sz;
+			if (func_rec_sz != sec->func_info.rec_sz) {
+				pr_warn("mismatch in func_info record size %zu != %u\n",
+					func_rec_sz, sec->func_info.rec_sz);
+				return -EINVAL;
+			}
+
+			funcs_sz += sizeof(struct btf_ext_info_sec) + func_rec_sz * sec->func_info.rec_cnt;
+		}
+		if (sec->line_info.rec_cnt) {
+			if (line_rec_sz == 0)
+				line_rec_sz = sec->line_info.rec_sz;
+			if (line_rec_sz != sec->line_info.rec_sz) {
+				pr_warn("mismatch in line_info record size %zu != %u\n",
+					line_rec_sz, sec->line_info.rec_sz);
+				return -EINVAL;
+			}
+
+			lines_sz += sizeof(struct btf_ext_info_sec) + line_rec_sz * sec->line_info.rec_cnt;
+		}
+		if (sec->core_relo_info.rec_cnt) {
+			if (core_relo_rec_sz == 0)
+				core_relo_rec_sz = sec->core_relo_info.rec_sz;
+			if (core_relo_rec_sz != sec->core_relo_info.rec_sz) {
+				pr_warn("mismatch in core_relo_info record size %zu != %u\n",
+					core_relo_rec_sz, sec->core_relo_info.rec_sz);
+				return -EINVAL;
+			}
+
+			core_relos_sz += sizeof(struct btf_ext_info_sec) + core_relo_rec_sz * sec->core_relo_info.rec_cnt;
+		}
+	}
+
+	if (!funcs_sz && !lines_sz && !core_relos_sz)
+		return 0;
+
+	total_sz += sizeof(struct btf_ext_header);
+	if (funcs_sz) {
+		funcs_sz += sizeof(__u32); /* record size prefix */
+		total_sz += funcs_sz;
+	}
+	if (lines_sz) {
+		lines_sz += sizeof(__u32); /* record size prefix */
+		total_sz += lines_sz;
+	}
+	if (core_relos_sz) {
+		core_relos_sz += sizeof(__u32); /* record size prefix */
+		total_sz += core_relos_sz;
+	}
+
+	cur = data = calloc(1, total_sz);
+	if (!data)
+		return -ENOMEM;
+
+	hdr = cur;
+	hdr->magic = BTF_MAGIC;
+	hdr->version = BTF_VERSION;
+	hdr->flags = 0;
+	hdr->hdr_len = sizeof(struct btf_ext_header);
+	cur += sizeof(struct btf_ext_header);
+
+	/* All offsets are in bytes relative to the end of this header */
+	hdr->func_info_off = 0;
+	hdr->func_info_len = funcs_sz;
+	hdr->line_info_off = funcs_sz;
+	hdr->line_info_len = lines_sz;
+	hdr->core_relo_off = funcs_sz + lines_sz;;
+	hdr->core_relo_len = core_relos_sz;
+
+	if (funcs_sz) {
+		*(__u32 *)cur = func_rec_sz;
+		cur += sizeof(__u32);
+
+		for (i = 1; i < linker->sec_cnt; i++) {
+			struct dst_sec *sec = &linker->secs[i];
+			size_t sz;
+
+			if (!sec->func_info.rec_cnt)
+				continue;
+
+			str_off = btf__add_str(linker->btf, sec->sec_name);
+			if (str_off < 0)
+				return -ENOMEM;
+
+			sec_info = cur;
+			sec_info->sec_name_off = str_off;
+			sec_info->num_info = sec->func_info.rec_cnt;
+			cur += sizeof(struct btf_ext_info_sec);
+
+			sz = sec->func_info.rec_cnt * func_rec_sz;
+			memcpy(cur, sec->func_info.recs, sz);
+			cur += sz;
+		}
+	}
+
+	if (lines_sz) {
+		*(__u32 *)cur = line_rec_sz;
+		cur += sizeof(__u32);
+
+		for (i = 1; i < linker->sec_cnt; i++) {
+			struct dst_sec *sec = &linker->secs[i];
+			size_t sz;
+
+			if (!sec->line_info.rec_cnt)
+				continue;
+
+			str_off = btf__add_str(linker->btf, sec->sec_name);
+			if (str_off < 0)
+				return -ENOMEM;
+
+			sec_info = cur;
+			sec_info->sec_name_off = str_off;
+			sec_info->num_info = sec->line_info.rec_cnt;
+			cur += sizeof(struct btf_ext_info_sec);
+
+			sz = sec->line_info.rec_cnt * line_rec_sz;
+			memcpy(cur, sec->line_info.recs, sz);
+			cur += sz;
+		}
+	}
+
+	if (core_relos_sz) {
+		*(__u32 *)cur = core_relo_rec_sz;
+		cur += sizeof(__u32);
+
+		for (i = 1; i < linker->sec_cnt; i++) {
+			struct dst_sec *sec = &linker->secs[i];
+			size_t sz;
+
+			if (!sec->core_relo_info.rec_cnt)
+				continue;
+
+			str_off = btf__add_str(linker->btf, sec->sec_name);
+			if (str_off < 0)
+				return -ENOMEM;
+
+			sec_info = cur;
+			sec_info->sec_name_off = str_off;
+			sec_info->num_info = sec->core_relo_info.rec_cnt;
+			cur += sizeof(struct btf_ext_info_sec);
+
+			sz = sec->core_relo_info.rec_cnt * core_relo_rec_sz;
+			memcpy(cur, sec->core_relo_info.recs, sz);
+			cur += sz;
+		}
+	}
+
+	linker->btf_ext = btf_ext__new(data, total_sz);
+	err = libbpf_get_error(linker->btf_ext);
+	if (err) {
+		linker->btf_ext = NULL;
+		pr_warn("failed to parse final .BTF.ext data: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking
  2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
                   ` (5 preceding siblings ...)
  2021-03-10  4:04 ` [PATCH bpf-next 06/10] libbpf: add BPF static linker BTF and BTF.ext support Andrii Nakryiko
@ 2021-03-10  4:04 ` Andrii Nakryiko
  2021-03-11 11:31   ` Quentin Monnet
  2021-03-10  4:04 ` [PATCH bpf-next 08/10] selftests/bpf: re-generate vmlinux.h and BPF skeletons if bpftool changed Andrii Nakryiko
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

Add `bpftool gen bpfo <output-file> <input_file>...` command to statically
link multiple BPF object files into a single output BPF object file.

Similarly to existing '*.o' convention, bpftool is establishing a '*.bpfo'
convention for statically-linked BPF object files. Both .o and .bpfo suffixes
will be stripped out during BPF skeleton generation to infer BPF object name.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/bpf/bpftool/gen.c | 46 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
index 4033c46d83e7..8b1ed6c0a62f 100644
--- a/tools/bpf/bpftool/gen.c
+++ b/tools/bpf/bpftool/gen.c
@@ -54,7 +54,9 @@ static void get_obj_name(char *name, const char *file)
 	strncpy(name, basename(file), MAX_OBJ_NAME_LEN - 1);
 	name[MAX_OBJ_NAME_LEN - 1] = '\0';
 	if (str_has_suffix(name, ".o"))
-		name[strlen(name) - 2] = '\0';
+		name[strlen(name) - sizeof(".o") + 1] = '\0';
+	else if (str_has_suffix(name, ".bpfo"))
+		name[strlen(name) - sizeof(".bpfo") + 1] = '\0';
 	sanitize_identifier(name);
 }
 
@@ -591,6 +593,47 @@ static int do_skeleton(int argc, char **argv)
 	return err;
 }
 
+static int do_bpfo(int argc, char **argv)
+{
+	struct bpf_linker *linker;
+	const char *output_file, *file;
+	int err;
+
+	if (!REQ_ARGS(2)) {
+		usage();
+		return -1;
+	}
+
+	output_file = GET_ARG();
+
+	linker = bpf_linker__new(output_file, NULL);
+	if (!linker) {
+		p_err("failed to create BPF linker instance");
+		return -1;
+	}
+
+	while (argc) {
+		file = GET_ARG();
+
+		err = bpf_linker__add_file(linker, file);
+		if (err) {
+			p_err("failed to link '%s': %d", file, err);
+			goto err_out;
+		}
+	}
+
+	err = bpf_linker__finalize(linker);
+	if (err) {
+		p_err("failed to finalize ELF file: %d", err);
+		goto err_out;
+	}
+
+	return 0;
+err_out:
+	bpf_linker__free(linker);
+	return -1;
+}
+
 static int do_help(int argc, char **argv)
 {
 	if (json_output) {
@@ -611,6 +654,7 @@ static int do_help(int argc, char **argv)
 
 static const struct cmd cmds[] = {
 	{ "skeleton",	do_skeleton },
+	{ "bpfo",	do_bpfo },
 	{ "help",	do_help },
 	{ 0 }
 };
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next 08/10] selftests/bpf: re-generate vmlinux.h and BPF skeletons if bpftool changed
  2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
                   ` (6 preceding siblings ...)
  2021-03-10  4:04 ` [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking Andrii Nakryiko
@ 2021-03-10  4:04 ` Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 09/10] selftests/bpf: pass all BPF .o's through BPF static linker Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 10/10] selftests/bpf: add multi-file statically linked BPF object file test Andrii Nakryiko
  9 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

Trigger vmlinux.h and BPF skeletons re-generation if detected that bpftool was
re-compiled. Otherwise full `make clean` is required to get updated skeletons,
if bpftool is modified.

Fixes: acbd06206bbb ("selftests/bpf: Add vmlinux.h selftest exercising tracing of syscalls")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index d0db2b673c6f..dbca39f45382 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -232,7 +232,7 @@ $(HOST_BPFOBJ): $(wildcard $(BPFDIR)/*.[ch] $(BPFDIR)/Makefile)                \
 		    DESTDIR=$(HOST_SCRATCH_DIR)/ prefix= all install_headers
 endif
 
-$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) | $(BPFTOOL) $(INCLUDE_DIR)
+$(INCLUDE_DIR)/vmlinux.h: $(VMLINUX_BTF) $(BPFTOOL) | $(INCLUDE_DIR)
 ifeq ($(VMLINUX_H),)
 	$(call msg,GEN,,$@)
 	$(Q)$(BPFTOOL) btf dump file $(VMLINUX_BTF) format c > $@
@@ -357,7 +357,8 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.o:				\
 
 $(TRUNNER_BPF_SKELS): $(TRUNNER_OUTPUT)/%.skel.h:			\
 		      $(TRUNNER_OUTPUT)/%.o				\
-		      | $(BPFTOOL) $(TRUNNER_OUTPUT)
+		      $(BPFTOOL)					\
+		      | $(TRUNNER_OUTPUT)
 	$$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@)
 	$(Q)$$(BPFTOOL) gen skeleton $$< > $$@
 endif
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next 09/10] selftests/bpf: pass all BPF .o's through BPF static linker
  2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
                   ` (7 preceding siblings ...)
  2021-03-10  4:04 ` [PATCH bpf-next 08/10] selftests/bpf: re-generate vmlinux.h and BPF skeletons if bpftool changed Andrii Nakryiko
@ 2021-03-10  4:04 ` Andrii Nakryiko
  2021-03-10  4:04 ` [PATCH bpf-next 10/10] selftests/bpf: add multi-file statically linked BPF object file test Andrii Nakryiko
  9 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

Pass all individual BPF object files (progs/*.o) through `bpftool gen bpfo`
command to validate that BPF static linker doesn't corrupt them.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/.gitignore | 1 +
 tools/testing/selftests/bpf/Makefile   | 8 +++-----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 4866f6a21901..811da0ea3ecd 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -36,4 +36,5 @@ test_cpp
 /runqslower
 /bench
 *.ko
+*.bpfo
 xdpxceiver
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index dbca39f45382..45a2ae1916ed 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -355,12 +355,10 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.o:				\
 	$$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@,			\
 					  $(TRUNNER_BPF_CFLAGS))
 
-$(TRUNNER_BPF_SKELS): $(TRUNNER_OUTPUT)/%.skel.h:			\
-		      $(TRUNNER_OUTPUT)/%.o				\
-		      $(BPFTOOL)					\
-		      | $(TRUNNER_OUTPUT)
+$(TRUNNER_BPF_SKELS): %.skel.h: %.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
 	$$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@)
-	$(Q)$$(BPFTOOL) gen skeleton $$< > $$@
+	$(Q)$$(BPFTOOL) gen bpfo $$(<:.o=.bpfo) $$<
+	$(Q)$$(BPFTOOL) gen skeleton $$(<:.o=.bpfo) > $$@
 endif
 
 # ensure we set up tests.h header generation rule just once
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH bpf-next 10/10] selftests/bpf: add multi-file statically linked BPF object file test
  2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
                   ` (8 preceding siblings ...)
  2021-03-10  4:04 ` [PATCH bpf-next 09/10] selftests/bpf: pass all BPF .o's through BPF static linker Andrii Nakryiko
@ 2021-03-10  4:04 ` Andrii Nakryiko
  9 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-10  4:04 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii, kernel-team

Add Makefile infra to specify multi-file BPF object files (and derivative
skeletons). Add first selftest validating BPF static linker can merge together
successfully two independent BPF object files and resulting object and
skeleton are correct and usable.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/testing/selftests/bpf/Makefile          | 12 ++++++
 .../selftests/bpf/prog_tests/static_linked.c  | 40 +++++++++++++++++++
 .../selftests/bpf/progs/test_static_linked1.c | 30 ++++++++++++++
 .../selftests/bpf/progs/test_static_linked2.c | 31 ++++++++++++++
 4 files changed, 113 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/static_linked.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_static_linked1.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_static_linked2.c

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 45a2ae1916ed..4d2f189d69dc 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -303,6 +303,10 @@ endef
 
 SKEL_BLACKLIST := btf__% test_pinning_invalid.c test_sk_assign.c
 
+LINKED_SKELS := test_static_linked.skel.h
+
+test_static_linked.skel.h-deps := test_static_linked1.o test_static_linked2.o
+
 # Set up extra TRUNNER_XXX "temporary" variables in the environment (relies on
 # $eval()) and pass control to DEFINE_TEST_RUNNER_RULES.
 # Parameters:
@@ -323,6 +327,7 @@ TRUNNER_BPF_OBJS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.o, $$(TRUNNER_BPF_SRCS)
 TRUNNER_BPF_SKELS := $$(patsubst %.c,$$(TRUNNER_OUTPUT)/%.skel.h,	\
 				 $$(filter-out $(SKEL_BLACKLIST),	\
 					       $$(TRUNNER_BPF_SRCS)))
+TRUNNER_BPF_SKELS_LINKED := $$(addprefix $$(TRUNNER_OUTPUT)/,$(LINKED_SKELS))
 TEST_GEN_FILES += $$(TRUNNER_BPF_OBJS)
 
 # Evaluate rules now with extra TRUNNER_XXX variables above already defined
@@ -359,6 +364,12 @@ $(TRUNNER_BPF_SKELS): %.skel.h: %.o $(BPFTOOL) | $(TRUNNER_OUTPUT)
 	$$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@)
 	$(Q)$$(BPFTOOL) gen bpfo $$(<:.o=.bpfo) $$<
 	$(Q)$$(BPFTOOL) gen skeleton $$(<:.o=.bpfo) > $$@
+
+$(TRUNNER_BPF_SKELS_LINKED): $(TRUNNER_BPF_OBJS) $(BPFTOOL) | $(TRUNNER_OUTPUT)
+	$$(call msg,LINK-BPF,$(TRUNNER_BINARY),$$(@:.skel.h=.bpfo))
+	$(Q)$$(BPFTOOL) gen bpfo $$(@:.skel.h=.bpfo) $$(addprefix $(TRUNNER_OUTPUT)/,$$($$(@F)-deps))
+	$$(call msg,GEN-SKEL,$(TRUNNER_BINARY),$$@)
+	$(Q)$$(BPFTOOL) gen skeleton $$(@:.skel.h=.bpfo) > $$@
 endif
 
 # ensure we set up tests.h header generation rule just once
@@ -380,6 +391,7 @@ $(TRUNNER_TEST_OBJS): $(TRUNNER_OUTPUT)/%.test.o:			\
 		      $(TRUNNER_EXTRA_HDRS)				\
 		      $(TRUNNER_BPF_OBJS)				\
 		      $(TRUNNER_BPF_SKELS)				\
+		      $(TRUNNER_BPF_SKELS_LINKED)			\
 		      $$(BPFOBJ) | $(TRUNNER_OUTPUT)
 	$$(call msg,TEST-OBJ,$(TRUNNER_BINARY),$$@)
 	$(Q)cd $$(@D) && $$(CC) -I. $$(CFLAGS) -c $(CURDIR)/$$< $$(LDLIBS) -o $$(@F)
diff --git a/tools/testing/selftests/bpf/prog_tests/static_linked.c b/tools/testing/selftests/bpf/prog_tests/static_linked.c
new file mode 100644
index 000000000000..46556976dccc
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/static_linked.c
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019 Facebook */
+
+#include <test_progs.h>
+#include "test_static_linked.skel.h"
+
+void test_static_linked(void)
+{
+	int err;
+	struct test_static_linked* skel;
+
+	skel = test_static_linked__open();
+	if (!ASSERT_OK_PTR(skel, "skel_open"))
+		return;
+
+	skel->rodata->rovar1 = 1;
+	skel->bss->static_var1 = 2;
+	skel->bss->static_var11 = 3;
+
+	skel->rodata->rovar2 = 4;
+	skel->bss->static_var2 = 5;
+	skel->bss->static_var22 = 6;
+
+	err = test_static_linked__load(skel);
+	if (!ASSERT_OK(err, "skel_load"))
+		goto cleanup;
+
+	err = test_static_linked__attach(skel);
+	if (!ASSERT_OK(err, "skel_attach"))
+		goto cleanup;
+
+	/* trigger */
+	usleep(1);
+
+	ASSERT_EQ(skel->bss->var1, 1 * 2 + 2 + 3, "var1");
+	ASSERT_EQ(skel->bss->var2, 4 * 3 + 5 + 6, "var2");
+
+cleanup:
+	test_static_linked__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_static_linked1.c b/tools/testing/selftests/bpf/progs/test_static_linked1.c
new file mode 100644
index 000000000000..ea1a6c4c7172
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_static_linked1.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Facebook */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+/* 8-byte aligned .bss */
+static volatile long static_var1;
+static volatile int static_var11;
+int var1 = 0;
+/* 4-byte aligned .rodata */
+const volatile int rovar1;
+
+/* same "subprog" name in both files */
+static __noinline int subprog(int x)
+{
+	/* but different formula */
+	return x * 2;
+}
+
+SEC("raw_tp/sys_enter")
+int handler1(const void *ctx)
+{
+	var1 = subprog(rovar1) + static_var1 + static_var11;
+
+	return 0;
+}
+
+char LICENSE[] SEC("license") = "GPL";
+int VERSION SEC("version") = 1;
diff --git a/tools/testing/selftests/bpf/progs/test_static_linked2.c b/tools/testing/selftests/bpf/progs/test_static_linked2.c
new file mode 100644
index 000000000000..54d8d1ab577c
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_static_linked2.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Facebook */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+/* 4-byte aligned .bss */
+static volatile int static_var2;
+static volatile int static_var22;
+int var2 = 0;
+/* 8-byte aligned .rodata */
+const volatile long rovar2;
+
+/* same "subprog" name in both files */
+static __noinline int subprog(int x)
+{
+	/* but different formula */
+	return x * 3;
+}
+
+SEC("raw_tp/sys_enter")
+int handler2(const void *ctx)
+{
+	var2 = subprog(rovar2) + static_var2 + static_var22;
+
+	return 0;
+}
+
+/* different name and/or type of the variable doesn't matter */
+char _license[] SEC("license") = "GPL";
+int _version SEC("version") = 1;
-- 
2.24.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next 05/10] libbpf: add BPF static linker APIs
  2021-03-10  4:04 ` [PATCH bpf-next 05/10] libbpf: add BPF static linker APIs Andrii Nakryiko
@ 2021-03-11  2:34   ` Alexei Starovoitov
  2021-03-11  3:29     ` Andrii Nakryiko
  0 siblings, 1 reply; 18+ messages in thread
From: Alexei Starovoitov @ 2021-03-11  2:34 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, netdev, ast, daniel, kernel-team

On Tue, Mar 09, 2021 at 08:04:26PM -0800, Andrii Nakryiko wrote:
> +
> +	struct btf *strtab_btf; /* we use struct btf to manage strings */
...
> +	str_off = btf__add_str(linker->strtab_btf, sec->sec_name);
> +	sec->shdr->sh_name = str_off;

That bit took me an hour to grok.
That single line comment above is far far from obvious.
What the logic is relying on is that string section in BTF format
has the same zero terminated set of strings as ELF's .strtab section.
There is no BTF anywhere here in this 'strtab_btf'.
The naming choice made it double hard.
My understanding that you're using that instead of renaming btf_add_mem()
into something generic to rely on string hashmap for string dedup?

The commit log in patch 2 that introduces btf_raw_strs() sort of talks about
this code puzzle, but I would never guessed that's what you meant based
on patch 2 alone.

Did you consider some renaming/generalizing of string management to
avoid btf__add_str() through out the patch 5?
The "btf_" prefix makes things challenging to read.
Especially when patch 6 is using btf__add_str() to add to real BTF.

Mainly pointing it out for others who might be looking at the patches.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next 05/10] libbpf: add BPF static linker APIs
  2021-03-11  2:34   ` Alexei Starovoitov
@ 2021-03-11  3:29     ` Andrii Nakryiko
  0 siblings, 0 replies; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-11  3:29 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Wed, Mar 10, 2021 at 6:34 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Tue, Mar 09, 2021 at 08:04:26PM -0800, Andrii Nakryiko wrote:
> > +
> > +     struct btf *strtab_btf; /* we use struct btf to manage strings */
> ...
> > +     str_off = btf__add_str(linker->strtab_btf, sec->sec_name);
> > +     sec->shdr->sh_name = str_off;
>
> That bit took me an hour to grok.
> That single line comment above is far far from obvious.

Heh, I guess I've been working with BTF, ELF and pahole for too long
to notice that it's so non-obvious. pahole wraps `struct btf` in a
similar fashion for deduplicated string management.

> What the logic is relying on is that string section in BTF format
> has the same zero terminated set of strings as ELF's .strtab section.
> There is no BTF anywhere here in this 'strtab_btf'.
> The naming choice made it double hard.

Right. strtab_strs would probably be a slightly better choice.

> My understanding that you're using that instead of renaming btf_add_mem()
> into something generic to rely on string hashmap for string dedup?

It's not about renaming btf_add_mem(). btf_add_mem() just implements
memory re-allocation (with exponential increase). But here we want to
not add a new string if it's already present. So it's much more
complicated logic than btf_add_mem().

>
> The commit log in patch 2 that introduces btf_raw_strs() sort of talks about
> this code puzzle, but I would never guessed that's what you meant based
> on patch 2 alone.
>
> Did you consider some renaming/generalizing of string management to
> avoid btf__add_str() through out the patch 5?
> The "btf_" prefix makes things challenging to read.
> Especially when patch 6 is using btf__add_str() to add to real BTF.

Right. I guess we can extract the "set of strings" data structure out
of `struct btf` into libbpf-internal data structure. Then use it from
struct btf and separately (and directly) from struct bpf_linker. I'll
see what that would involve in terms of refactoring.

>
> Mainly pointing it out for others who might be looking at the patches.

That's a good point, I should have probably at least mentioned that
bit more explicitly.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking
  2021-03-10  4:04 ` [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking Andrii Nakryiko
@ 2021-03-11 11:31   ` Quentin Monnet
  2021-03-11 18:45     ` Andrii Nakryiko
  0 siblings, 1 reply; 18+ messages in thread
From: Quentin Monnet @ 2021-03-11 11:31 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel; +Cc: kernel-team

2021-03-09 20:04 UTC-0800 ~ Andrii Nakryiko <andrii@kernel.org>
> Add `bpftool gen bpfo <output-file> <input_file>...` command to statically
> link multiple BPF object files into a single output BPF object file.
> 
> Similarly to existing '*.o' convention, bpftool is establishing a '*.bpfo'
> convention for statically-linked BPF object files. Both .o and .bpfo suffixes
> will be stripped out during BPF skeleton generation to infer BPF object name.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
>  tools/bpf/bpftool/gen.c | 46 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> index 4033c46d83e7..8b1ed6c0a62f 100644
> --- a/tools/bpf/bpftool/gen.c
> +++ b/tools/bpf/bpftool/gen.c
> +static int do_bpfo(int argc, char **argv)

> +{
> +	struct bpf_linker *linker;
> +	const char *output_file, *file;
> +	int err;
> +
> +	if (!REQ_ARGS(2)) {
> +		usage();
> +		return -1;
> +	}
> +
> +	output_file = GET_ARG();
> +
> +	linker = bpf_linker__new(output_file, NULL);
> +	if (!linker) {
> +		p_err("failed to create BPF linker instance");
> +		return -1;
> +	}
> +
> +	while (argc) {
> +		file = GET_ARG();
> +
> +		err = bpf_linker__add_file(linker, file);
> +		if (err) {
> +			p_err("failed to link '%s': %d", file, err);

I think you mentioned before that your preference was for having just
the error code instead of using strerror(), but I think it would be more
user-friendly for the majority of users who don't know the error codes
if we had something more verbose? How about having both strerror()
output and the error code?

> +			goto err_out;
> +		}
> +	}
> +
> +	err = bpf_linker__finalize(linker);
> +	if (err) {
> +		p_err("failed to finalize ELF file: %d", err);
> +		goto err_out;
> +	}
> +
> +	return 0;
> +err_out:
> +	bpf_linker__free(linker);
> +	return -1;

Should you call bpf_linker__free() even on success? I see that
bpf_linker__finalize() frees some of the resources, but it seems that
bpf_linker__free() does a more thorough job?

> +}
> +
>  static int do_help(int argc, char **argv)
>  {
>  	if (json_output) {
> @@ -611,6 +654,7 @@ static int do_help(int argc, char **argv)
>  
>  static const struct cmd cmds[] = {
>  	{ "skeleton",	do_skeleton },
> +	{ "bpfo",	do_bpfo },
>  	{ "help",	do_help },
>  	{ 0 }
>  };
> 

Please update the usage help message, man page, and bash completion,
thanks. Especially because what "bpftool gen bpfo" does is not intuitive
(but I don't have a better name suggestion at the moment).

Great work!

Quentin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking
  2021-03-11 11:31   ` Quentin Monnet
@ 2021-03-11 18:45     ` Andrii Nakryiko
  2021-03-12 18:07       ` Quentin Monnet
  0 siblings, 1 reply; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-11 18:45 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Thu, Mar 11, 2021 at 3:31 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2021-03-09 20:04 UTC-0800 ~ Andrii Nakryiko <andrii@kernel.org>
> > Add `bpftool gen bpfo <output-file> <input_file>...` command to statically
> > link multiple BPF object files into a single output BPF object file.
> >
> > Similarly to existing '*.o' convention, bpftool is establishing a '*.bpfo'
> > convention for statically-linked BPF object files. Both .o and .bpfo suffixes
> > will be stripped out during BPF skeleton generation to infer BPF object name.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > ---
> >  tools/bpf/bpftool/gen.c | 46 ++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> > index 4033c46d83e7..8b1ed6c0a62f 100644
> > --- a/tools/bpf/bpftool/gen.c
> > +++ b/tools/bpf/bpftool/gen.c
> > +static int do_bpfo(int argc, char **argv)
>
> > +{
> > +     struct bpf_linker *linker;
> > +     const char *output_file, *file;
> > +     int err;
> > +
> > +     if (!REQ_ARGS(2)) {
> > +             usage();
> > +             return -1;
> > +     }
> > +
> > +     output_file = GET_ARG();
> > +
> > +     linker = bpf_linker__new(output_file, NULL);
> > +     if (!linker) {
> > +             p_err("failed to create BPF linker instance");
> > +             return -1;
> > +     }
> > +
> > +     while (argc) {
> > +             file = GET_ARG();
> > +
> > +             err = bpf_linker__add_file(linker, file);
> > +             if (err) {
> > +                     p_err("failed to link '%s': %d", file, err);
>
> I think you mentioned before that your preference was for having just
> the error code instead of using strerror(), but I think it would be more
> user-friendly for the majority of users who don't know the error codes
> if we had something more verbose? How about having both strerror()
> output and the error code?

Sure, I'll add strerror(). My earlier point was that those messages
are more often misleading (e.g., "file not found" for ENOENT or
something similar) than helpful. I should check if bpftool is passing
through warn-level messages from libbpf. Those are going to be very
helpful, if anything goes wrong. --verbose should pass through all of
libbpf messages, if it's not already the case.

>
> > +                     goto err_out;
> > +             }
> > +     }
> > +
> > +     err = bpf_linker__finalize(linker);
> > +     if (err) {
> > +             p_err("failed to finalize ELF file: %d", err);
> > +             goto err_out;
> > +     }
> > +
> > +     return 0;
> > +err_out:
> > +     bpf_linker__free(linker);
> > +     return -1;
>
> Should you call bpf_linker__free() even on success? I see that
> bpf_linker__finalize() frees some of the resources, but it seems that
> bpf_linker__free() does a more thorough job?

yep, it should really be just

err_out:
    bpf_linker__free(linker);
    return err;


>
> > +}
> > +
> >  static int do_help(int argc, char **argv)
> >  {
> >       if (json_output) {
> > @@ -611,6 +654,7 @@ static int do_help(int argc, char **argv)
> >
> >  static const struct cmd cmds[] = {
> >       { "skeleton",   do_skeleton },
> > +     { "bpfo",       do_bpfo },
> >       { "help",       do_help },
> >       { 0 }
> >  };
> >
>
> Please update the usage help message, man page, and bash completion,
> thanks. Especially because what "bpftool gen bpfo" does is not intuitive
> (but I don't have a better name suggestion at the moment).

Yeah, forgot about manpage and bash completions, as usual.

re: "gen bpfo". I don't have much better naming as well. `bpftool
link` is already taken for bpf_link-related commands. It felt like
keeping this under "gen" command makes sense. But maybe `bpftool
linker link <out> <in1> <in2> ...` would be a bit less confusing
convention?

>
> Great work!
>
> Quentin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking
  2021-03-11 18:45     ` Andrii Nakryiko
@ 2021-03-12 18:07       ` Quentin Monnet
  2021-03-13 18:37         ` Andrii Nakryiko
  0 siblings, 1 reply; 18+ messages in thread
From: Quentin Monnet @ 2021-03-12 18:07 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

2021-03-11 10:45 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, Mar 11, 2021 at 3:31 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> 2021-03-09 20:04 UTC-0800 ~ Andrii Nakryiko <andrii@kernel.org>
>>> Add `bpftool gen bpfo <output-file> <input_file>...` command to statically
>>> link multiple BPF object files into a single output BPF object file.
>>>
>>> Similarly to existing '*.o' convention, bpftool is establishing a '*.bpfo'
>>> convention for statically-linked BPF object files. Both .o and .bpfo suffixes
>>> will be stripped out during BPF skeleton generation to infer BPF object name.
>>>
>>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>>> ---
>>>  tools/bpf/bpftool/gen.c | 46 ++++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 45 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
>>> index 4033c46d83e7..8b1ed6c0a62f 100644
>>> --- a/tools/bpf/bpftool/gen.c
>>> +++ b/tools/bpf/bpftool/gen.c
>>> +static int do_bpfo(int argc, char **argv)
>>
>>> +{
>>> +     struct bpf_linker *linker;
>>> +     const char *output_file, *file;
>>> +     int err;
>>> +
>>> +     if (!REQ_ARGS(2)) {
>>> +             usage();
>>> +             return -1;
>>> +     }
>>> +
>>> +     output_file = GET_ARG();
>>> +
>>> +     linker = bpf_linker__new(output_file, NULL);
>>> +     if (!linker) {
>>> +             p_err("failed to create BPF linker instance");
>>> +             return -1;
>>> +     }
>>> +
>>> +     while (argc) {
>>> +             file = GET_ARG();
>>> +
>>> +             err = bpf_linker__add_file(linker, file);
>>> +             if (err) {
>>> +                     p_err("failed to link '%s': %d", file, err);
>>
>> I think you mentioned before that your preference was for having just
>> the error code instead of using strerror(), but I think it would be more
>> user-friendly for the majority of users who don't know the error codes
>> if we had something more verbose? How about having both strerror()
>> output and the error code?
> 
> Sure, I'll add strerror(). My earlier point was that those messages
> are more often misleading (e.g., "file not found" for ENOENT or
> something similar) than helpful. I should check if bpftool is passing
> through warn-level messages from libbpf. Those are going to be very
> helpful, if anything goes wrong. --verbose should pass through all of
> libbpf messages, if it's not already the case.

Thanks. Yes, --verbose should do it, but it's worth a double-check.

>>> +                     goto err_out;
>>> +             }
>>> +     }
>>> +
>>> +     err = bpf_linker__finalize(linker);
>>> +     if (err) {
>>> +             p_err("failed to finalize ELF file: %d", err);
>>> +             goto err_out;
>>> +     }
>>> +
>>> +     return 0;
>>> +err_out:
>>> +     bpf_linker__free(linker);
>>> +     return -1;
>>
>> Should you call bpf_linker__free() even on success? I see that
>> bpf_linker__finalize() frees some of the resources, but it seems that
>> bpf_linker__free() does a more thorough job?
> 
> yep, it should really be just
> 
> err_out:
>     bpf_linker__free(linker);
>     return err;
> 
> 
>>
>>> +}
>>> +
>>>  static int do_help(int argc, char **argv)
>>>  {
>>>       if (json_output) {
>>> @@ -611,6 +654,7 @@ static int do_help(int argc, char **argv)
>>>
>>>  static const struct cmd cmds[] = {
>>>       { "skeleton",   do_skeleton },
>>> +     { "bpfo",       do_bpfo },
>>>       { "help",       do_help },
>>>       { 0 }
>>>  };
>>>
>>
>> Please update the usage help message, man page, and bash completion,
>> thanks. Especially because what "bpftool gen bpfo" does is not intuitive
>> (but I don't have a better name suggestion at the moment).
> 
> Yeah, forgot about manpage and bash completions, as usual.
> 
> re: "gen bpfo". I don't have much better naming as well. `bpftool
> link` is already taken for bpf_link-related commands. It felt like
> keeping this under "gen" command makes sense. But maybe `bpftool
> linker link <out> <in1> <in2> ...` would be a bit less confusing
> convention?

"bpftool linker" would have been nice, but having "bpftool link", I
think it would be even more confusing. We can pass commands by their
prefixes, so is "bpftool link" the command "link" or a prefix for
"linker"? (I know it would be easy to sort out from our point of view,
but for regular users I'm sure that would be confusing).

I don't mind leaving it under "bpftool gen", it's probably the most
relevant command we have. As for replacing the "bpfo" keyword, I've
thought of "combined", "static_linked", "archive", "concat". I write
them in case it's any inspiration, but I find none of them ideal :/.

Quentin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking
  2021-03-12 18:07       ` Quentin Monnet
@ 2021-03-13 18:37         ` Andrii Nakryiko
  2021-03-13 21:27           ` Quentin Monnet
  0 siblings, 1 reply; 18+ messages in thread
From: Andrii Nakryiko @ 2021-03-13 18:37 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Fri, Mar 12, 2021 at 10:07 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2021-03-11 10:45 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Thu, Mar 11, 2021 at 3:31 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>
> >> 2021-03-09 20:04 UTC-0800 ~ Andrii Nakryiko <andrii@kernel.org>
> >>> Add `bpftool gen bpfo <output-file> <input_file>...` command to statically
> >>> link multiple BPF object files into a single output BPF object file.
> >>>
> >>> Similarly to existing '*.o' convention, bpftool is establishing a '*.bpfo'
> >>> convention for statically-linked BPF object files. Both .o and .bpfo suffixes
> >>> will be stripped out during BPF skeleton generation to infer BPF object name.
> >>>
> >>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> >>> ---
> >>>  tools/bpf/bpftool/gen.c | 46 ++++++++++++++++++++++++++++++++++++++++-
> >>>  1 file changed, 45 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> >>> index 4033c46d83e7..8b1ed6c0a62f 100644
> >>> --- a/tools/bpf/bpftool/gen.c
> >>> +++ b/tools/bpf/bpftool/gen.c
> >>> +static int do_bpfo(int argc, char **argv)
> >>
> >>> +{
> >>> +     struct bpf_linker *linker;
> >>> +     const char *output_file, *file;
> >>> +     int err;
> >>> +
> >>> +     if (!REQ_ARGS(2)) {
> >>> +             usage();
> >>> +             return -1;
> >>> +     }
> >>> +
> >>> +     output_file = GET_ARG();
> >>> +
> >>> +     linker = bpf_linker__new(output_file, NULL);
> >>> +     if (!linker) {
> >>> +             p_err("failed to create BPF linker instance");
> >>> +             return -1;
> >>> +     }
> >>> +
> >>> +     while (argc) {
> >>> +             file = GET_ARG();
> >>> +
> >>> +             err = bpf_linker__add_file(linker, file);
> >>> +             if (err) {
> >>> +                     p_err("failed to link '%s': %d", file, err);
> >>
> >> I think you mentioned before that your preference was for having just
> >> the error code instead of using strerror(), but I think it would be more
> >> user-friendly for the majority of users who don't know the error codes
> >> if we had something more verbose? How about having both strerror()
> >> output and the error code?
> >
> > Sure, I'll add strerror(). My earlier point was that those messages
> > are more often misleading (e.g., "file not found" for ENOENT or
> > something similar) than helpful. I should check if bpftool is passing
> > through warn-level messages from libbpf. Those are going to be very
> > helpful, if anything goes wrong. --verbose should pass through all of
> > libbpf messages, if it's not already the case.
>
> Thanks. Yes, --verbose should do it, but it's worth a double-check.
>
> >>> +                     goto err_out;
> >>> +             }
> >>> +     }
> >>> +
> >>> +     err = bpf_linker__finalize(linker);
> >>> +     if (err) {
> >>> +             p_err("failed to finalize ELF file: %d", err);
> >>> +             goto err_out;
> >>> +     }
> >>> +
> >>> +     return 0;
> >>> +err_out:
> >>> +     bpf_linker__free(linker);
> >>> +     return -1;
> >>
> >> Should you call bpf_linker__free() even on success? I see that
> >> bpf_linker__finalize() frees some of the resources, but it seems that
> >> bpf_linker__free() does a more thorough job?
> >
> > yep, it should really be just
> >
> > err_out:
> >     bpf_linker__free(linker);
> >     return err;
> >
> >
> >>
> >>> +}
> >>> +
> >>>  static int do_help(int argc, char **argv)
> >>>  {
> >>>       if (json_output) {
> >>> @@ -611,6 +654,7 @@ static int do_help(int argc, char **argv)
> >>>
> >>>  static const struct cmd cmds[] = {
> >>>       { "skeleton",   do_skeleton },
> >>> +     { "bpfo",       do_bpfo },
> >>>       { "help",       do_help },
> >>>       { 0 }
> >>>  };
> >>>
> >>
> >> Please update the usage help message, man page, and bash completion,
> >> thanks. Especially because what "bpftool gen bpfo" does is not intuitive
> >> (but I don't have a better name suggestion at the moment).
> >
> > Yeah, forgot about manpage and bash completions, as usual.
> >
> > re: "gen bpfo". I don't have much better naming as well. `bpftool
> > link` is already taken for bpf_link-related commands. It felt like
> > keeping this under "gen" command makes sense. But maybe `bpftool
> > linker link <out> <in1> <in2> ...` would be a bit less confusing
> > convention?
>
> "bpftool linker" would have been nice, but having "bpftool link", I
> think it would be even more confusing. We can pass commands by their
> prefixes, so is "bpftool link" the command "link" or a prefix for
> "linker"? (I know it would be easy to sort out from our point of view,
> but for regular users I'm sure that would be confusing).

right

>
> I don't mind leaving it under "bpftool gen", it's probably the most
> relevant command we have. As for replacing the "bpfo" keyword, I've
> thought of "combined", "static_linked", "archive", "concat". I write
> them in case it's any inspiration, but I find none of them ideal :/.

How about "bpftool gen object", which can be shortened in typing to
just `bpftool gen obj`. It seems complementary to `gen skeleton`. You
first generate object (from other objects generated by compiler, which
might be a bit confusing at first), then you generate skeleton from
the object. WDYT?

>
> Quentin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking
  2021-03-13 18:37         ` Andrii Nakryiko
@ 2021-03-13 21:27           ` Quentin Monnet
  0 siblings, 0 replies; 18+ messages in thread
From: Quentin Monnet @ 2021-03-13 21:27 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team

On Sat, 13 Mar 2021 at 18:37, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Fri, Mar 12, 2021 at 10:07 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >
> > 2021-03-11 10:45 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > > On Thu, Mar 11, 2021 at 3:31 AM Quentin Monnet <quentin@isovalent.com> wrote:
> > >>
> > >> 2021-03-09 20:04 UTC-0800 ~ Andrii Nakryiko <andrii@kernel.org>
> > >>> Add `bpftool gen bpfo <output-file> <input_file>...` command to statically
> > >>> link multiple BPF object files into a single output BPF object file.
> > >>>
> > >>> Similarly to existing '*.o' convention, bpftool is establishing a '*.bpfo'
> > >>> convention for statically-linked BPF object files. Both .o and .bpfo suffixes
> > >>> will be stripped out during BPF skeleton generation to infer BPF object name.
> > >>>
> > >>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> > >>> ---
> > >>>  tools/bpf/bpftool/gen.c | 46 ++++++++++++++++++++++++++++++++++++++++-
> > >>>  1 file changed, 45 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/tools/bpf/bpftool/gen.c b/tools/bpf/bpftool/gen.c
> > >>> index 4033c46d83e7..8b1ed6c0a62f 100644
> > >>> --- a/tools/bpf/bpftool/gen.c
> > >>> +++ b/tools/bpf/bpftool/gen.c

> > >>> @@ -611,6 +654,7 @@ static int do_help(int argc, char **argv)
> > >>>
> > >>>  static const struct cmd cmds[] = {
> > >>>       { "skeleton",   do_skeleton },
> > >>> +     { "bpfo",       do_bpfo },
> > >>>       { "help",       do_help },
> > >>>       { 0 }
> > >>>  };
> > >>>
> > >>
> > >> Please update the usage help message, man page, and bash completion,
> > >> thanks. Especially because what "bpftool gen bpfo" does is not intuitive
> > >> (but I don't have a better name suggestion at the moment).
> > >
> > > Yeah, forgot about manpage and bash completions, as usual.
> > >
> > > re: "gen bpfo". I don't have much better naming as well. `bpftool
> > > link` is already taken for bpf_link-related commands. It felt like
> > > keeping this under "gen" command makes sense. But maybe `bpftool
> > > linker link <out> <in1> <in2> ...` would be a bit less confusing
> > > convention?
> >
> > "bpftool linker" would have been nice, but having "bpftool link", I
> > think it would be even more confusing. We can pass commands by their
> > prefixes, so is "bpftool link" the command "link" or a prefix for
> > "linker"? (I know it would be easy to sort out from our point of view,
> > but for regular users I'm sure that would be confusing).
>
> right
>
> >
> > I don't mind leaving it under "bpftool gen", it's probably the most
> > relevant command we have. As for replacing the "bpfo" keyword, I've
> > thought of "combined", "static_linked", "archive", "concat". I write
> > them in case it's any inspiration, but I find none of them ideal :/.
>
> How about "bpftool gen object", which can be shortened in typing to
> just `bpftool gen obj`. It seems complementary to `gen skeleton`. You
> first generate object (from other objects generated by compiler, which
> might be a bit confusing at first), then you generate skeleton from
> the object. WDYT?

Sounds good, better than "bpfo" I think.

Quentin

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-03-13 21:32 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10  4:04 [PATCH bpf-next 00/10] BPF static linking Andrii Nakryiko
2021-03-10  4:04 ` [PATCH bpf-next 01/10] libbpf: expose btf_type_by_id() internally Andrii Nakryiko
2021-03-10  4:04 ` [PATCH bpf-next 02/10] libbpf: add internal helper to get raw BTF strings section Andrii Nakryiko
2021-03-10  4:04 ` [PATCH bpf-next 03/10] libbpf: generalize BTF and BTF.ext type ID and strings iteration Andrii Nakryiko
2021-03-10  4:04 ` [PATCH bpf-next 04/10] libbpf: add generic BTF type shallow copy API Andrii Nakryiko
2021-03-10  4:04 ` [PATCH bpf-next 05/10] libbpf: add BPF static linker APIs Andrii Nakryiko
2021-03-11  2:34   ` Alexei Starovoitov
2021-03-11  3:29     ` Andrii Nakryiko
2021-03-10  4:04 ` [PATCH bpf-next 06/10] libbpf: add BPF static linker BTF and BTF.ext support Andrii Nakryiko
2021-03-10  4:04 ` [PATCH bpf-next 07/10] bpftool: add `gen bpfo` command to perform BPF static linking Andrii Nakryiko
2021-03-11 11:31   ` Quentin Monnet
2021-03-11 18:45     ` Andrii Nakryiko
2021-03-12 18:07       ` Quentin Monnet
2021-03-13 18:37         ` Andrii Nakryiko
2021-03-13 21:27           ` Quentin Monnet
2021-03-10  4:04 ` [PATCH bpf-next 08/10] selftests/bpf: re-generate vmlinux.h and BPF skeletons if bpftool changed Andrii Nakryiko
2021-03-10  4:04 ` [PATCH bpf-next 09/10] selftests/bpf: pass all BPF .o's through BPF static linker Andrii Nakryiko
2021-03-10  4:04 ` [PATCH bpf-next 10/10] selftests/bpf: add multi-file statically linked BPF object file test Andrii Nakryiko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.