| Message ID | 20200408232528.2675856-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:26 PM Yonghong Song <yhs@fb.com> wrote: > > This patch added bpf_map target. Traversing all bpf_maps > through map_idr. A reference is held for the map during > the show() to ensure safety and correctness for field accesses. > > Signed-off-by: Yonghong Song <yhs@fb.com> > --- > kernel/bpf/syscall.c | 104 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 104 insertions(+) > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index b5e4f18cc633..62a872a406ca 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -3797,3 +3797,107 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz > > return err; > } > + > +struct bpfdump_seq_map_info { > + struct bpf_map *map; > + u32 id; > +}; > + > +static struct bpf_map *bpf_map_seq_get_next(u32 *id) > +{ > + struct bpf_map *map; > + > + spin_lock_bh(&map_idr_lock); > + map = idr_get_next(&map_idr, id); > + if (map) > + map = __bpf_map_inc_not_zero(map, false); > + spin_unlock_bh(&map_idr_lock); > + > + return map; > +} > + > +static void *bpf_map_seq_start(struct seq_file *seq, loff_t *pos) > +{ > + struct bpfdump_seq_map_info *info = seq->private; > + struct bpf_map *map; > + u32 id = info->id + 1; shouldn't it always start from id=0? This seems buggy and should break on seq_file restart. > + > + map = bpf_map_seq_get_next(&id); > + if (!map) bpf_map_seq_get_next will return error code, not NULL, if bpf_map refcount couldn't be incremented. So this must be IS_ERR(map). > + return NULL; > + > + ++*pos; > + info->map = map; > + info->id = id; > + return map; > +} > + > +static void *bpf_map_seq_next(struct seq_file *seq, void *v, loff_t *pos) > +{ > + struct bpfdump_seq_map_info *info = seq->private; > + struct bpf_map *map; > + u32 id = info->id + 1; > + > + ++*pos; > + map = bpf_map_seq_get_next(&id); > + if (!map) same here, IS_ERR(map) > + return NULL; > + > + __bpf_map_put(info->map, true); > + info->map = map; > + info->id = id; > + return map; > +} > + [...]
On Mon, Apr 13, 2020 at 3:18 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Wed, Apr 8, 2020 at 4:26 PM Yonghong Song <yhs@fb.com> wrote: > > > > This patch added bpf_map target. Traversing all bpf_maps > > through map_idr. A reference is held for the map during > > the show() to ensure safety and correctness for field accesses. > > > > Signed-off-by: Yonghong Song <yhs@fb.com> > > --- > > kernel/bpf/syscall.c | 104 +++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 104 insertions(+) > > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > index b5e4f18cc633..62a872a406ca 100644 > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -3797,3 +3797,107 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz > > > > return err; > > } > > + > > +struct bpfdump_seq_map_info { > > + struct bpf_map *map; > > + u32 id; > > +}; > > + > > +static struct bpf_map *bpf_map_seq_get_next(u32 *id) > > +{ > > + struct bpf_map *map; > > + > > + spin_lock_bh(&map_idr_lock); > > + map = idr_get_next(&map_idr, id); > > + if (map) > > + map = __bpf_map_inc_not_zero(map, false); > > + spin_unlock_bh(&map_idr_lock); > > + > > + return map; > > +} > > + > > +static void *bpf_map_seq_start(struct seq_file *seq, loff_t *pos) > > +{ > > + struct bpfdump_seq_map_info *info = seq->private; > > + struct bpf_map *map; > > + u32 id = info->id + 1; > > shouldn't it always start from id=0? This seems buggy and should break > on seq_file restart. Actually never mind this, from reading fs/seq_file.c code I've been under impression that start is only called for full restarts, but that's not true. > > > + > > + map = bpf_map_seq_get_next(&id); > > + if (!map) > > bpf_map_seq_get_next will return error code, not NULL, if bpf_map > refcount couldn't be incremented. So this must be IS_ERR(map). > > > + return NULL; > > + > > + ++*pos; > > + info->map = map; > > + info->id = id; > > + return map; > > +} > > + > > +static void *bpf_map_seq_next(struct seq_file *seq, void *v, loff_t *pos) > > +{ > > + struct bpfdump_seq_map_info *info = seq->private; > > + struct bpf_map *map; > > + u32 id = info->id + 1; > > + > > + ++*pos; > > + map = bpf_map_seq_get_next(&id); > > + if (!map) > > same here, IS_ERR(map) > > > + return NULL; > > + > > + __bpf_map_put(info->map, true); > > + info->map = map; > > + info->id = id; > > + return map; > > +} > > + > > [...]
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index b5e4f18cc633..62a872a406ca 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3797,3 +3797,107 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz return err; } + +struct bpfdump_seq_map_info { + struct bpf_map *map; + u32 id; +}; + +static struct bpf_map *bpf_map_seq_get_next(u32 *id) +{ + struct bpf_map *map; + + spin_lock_bh(&map_idr_lock); + map = idr_get_next(&map_idr, id); + if (map) + map = __bpf_map_inc_not_zero(map, false); + spin_unlock_bh(&map_idr_lock); + + return map; +} + +static void *bpf_map_seq_start(struct seq_file *seq, loff_t *pos) +{ + struct bpfdump_seq_map_info *info = seq->private; + struct bpf_map *map; + u32 id = info->id + 1; + + map = bpf_map_seq_get_next(&id); + if (!map) + return NULL; + + ++*pos; + info->map = map; + info->id = id; + return map; +} + +static void *bpf_map_seq_next(struct seq_file *seq, void *v, loff_t *pos) +{ + struct bpfdump_seq_map_info *info = seq->private; + struct bpf_map *map; + u32 id = info->id + 1; + + ++*pos; + map = bpf_map_seq_get_next(&id); + if (!map) + return NULL; + + __bpf_map_put(info->map, true); + info->map = map; + info->id = id; + return map; +} + +static void bpf_map_seq_stop(struct seq_file *seq, void *v) +{ + struct bpfdump_seq_map_info *info = seq->private; + + if (info->map) { + __bpf_map_put(info->map, true); + info->map = NULL; + } +} + +static int bpf_map_seq_show(struct seq_file *seq, void *v) +{ + struct { + struct bpf_map *map; + struct seq_file *seq; + u64 seq_num; + } ctx = { + .map = v, + .seq = seq, + }; + struct bpf_prog *prog; + int ret; + + prog = bpf_dump_get_prog(seq, sizeof(struct bpfdump_seq_map_info), + &ctx.seq_num); + ret = bpf_dump_run_prog(prog, &ctx); + + return ret == 0 ? 0 : -EINVAL; +} + +static const struct seq_operations bpf_map_seq_ops = { + .start = bpf_map_seq_start, + .next = bpf_map_seq_next, + .stop = bpf_map_seq_stop, + .show = bpf_map_seq_show, +}; + +int __init bpfdump__bpf_map(struct bpf_map *map, struct seq_file *seq, + u64 seq_num) +{ + return 0; +} + +static int __init bpf_map_dump_init(void) +{ + return bpf_dump_reg_target("bpf_map", + "bpfdump__bpf_map", + &bpf_map_seq_ops, + sizeof(struct bpfdump_seq_map_info), 0); +} + +late_initcall(bpf_map_dump_init);
This patch added bpf_map target. Traversing all bpf_maps through map_idr. A reference is held for the map during the show() to ensure safety and correctness for field accesses. Signed-off-by: Yonghong Song <yhs@fb.com> --- kernel/bpf/syscall.c | 104 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+)