bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii@kernel.org>
To: <bpf@vger.kernel.org>, <netdev@vger.kernel.org>, <ast@fb.com>,
	<daniel@iogearbox.net>
Cc: <andrii@kernel.org>, <kernel-team@fb.com>
Subject: [PATCH v2 bpf-next 12/17] libbpf: support extern resolution for BTF-defined maps in .maps section
Date: Fri, 16 Apr 2021 13:23:59 -0700	[thread overview]
Message-ID: <20210416202404.3443623-13-andrii@kernel.org> (raw)
In-Reply-To: <20210416202404.3443623-1-andrii@kernel.org>

Add extra logic to handle map externs (only BTF-defined maps are supported for
linking). Re-use the map parsing logic used during bpf_object__open(). Map
externs are currently restricted to always match complete map definition. So
all the specified attributes will be compared (down to pining, map_flags,
numa_node, etc). In the future this restriction might be relaxed with no
backwards compatibility issues. If any attribute is mismatched between extern
and actual map definition, linker will report an error, pointing out which one
mismatches.

The original intent was to allow for extern to specify attributes that matters
(to user) to enforce. E.g., if you specify just key information and omit
value, then any value fits. Similarly, it should have been possible to enforce
map_flags, pinning, and any other possible map attribute. Unfortunately, that
means that multiple externs can be only partially overlapping with each other,
which means linker would need to combine their type definitions to end up with
the most restrictive and fullest map definition. This requires an extra amount
of BTF manipulation which at this time was deemed unnecessary and would
require further extending generic BTF writer APIs. So that is left for future
follow ups, if there will be demand for that. But the idea seems intresting
and useful, so I want to document it here.

Weak definitions are also supported, but are pretty strict as well, just
like externs: all weak map definitions have to match exactly. In the follow up
patches this most probably will be relaxed, with __weak map definitions being
able to differ between each other (with non-weak definition always winning, of
course).

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

diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
index 67d2d06e3cb6..84d444427b65 100644
--- a/tools/lib/bpf/linker.c
+++ b/tools/lib/bpf/linker.c
@@ -1463,6 +1463,134 @@ static bool glob_sym_btf_matches(const char *sym_name, bool exact,
 	}
 }
 
