On Wed, Sep 20, 2023 at 9:00 AM <thinker.li@xxxxxxxxx> wrote:
>
> From: Kui-Feng Lee <thinker.li@xxxxxxxxx>
>
> The type info of a struct_ops type may be in a module. So, we need to know
> which module BTF to look for type information. The later patches will make
> libbpf to attach module BTFs to programs. This patch passes attached BTF
> from syscall to bpf_struct_ops subsystem to make sure attached BTF is
> available when the bpf_struct_ops subsystem is ready to use it.
>
> bpf_prog has attach_btf in aux from attach_btf_obj_fd, that is pass along
> with the bpf_attr loading the program. attach_btf is used to find the btf
> type of attach_btf_id. attach_btf_id is used to identify the traced
> function for a trace program. For struct_ops programs, it is used to
> identify the struct_ops type of the struct_ops object a program attached
> to.
>
> Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
> ---
> include/uapi/linux/bpf.h | 4 ++++
> kernel/bpf/bpf_struct_ops.c | 12 +++++++++++-
> kernel/bpf/syscall.c | 2 +-
> kernel/bpf/verifier.c | 4 +++-
> tools/include/uapi/linux/bpf.h | 4 ++++
> 5 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 73b155e52204..178d6fa45fa0 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1390,6 +1390,10 @@ union bpf_attr {
> * to using 5 hash functions).
> */
> __u64 map_extra;
> +
> + __u32 mod_btf_fd; /* fd pointing to a BTF type data
> + * for btf_vmlinux_value_type_id.
> + */
we have attach_btf_obj_fd for BPF_PROG_LOAD command, so I guess
consistent naming would be "<something>_btf_obj_fd" where <something>
would make it more-or-less clear that this is BTF for
btf_vmlinux_value_type_id?
> };
>
> struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 8b5c859377e9..d5600d9ad302 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -765,9 +765,19 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
> struct bpf_struct_ops_map *st_map;
> const struct btf_type *t, *vt;
> struct bpf_map *map;
> + struct btf *btf;
> int ret;
>
> - st_ops = bpf_struct_ops_find_value(attr->btf_vmlinux_value_type_id, btf_vmlinux);
> + /* XXX: We need a module name or ID to find a BTF type. */
> + /* XXX: should use btf from attr->btf_fd */
Do we need these XXX: comments? I think you had some more in previous patches
> + if (attr->mod_btf_fd) {
> + btf = btf_get_by_fd(attr->mod_btf_fd);
> + if (IS_ERR(btf))
> + return ERR_PTR(PTR_ERR(btf));
> + } else {
> + btf = btf_vmlinux;
> + }
> + st_ops = bpf_struct_ops_find_value(attr->btf_vmlinux_value_type_id, btf);
> if (!st_ops)
> return ERR_PTR(-ENOTSUPP);
should we make sure that module's BTF is put properly on error?
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 85c1d908f70f..fed3870fec7a 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1097,7 +1097,7 @@ static int map_check_btf(struct bpf_map *map, const struct btf *btf,
> return ret;
> }
>
> -#define BPF_MAP_CREATE_LAST_FIELD map_extra
> +#define BPF_MAP_CREATE_LAST_FIELD mod_btf_fd
> /* called via syscall */
> static int map_create(union bpf_attr *attr)
> {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 99b45501951c..11f85dbc911b 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -19623,6 +19623,7 @@ static int check_struct_ops_btf_id(struct bpf_verifier_env *env)
> const struct btf_member *member;
> struct bpf_prog *prog = env->prog;
> u32 btf_id, member_idx;
> + struct btf *btf;
> const char *mname;
>
> if (!prog->gpl_compatible) {
> @@ -19630,8 +19631,9 @@ static int check_struct_ops_btf_id(struct bpf_verifier_env *env)
> return -EINVAL;
> }
>
> + btf = prog->aux->attach_btf;
> btf_id = prog->aux->attach_btf_id;
> - st_ops = bpf_struct_ops_find(btf_id, btf_vmlinux);
> + st_ops = bpf_struct_ops_find(btf_id, btf);
> if (!st_ops) {
> verbose(env, "attach_btf_id %u is not a supported struct\n",
> btf_id);
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 73b155e52204..178d6fa45fa0 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1390,6 +1390,10 @@ union bpf_attr {
> * to using 5 hash functions).
> */
> __u64 map_extra;
> +
> + __u32 mod_btf_fd; /* fd pointing to a BTF type data
> + * for btf_vmlinux_value_type_id.
> + */
> };
>
> struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
> --
> 2.34.1
>
