From: Yonghong Song <yonghong.song@linux.dev>
To: Juntong Deng <juntong.deng@outlook.com>,
ast@kernel.org, daniel@iogearbox.net, john.fastabend@gmail.com,
andrii@kernel.org, martin.lau@linux.dev, eddyz87@gmail.com,
song@kernel.org, kpsingh@kernel.org, sdf@fomichev.me,
haoluo@google.com, jolsa@kernel.org, memxor@gmail.com,
snorcht@gmail.com, brauner@kernel.org
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH bpf-next v6 2/5] selftests/bpf: Add tests for open-coded style process file iterator
Date: Thu, 19 Dec 2024 08:35:09 -0800 [thread overview]
Message-ID: <2d389258-ad08-4d28-a347-667909b0e190@linux.dev> (raw)
In-Reply-To: <AM6PR03MB508014EBAC89D14D0C3ADA6899042@AM6PR03MB5080.eurprd03.prod.outlook.com>
On 12/17/24 3:37 PM, Juntong Deng wrote:
> This patch adds test cases for open-coded style process file iterator.
>
> Test cases related to process files are run in the newly created child
> process. Close all opened files inherited from the parent process in
> the child process to avoid the files opened by the parent process
> affecting the test results.
>
> In addition, this patch adds failure test cases where bpf programs
> cannot pass the verifier due to uninitialized or untrusted
> arguments, etc.
>
> Signed-off-by: Juntong Deng <juntong.deng@outlook.com>
> ---
> .../testing/selftests/bpf/bpf_experimental.h | 7 ++
> .../testing/selftests/bpf/prog_tests/iters.c | 79 ++++++++++++++++
> .../selftests/bpf/progs/iters_task_file.c | 86 ++++++++++++++++++
> .../bpf/progs/iters_task_file_failure.c | 91 +++++++++++++++++++
> 4 files changed, 263 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/iters_task_file.c
> create mode 100644 tools/testing/selftests/bpf/progs/iters_task_file_failure.c
>
> diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h
> index cd8ecd39c3f3..ce1520c56b55 100644
> --- a/tools/testing/selftests/bpf/bpf_experimental.h
> +++ b/tools/testing/selftests/bpf/bpf_experimental.h
> @@ -588,4 +588,11 @@ extern int bpf_iter_kmem_cache_new(struct bpf_iter_kmem_cache *it) __weak __ksym
> extern struct kmem_cache *bpf_iter_kmem_cache_next(struct bpf_iter_kmem_cache *it) __weak __ksym;
> extern void bpf_iter_kmem_cache_destroy(struct bpf_iter_kmem_cache *it) __weak __ksym;
>
> +struct bpf_iter_task_file;
> +struct bpf_iter_task_file_item;
> +extern int bpf_iter_task_file_new(struct bpf_iter_task_file *it, struct task_struct *task) __ksym;
> +extern struct bpf_iter_task_file_item *
> +bpf_iter_task_file_next(struct bpf_iter_task_file *it) __ksym;
> +extern void bpf_iter_task_file_destroy(struct bpf_iter_task_file *it) __ksym;
All the above declarations should be in vmlinux.h already and I see your below bpf prog already
included vmlinux.h, there is no need to put them here.
> +
> #endif
> diff --git a/tools/testing/selftests/bpf/prog_tests/iters.c b/tools/testing/selftests/bpf/prog_tests/iters.c
> index 3cea71f9c500..cfe5b56cc027 100644
> --- a/tools/testing/selftests/bpf/prog_tests/iters.c
> +++ b/tools/testing/selftests/bpf/prog_tests/iters.c
> @@ -1,6 +1,8 @@
> // SPDX-License-Identifier: GPL-2.0
> /* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
>
> +#define _GNU_SOURCE
> +#include <sys/socket.h>
> #include <sys/syscall.h>
> #include <sys/mman.h>
> #include <sys/wait.h>
> @@ -16,11 +18,13 @@
> #include "iters_num.skel.h"
> #include "iters_testmod.skel.h"
> #include "iters_testmod_seq.skel.h"
> +#include "iters_task_file.skel.h"
> #include "iters_task_vma.skel.h"
> #include "iters_task.skel.h"
> #include "iters_css_task.skel.h"
> #include "iters_css.skel.h"
> #include "iters_task_failure.skel.h"
> +#include "iters_task_file_failure.skel.h"
>
> static void subtest_num_iters(void)
> {
> @@ -291,6 +295,78 @@ static void subtest_css_iters(void)
> iters_css__destroy(skel);
> }
>
> +static int task_file_test_process(void *args)
> +{
> + int pipefd[2], sockfd, err = 0;
> +
> + /* Create a clean file descriptor table for the test process */
> + close_range(0, ~0U, 0);
> +
> + if (pipe(pipefd) < 0)
> + return 1;
> +
> + sockfd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
> + if (sockfd < 0) {
> + err = 2;
> + goto cleanup_pipe;
> + }
> +
> + usleep(1);
> +
> + close(sockfd);
> +cleanup_pipe:
> + close(pipefd[0]);
> + close(pipefd[1]);
> + return err;
> +}
> +
> +static void subtest_task_file_iters(void)
> +{
> + const int stack_size = 1024 * 1024;
> + struct iters_task_file *skel;
> + int child_pid, wstatus, err;
> + char *stack;
> +
> + skel = iters_task_file__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "open_and_load"))
> + return;
> +
> + if (!ASSERT_OK(skel->bss->err, "pre_test_err"))
> + goto cleanup_skel;
> +
> + skel->bss->parent_pid = getpid();
> + skel->bss->count = 0;
> +
> + err = iters_task_file__attach(skel);
> + if (!ASSERT_OK(err, "skel_attach"))
> + goto cleanup_skel;
> +
> + stack = (char *)malloc(stack_size);
> + if (!ASSERT_OK_PTR(stack, "clone_stack"))
> + goto cleanup_attach;
> +
> + /* Note that there is no CLONE_FILES */
> + child_pid = clone(task_file_test_process, stack + stack_size, CLONE_VM | SIGCHLD, NULL);
> + if (!ASSERT_GT(child_pid, -1, "child_pid"))
> + goto cleanup_stack;
> +
> + if (!ASSERT_GT(waitpid(child_pid, &wstatus, 0), -1, "waitpid"))
> + goto cleanup_stack;
> +
> + if (!ASSERT_OK(WEXITSTATUS(wstatus), "run_task_file_iters_test_err"))
> + goto cleanup_stack;
> +
> + ASSERT_EQ(skel->bss->count, 1, "run_task_file_iters_test_count_err");
> + ASSERT_OK(skel->bss->err, "run_task_file_iters_test_failure");
> +
> +cleanup_stack:
> + free(stack);
> +cleanup_attach:
> + iters_task_file__detach(skel);
> +cleanup_skel:
> + iters_task_file__destroy(skel);
> +}
> +
> void test_iters(void)
> {
> RUN_TESTS(iters_state_safety);
> @@ -315,5 +391,8 @@ void test_iters(void)
> subtest_css_task_iters();
> if (test__start_subtest("css"))
> subtest_css_iters();
> + if (test__start_subtest("task_file"))
> + subtest_task_file_iters();
> RUN_TESTS(iters_task_failure);
> + RUN_TESTS(iters_task_file_failure);
> }
> diff --git a/tools/testing/selftests/bpf/progs/iters_task_file.c b/tools/testing/selftests/bpf/progs/iters_task_file.c
> new file mode 100644
> index 000000000000..47941530e51b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/iters_task_file.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +#include "bpf_experimental.h"
> +#include "task_kfunc_common.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +int err, parent_pid, count;
> +
> +extern const void pipefifo_fops __ksym;
> +extern const void socket_file_ops __ksym;
There is no need to have 'const' in the above two extern declarations.
> +
> +SEC("fentry/" SYS_PREFIX "sys_nanosleep")
> +int test_bpf_iter_task_file(void *ctx)
> +{
> + struct bpf_iter_task_file task_file_it;
> + struct bpf_iter_task_file_item *item;
> + struct task_struct *task;
> +
> + task = bpf_get_current_task_btf();
> + if (task->parent->pid != parent_pid)
> + return 0;
> +
> + count++;
> +
> + bpf_iter_task_file_new(&task_file_it, task);
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item == NULL) {
> + err = 1;
> + goto cleanup;
> + }
> +
> + if (item->fd != 0) {
> + err = 2;
> + goto cleanup;
> + }
> +
> + if (item->file->f_op != &pipefifo_fops) {
> + err = 3;
> + goto cleanup;
> + }
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item == NULL) {
> + err = 4;
> + goto cleanup;
> + }
> +
> + if (item->fd != 1) {
> + err = 5;
> + goto cleanup;
> + }
> +
> + if (item->file->f_op != &pipefifo_fops) {
> + err = 6;
> + goto cleanup;
> + }
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item == NULL) {
> + err = 7;
> + goto cleanup;
> + }
> +
> + if (item->fd != 2) {
> + err = 8;
> + goto cleanup;
> + }
> +
> + if (item->file->f_op != &socket_file_ops) {
> + err = 9;
> + goto cleanup;
> + }
> +
> + item = bpf_iter_task_file_next(&task_file_it);
> + if (item != NULL)
> + err = 10;
> +cleanup:
> + bpf_iter_task_file_destroy(&task_file_it);
> + return 0;
> +}
[...]
next prev parent reply other threads:[~2024-12-19 16:35 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-17 23:34 [PATCH bpf-next v6 0/5] bpf: Add open-coded style process file iterator and bpf_fget_task() kfunc Juntong Deng
2024-12-17 23:37 ` [PATCH bpf-next v6 1/5] bpf: Introduce task_file open-coded iterator kfuncs Juntong Deng
2024-12-19 16:19 ` Yonghong Song
2024-12-17 23:37 ` [PATCH bpf-next v6 2/5] selftests/bpf: Add tests for open-coded style process file iterator Juntong Deng
2024-12-19 16:35 ` Yonghong Song [this message]
2024-12-24 0:43 ` Juntong Deng
2024-12-17 23:37 ` [PATCH bpf-next v6 3/5] bpf: Add bpf_fget_task() kfunc Juntong Deng
2024-12-17 23:37 ` [PATCH bpf-next v6 4/5] bpf: Make fs kfuncs available for SYSCALL program type Juntong Deng
2024-12-19 16:41 ` Alexei Starovoitov
2024-12-24 0:51 ` Juntong Deng
2024-12-24 12:10 ` Juntong Deng
2025-01-09 19:23 ` per st_ops kfunc allow/deny mask. Was: " Alexei Starovoitov
2025-01-09 20:49 ` Song Liu
2025-01-09 21:24 ` Alexei Starovoitov
2025-01-10 20:19 ` Tejun Heo
2025-01-10 20:50 ` Juntong Deng
2025-01-10 20:42 ` Juntong Deng
2025-01-16 19:52 ` Juntong Deng
2024-12-17 23:37 ` [PATCH bpf-next v6 5/5] selftests/bpf: Add tests for bpf_fget_task() kfunc Juntong Deng
2024-12-19 16:11 ` [PATCH bpf-next v6 0/5] bpf: Add open-coded style process file iterator and " Yonghong Song
2024-12-24 0:42 ` Juntong Deng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2d389258-ad08-4d28-a347-667909b0e190@linux.dev \
--to=yonghong.song@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brauner@kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=juntong.deng@outlook.com \
--cc=kpsingh@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=sdf@fomichev.me \
--cc=snorcht@gmail.com \
--cc=song@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.