* [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
* [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 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 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
* 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
* 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 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
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).