From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Yonghong Song <yhs@fb.com>
Cc: Andrii Nakryiko <andriin@fb.com>, bpf <bpf@vger.kernel.org>,
Martin KaFai Lau <kafai@fb.com>,
Networking <netdev@vger.kernel.org>,
Alexei Starovoitov <ast@fb.com>,
Daniel Borkmann <daniel@iogearbox.net>,
Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH bpf-next v1 15/19] tools/libbpf: add bpf_iter support
Date: Wed, 29 Apr 2020 18:41:56 -0700 [thread overview]
Message-ID: <CAEf4BzZKaBpQfohsWcF5qJpMU96vxDVniaPie=54Gx6kK66KQw@mail.gmail.com> (raw)
In-Reply-To: <20200427201252.2996037-1-yhs@fb.com>
On Mon, Apr 27, 2020 at 1:17 PM Yonghong Song <yhs@fb.com> wrote:
>
> Three new libbpf APIs are added to support bpf_iter:
> - bpf_program__attach_iter
> Given a bpf program and additional parameters, which is
> none now, returns a bpf_link.
> - bpf_link__create_iter
> Given a bpf_link, create a bpf_iter and return a fd
> so user can then do read() to get seq_file output data.
> - bpf_iter_create
> syscall level API to create a bpf iterator.
>
> Two macros, BPF_SEQ_PRINTF0 and BPF_SEQ_PRINTF, are also introduced.
> These two macros can help bpf program writers with
> nicer bpf_seq_printf syntax similar to the kernel one.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> tools/lib/bpf/bpf.c | 11 +++++++
> tools/lib/bpf/bpf.h | 2 ++
> tools/lib/bpf/bpf_tracing.h | 23 ++++++++++++++
> tools/lib/bpf/libbpf.c | 60 +++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 11 +++++++
> tools/lib/bpf/libbpf.map | 7 +++++
> 6 files changed, 114 insertions(+)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 5cc1b0785d18..7ffd6c0ad95f 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -619,6 +619,17 @@ int bpf_link_update(int link_fd, int new_prog_fd,
> return sys_bpf(BPF_LINK_UPDATE, &attr, sizeof(attr));
> }
>
> +int bpf_iter_create(int link_fd, unsigned int flags)
Do you envision anything more than just flags being passed for
bpf_iter_create? I wonder if we should just go ahead with options
struct here?
> +{
> + union bpf_attr attr;
> +
> + memset(&attr, 0, sizeof(attr));
> + attr.iter_create.link_fd = link_fd;
> + attr.iter_create.flags = flags;
> +
> + return sys_bpf(BPF_ITER_CREATE, &attr, sizeof(attr));
> +}
> +
[...]
> +/*
> + * BPF_SEQ_PRINTF to wrap bpf_seq_printf to-be-printed values
> + * in a structure. BPF_SEQ_PRINTF0 is a simple wrapper for
> + * bpf_seq_printf().
> + */
> +#define BPF_SEQ_PRINTF0(seq, fmt) \
> + ({ \
> + int ret = bpf_seq_printf(seq, fmt, sizeof(fmt), \
> + (void *)0, 0); \
> + ret; \
> + })
> +
> +#define BPF_SEQ_PRINTF(seq, fmt, args...) \
You can unify BPF_SEQ_PRINTF and BPF_SEQ_PRINTF0 by using
___bpf_empty() macro. See bpf_core_read.h for similar use case.
Specifically, look at ___empty (equivalent of ___bpf_empty) and
___core_read, ___core_read0, ___core_readN macro.
> + ({ \
> + _Pragma("GCC diagnostic push") \
> + _Pragma("GCC diagnostic ignored \"-Wint-conversion\"") \
> + __u64 param[___bpf_narg(args)] = { args }; \
Do you need to provide the size of array here? If you omit
__bpf_narg(args), wouldn't compiler automatically calculate the right
size?
Also, can you please use "unsigned long long" to not have any implicit
dependency on __u64 being defined?
> + _Pragma("GCC diagnostic pop") \
> + int ret = bpf_seq_printf(seq, fmt, sizeof(fmt), \
> + param, sizeof(param)); \
> + ret; \
> + })
> +
> #endif
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8e1dc6980fac..ffdc4d8e0cc0 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -6366,6 +6366,9 @@ static const struct bpf_sec_def section_defs[] = {
> .is_attach_btf = true,
> .expected_attach_type = BPF_LSM_MAC,
> .attach_fn = attach_lsm),
> + SEC_DEF("iter/", TRACING,
> + .expected_attach_type = BPF_TRACE_ITER,
> + .is_attach_btf = true),
It would be nice to implement auto-attach capabilities (similar to
fentry/fexit, lsm and raw_tracepoint). Section name should have enough
information for this, no?
> BPF_PROG_SEC("xdp", BPF_PROG_TYPE_XDP),
> BPF_PROG_SEC("perf_event", BPF_PROG_TYPE_PERF_EVENT),
> BPF_PROG_SEC("lwt_in", BPF_PROG_TYPE_LWT_IN),
> @@ -6629,6 +6632,7 @@ static int bpf_object__collect_struct_ops_map_reloc(struct bpf_object *obj,
>
[...]
> +
> + link = calloc(1, sizeof(*link));
> + if (!link)
> + return ERR_PTR(-ENOMEM);
> + link->detach = &bpf_link__detach_fd;
> +
> + attach_type = bpf_program__get_expected_attach_type(prog);
Given you know it has to be BPF_TRACE_ITER, it's better to explicitly
specify that. If provided program wasn't loaded with correct
expected_attach_type, kernel will reject it. But if you don't do it,
then you can accidentally create some other type of bpf_link.
> + link_fd = bpf_link_create(prog_fd, 0, attach_type, NULL);
> + if (link_fd < 0) {
> + link_fd = -errno;
> + free(link);
> + pr_warn("program '%s': failed to attach to iterator: %s\n",
> + bpf_program__title(prog, false),
> + libbpf_strerror_r(link_fd, errmsg, sizeof(errmsg)));
> + return ERR_PTR(link_fd);
> + }
> + link->fd = link_fd;
> + return link;
> +}
> +
> +int bpf_link__create_iter(struct bpf_link *link, unsigned int flags)
> +{
Same question as for low-level bpf_link_create(). If we expect the
need to extend optional things in the future, I'd add opts right now.
But I wonder if bpf_link__create_iter() provides any additional value
beyond bpf_iter_create(). Maybe let's not add it (yet)?
> + char errmsg[STRERR_BUFSIZE];
> + int iter_fd;
> +
> + iter_fd = bpf_iter_create(bpf_link__fd(link), flags);
> + if (iter_fd < 0) {
> + iter_fd = -errno;
> + pr_warn("failed to create an iterator: %s\n",
> + libbpf_strerror_r(iter_fd, errmsg, sizeof(errmsg)));
> + }
> +
> + return iter_fd;
> +}
> +
[...]
next prev parent reply other threads:[~2020-04-30 1:42 UTC|newest]
Thread overview: 81+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-27 20:12 [PATCH bpf-next v1 00/19] bpf: implement bpf iterator for kernel data Yonghong Song
2020-04-27 20:12 ` [PATCH bpf-next v1 01/19] net: refactor net assignment for seq_net_private structure Yonghong Song
2020-04-29 5:38 ` Andrii Nakryiko
2020-04-27 20:12 ` [PATCH bpf-next v1 02/19] bpf: implement an interface to register bpf_iter targets Yonghong Song
2020-04-28 16:20 ` Martin KaFai Lau
2020-04-28 16:50 ` Yonghong Song
2020-04-27 20:12 ` [PATCH bpf-next v1 03/19] bpf: add bpf_map iterator Yonghong Song
2020-04-29 0:37 ` Martin KaFai Lau
2020-04-29 0:48 ` Alexei Starovoitov
2020-04-29 1:15 ` Yonghong Song
2020-04-29 2:44 ` Alexei Starovoitov
2020-04-29 5:09 ` Yonghong Song
2020-04-29 6:08 ` Andrii Nakryiko
2020-04-29 6:20 ` Yonghong Song
2020-04-29 6:30 ` Alexei Starovoitov
2020-04-29 6:40 ` Andrii Nakryiko
2020-04-29 6:44 ` Yonghong Song
2020-04-29 15:34 ` Alexei Starovoitov
2020-04-29 18:14 ` Yonghong Song
2020-04-29 19:19 ` Andrii Nakryiko
2020-04-29 20:15 ` Yonghong Song
2020-04-30 3:06 ` Alexei Starovoitov
2020-04-30 4:01 ` Yonghong Song
2020-04-29 6:34 ` Martin KaFai Lau
2020-04-29 6:51 ` Yonghong Song
2020-04-29 19:25 ` Andrii Nakryiko
2020-04-29 1:02 ` Yonghong Song
2020-04-29 6:04 ` Andrii Nakryiko
2020-04-27 20:12 ` [PATCH bpf-next v1 04/19] bpf: allow loading of a bpf_iter program Yonghong Song
2020-04-29 0:54 ` Martin KaFai Lau
2020-04-29 1:27 ` Yonghong Song
2020-04-27 20:12 ` [PATCH bpf-next v1 05/19] bpf: support bpf tracing/iter programs for BPF_LINK_CREATE Yonghong Song
2020-04-29 1:17 ` [Potential Spoof] " Martin KaFai Lau
2020-04-29 6:25 ` Andrii Nakryiko
2020-04-27 20:12 ` [PATCH bpf-next v1 06/19] bpf: support bpf tracing/iter programs for BPF_LINK_UPDATE Yonghong Song
2020-04-29 1:32 ` Martin KaFai Lau
2020-04-29 5:04 ` Yonghong Song
2020-04-29 5:58 ` Martin KaFai Lau
2020-04-29 6:32 ` Andrii Nakryiko
2020-04-29 6:41 ` Martin KaFai Lau
2020-04-27 20:12 ` [PATCH bpf-next v1 07/19] bpf: create anonymous bpf iterator Yonghong Song
2020-04-29 5:39 ` Martin KaFai Lau
2020-04-29 6:56 ` Andrii Nakryiko
2020-04-29 7:06 ` Yonghong Song
2020-04-29 18:16 ` Andrii Nakryiko
2020-04-29 18:46 ` Martin KaFai Lau
2020-04-29 19:20 ` Yonghong Song
2020-04-29 20:50 ` Martin KaFai Lau
2020-04-29 20:54 ` Yonghong Song
2020-04-29 19:39 ` Andrii Nakryiko
2020-04-27 20:12 ` [PATCH bpf-next v1 08/19] bpf: create file " Yonghong Song
2020-04-29 20:40 ` Andrii Nakryiko
2020-04-30 18:02 ` Yonghong Song
2020-04-27 20:12 ` [PATCH bpf-next v1 09/19] bpf: add PTR_TO_BTF_ID_OR_NULL support Yonghong Song
2020-04-29 20:46 ` Andrii Nakryiko
2020-04-29 20:51 ` Yonghong Song
2020-04-27 20:12 ` [PATCH bpf-next v1 10/19] bpf: add netlink and ipv6_route targets Yonghong Song
2020-04-28 19:49 ` kbuild test robot
2020-04-28 19:50 ` [RFC PATCH] bpf: __bpf_iter__netlink() can be static kbuild test robot
2020-04-27 20:12 ` [PATCH bpf-next v1 11/19] bpf: add task and task/file targets Yonghong Song
2020-04-30 2:08 ` Andrii Nakryiko
2020-05-01 17:23 ` Yonghong Song
2020-05-01 19:01 ` Andrii Nakryiko
2020-04-27 20:12 ` [PATCH bpf-next v1 12/19] bpf: add bpf_seq_printf and bpf_seq_write helpers Yonghong Song
2020-04-28 6:02 ` kbuild test robot
2020-04-28 16:35 ` Yonghong Song
2020-04-30 20:06 ` Andrii Nakryiko
2020-04-27 20:12 ` [PATCH bpf-next v1 13/19] bpf: handle spilled PTR_TO_BTF_ID properly when checking stack_boundary Yonghong Song
2020-04-27 20:12 ` [PATCH bpf-next v1 14/19] bpf: support variable length array in tracing programs Yonghong Song
2020-04-30 20:04 ` Andrii Nakryiko
2020-04-27 20:12 ` [PATCH bpf-next v1 15/19] tools/libbpf: add bpf_iter support Yonghong Song
2020-04-30 1:41 ` Andrii Nakryiko [this message]
2020-05-02 7:17 ` Yonghong Song
2020-04-27 20:12 ` [PATCH bpf-next v1 16/19] tools/bpftool: add bpf_iter support for bptool Yonghong Song
2020-04-28 9:27 ` Quentin Monnet
2020-04-28 17:35 ` Yonghong Song
2020-04-29 8:37 ` Quentin Monnet
2020-04-27 20:12 ` [PATCH bpf-next v1 17/19] tools/bpf: selftests: add iterator programs for ipv6_route and netlink Yonghong Song
2020-04-30 2:12 ` Andrii Nakryiko
2020-04-27 20:12 ` [PATCH bpf-next v1 18/19] tools/bpf: selftests: add iter progs for bpf_map/task/task_file Yonghong Song
2020-04-27 20:12 ` [PATCH bpf-next v1 19/19] tools/bpf: selftests: add bpf_iter selftests Yonghong Song
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='CAEf4BzZKaBpQfohsWcF5qJpMU96vxDVniaPie=54Gx6kK66KQw@mail.gmail.com' \
--to=andrii.nakryiko@gmail.com \
--cc=andriin@fb.com \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kafai@fb.com \
--cc=kernel-team@fb.com \
--cc=netdev@vger.kernel.org \
--cc=yhs@fb.com \
/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).