[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d220154c-6aad-4bc1-ab86-6ae72e351dca@fb.com>
Date: Thu, 19 Dec 2019 16:24:14 +0000
From: Yonghong Song <yhs@...com>
To: Toke Høiland-Jørgensen <toke@...hat.com>,
"Daniel Borkmann" <daniel@...earbox.net>
CC: Alexei Starovoitov <ast@...nel.org>, Martin Lau <kafai@...com>,
Song Liu <songliubraving@...com>,
Jesper Dangaard Brouer <brouer@...hat.com>,
"Andrii Nakryiko" <andrii.nakryiko@...il.com>,
David Miller <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"bpf@...r.kernel.org" <bpf@...r.kernel.org>
Subject: Re: [PATCH RFC bpf-next 2/3] libbpf: Handle function externs and
support static linking
On 12/19/19 6:29 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@...hat.com>
>
> This adds support for resolving function externs to libbpf, with a new API
> to resolve external function calls by static linking at load-time. The API
> for this requires the caller to supply the object files containing the
> target functions, and to specify an explicit mapping between extern
> function names in the calling program, and function names in the target
> object file. This is to support the XDP multi-prog case, where the
> dispatcher program may not necessarily have control over function names in
> the target programs, so simple function name resolution can't be used.
>
> The target object files must be loaded into the kernel before the calling
> program, to ensure all relocations are done on the target functions, so we
> can just copy over the instructions.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> ---
> tools/lib/bpf/btf.c | 10 +-
> tools/lib/bpf/libbpf.c | 268 +++++++++++++++++++++++++++++++++++++++---------
> tools/lib/bpf/libbpf.h | 17 +++
> 3 files changed, 244 insertions(+), 51 deletions(-)
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index 5f04f56e1eb6..2740d4a6b2eb 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -246,6 +246,7 @@ __s64 btf__resolve_size(const struct btf *btf, __u32 type_id)
> size = t->size;
> goto done;
> case BTF_KIND_PTR:
> + case BTF_KIND_FUNC_PROTO:
> size = sizeof(void *);
> goto done;
> case BTF_KIND_TYPEDEF:
> @@ -288,6 +289,7 @@ int btf__align_of(const struct btf *btf, __u32 id)
> case BTF_KIND_ENUM:
> return min(sizeof(void *), t->size);
> case BTF_KIND_PTR:
> + case BTF_KIND_FUNC_PROTO:
> return sizeof(void *);
> case BTF_KIND_TYPEDEF:
> case BTF_KIND_VOLATILE:
> @@ -640,12 +642,16 @@ int btf__finalize_data(struct bpf_object *obj, struct btf *btf)
> */
> if (btf_is_datasec(t)) {
> err = btf_fixup_datasec(obj, btf, t);
> - if (err)
> + /* FIXME: With function externs we can get a BTF DATASEC
> + * entry for .extern, but the section doesn't exist; so
> + * make ENOENT non-fatal
> + */
> + if (err && err != -ENOENT)
> break;
> }
> }
>
> - return err;
> + return err == -ENOENT ? err : 0;
> }
>
> int btf__load(struct btf *btf)
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 266b725e444b..b2c0a2f927e7 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -172,13 +172,17 @@ enum reloc_type {
> RELO_CALL,
> RELO_DATA,
> RELO_EXTERN,
> + RELO_EXTERN_CALL,
> };
>
> +struct extern_desc;
> +
> struct reloc_desc {
> enum reloc_type type;
> int insn_idx;
> int map_idx;
> int sym_off;
> + struct extern_desc *ext;
> };
>
> /*
> @@ -274,6 +278,7 @@ enum extern_type {
> EXT_INT,
> EXT_TRISTATE,
> EXT_CHAR_ARR,
> + EXT_FUNC
> };
>
> struct extern_desc {
> @@ -287,6 +292,7 @@ struct extern_desc {
> bool is_signed;
> bool is_weak;
> bool is_set;
> + struct bpf_program *tgt_prog;
> };
>
> static LIST_HEAD(bpf_objects_list);
> @@ -305,6 +311,7 @@ struct bpf_object {
> char *kconfig;
> struct extern_desc *externs;
> int nr_extern;
> + int nr_data_extern;
> int kconfig_map_idx;
>
> bool loaded;
> @@ -1041,6 +1048,7 @@ static int set_ext_value_tri(struct extern_desc *ext, void *ext_val,
> case EXT_UNKNOWN:
> case EXT_INT:
> case EXT_CHAR_ARR:
> + case EXT_FUNC:
> default:
> pr_warn("extern %s=%c should be bool, tristate, or char\n",
> ext->name, value);
> @@ -1281,7 +1289,7 @@ static int bpf_object__init_kconfig_map(struct bpf_object *obj)
> size_t map_sz;
> int err;
>
> - if (obj->nr_extern == 0)
> + if (obj->nr_data_extern == 0)
> return 0;
>
> last_ext = &obj->externs[obj->nr_extern - 1];
> @@ -1822,29 +1830,51 @@ static void bpf_object__sanitize_btf(struct bpf_object *obj)
> struct btf_type *t;
> int i, j, vlen;
>
> - if (!obj->btf || (has_func && has_datasec))
> + if (!obj->btf)
> return;
> -
> for (i = 1; i <= btf__get_nr_types(btf); i++) {
> t = (struct btf_type *)btf__type_by_id(btf, i);
>
> - if (!has_datasec && btf_is_var(t)) {
> - /* replace VAR with INT */
> - t->info = BTF_INFO_ENC(BTF_KIND_INT, 0, 0);
> - /*
> - * using size = 1 is the safest choice, 4 will be too
> - * big and cause kernel BTF validation failure if
> - * original variable took less than 4 bytes
> + if (btf_is_var(t)) {
> + struct btf_type *var_t;
> +
> + var_t = (struct btf_type *)btf__type_by_id(btf,
> + t->type);
> +
> + /* FIXME: The kernel doesn't understand func_proto with
> + * BTF_VAR_GLOBAL_EXTERN linkage, so we just replace
> + * them with INTs here. What's the right thing to do?
> */
> - t->size = 1;
> - *(int *)(t + 1) = BTF_INT_ENC(0, 0, 8);
> - } else if (!has_datasec && btf_is_datasec(t)) {
> + if (!has_datasec ||
> + (btf_kind(var_t) == BTF_KIND_FUNC_PROTO &&
> + btf_var(t)->linkage == BTF_VAR_GLOBAL_EXTERN)) {
You are the first user to use extern function encoding in BTF! Thanks!
Recently, we have discussion with Alexei and felt that putting extern
function into datasec/var is not pretty. So we have the following llvm patch
https://reviews.llvm.org/D71638
to put extern function as a BTF_KIND_FUNC, i.e.,
BTF_KIND_FUNC
.info (lower 2 bits) -> FUNC_STATIC, FUNC_GLOBAL, FUNC_EXTERN
.type -> BTF_KIND_FUNC_PROTO
Alexei is working on kernel side to ensure this is handled properly
before llvm patch can be merged.
Just let you know for the future potential BTF interface change.
> + /* replace VAR with INT */
> + t->info = BTF_INFO_ENC(BTF_KIND_INT, 0, 0);
> + /*
> + * using size = 1 is the safest choice, 4 will
> + * be too big and cause kernel BTF validation
> + * failure if original variable took less than 4
> + * bytes
> + */
> + t->size = 1;
> + *(int *)(t + 1) = BTF_INT_ENC(0, 0, 8);
> + }
> + } else if (btf_is_datasec(t)) {
> /* replace DATASEC with STRUCT */
> const struct btf_var_secinfo *v = btf_var_secinfos(t);
> struct btf_member *m = btf_members(t);
> struct btf_type *vt;
> + size_t tot_size = 0;
> char *name;
>
> + /* FIXME: The .extern datasec can be 0-sized when there
> + * are only function signatures but no variables marked
> + * as extern. Kernel doesn't understand this, so we need
> + * to get rid of those.
> + */
> + if (has_datasec && t->size > 0)
> + continue;
> +
> name = (char *)btf__name_by_offset(btf, t->name_off);
> while (*name) {
> if (*name == '.')
> @@ -1861,7 +1891,10 @@ static void bpf_object__sanitize_btf(struct bpf_object *obj)
> /* preserve variable name as member name */
> vt = (void *)btf__type_by_id(btf, v->type);
> m->name_off = vt->name_off;
> + tot_size += vt->size;
> }
> + if (t->size < tot_size)
> + t->size = tot_size;
> } else if (!has_func && btf_is_func_proto(t)) {
> /* replace FUNC_PROTO with ENUM */
> vlen = btf_vlen(t);
> @@ -2205,6 +2238,8 @@ static enum extern_type find_extern_type(const struct btf *btf, int id,
> if (find_extern_type(btf, btf_array(t)->type, NULL) != EXT_CHAR)
> return EXT_UNKNOWN;
> return EXT_CHAR_ARR;
> + case BTF_KIND_FUNC_PROTO:
> + return EXT_FUNC;
> default:
> return EXT_UNKNOWN;
> }
> @@ -2215,6 +2250,10 @@ static int cmp_externs(const void *_a, const void *_b)
[...]
Powered by blists - more mailing lists