netdev - Re: [RFC PATCH bpf-next 06/16] bpf: add netlink and ipv6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 10 Apr 2020 16:13:36 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Yonghong Song <yhs@...com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Martin KaFai Lau <kafai@...com>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [RFC PATCH bpf-next 06/16] bpf: add netlink and ipv6_route targets

On Wed, Apr 8, 2020 at 4:25 PM Yonghong Song <yhs@...com> wrote:
>
> This patch added netlink and ipv6_route targets, using
> the same seq_ops (except show()) for /proc/net/{netlink,ipv6_route}.
>
> Since module is not supported for now, ipv6_route is
> supported only if the IPV6 is built-in, i.e., not compiled
> as a module. The restriction can be lifted once module
> is properly supported for bpfdump.
>
> Signed-off-by: Yonghong Song <yhs@...com>
> ---
>  include/linux/bpf.h      |  1 +
>  kernel/bpf/dump.c        | 13 ++++++++++
>  net/ipv6/ip6_fib.c       | 41 +++++++++++++++++++++++++++++-
>  net/ipv6/route.c         | 22 ++++++++++++++++
>  net/netlink/af_netlink.c | 54 +++++++++++++++++++++++++++++++++++++++-
>  5 files changed, 129 insertions(+), 2 deletions(-)
>

[...]

>
> +#if IS_BUILTIN(CONFIG_IPV6)
> +static int ipv6_route_prog_seq_show(struct bpf_prog *prog, struct seq_file *seq,
> +                                   u64 seq_num, void *v)
> +{
> +       struct ipv6_route_iter *iter = seq->private;
> +       struct {
> +               struct fib6_info *rt;
> +               struct seq_file *seq;
> +               u64 seq_num;
> +       } ctx = {

So this anonymous struct definition has to match bpfdump__ipv6_route
function prototype, if I understand correctly. So this means that BTF
will have a very useful struct, that can be used directly in BPF
program, but it won't have a canonical name. This is very sad... Would
it be possible to instead use a struct as a prototype for these
dumpers? Here's why it matters. Instead of currently requiring BPF
users to declare their dumpers as (just copy-pasted):

int BPF_PROG(some_name, struct fib6_info *rt, struct seq_file *seq,
u64 seq_num) {
   ...
}

if bpfdump__ipv6_route was actually a struct definition:


struct bpfdump__ipv6_route {
    struct fib6_info *rt;
    struct seq_file *seq;
    u64 seq_num;
};

Then with vmlinux.h, such program would be very nicely declared and used as:

int some_name(struct bpfdump__ipv6_route *ctx) {
  /* here use ctx->rt, ctx->seq, ctx->seqnum */
}

This is would would be nice to have for raw_tp and tp_btf as well.


Of course we can also code-generate such types from func_protos in
bpftool, and that's a plan B for this, IMO. But seem like in this case
you already have two keep two separate entities in sync: func proto
and struct for context, so I thought I'd bring it up.

> +               .rt = v,
> +               .seq = seq,
> +               .seq_num = seq_num,
> +       };
> +       int ret;
> +
> +       ret = bpf_dump_run_prog(prog, &ctx);
> +       iter->w.leaf = NULL;
> +       return ret == 0 ? 0 : -EINVAL;
> +}
> +