| Message ID | 20200408232527.2675717-1-yhs@fb.com |
|---|---|
| State | RFC |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: implement bpf based dumping of kernel data structures | expand |
On Wed, Apr 8, 2020 at 4:25 PM Yonghong Song <yhs@fb.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@fb.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; > +} > +
On 4/10/20 4:13 PM, Andrii Nakryiko wrote: > On Wed, Apr 8, 2020 at 4:25 PM Yonghong Song <yhs@fb.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@fb.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 */ > } Thanks, I do not know this! This definitely better and may make kernel code simpler. Will experiment. > > 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; >> +} >> +
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 8171e01ff4be..f7d4269d77b8 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1119,6 +1119,7 @@ int bpf_dump_set_target_info(u32 target_fd, struct bpf_prog *prog); int bpf_dump_create(u32 prog_fd, const char __user *dumper_name); struct bpf_prog *bpf_dump_get_prog(struct seq_file *seq, u32 priv_data_size, u64 *seq_num); +int bpf_dump_run_prog(struct bpf_prog *prog, void *ctx); int bpf_percpu_hash_copy(struct bpf_map *map, void *key, void *value); int bpf_percpu_array_copy(struct bpf_map *map, void *key, void *value); diff --git a/kernel/bpf/dump.c b/kernel/bpf/dump.c index ac6856abb711..4e009b2612c2 100644 --- a/kernel/bpf/dump.c +++ b/kernel/bpf/dump.c @@ -412,6 +412,19 @@ struct bpf_prog *bpf_dump_get_prog(struct seq_file *seq, u32 priv_data_size, return extra_data->prog; } +int bpf_dump_run_prog(struct bpf_prog *prog, void *ctx) +{ + int ret; + + migrate_disable(); + rcu_read_lock(); + ret = BPF_PROG_RUN(prog, ctx); + rcu_read_unlock(); + migrate_enable(); + + return ret; +} + int bpf_dump_reg_target(const char *target, const char *target_proto, const struct seq_operations *seq_ops, diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c index 46ed56719476..0a8dbdcf5f12 100644 --- a/net/ipv6/ip6_fib.c +++ b/net/ipv6/ip6_fib.c @@ -2467,7 +2467,7 @@ void fib6_gc_cleanup(void) } #ifdef CONFIG_PROC_FS -static int ipv6_route_seq_show(struct seq_file *seq, void *v) +static int ipv6_route_native_seq_show(struct seq_file *seq, void *v) { struct fib6_info *rt = v; struct ipv6_route_iter *iter = seq->private; @@ -2637,6 +2637,45 @@ static void ipv6_route_seq_stop(struct seq_file *seq, void *v) rcu_read_unlock_bh(); } +#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 = { + .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; +} + +static int ipv6_route_seq_show(struct seq_file *seq, void *v) +{ + struct bpf_prog *prog; + u64 seq_num; + + prog = bpf_dump_get_prog(seq, sizeof(struct ipv6_route_iter), &seq_num); + if (!prog) + return ipv6_route_native_seq_show(seq, v); + + return ipv6_route_prog_seq_show(prog, seq, seq_num, v); +} +#else +static int ipv6_route_seq_show(struct seq_file *seq, void *v) +{ + return ipv6_route_native_seq_show(seq, v); +} +#endif + const struct seq_operations ipv6_route_seq_ops = { .start = ipv6_route_seq_start, .next = ipv6_route_seq_next, diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 310cbddaa533..f3457d9d5a8b 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -6390,6 +6390,16 @@ void __init ip6_route_init_special_entries(void) #endif } +#if IS_BUILTIN(CONFIG_IPV6) +#ifdef CONFIG_PROC_FS +int __init bpfdump__ipv6_route(struct fib6_info *rt, struct seq_file *seq, + u64 seq_num) +{ + return 0; +} +#endif +#endif + int __init ip6_route_init(void) { int ret; @@ -6452,6 +6462,18 @@ int __init ip6_route_init(void) if (ret) goto out_register_late_subsys; +#if IS_BUILTIN(CONFIG_IPV6) +#ifdef CONFIG_PROC_FS + ret = bpf_dump_reg_target("ipv6_route", + "bpfdump__ipv6_route", + &ipv6_route_seq_ops, + sizeof(struct ipv6_route_iter), + BPF_DUMP_SEQ_NET_PRIVATE); + if (ret) + goto out_register_late_subsys; +#endif +#endif + for_each_possible_cpu(cpu) { struct uncached_list *ul = per_cpu_ptr(&rt6_uncached_list, cpu); diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 5ded01ca8b20..b6ab827e8d47 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -2607,7 +2607,7 @@ static void netlink_seq_stop(struct seq_file *seq, void *v) } -static int netlink_seq_show(struct seq_file *seq, void *v) +static int netlink_native_seq_show(struct seq_file *seq, void *v) { if (v == SEQ_START_TOKEN) { seq_puts(seq, @@ -2634,6 +2634,40 @@ static int netlink_seq_show(struct seq_file *seq, void *v) return 0; } +static int netlink_prog_seq_show(struct bpf_prog *prog, struct seq_file *seq, + u64 seq_num, void *v) +{ + struct { + struct netlink_sock *sk; + struct seq_file *seq; + u64 seq_num; + } ctx = { + .seq = seq, + .seq_num = seq_num - 1, + }; + int ret = 0; + + if (v == SEQ_START_TOKEN) + return 0; + + ctx.sk = nlk_sk((struct sock *)v); + ret = bpf_dump_run_prog(prog, &ctx); + + return ret == 0 ? 0 : -EINVAL; +} + +static int netlink_seq_show(struct seq_file *seq, void *v) +{ + struct bpf_prog *prog; + u64 seq_num; + + prog = bpf_dump_get_prog(seq, sizeof(struct nl_seq_iter), &seq_num); + if (!prog) + return netlink_native_seq_show(seq, v); + + return netlink_prog_seq_show(prog, seq, seq_num, v); +} + static const struct seq_operations netlink_seq_ops = { .start = netlink_seq_start, .next = netlink_seq_next, @@ -2740,6 +2774,14 @@ static const struct rhashtable_params netlink_rhashtable_params = { .automatic_shrinking = true, }; +#ifdef CONFIG_PROC_FS +int __init bpfdump__netlink(struct netlink_sock *sk, struct seq_file *seq, + u64 seq_num) +{ + return 0; +} +#endif + static int __init netlink_proto_init(void) { int i; @@ -2764,6 +2806,16 @@ static int __init netlink_proto_init(void) } } +#ifdef CONFIG_PROC_FS + err = bpf_dump_reg_target("netlink", + "bpfdump__netlink", + &netlink_seq_ops, + sizeof(struct nl_seq_iter), + BPF_DUMP_SEQ_NET_PRIVATE); + if (err) + goto out; +#endif + netlink_add_usersock_entry(); sock_register(&netlink_family_ops);
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@fb.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(-)