Re: [PATCH v4 bpf-next 2/4] bpf: Add BPF program and map iterators as built-in BPF programs.

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

On Fri, Jul 24, 2020 at 1:39 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> The program and map iterators work similar to seq_file-s.
> Once the program is pinned in bpffs it can be read with "cat" tool
> to print human readable output. In this case about BPF programs and maps.
> For example:
> $ cat /sys/fs/bpf/progs.debug
>   id name            attached
>    5 dump_bpf_map    bpf_iter_bpf_map
>    6 dump_bpf_prog   bpf_iter_bpf_prog
> $ cat /sys/fs/bpf/maps.debug
>   id name            pages
>    3 iterator.rodata     2
>
> To avoid kernel build dependency on clang 10 separate bpf skeleton generation
> into manual "make" step and instead check-in generated .skel.h into git.
>
> Unlike 'bpftool prog show' in-kernel BTF name is used (when available)
> to print full name of BPF program instead of 16-byte truncated name.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---

Tiny bug below, otherwise looks good.

Acked-by: Andrii Nakryiko <andriin@fb.com>


>  kernel/bpf/preload/iterators/.gitignore       |   2 +
>  kernel/bpf/preload/iterators/Makefile         |  57 +++
>  kernel/bpf/preload/iterators/README           |   4 +
>  kernel/bpf/preload/iterators/iterators.bpf.c  | 118 +++++
>  kernel/bpf/preload/iterators/iterators.skel.h | 411 ++++++++++++++++++
>  5 files changed, 592 insertions(+)
>  create mode 100644 kernel/bpf/preload/iterators/.gitignore
>  create mode 100644 kernel/bpf/preload/iterators/Makefile
>  create mode 100644 kernel/bpf/preload/iterators/README
>  create mode 100644 kernel/bpf/preload/iterators/iterators.bpf.c
>  create mode 100644 kernel/bpf/preload/iterators/iterators.skel.h
>

[...]

> +
> +static const char *get_name(struct btf *btf, long btf_id, const char *fallback)
> +{
> +       struct btf_type **types, *t;
> +       unsigned int name_off;
> +       const char *str;
> +
> +       if (!btf)
> +               return fallback;
> +       str = btf->strings;
> +       types = btf->types;
> +       bpf_probe_read_kernel(&t, sizeof(t), types + btf_id);
> +       name_off = BPF_CORE_READ(t, name_off);
> +       if (name_off > btf->hdr.str_len)

>= here?

> +               return fallback;
> +       return str + name_off;
> +}
> +
> +SEC("iter/bpf_map")
> +int dump_bpf_map(struct bpf_iter__bpf_map *ctx)
> +{
> +       struct seq_file *seq = ctx->meta->seq;
> +       __u64 seq_num = ctx->meta->seq_num;
> +       struct bpf_map *map = ctx->map;
> +
> +       if (!map)
> +               return 0;
> +
> +       if (seq_num == 0)
> +               BPF_SEQ_PRINTF(seq, "  id name             pages\n");
> +
> +       BPF_SEQ_PRINTF(seq, "%4u %-16s%6d\n", map->id, map->name, map->memory.pages);

map->memory.pages won't be meaningful, once Roman's patches removing
RLIMIT_MEMLOCK usage land, so might just drop them now

> +       return 0;
> +}
> +
> +SEC("iter/bpf_prog")
> +int dump_bpf_prog(struct bpf_iter__bpf_prog *ctx)
> +{
> +       struct seq_file *seq = ctx->meta->seq;
> +       __u64 seq_num = ctx->meta->seq_num;
> +       struct bpf_prog *prog = ctx->prog;
> +       struct bpf_prog_aux *aux;
> +
> +       if (!prog)
> +               return 0;
> +
> +       aux = prog->aux;
> +       if (seq_num == 0)
> +               BPF_SEQ_PRINTF(seq, "  id name             attached\n");
> +
> +       BPF_SEQ_PRINTF(seq, "%4u %-16s %s %s\n", aux->id,
> +                      get_name(aux->btf, aux->func_info[0].type_id, aux->name),
> +                      aux->attach_func_name, aux->linked_prog->aux->name);
> +       return 0;
> +}
> +char LICENSE[] SEC("license") = "GPL";


[...]