On Thu, Jan 13, 2022 at 4:49 PM Kenny Yu <kennyyu@xxxxxx> wrote:
>
> This adds a test for bpf iterator programs to make use of sleepable
> bpf helpers.
>
> Signed-off-by: Kenny Yu <kennyyu@xxxxxx>
> ---
> .../selftests/bpf/prog_tests/bpf_iter.c | 16 ++++++
> .../selftests/bpf/progs/bpf_iter_task.c | 54 +++++++++++++++++++
> 2 files changed, 70 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> index b84f859b1267..fcda0ecd8746 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> @@ -138,6 +138,20 @@ static void test_task(void)
> bpf_iter_task__destroy(skel);
> }
>
> +static void test_task_sleepable(void)
> +{
> + struct bpf_iter_task *skel;
> +
> + skel = bpf_iter_task__open_and_load();
> + if (CHECK(!skel, "bpf_iter_task__open_and_load",
> + "skeleton open_and_load failed\n"))
> + return;
> +
> + do_dummy_read(skel->progs.dump_task_sleepable);
> +
> + bpf_iter_task__destroy(skel);
> +}
> +
> static void test_task_stack(void)
> {
> struct bpf_iter_task_stack *skel;
> @@ -1252,6 +1266,8 @@ void test_bpf_iter(void)
> test_bpf_map();
> if (test__start_subtest("task"))
> test_task();
> + if (test__start_subtest("task_sleepable"))
> + test_task_sleepable();
> if (test__start_subtest("task_stack"))
> test_task_stack();
> if (test__start_subtest("task_file"))
> diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_task.c b/tools/testing/selftests/bpf/progs/bpf_iter_task.c
> index c86b93f33b32..bb4b63043533 100644
> --- a/tools/testing/selftests/bpf/progs/bpf_iter_task.c
> +++ b/tools/testing/selftests/bpf/progs/bpf_iter_task.c
> @@ -2,6 +2,7 @@
> /* Copyright (c) 2020 Facebook */
> #include "bpf_iter.h"
> #include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
>
> char _license[] SEC("license") = "GPL";
>
> @@ -23,3 +24,56 @@ int dump_task(struct bpf_iter__task *ctx)
> BPF_SEQ_PRINTF(seq, "%8d %8d\n", task->tgid, task->pid);
> return 0;
> }
> +
> +// New helper added
> +static long (*bpf_access_process_vm)(
> + struct task_struct *tsk,
> + unsigned long addr,
> + void *buf,
> + int len,
> + unsigned int gup_flags) = (void *)186;
This shouldn't be needed.
Since patch 1 updates tools/include/uapi/linux/bpf.h
it will be in bpf_helper_defs.h automatically.
> +
> +// Copied from include/linux/mm.h
> +#define FOLL_REMOTE 0x2000 /* we are working on non-current tsk/mm */
Please use C style comments only.
> + numread = bpf_access_process_vm(task,
> + (unsigned long)ptr,
> + (void *)&user_data,
> + sizeof(uint32_t),
> + FOLL_REMOTE);
We probably would need to hide flags like FOLL_REMOTE
inside the helper otherwise prog might confuse the kernel.
In this case I'm not even sure that FOLL_REMOTE is needed.
I suspect gup_flags=0 in all cases will work fine.
We're not doing write here and not pining anything.
fast_gup is not necessary either.
