* [RFC PATCH bpf-next 0/2] bpf: bpf link iterator @ 2022-04-22 18:22 Dmitrii Dolgov 2022-04-22 18:22 ` [RFC PATCH bpf-next 1/2] bpf: Add bpf_link iterator Dmitrii Dolgov ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Dmitrii Dolgov @ 2022-04-22 18:22 UTC (permalink / raw) To: bpf, ast, daniel, andrii, yhs, songliubraving; +Cc: Dmitrii Dolgov Bpf links seem to be one of the important structures for which no iterator is provided. Such iterator could be useful in those cases when generic 'task/file' is not suitable or better performance is needed. The implementation is mostly copied from prog iterator, and I would like to get any high-level feedback about what needs to be different or have to be taken into account. As a side note, I would also appreciate if someone could point me out to some guide about writing selftests for bpf subsystem -- for some unclear reason I couldn't compile the test from this changeset, and was testing it only manually with a custom test program. Dmitrii Dolgov (2): bpf: Add bpf_link iterator selftests/bpf: Add bpf_link test include/linux/bpf.h | 1 + kernel/bpf/Makefile | 2 +- kernel/bpf/link_iter.c | 107 ++++++++++++++++++ kernel/bpf/syscall.c | 19 ++++ .../selftests/bpf/prog_tests/bpf_iter.c | 15 +++ .../selftests/bpf/progs/bpf_iter_bpf_link.c | 18 +++ 6 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 kernel/bpf/link_iter.c create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c -- 2.32.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC PATCH bpf-next 1/2] bpf: Add bpf_link iterator 2022-04-22 18:22 [RFC PATCH bpf-next 0/2] bpf: bpf link iterator Dmitrii Dolgov @ 2022-04-22 18:22 ` Dmitrii Dolgov 2022-04-27 19:03 ` Andrii Nakryiko 2022-04-28 16:14 ` Yonghong Song 2022-04-22 18:22 ` [RFC PATCH bpf-next 2/2] selftests/bpf: Add bpf link iter test Dmitrii Dolgov 2022-04-27 19:04 ` [RFC PATCH bpf-next 0/2] bpf: bpf link iterator Andrii Nakryiko 2 siblings, 2 replies; 9+ messages in thread From: Dmitrii Dolgov @ 2022-04-22 18:22 UTC (permalink / raw) To: bpf, ast, daniel, andrii, yhs, songliubraving; +Cc: Dmitrii Dolgov Implement bpf_link iterator to traverse links via bpf_seq_file operations. The changeset is mostly shamelessly copied from commit a228a64fc1e4 ("bpf: Add bpf_prog iterator") Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com> --- include/linux/bpf.h | 1 + kernel/bpf/Makefile | 2 +- kernel/bpf/link_iter.c | 107 +++++++++++++++++++++++++++++++++++++++++ kernel/bpf/syscall.c | 19 ++++++++ 4 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 kernel/bpf/link_iter.c diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 7bf441563ffc..330e88fcc50e 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -1489,6 +1489,7 @@ void bpf_link_put(struct bpf_link *link); int bpf_link_new_fd(struct bpf_link *link); struct file *bpf_link_new_file(struct bpf_link *link, int *reserved_fd); struct bpf_link *bpf_link_get_from_fd(u32 ufd); +struct bpf_link *bpf_link_get_curr_or_next(u32 *id); int bpf_obj_pin_user(u32 ufd, const char __user *pathname); int bpf_obj_get_user(const char __user *pathname, int flags); diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile index c1a9be6a4b9f..057ba8e01e70 100644 --- a/kernel/bpf/Makefile +++ b/kernel/bpf/Makefile @@ -6,7 +6,7 @@ cflags-nogcse-$(CONFIG_X86)$(CONFIG_CC_IS_GCC) := -fno-gcse endif CFLAGS_core.o += $(call cc-disable-warning, override-init) $(cflags-nogcse-yy) -obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o bpf_iter.o map_iter.o task_iter.o prog_iter.o +obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o bpf_iter.o map_iter.o task_iter.o prog_iter.o link_iter.o obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o bloom_filter.o obj-$(CONFIG_BPF_SYSCALL) += local_storage.o queue_stack_maps.o ringbuf.o obj-$(CONFIG_BPF_SYSCALL) += bpf_local_storage.o bpf_task_storage.o diff --git a/kernel/bpf/link_iter.c b/kernel/bpf/link_iter.c new file mode 100644 index 000000000000..fde41d09f26b --- /dev/null +++ b/kernel/bpf/link_iter.c @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright (c) 2020 Facebook */ +#include <linux/bpf.h> +#include <linux/fs.h> +#include <linux/filter.h> +#include <linux/kernel.h> +#include <linux/btf_ids.h> + +struct bpf_iter_seq_link_info { + u32 link_id; +}; + +static void *bpf_link_seq_start(struct seq_file *seq, loff_t *pos) +{ + struct bpf_iter_seq_link_info *info = seq->private; + struct bpf_link *link; + + link = bpf_link_get_curr_or_next(&info->link_id); + if (!link) + return NULL; + + if (*pos == 0) + ++*pos; + return link; +} + +static void *bpf_link_seq_next(struct seq_file *seq, void *v, loff_t *pos) +{ + struct bpf_iter_seq_link_info *info = seq->private; + + ++*pos; + ++info->link_id; + bpf_link_put((struct bpf_link *)v); + return bpf_link_get_curr_or_next(&info->link_id); +} + +struct bpf_iter__bpf_link { + __bpf_md_ptr(struct bpf_iter_meta *, meta); + __bpf_md_ptr(struct bpf_link *, link); +}; + +DEFINE_BPF_ITER_FUNC(bpf_link, struct bpf_iter_meta *meta, struct bpf_link *link) + +static int __bpf_link_seq_show(struct seq_file *seq, void *v, bool in_stop) +{ + struct bpf_iter__bpf_link ctx; + struct bpf_iter_meta meta; + struct bpf_prog *prog; + int ret = 0; + + ctx.meta = &meta; + ctx.link = v; + meta.seq = seq; + prog = bpf_iter_get_info(&meta, in_stop); + if (prog) + ret = bpf_iter_run_prog(prog, &ctx); + + return ret; +} + +static int bpf_link_seq_show(struct seq_file *seq, void *v) +{ + return __bpf_link_seq_show(seq, v, false); +} + +static void bpf_link_seq_stop(struct seq_file *seq, void *v) +{ + if (!v) + (void)__bpf_link_seq_show(seq, v, true); + else + bpf_link_put((struct bpf_link *)v); +} + +static const struct seq_operations bpf_link_seq_ops = { + .start = bpf_link_seq_start, + .next = bpf_link_seq_next, + .stop = bpf_link_seq_stop, + .show = bpf_link_seq_show, +}; + +BTF_ID_LIST(btf_bpf_link_id) +BTF_ID(struct, bpf_link) + +static const struct bpf_iter_seq_info bpf_link_seq_info = { + .seq_ops = &bpf_link_seq_ops, + .init_seq_private = NULL, + .fini_seq_private = NULL, + .seq_priv_size = sizeof(struct bpf_iter_seq_link_info), +}; + +static struct bpf_iter_reg bpf_link_reg_info = { + .target = "bpf_link", + .ctx_arg_info_size = 1, + .ctx_arg_info = { + { offsetof(struct bpf_iter__bpf_link, link), + PTR_TO_BTF_ID_OR_NULL }, + }, + .seq_info = &bpf_link_seq_info, +}; + +static int __init bpf_link_iter_init(void) +{ + bpf_link_reg_info.ctx_arg_info[0].btf_id = *btf_bpf_link_id; + return bpf_iter_reg_target(&bpf_link_reg_info); +} + +late_initcall(bpf_link_iter_init); diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index e9621cfa09f2..7d2d4cedea3f 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -4454,6 +4454,25 @@ struct bpf_link *bpf_link_by_id(u32 id) return link; } +struct bpf_link *bpf_link_get_curr_or_next(u32 *id) +{ + struct bpf_link *link; + + spin_lock_bh(&link_idr_lock); +again: + link = idr_get_next(&link_idr, id); + if (link) { + link = bpf_link_inc_not_zero(link); + if (IS_ERR(link)) { + (*id)++; + goto again; + } + } + spin_unlock_bh(&link_idr_lock); + + return link; +} + #define BPF_LINK_GET_FD_BY_ID_LAST_FIELD link_id static int bpf_link_get_fd_by_id(const union bpf_attr *attr) -- 2.32.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC PATCH bpf-next 1/2] bpf: Add bpf_link iterator 2022-04-22 18:22 ` [RFC PATCH bpf-next 1/2] bpf: Add bpf_link iterator Dmitrii Dolgov @ 2022-04-27 19:03 ` Andrii Nakryiko 2022-04-28 16:14 ` Yonghong Song 1 sibling, 0 replies; 9+ messages in thread From: Andrii Nakryiko @ 2022-04-27 19:03 UTC (permalink / raw) To: Dmitrii Dolgov Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Yonghong Song, Song Liu On Fri, Apr 22, 2022 at 11:23 AM Dmitrii Dolgov <9erthalion6@gmail.com> wrote: > > Implement bpf_link iterator to traverse links via bpf_seq_file > operations. The changeset is mostly shamelessly copied from > commit a228a64fc1e4 ("bpf: Add bpf_prog iterator") > > Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com> > --- > include/linux/bpf.h | 1 + > kernel/bpf/Makefile | 2 +- > kernel/bpf/link_iter.c | 107 +++++++++++++++++++++++++++++++++++++++++ > kernel/bpf/syscall.c | 19 ++++++++ > 4 files changed, 128 insertions(+), 1 deletion(-) > create mode 100644 kernel/bpf/link_iter.c > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 7bf441563ffc..330e88fcc50e 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1489,6 +1489,7 @@ void bpf_link_put(struct bpf_link *link); > int bpf_link_new_fd(struct bpf_link *link); > struct file *bpf_link_new_file(struct bpf_link *link, int *reserved_fd); > struct bpf_link *bpf_link_get_from_fd(u32 ufd); > +struct bpf_link *bpf_link_get_curr_or_next(u32 *id); > > int bpf_obj_pin_user(u32 ufd, const char __user *pathname); > int bpf_obj_get_user(const char __user *pathname, int flags); > diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile > index c1a9be6a4b9f..057ba8e01e70 100644 > --- a/kernel/bpf/Makefile > +++ b/kernel/bpf/Makefile > @@ -6,7 +6,7 @@ cflags-nogcse-$(CONFIG_X86)$(CONFIG_CC_IS_GCC) := -fno-gcse > endif > CFLAGS_core.o += $(call cc-disable-warning, override-init) $(cflags-nogcse-yy) > > -obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o bpf_iter.o map_iter.o task_iter.o prog_iter.o > +obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o bpf_iter.o map_iter.o task_iter.o prog_iter.o link_iter.o > obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o bloom_filter.o > obj-$(CONFIG_BPF_SYSCALL) += local_storage.o queue_stack_maps.o ringbuf.o > obj-$(CONFIG_BPF_SYSCALL) += bpf_local_storage.o bpf_task_storage.o > diff --git a/kernel/bpf/link_iter.c b/kernel/bpf/link_iter.c > new file mode 100644 > index 000000000000..fde41d09f26b > --- /dev/null > +++ b/kernel/bpf/link_iter.c > @@ -0,0 +1,107 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright (c) 2020 Facebook */ this probably needs a bit of update? > +#include <linux/bpf.h> > +#include <linux/fs.h> > +#include <linux/filter.h> > +#include <linux/kernel.h> > +#include <linux/btf_ids.h> > + > +struct bpf_iter_seq_link_info { > + u32 link_id; > +}; > + [...] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH bpf-next 1/2] bpf: Add bpf_link iterator 2022-04-22 18:22 ` [RFC PATCH bpf-next 1/2] bpf: Add bpf_link iterator Dmitrii Dolgov 2022-04-27 19:03 ` Andrii Nakryiko @ 2022-04-28 16:14 ` Yonghong Song [not found] ` <CA+q6zcXkSBrmnUt3jS+zggqJjUFJQ2J_qUmA4HXtcFmYzYppMg@mail.gmail.com> 1 sibling, 1 reply; 9+ messages in thread From: Yonghong Song @ 2022-04-28 16:14 UTC (permalink / raw) To: Dmitrii Dolgov, bpf, ast, daniel, andrii, songliubraving On 4/22/22 11:22 AM, Dmitrii Dolgov wrote: > Implement bpf_link iterator to traverse links via bpf_seq_file > operations. The changeset is mostly shamelessly copied from > commit a228a64fc1e4 ("bpf: Add bpf_prog iterator") LGTM except one copyright issue mentioned by Andrii early. Acked-by: Yonghong Song <yhs@fb.com> > > Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com> > --- > include/linux/bpf.h | 1 + > kernel/bpf/Makefile | 2 +- > kernel/bpf/link_iter.c | 107 +++++++++++++++++++++++++++++++++++++++++ > kernel/bpf/syscall.c | 19 ++++++++ > 4 files changed, 128 insertions(+), 1 deletion(-) > create mode 100644 kernel/bpf/link_iter.c > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 7bf441563ffc..330e88fcc50e 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -1489,6 +1489,7 @@ void bpf_link_put(struct bpf_link *link); > int bpf_link_new_fd(struct bpf_link *link); > struct file *bpf_link_new_file(struct bpf_link *link, int *reserved_fd); > struct bpf_link *bpf_link_get_from_fd(u32 ufd); > +struct bpf_link *bpf_link_get_curr_or_next(u32 *id); > > int bpf_obj_pin_user(u32 ufd, const char __user *pathname); > int bpf_obj_get_user(const char __user *pathname, int flags); > diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile > index c1a9be6a4b9f..057ba8e01e70 100644 > --- a/kernel/bpf/Makefile > +++ b/kernel/bpf/Makefile > @@ -6,7 +6,7 @@ cflags-nogcse-$(CONFIG_X86)$(CONFIG_CC_IS_GCC) := -fno-gcse > endif > CFLAGS_core.o += $(call cc-disable-warning, override-init) $(cflags-nogcse-yy) > > -obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o bpf_iter.o map_iter.o task_iter.o prog_iter.o > +obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o bpf_iter.o map_iter.o task_iter.o prog_iter.o link_iter.o > obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o bloom_filter.o > obj-$(CONFIG_BPF_SYSCALL) += local_storage.o queue_stack_maps.o ringbuf.o > obj-$(CONFIG_BPF_SYSCALL) += bpf_local_storage.o bpf_task_storage.o > diff --git a/kernel/bpf/link_iter.c b/kernel/bpf/link_iter.c > new file mode 100644 > index 000000000000..fde41d09f26b > --- /dev/null > +++ b/kernel/bpf/link_iter.c > @@ -0,0 +1,107 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* Copyright (c) 2020 Facebook */ Change to your own copyright. > +#include <linux/bpf.h> > +#include <linux/fs.h> > +#include <linux/filter.h> > +#include <linux/kernel.h> > +#include <linux/btf_ids.h> > + [...] ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <CA+q6zcXkSBrmnUt3jS+zggqJjUFJQ2J_qUmA4HXtcFmYzYppMg@mail.gmail.com>]
* Re: [RFC PATCH bpf-next 1/2] bpf: Add bpf_link iterator [not found] ` <CA+q6zcXkSBrmnUt3jS+zggqJjUFJQ2J_qUmA4HXtcFmYzYppMg@mail.gmail.com> @ 2022-04-28 21:02 ` Yonghong Song 0 siblings, 0 replies; 9+ messages in thread From: Yonghong Song @ 2022-04-28 21:02 UTC (permalink / raw) To: Dmitry Dolgov Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Song Liu On 4/28/22 10:07 AM, Dmitry Dolgov wrote: > On Thu, Apr 28, 2022, 6:14 PM Yonghong Song <yhs@fb.com > <mailto:yhs@fb.com>> wrote: > > >> diff --git a/kernel/bpf/link_iter.c b/kernel/bpf/link_iter.c > >> new file mode 100644 > >> index 000000000000..fde41d09f26b > >> --- /dev/null > >> +++ b/kernel/bpf/link_iter.c > >> @@ -0,0 +1,107 @@ > >> +// SPDX-License-Identifier: GPL-2.0-only > >> +/* Copyright (c) 2020 Facebook */ > > > Change to your own copyright. > > Thanks for reviewing. I have to admit I'm not sure how should it look > like, is there anything like a guideline/best practices about what to > put into the copyright line? You can do /* Copyright (c) 2022 <Your Company Name> */ or /* Copyright (c) 2022 <Your Name> */ ^ permalink raw reply [flat|nested] 9+ messages in thread
* [RFC PATCH bpf-next 2/2] selftests/bpf: Add bpf link iter test 2022-04-22 18:22 [RFC PATCH bpf-next 0/2] bpf: bpf link iterator Dmitrii Dolgov 2022-04-22 18:22 ` [RFC PATCH bpf-next 1/2] bpf: Add bpf_link iterator Dmitrii Dolgov @ 2022-04-22 18:22 ` Dmitrii Dolgov 2022-04-27 19:03 ` Andrii Nakryiko 2022-04-28 16:42 ` Yonghong Song 2022-04-27 19:04 ` [RFC PATCH bpf-next 0/2] bpf: bpf link iterator Andrii Nakryiko 2 siblings, 2 replies; 9+ messages in thread From: Dmitrii Dolgov @ 2022-04-22 18:22 UTC (permalink / raw) To: bpf, ast, daniel, andrii, yhs, songliubraving; +Cc: Dmitrii Dolgov Add a simple test for bpf link iterator Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com> --- .../selftests/bpf/prog_tests/bpf_iter.c | 15 +++++++++++++++ .../selftests/bpf/progs/bpf_iter_bpf_link.c | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+) create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c index 2c403ddc8076..e14a7a6d925c 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c @@ -26,6 +26,7 @@ #include "bpf_iter_bpf_sk_storage_map.skel.h" #include "bpf_iter_test_kern5.skel.h" #include "bpf_iter_test_kern6.skel.h" +#include "bpf_iter_bpf_link.skel.h" static int duration; @@ -1172,6 +1173,20 @@ static void test_buf_neg_offset(void) bpf_iter_test_kern6__destroy(skel); } +static void test_link_iter(void) +{ + struct bpf_iter_bpf_link *skel; + + skel = bpf_iter_bpf_link__open_and_load(); + if (CHECK(skel, "bpf_iter_bpf_link__open_and_load", + "skeleton open_and_load unexpected success\n")) + return; + + do_dummy_read(skel->progs.dump_bpf_link); + + bpf_iter_bpf_link__destroy(skel); +} + #define CMP_BUFFER_SIZE 1024 static char task_vma_output[CMP_BUFFER_SIZE]; static char proc_maps_output[CMP_BUFFER_SIZE]; diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c b/tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c new file mode 100644 index 000000000000..a5041fa1cda9 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2020 Facebook */ +#include "bpf_iter.h" +#include <bpf/bpf_helpers.h> + +char _license[] SEC("license") = "GPL"; + +SEC("iter/bpf_link") +int dump_bpf_link(struct bpf_iter__bpf_link *ctx) +{ + struct seq_file *seq = ctx->meta->seq; + struct bpf_link *link = ctx->link; + int link_id; + + link_id = link->id; + bpf_seq_write(seq, &link_id, sizeof(link_id)); + return 0; +} -- 2.32.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RFC PATCH bpf-next 2/2] selftests/bpf: Add bpf link iter test 2022-04-22 18:22 ` [RFC PATCH bpf-next 2/2] selftests/bpf: Add bpf link iter test Dmitrii Dolgov @ 2022-04-27 19:03 ` Andrii Nakryiko 2022-04-28 16:42 ` Yonghong Song 1 sibling, 0 replies; 9+ messages in thread From: Andrii Nakryiko @ 2022-04-27 19:03 UTC (permalink / raw) To: Dmitrii Dolgov Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Yonghong Song, Song Liu On Fri, Apr 22, 2022 at 11:23 AM Dmitrii Dolgov <9erthalion6@gmail.com> wrote: > > Add a simple test for bpf link iterator > > Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com> > --- > .../selftests/bpf/prog_tests/bpf_iter.c | 15 +++++++++++++++ > .../selftests/bpf/progs/bpf_iter_bpf_link.c | 18 ++++++++++++++++++ > 2 files changed, 33 insertions(+) > create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > index 2c403ddc8076..e14a7a6d925c 100644 > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > @@ -26,6 +26,7 @@ > #include "bpf_iter_bpf_sk_storage_map.skel.h" > #include "bpf_iter_test_kern5.skel.h" > #include "bpf_iter_test_kern6.skel.h" > +#include "bpf_iter_bpf_link.skel.h" > > static int duration; > > @@ -1172,6 +1173,20 @@ static void test_buf_neg_offset(void) > bpf_iter_test_kern6__destroy(skel); > } > > +static void test_link_iter(void) > +{ > + struct bpf_iter_bpf_link *skel; > + > + skel = bpf_iter_bpf_link__open_and_load(); > + if (CHECK(skel, "bpf_iter_bpf_link__open_and_load", > + "skeleton open_and_load unexpected success\n")) > + return; > + please don't use CHECK() for new tests, you need ASSERT_OK_PTR() here > + do_dummy_read(skel->progs.dump_bpf_link); > + > + bpf_iter_bpf_link__destroy(skel); > +} > + > #define CMP_BUFFER_SIZE 1024 > static char task_vma_output[CMP_BUFFER_SIZE]; > static char proc_maps_output[CMP_BUFFER_SIZE]; > diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c b/tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c > new file mode 100644 > index 000000000000..a5041fa1cda9 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2020 Facebook */ > +#include "bpf_iter.h" > +#include <bpf/bpf_helpers.h> > + > +char _license[] SEC("license") = "GPL"; > + > +SEC("iter/bpf_link") > +int dump_bpf_link(struct bpf_iter__bpf_link *ctx) see progs/bpf_iter.h, let's add bpf_iter__bpf_link there as well > +{ > + struct seq_file *seq = ctx->meta->seq; > + struct bpf_link *link = ctx->link; > + int link_id; > + > + link_id = link->id; > + bpf_seq_write(seq, &link_id, sizeof(link_id)); > + return 0; > +} > -- > 2.32.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH bpf-next 2/2] selftests/bpf: Add bpf link iter test 2022-04-22 18:22 ` [RFC PATCH bpf-next 2/2] selftests/bpf: Add bpf link iter test Dmitrii Dolgov 2022-04-27 19:03 ` Andrii Nakryiko @ 2022-04-28 16:42 ` Yonghong Song 1 sibling, 0 replies; 9+ messages in thread From: Yonghong Song @ 2022-04-28 16:42 UTC (permalink / raw) To: Dmitrii Dolgov, bpf, ast, daniel, andrii, songliubraving On 4/22/22 11:22 AM, Dmitrii Dolgov wrote: > Add a simple test for bpf link iterator > > Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com> > --- > .../selftests/bpf/prog_tests/bpf_iter.c | 15 +++++++++++++++ > .../selftests/bpf/progs/bpf_iter_bpf_link.c | 18 ++++++++++++++++++ > 2 files changed, 33 insertions(+) > create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > index 2c403ddc8076..e14a7a6d925c 100644 > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c > @@ -26,6 +26,7 @@ > #include "bpf_iter_bpf_sk_storage_map.skel.h" > #include "bpf_iter_test_kern5.skel.h" > #include "bpf_iter_test_kern6.skel.h" > +#include "bpf_iter_bpf_link.skel.h" > > static int duration; > > @@ -1172,6 +1173,20 @@ static void test_buf_neg_offset(void) > bpf_iter_test_kern6__destroy(skel); > } > > +static void test_link_iter(void) This function is used. Please add a proper subtest for this in function test_bpf_iter(). > +{ > + struct bpf_iter_bpf_link *skel; > + > + skel = bpf_iter_bpf_link__open_and_load(); > + if (CHECK(skel, "bpf_iter_bpf_link__open_and_load", > + "skeleton open_and_load unexpected success\n")) > + return; This is not correct. You should have CHECK(!skel, ...) to return only if skel is NULL. The error message "skeleton open_and_load unexpected success\n" is not correct either. Probably a copy-paste error. Also, since you are working on this file, probably convert all CHECK's in this file to ASSERT_*() macros as patch #2. Then this patch itself can be patch #3 using ASSERT_*() as well. > + > + do_dummy_read(skel->progs.dump_bpf_link); > + > + bpf_iter_bpf_link__destroy(skel); > +} > + > #define CMP_BUFFER_SIZE 1024 > static char task_vma_output[CMP_BUFFER_SIZE]; > static char proc_maps_output[CMP_BUFFER_SIZE]; > diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c b/tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c > new file mode 100644 > index 000000000000..a5041fa1cda9 > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c > @@ -0,0 +1,18 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* Copyright (c) 2020 Facebook */ copyright issue. > +#include "bpf_iter.h" > +#include <bpf/bpf_helpers.h> > + > +char _license[] SEC("license") = "GPL"; > + > +SEC("iter/bpf_link") > +int dump_bpf_link(struct bpf_iter__bpf_link *ctx) Please put bpf_iter__bpf_link definition in bpf_iter.h so the test can work with an old version of vmlinux.h. > +{ > + struct seq_file *seq = ctx->meta->seq; > + struct bpf_link *link = ctx->link; > + int link_id; The 'link' pointer could be NULL as in previous patch we have: + .ctx_arg_info = { + { offsetof(struct bpf_iter__bpf_link, link), + PTR_TO_BTF_ID_OR_NULL }, + }, So you need to add a check below. if (!link) return 0; > + > + link_id = link->id; > + bpf_seq_write(seq, &link_id, sizeof(link_id)); > + return 0; > +} ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC PATCH bpf-next 0/2] bpf: bpf link iterator 2022-04-22 18:22 [RFC PATCH bpf-next 0/2] bpf: bpf link iterator Dmitrii Dolgov 2022-04-22 18:22 ` [RFC PATCH bpf-next 1/2] bpf: Add bpf_link iterator Dmitrii Dolgov 2022-04-22 18:22 ` [RFC PATCH bpf-next 2/2] selftests/bpf: Add bpf link iter test Dmitrii Dolgov @ 2022-04-27 19:04 ` Andrii Nakryiko 2 siblings, 0 replies; 9+ messages in thread From: Andrii Nakryiko @ 2022-04-27 19:04 UTC (permalink / raw) To: Dmitrii Dolgov, Yonghong Song Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Song Liu On Fri, Apr 22, 2022 at 11:23 AM Dmitrii Dolgov <9erthalion6@gmail.com> wrote: > > Bpf links seem to be one of the important structures for which no > iterator is provided. Such iterator could be useful in those cases when > generic 'task/file' is not suitable or better performance is needed. > Overall looks good, I'll defer to Yonghong to validate kernel-side iterator logic. Yonghong, can you please take a look? Thanks! > The implementation is mostly copied from prog iterator, and I would like > to get any high-level feedback about what needs to be different or have > to be taken into account. As a side note, I would also appreciate if > someone could point me out to some guide about writing selftests for bpf > subsystem -- for some unclear reason I couldn't compile the test from > this changeset, and was testing it only manually with a custom test > program. > What was the error? Generally, you need very recent Clang (probably built from sources), latest pahole built from sources, and you should compile kernel before building selftests/bpf. > Dmitrii Dolgov (2): > bpf: Add bpf_link iterator > selftests/bpf: Add bpf_link test > > include/linux/bpf.h | 1 + > kernel/bpf/Makefile | 2 +- > kernel/bpf/link_iter.c | 107 ++++++++++++++++++ > kernel/bpf/syscall.c | 19 ++++ > .../selftests/bpf/prog_tests/bpf_iter.c | 15 +++ > .../selftests/bpf/progs/bpf_iter_bpf_link.c | 18 +++ > 6 files changed, 161 insertions(+), 1 deletion(-) > create mode 100644 kernel/bpf/link_iter.c > create mode 100644 tools/testing/selftests/bpf/progs/bpf_iter_bpf_link.c > > -- > 2.32.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-04-28 21:02 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-22 18:22 [RFC PATCH bpf-next 0/2] bpf: bpf link iterator Dmitrii Dolgov 2022-04-22 18:22 ` [RFC PATCH bpf-next 1/2] bpf: Add bpf_link iterator Dmitrii Dolgov 2022-04-27 19:03 ` Andrii Nakryiko 2022-04-28 16:14 ` Yonghong Song [not found] ` <CA+q6zcXkSBrmnUt3jS+zggqJjUFJQ2J_qUmA4HXtcFmYzYppMg@mail.gmail.com> 2022-04-28 21:02 ` Yonghong Song 2022-04-22 18:22 ` [RFC PATCH bpf-next 2/2] selftests/bpf: Add bpf link iter test Dmitrii Dolgov 2022-04-27 19:03 ` Andrii Nakryiko 2022-04-28 16:42 ` Yonghong Song 2022-04-27 19:04 ` [RFC PATCH bpf-next 0/2] bpf: bpf link iterator Andrii Nakryiko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.