+static bool map_defs_match(const char *sym_name,
+			   const struct btf *main_btf,
+			   const struct btf_map_def *main_def,
+			   const struct btf_map_def *main_inner_def,
+			   const struct btf *extra_btf,
+			   const struct btf_map_def *extra_def,
+			   const struct btf_map_def *extra_inner_def)
+{
+	const char *reason;
+
+	if (main_def->map_type != extra_def->map_type) {
+		reason = "type";
+		goto mismatch;
+	}
+
+	/* check key type/size match */
+	if (main_def->key_size != extra_def->key_size) {
+		reason = "key_size";
+		goto mismatch;
+	}
+	if (!!main_def->key_type_id != !!extra_def->key_type_id) {
+		reason = "key type";
+		goto mismatch;
+	}
+	if ((main_def->parts & MAP_DEF_KEY_TYPE)
+	     && !glob_sym_btf_matches(sym_name, true /*exact*/,
+				      main_btf, main_def->key_type_id,
+				      extra_btf, extra_def->key_type_id)) {
+		reason = "key type";
+		goto mismatch;
+	}
+
+	/* validate value type/size match */
+	if (main_def->value_size != extra_def->value_size) {
+		reason = "value_size";
+		goto mismatch;
+	}
+	if (!!main_def->value_type_id != !!extra_def->value_type_id) {
+		reason = "value type";
+		goto mismatch;
+	}
+	if ((main_def->parts & MAP_DEF_VALUE_TYPE)
+	     && !glob_sym_btf_matches(sym_name, true /*exact*/,
+				      main_btf, main_def->value_type_id,
+				      extra_btf, extra_def->value_type_id)) {
+		reason = "key type";
+		goto mismatch;
+	}
+
+	if (main_def->max_entries != extra_def->max_entries) {
+		reason = "max_entries";
+		goto mismatch;
+	}
+	if (main_def->map_flags != extra_def->map_flags) {
+		reason = "map_flags";
+		goto mismatch;
+	}
+	if (main_def->numa_node != extra_def->numa_node) {
+		reason = "numa_node";
+		goto mismatch;
+	}
+	if (main_def->pinning != extra_def->pinning) {
+		reason = "pinning";
+		goto mismatch;
+	}
+
+	if ((main_def->parts & MAP_DEF_INNER_MAP) != (extra_def->parts & MAP_DEF_INNER_MAP)) {
+		reason = "inner map";
+		goto mismatch;
+	}
+
+	if (main_def->parts & MAP_DEF_INNER_MAP) {
+		char inner_map_name[128];
+
+		snprintf(inner_map_name, sizeof(inner_map_name), "%s.inner", sym_name);
+
+		return map_defs_match(inner_map_name,
+				      main_btf, main_inner_def, NULL,
+				      extra_btf, extra_inner_def, NULL);
+	}
+
+	return true;
+
+mismatch:
+	pr_warn("global '%s': map %s mismatch\n", sym_name, reason);
+	return false;
+}
+
+static bool glob_map_defs_match(const char *sym_name,
+				struct bpf_linker *linker, struct glob_sym *glob_sym,
+				struct src_obj *obj, Elf64_Sym *sym, int btf_id)
+{
+	struct btf_map_def dst_def = {}, dst_inner_def = {};
+	struct btf_map_def src_def = {}, src_inner_def = {};
+	const struct btf_type *t;
+	int err;
+
+	t = btf__type_by_id(obj->btf, btf_id);
+	if (!btf_is_var(t)) {
+		pr_warn("global '%s': invalid map definition type [%d]\n", sym_name, btf_id);
+		return false;
+	}
+	t = skip_mods_and_typedefs(obj->btf, t->type, NULL);
+
+	err = parse_btf_map_def(sym_name, obj->btf, t, true /*strict*/, &src_def, &src_inner_def);
+	if (err) {
+		pr_warn("global '%s': invalid map definition\n", sym_name);
+		return false;
+	}
+
+	/* re-parse existing map definition */
+	t = btf__type_by_id(linker->btf, glob_sym->btf_id);
+	t = skip_mods_and_typedefs(linker->btf, t->type, NULL);
+	err = parse_btf_map_def(sym_name, linker->btf, t, true /*strict*/, &dst_def, &dst_inner_def);
+	if (err) {
+		/* this should not happen, because we already validated it */
+		pr_warn("global '%s': invalid dst map definition\n", sym_name);
+		return false;
+	}
+
+	/* Currently extern map definition has to be complete and match
+	 * concrete map definition exactly. This restriction might be lifted
+	 * in the future.
+	 */
+	return map_defs_match(sym_name, linker->btf, &dst_def, &dst_inner_def,
+			      obj->btf, &src_def, &src_inner_def);
+}
+
 static bool glob_syms_match(const char *sym_name,
 			    struct bpf_linker *linker, struct glob_sym *glob_sym,
 			    struct src_obj *obj, Elf64_Sym *sym, size_t sym_idx, int btf_id)
@@ -1484,6 +1612,10 @@ static bool glob_syms_match(const char *sym_name,
 		return false;
 	}
 
+	/* deal with .maps definitions specially */
+	if (glob_sym->sec_id && strcmp(linker->secs[glob_sym->sec_id].sec_name, MAPS_ELF_SEC) == 0)
+		return glob_map_defs_match(sym_name, linker, glob_sym, obj, sym, btf_id);
+
 	if (!glob_sym_btf_matches(sym_name, true /*exact*/,
 				  linker->btf, glob_sym->btf_id, obj->btf, btf_id))
 		return false;
