Re: [PATCH RFC bpf-next 2/3] libbpf: Handle function externs and support static linking

[Yonghong Song <yhs@fb.com>]

On 12/19/19 6:29 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.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@redhat.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)
[...]