Re: [PATCH v3 bpf-next 11/18] libbpf: add linker extern resolution support for functions and global variables

From: Yonghong Song <yhs@fb.com>
To: Andrii Nakryiko <andrii@kernel.org>, <bpf@vger.kernel.org>,
	<netdev@vger.kernel.org>, <ast@fb.com>, <daniel@iogearbox.net>
Cc: <kernel-team@fb.com>
Subject: Re: [PATCH v3 bpf-next 11/18] libbpf: add linker extern resolution support for functions and global variables
Date: Fri, 23 Apr 2021 11:46:39 -0700	[thread overview]
Message-ID: <56eaffb0-b9da-371c-d1e1-6ae5f998e158@fb.com> (raw)
In-Reply-To: <20210423181348.1801389-12-andrii@kernel.org>


On 4/23/21 11:13 AM, Andrii Nakryiko wrote:
> Add BPF static linker logic to resolve extern variables and functions across
> multiple linked together BPF object files.
> 
> For that, linker maintains a separate list of struct glob_sym structures,
> which keeps track of few pieces of metadata (is it extern or resolved global,
> is it a weak symbol, which ELF section it belongs to, etc) and ties together
> BTF type info and ELF symbol information and keeps them in sync.
> 
> With adding support for extern variables/funcs, it's now possible for some
> sections to contain both extern and non-extern definitions. This means that
> some sections may start out as ephemeral (if only externs are present and thus
> there is not corresponding ELF section), but will be "upgraded" to actual ELF
> section as symbols are resolved or new non-extern definitions are appended.
> 
> Additional care is taken to not duplicate extern entries in sections like
> .kconfig and .ksyms.
> 
> Given libbpf requires BTF type to always be present for .kconfig/.ksym
> externs, linker extends this requirement to all the externs, even those that
> are supposed to be resolved during static linking and which won't be visible
> to libbpf. With BTF information always present, static linker will check not
> just ELF symbol matches, but entire BTF type signature match as well. That
> logic is stricter that BPF CO-RE checks. It probably should be re-used by
> .ksym resolution logic in libbpf as well, but that's left for follow up
> patches.
> 
> To make it unnecessary to rewrite ELF symbols and minimize BTF type
> rewriting/removal, ELF symbols that correspond to externs initially will be
> updated in place once they are resolved. Similarly for BTF type info, VAR/FUNC
> and var_secinfo's (sec_vars in struct bpf_linker) are staying stable, but
> types they point to might get replaced when extern is resolved. This might
> leave some left-over types (even though we try to minimize this for common
> cases of having extern funcs with not argument names vs concrete function with
> names properly specified). That can be addresses later with a generic BTF
> garbage collection. That's left for a follow up as well.
> 
> Given BTF type appending phase is separate from ELF symbol
> appending/resolution, special struct glob_sym->underlying_btf_id variable is
> used to communicate resolution and rewrite decisions. 0 means
> underlying_btf_id needs to be appended (it's not yet in final linker->btf), <0
> values are used for temporary storage of source BTF type ID (not yet
> rewritten), so -glob_sym->underlying_btf_id is BTF type id in obj-btf. But by
> the end of linker_append_btf() phase, that underlying_btf_id will be remapped
> and will always be > 0. This is the uglies part of the whole process, but
> keeps the other parts much simpler due to stability of sec_var and VAR/FUNC
> types, as well as ELF symbol, so please keep that in mind while reviewing.
> 
> BTF-defined maps require some extra custom logic and is addressed separate in
> the next patch, so that to keep this one smaller and easier to review.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>