bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
	Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH v2 bpf-next 10/16] bpf: Add bpf_btf_find_by_name_kind() helper.
Date: Mon, 26 Apr 2021 20:37:07 -0700	[thread overview]
Message-ID: <20210427033707.fu7hsm6xi5ayx6he@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <CAEf4BzYkzzN=ZD2X1bOg8U39Whbe6oTPuUEMOpACw6NPEW69NA@mail.gmail.com>

On Mon, Apr 26, 2021 at 03:46:29PM -0700, Andrii Nakryiko wrote:
> 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

+1

> specified? KSYM_NAME_LEN is a pretty arbitrary restriction.

that's implementation detail that shouldn't leak into uapi.

> 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?

That's an option too, but then the helper will have 5 args and 'flags'
would be likely useless. I mean unlikely it will help extending it.
I was thinking about ARG_PTR_TO_CONST_STR, but it doesn't work,
since blob is writeable by the prog. It's read only from user space.
I'm fine with name, name_sz though.

> 
> > + *             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?

just like bpf_sys_bpf. Who owns returned FD? The program that called
the helper, of course.
In the current shape of loader prog these btf fds are cleaned up correctly
in success and in error case.
Not all FDs though. map fds will stay around if bpf_sys_bpf(prog_load) fails to load.
Tweaking loader prog to close all FDs in error case is on todo list.

  reply	other threads:[~2021-04-27  3:37 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-23  0:26 [PATCH v2 bpf-next 00/16] bpf: syscall program, FD array, loader program, light skeleton Alexei Starovoitov
2021-04-23  0:26 ` [PATCH v2 bpf-next 01/16] bpf: Introduce bpf_sys_bpf() helper and program type Alexei Starovoitov
2021-04-23 18:15   ` Yonghong Song
2021-04-23 18:28     ` Alexei Starovoitov
2021-04-23 19:32       ` Yonghong Song
2021-04-26 16:51   ` Andrii Nakryiko
2021-04-27 18:45   ` John Fastabend
2021-04-23  0:26 ` [PATCH v2 bpf-next 02/16] bpf: Introduce bpfptr_t user/kernel pointer Alexei Starovoitov
2021-04-23  0:26 ` [PATCH v2 bpf-next 03/16] bpf: Prepare bpf syscall to be used from kernel and user space Alexei Starovoitov
2021-04-23  0:26 ` [PATCH v2 bpf-next 04/16] libbpf: Support for syscall program type Alexei Starovoitov
2021-04-26 22:24   ` Andrii Nakryiko
2021-04-23  0:26 ` [PATCH v2 bpf-next 05/16] selftests/bpf: Test " Alexei Starovoitov
2021-04-26 17:02   ` Andrii Nakryiko
2021-04-27  2:43     ` Alexei Starovoitov
2021-04-27 16:28       ` Andrii Nakryiko
2021-04-23  0:26 ` [PATCH v2 bpf-next 06/16] bpf: Make btf_load command to be bpfptr_t compatible Alexei Starovoitov
2021-04-23  0:26 ` [PATCH v2 bpf-next 07/16] selftests/bpf: Test for btf_load command Alexei Starovoitov
2021-04-23  0:26 ` [PATCH v2 bpf-next 08/16] bpf: Introduce fd_idx Alexei Starovoitov
2021-04-23  0:26 ` [PATCH v2 bpf-next 09/16] libbpf: Support for fd_idx Alexei Starovoitov
2021-04-26 17:14   ` Andrii Nakryiko
2021-04-27  2:53     ` Alexei Starovoitov
2021-04-27 16:36       ` Andrii Nakryiko
2021-04-28  1:32         ` Alexei Starovoitov
2021-04-28 18:40           ` Andrii Nakryiko
2021-04-23  0:26 ` [PATCH v2 bpf-next 10/16] bpf: Add bpf_btf_find_by_name_kind() helper Alexei Starovoitov
2021-04-26 22:46   ` Andrii Nakryiko
2021-04-27  3:37     ` Alexei Starovoitov [this message]
2021-04-27 17:45       ` Andrii Nakryiko
2021-04-28  1:55         ` Alexei Starovoitov
2021-04-28 18:44           ` Andrii Nakryiko
2021-04-27 21:00   ` John Fastabend
2021-04-27 21:05     ` John Fastabend
2021-04-28  2:10     ` Alexei Starovoitov
2021-04-23  0:26 ` [PATCH v2 bpf-next 11/16] bpf: Add bpf_sys_close() helper Alexei Starovoitov
2021-04-23  0:26 ` [PATCH v2 bpf-next 12/16] libbpf: Change the order of data and text relocations Alexei Starovoitov
2021-04-26 17:29   ` Andrii Nakryiko
2021-04-27  3:00     ` Alexei Starovoitov
2021-04-27 16:47       ` Andrii Nakryiko
2021-04-23  0:26 ` [PATCH v2 bpf-next 13/16] libbpf: Add bpf_object pointer to kernel_supports() Alexei Starovoitov
2021-04-26 17:30   ` Andrii Nakryiko
2021-04-23  0:26 ` [PATCH v2 bpf-next 14/16] libbpf: Generate loader program out of BPF ELF file Alexei Starovoitov
2021-04-26 22:22   ` Andrii Nakryiko
2021-04-27  3:25     ` Alexei Starovoitov
2021-04-27 17:34       ` Andrii Nakryiko
2021-04-28  1:42         ` Alexei Starovoitov
2021-04-23  0:26 ` [PATCH v2 bpf-next 15/16] bpftool: Use syscall/loader program in "prog load" and "gen skeleton" command Alexei Starovoitov
2021-04-26 22:35   ` Andrii Nakryiko
2021-04-27  3:28     ` Alexei Starovoitov
2021-04-27 17:38       ` Andrii Nakryiko
2021-04-23  0:26 ` [PATCH v2 bpf-next 16/16] selftests/bpf: Convert few tests to light skeleton Alexei Starovoitov
2021-04-23 21:36 ` [PATCH v2 bpf-next 00/16] bpf: syscall program, FD array, loader program, " Yonghong Song
2021-04-23 23:16   ` Alexei Starovoitov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210427033707.fu7hsm6xi5ayx6he@ast-mbp.dhcp.thefacebook.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).