On Sun, May 3, 2020 at 11:27 PM Yonghong Song <yhs@xxxxxx> wrote:
>
> Two 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_iter_create
> syscall level API to create a bpf iterator.
>
> The macro BPF_SEQ_PRINTF are also introduced. The format
> looks like:
> BPF_SEQ_PRINTF(seq, "task id %d\n", pid);
>
> This macro can help bpf program writers with
> nicer bpf_seq_printf syntax similar to the kernel one.
>
> Signed-off-by: Yonghong Song <yhs@xxxxxx>
> ---
Looks great! Just few nits.
> tools/lib/bpf/bpf.c | 11 +++++++++
> tools/lib/bpf/bpf.h | 2 ++
> tools/lib/bpf/bpf_tracing.h | 16 +++++++++++++
> tools/lib/bpf/libbpf.c | 45 +++++++++++++++++++++++++++++++++++++
> tools/lib/bpf/libbpf.h | 9 ++++++++
> tools/lib/bpf/libbpf.map | 2 ++
> 6 files changed, 85 insertions(+)
>
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 43322f0d6c7f..1756ae47ddf2 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)
As discussed in previous thread, given we don't anticipate needing
anything beyond link_fd, let's do bpf_iter_create(int link_fd), nice
and simple. Once we need to add any extensibility, we can add
bpf_iter_create_xattr() variant with opts.
> +{
> + 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));
> +}
> +
[...]
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 977add1b73e2..93355a257405 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -6629,6 +6629,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),
Another nit. As discussed, I think auto-attach is a nice feature,
which, if user doesn't want/need, can be skipped.
> 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),
> @@ -6891,6 +6894,7 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
>
[...]
>
> +struct bpf_link *
> +bpf_program__attach_iter(struct bpf_program *prog,
> + const struct bpf_iter_attach_opts *opts)
> +{
> + enum bpf_attach_type attach_type;
> + char errmsg[STRERR_BUFSIZE];
> + struct bpf_link *link;
> + int prog_fd, link_fd;
> +
> + if (!OPTS_VALID(opts, bpf_iter_attach_opts))
> + return ERR_PTR(-EINVAL);
> +
> + prog_fd = bpf_program__fd(prog);
> + if (prog_fd < 0) {
> + pr_warn("program '%s': can't attach before loaded\n",
> + bpf_program__title(prog, false));
> + return ERR_PTR(-EINVAL);
> + }
> +
> + link = calloc(1, sizeof(*link));
> + if (!link)
> + return ERR_PTR(-ENOMEM);
> + link->detach = &bpf_link__detach_fd;
> +
> + attach_type = BPF_TRACE_ITER;
> + link_fd = bpf_link_create(prog_fd, 0, attach_type, NULL);
nit: attach_type variable doesn't seem to be necessary
> + 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;
> +}
> +
[...]
