Re: [PATCH v2 bpf-next 10/16] bpf: Add bpf_btf_find_by_name_kind() helper.

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Thu, Apr 22, 2021 at 5:27 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> Add new helper:
>
> long bpf_btf_find_by_name_kind(u32 btf_fd, char *name, u32 kind, int flags)
>         Description
>                 Find given name with given type in BTF pointed to by btf_fd.
>                 If btf_fd is zero look for the name in vmlinux BTF and in module's BTFs.
>         Return
>                 Returns btf_id and btf_obj_fd in lower and upper 32 bits.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  include/linux/bpf.h            |  1 +
>  include/uapi/linux/bpf.h       |  8 ++++
>  kernel/bpf/btf.c               | 68 ++++++++++++++++++++++++++++++++++
>  kernel/bpf/syscall.c           |  2 +
>  tools/include/uapi/linux/bpf.h |  8 ++++
>  5 files changed, 87 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 0f841bd0cb85..4cf361eb6a80 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1972,6 +1972,7 @@ extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto;
>  extern const struct bpf_func_proto bpf_task_storage_get_proto;
>  extern const struct bpf_func_proto bpf_task_storage_delete_proto;
>  extern const struct bpf_func_proto bpf_for_each_map_elem_proto;
> +extern const struct bpf_func_proto bpf_btf_find_by_name_kind_proto;
>
>  const struct bpf_func_proto *bpf_tracing_func_proto(
>         enum bpf_func_id func_id, const struct bpf_prog *prog);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index de58a714ed36..253f5f031f08 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -4748,6 +4748,13 @@ union bpf_attr {
>   *             Execute bpf syscall with given arguments.
>   *     Return
>   *             A syscall result.
> + *
> + * long bpf_btf_find_by_name_kind(u32 btf_fd, char *name, u32 kind, int flags)
> + *     Description
> + *             Find given name with given type in BTF pointed to by btf_fd.

"Find BTF type with given name"? Should the limits on name length be
specified? KSYM_NAME_LEN is a pretty arbitrary restriction. Also,
would it still work fine if the caller provides a pointer to a much
shorter piece of memory?

Why not add name_sz right after name, as we do with a lot of other
arguments like this?

> + *             If btf_fd is zero look for the name in vmlinux BTF and in module's BTFs.
> + *     Return
> + *             Returns btf_id and btf_obj_fd in lower and upper 32 bits.

Mention that for vmlinux BTF btf_obj_fd will be zero? Also who "owns"
the FD? If the BPF program doesn't close it, when are they going to be
cleaned up?

>   */
>  #define __BPF_FUNC_MAPPER(FN)          \
>         FN(unspec),                     \
> @@ -4917,6 +4924,7 @@ union bpf_attr {
>         FN(for_each_map_elem),          \
>         FN(snprintf),                   \
>         FN(sys_bpf),                    \
> +       FN(btf_find_by_name_kind),      \
>         /* */
>

[...]