From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D8965C43603 for ; Thu, 19 Dec 2019 16:59:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 96D662146E for ; Thu, 19 Dec 2019 16:59:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="fGU1+3We" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726943AbfLSQ7h (ORCPT ); Thu, 19 Dec 2019 11:59:37 -0500 Received: from us-smtp-2.mimecast.com ([207.211.31.81]:60675 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726760AbfLSQ7h (ORCPT ); Thu, 19 Dec 2019 11:59:37 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1576774775; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=MGL6WDhpvx9WJXLOC66ZSgn8G/Jlbj4ZPZWGZkIIBLc=; b=fGU1+3We2dW9r4DiHCKArqf6UhpGmIK0hAjTsVpUWxRXjhcPFtXd2j00Sp/xXbaIb95LX0 WraNkQ/b/K4/Dcm59hQLcL0UqB7aiz1U/zxs+o4CvAqJYqF7Fm0JtnnnFROIHB4yqaO1lf 2sOxI33x/agXkQio19Nezsf6OF+2Fzs= Received: from mail-lf1-f69.google.com (mail-lf1-f69.google.com [209.85.167.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-153-UAZaXEzkM3uTV_1vXRKawA-1; Thu, 19 Dec 2019 11:59:34 -0500 X-MC-Unique: UAZaXEzkM3uTV_1vXRKawA-1 Received: by mail-lf1-f69.google.com with SMTP id a11so623369lff.12 for ; Thu, 19 Dec 2019 08:59:33 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version:content-transfer-encoding; bh=MGL6WDhpvx9WJXLOC66ZSgn8G/Jlbj4ZPZWGZkIIBLc=; b=FdDi5d7d14vWPL0Q0XHhP3N4cj5ZwIh2Mmhc7UOgCOpvYtZa7o8fDWMQrAXM8ekDkE QuAr0vBXHvWtG29GsWXljwUBQoQMnti5AbMN7zofqd0GC72l//lWClQGP3I342eTFZbg ejedtHgUOgEwYaXMjexe3Au8sWK//O7iGJVLOovG9YmFxRkUuNoYELY+LBCbj+oMn88d TSR8HQiBsLO8uZe09pEj4vU8iq2ilLkfKStIyUICrKqL0GdtsySGV/00gorkiNvujaJa FDMy+u0W5wENPJq2ICCcwCUEw/8KKX02jJ4XqgYIu+/lPp3/gOm4YkQ+Vmuj3vwcUg9K 9yMw== X-Gm-Message-State: APjAAAXMMwfWJV7L/L2vZaG6OH/6CQIpGO87jGkACI6JB3j8wygvTuZ7 1kRrgEcYrUGsK7DBFLansXcTUrb2/hzL+IlDvZWaSkOIZcbHjYqMqfdhXZlgfNIcpM2Xun0DMDk eVyhWGTpILNYO X-Received: by 2002:a05:651c:8f:: with SMTP id 15mr6763374ljq.109.1576774772595; Thu, 19 Dec 2019 08:59:32 -0800 (PST) X-Google-Smtp-Source: APXvYqzhzAk06AnWNSsOXuDYDdgrouti3i9ncVXsU5b1ct6nC/BdZGwcfsJxSEzEce7UZBH9WoThTg== X-Received: by 2002:a05:651c:8f:: with SMTP id 15mr6763361ljq.109.1576774772337; Thu, 19 Dec 2019 08:59:32 -0800 (PST) Received: from alrua-x1.borgediget.toke.dk ([2a0c:4d80:42:443::2]) by smtp.gmail.com with ESMTPSA id k23sm3241541ljj.85.2019.12.19.08.59.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Dec 2019 08:59:31 -0800 (PST) Received: by alrua-x1.borgediget.toke.dk (Postfix, from userid 1000) id A9AAA180969; Thu, 19 Dec 2019 17:59:29 +0100 (CET) From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Yonghong Song , Daniel Borkmann Cc: Alexei Starovoitov , Martin Lau , Song Liu , Jesper Dangaard Brouer , Andrii Nakryiko , David Miller , "netdev\@vger.kernel.org" , "bpf\@vger.kernel.org" Subject: Re: [PATCH RFC bpf-next 2/3] libbpf: Handle function externs and support static linking In-Reply-To: References: <157676577049.957277.3346427306600998172.stgit@toke.dk> <157676577267.957277.6240503077867756432.stgit@toke.dk> X-Clacks-Overhead: GNU Terry Pratchett Date: Thu, 19 Dec 2019 17:59:29 +0100 Message-ID: <87v9qc2qvi.fsf@toke.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Sender: bpf-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org Yonghong Song writes: > On 12/19/19 6:29 AM, Toke H=C3=B8iland-J=C3=B8rgensen wrote: >> From: Toke H=C3=B8iland-J=C3=B8rgensen >>=20 >> This adds support for resolving function externs to libbpf, with a new A= PI >> to resolve external function calls by static linking at load-time. The A= PI >> 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. >>=20 >> 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. >>=20 >> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen >> --- >> 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(-) >>=20 >> 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 =3D t->size; >> goto done; >> case BTF_KIND_PTR: >> + case BTF_KIND_FUNC_PROTO: >> size =3D 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, str= uct btf *btf) >> */ >> if (btf_is_datasec(t)) { >> err =3D 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 !=3D -ENOENT) >> break; >> } >> } >>=20=20=20 >> - return err; >> + return err =3D=3D -ENOENT ? err : 0; >> } >>=20=20=20 >> 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, >> }; >>=20=20=20 >> +struct extern_desc; >> + >> struct reloc_desc { >> enum reloc_type type; >> int insn_idx; >> int map_idx; >> int sym_off; >> + struct extern_desc *ext; >> }; >>=20=20=20 >> /* >> @@ -274,6 +278,7 @@ enum extern_type { >> EXT_INT, >> EXT_TRISTATE, >> EXT_CHAR_ARR, >> + EXT_FUNC >> }; >>=20=20=20 >> struct extern_desc { >> @@ -287,6 +292,7 @@ struct extern_desc { >> bool is_signed; >> bool is_weak; >> bool is_set; >> + struct bpf_program *tgt_prog; >> }; >>=20=20=20 >> 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; >>=20=20=20 >> bool loaded; >> @@ -1041,6 +1048,7 @@ static int set_ext_value_tri(struct extern_desc *e= xt, void *ext_val, >> case EXT_UNKNOWN: >> case EXT_INT: >> case EXT_CHAR_ARR: >> + case EXT_FUNC: >> default: >> pr_warn("extern %s=3D%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; >>=20=20=20 >> - if (obj->nr_extern =3D=3D 0) >> + if (obj->nr_data_extern =3D=3D 0) >> return 0; >>=20=20=20 >> last_ext =3D &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; >>=20=20=20 >> - if (!obj->btf || (has_func && has_datasec)) >> + if (!obj->btf) >> return; >> - >> for (i =3D 1; i <=3D btf__get_nr_types(btf); i++) { >> t =3D (struct btf_type *)btf__type_by_id(btf, i); >>=20=20=20 >> - if (!has_datasec && btf_is_var(t)) { >> - /* replace VAR with INT */ >> - t->info =3D BTF_INFO_ENC(BTF_KIND_INT, 0, 0); >> - /* >> - * using size =3D 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 =3D (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 =3D 1; >> - *(int *)(t + 1) =3D BTF_INT_ENC(0, 0, 8); >> - } else if (!has_datasec && btf_is_datasec(t)) { >> + if (!has_datasec || >> + (btf_kind(var_t) =3D=3D BTF_KIND_FUNC_PROTO && >> + btf_var(t)->linkage =3D=3D BTF_VAR_GLOBAL_EXTERN)) { > > You are the first user to use extern function encoding in BTF! Thanks! Haha, you're welcome! And yeah, I realise this is pretty bleeding edge stuff, and as you can probably tell I'm sort of fumbling my way forward here ;) > Recently, we have discussion with Alexei and felt that putting extern=20 > function into datasec/var is not pretty. So we have the following llvm pa= tch > 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=20 > before llvm patch can be merged. > > Just let you know for the future potential BTF interface change. OK, thanks for the head's up. And yeah, I agree this sounds like an improvement; I was a little puzzled by the datasec/var thing, and a lot of the weird hacks I had to do were related to working around that. So if this is just going to go away, that's great! If you guys can look the rest of the patch over and give me some pointers on the other FIXME items (and the API), that would be great! No great rush, though; I'm leaving for the holidays tomorrow, so won't have any more time to work on this before the new year. I guess I'll and see how far along the kernel/llvm changes are, and deal with any other feedback you guys have by then. :) -Toke