-- 
2.30.2


  parent reply	other threads:[~2021-04-16 20:24 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-16 20:23 [PATCH v2 bpf-next 00/17] BPF static linker: support externs Andrii Nakryiko
2021-04-16 20:23 ` [PATCH v2 bpf-next 01/17] bpftool: support dumping BTF VAR's "extern" linkage Andrii Nakryiko
2021-04-22  3:02   ` Yonghong Song
2021-04-16 20:23 ` [PATCH v2 bpf-next 02/17] bpftool: dump more info about DATASEC members Andrii Nakryiko
2021-04-22  3:09   ` Yonghong Song
2021-04-16 20:23 ` [PATCH v2 bpf-next 03/17] libbpf: suppress compiler warning when using SEC() macro with externs Andrii Nakryiko
2021-04-22  3:47   ` Yonghong Song
2021-04-22  3:59     ` Andrii Nakryiko
2021-04-16 20:23 ` [PATCH v2 bpf-next 04/17] libbpf: mark BPF subprogs with hidden visibility as static for BPF verifier Andrii Nakryiko
2021-04-22  5:43   ` Yonghong Song
2021-04-22 18:09     ` Andrii Nakryiko
2021-04-22 23:00       ` Yonghong Song
2021-04-22 23:28         ` Andrii Nakryiko
2021-04-16 20:23 ` [PATCH v2 bpf-next 05/17] libbpf: allow gaps in BPF program sections to support overriden weak functions Andrii Nakryiko
2021-04-22  6:25   ` Yonghong Song
2021-04-22 18:10     ` Andrii Nakryiko
2021-04-16 20:23 ` [PATCH v2 bpf-next 06/17] libbpf: refactor BTF map definition parsing Andrii Nakryiko
2021-04-22 15:33   ` Yonghong Song
2021-04-22 18:40     ` Andrii Nakryiko
2021-04-16 20:23 ` [PATCH v2 bpf-next 07/17] libbpf: factor out symtab and relos sanity checks Andrii Nakryiko
2021-04-22 16:06   ` Yonghong Song
2021-04-22 18:16     ` Andrii Nakryiko
2021-04-16 20:23 ` [PATCH v2 bpf-next 08/17] libbpf: make few internal helpers available outside of libbpf.c Andrii Nakryiko
2021-04-22 16:19   ` Yonghong Song
2021-04-22 18:18     ` Andrii Nakryiko
2021-04-16 20:23 ` [PATCH v2 bpf-next 09/17] libbpf: extend sanity checking ELF symbols with externs validation Andrii Nakryiko
2021-04-22 16:35   ` Yonghong Song
2021-04-22 18:20     ` Andrii Nakryiko
2021-04-22 23:13       ` Yonghong Song
2021-04-16 20:23 ` [PATCH v2 bpf-next 10/17] libbpf: tighten BTF type ID rewriting with error checking Andrii Nakryiko
2021-04-22 16:50   ` Yonghong Song
2021-04-22 18:24     ` Andrii Nakryiko
2021-04-23  2:54       ` Alexei Starovoitov
2021-04-23  4:25         ` Andrii Nakryiko
2021-04-23  5:11           ` Alexei Starovoitov
2021-04-23 16:09             ` Andrii Nakryiko
2021-04-23 16:18               ` Alexei Starovoitov
2021-04-23 16:30                 ` Andrii Nakryiko
2021-04-23 16:34                   ` Alexei Starovoitov
2021-04-23 17:02                     ` Andrii Nakryiko
2021-04-16 20:23 ` [PATCH v2 bpf-next 11/17] libbpf: add linker extern resolution support for functions and global variables Andrii Nakryiko
2021-04-22 21:27   ` Yonghong Song
2021-04-22 22:12     ` Andrii Nakryiko
2021-04-22 23:57       ` Yonghong Song
2021-04-23  2:36         ` Yonghong Song
2021-04-23  4:28           ` Andrii Nakryiko
2021-04-23  4:27         ` Andrii Nakryiko
2021-04-16 20:23 ` Andrii Nakryiko [this message]
2021-04-22 22:56   ` [PATCH v2 bpf-next 12/17] libbpf: support extern resolution for BTF-defined maps in .maps section Yonghong Song
2021-04-22 23:32     ` Andrii Nakryiko
2021-04-16 20:24 ` [PATCH v2 bpf-next 13/17] selftests/bpf: use -O0 instead of -Og in selftests builds Andrii Nakryiko
2021-04-23  0:05   ` Yonghong Song
2021-04-16 20:24 ` [PATCH v2 bpf-next 14/17] selftests/bpf: omit skeleton generation for multi-linked BPF object files Andrii Nakryiko
2021-04-23  0:13   ` Yonghong Song
2021-04-16 20:24 ` [PATCH v2 bpf-next 15/17] selftests/bpf: add function linking selftest Andrii Nakryiko
2021-04-23  0:50   ` Yonghong Song
2021-04-23  2:34     ` Alexei Starovoitov
2021-04-23  4:29       ` Andrii Nakryiko
2021-04-23 17:18     ` Andrii Nakryiko
2021-04-23 17:35       ` Yonghong Song
2021-04-23 17:55         ` Andrii Nakryiko
2021-04-23 17:58           ` Yonghong Song
2021-04-23 17:59             ` Andrii Nakryiko
2021-04-16 20:24 ` [PATCH v2 bpf-next 16/17] selftests/bpf: add global variables " Andrii Nakryiko
2021-04-23  1:01   ` Yonghong Song
2021-04-16 20:24 ` [PATCH v2 bpf-next 17/17] sleftests/bpf: add map " Andrii Nakryiko
2021-04-23  1:20   ` Yonghong Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210416202404.3443623-13-andrii@kernel.org \
    --to=andrii@kernel.org \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).