Re: [PATCH bpf-next 2/2] selftests/bpf: Test BPF_KPROBE_SYSCALL/BPF_KRETPROBE_SYSCALL macros

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Dec 20, 2021 at 9:54 PM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote:
>
> Add tests for the newly added BPF_KPROBE_SYSCALL/BPF_KRETPROBE_SYSCALL macros.
>
> Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx>
> ---
>  .../selftests/bpf/prog_tests/kprobe_syscall.c | 40 ++++++++++++++++++
>  .../selftests/bpf/progs/test_kprobe_syscall.c | 41 +++++++++++++++++++
>  2 files changed, 81 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c
> new file mode 100644
> index 000000000000..a1fad70bbb69
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_syscall.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2021 Hengqi Chen */
> +
> +#include <test_progs.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +#include "test_kprobe_syscall.skel.h"
> +
> +void test_kprobe_syscall(void)
> +{
> +       struct test_kprobe_syscall *skel;
> +       int err, fd = 0;
> +
> +       skel = test_kprobe_syscall__open();
> +       if (!ASSERT_OK_PTR(skel, "could not open BPF object"))

"could not open BPF object" is not an error message, it's an
identifier of what you are checking (skel here).  If assertion fails,
we'll see something like: "<identifier> is not a valid pointer". So
please pick it properly here and below.

> +               return;
> +
> +       skel->rodata->my_pid = getpid();
> +
> +       err = test_kprobe_syscall__load(skel);
> +       if (!ASSERT_OK(err, "could not load BPF object"))
> +               goto cleanup;
> +
> +       err = test_kprobe_syscall__attach(skel);
> +       if (!ASSERT_OK(err, "could not attach BPF object"))
> +               goto cleanup;
> +
> +       fd = socket(AF_UNIX, SOCK_STREAM, 0);

maybe use something non-zero for the 3rd argument? Also see discussion
on previous patch, let's test something that has at least 4 arguments.

> +
> +       ASSERT_GT(fd, 0, "socket failed");

see comment below, it should be GE

> +       ASSERT_EQ(skel->bss->domain, AF_UNIX, "BPF_KPROBE_SYSCALL failed");
> +       ASSERT_EQ(skel->bss->type, SOCK_STREAM, "BPF_KPROBE_SYSCALL failed");
> +       ASSERT_EQ(skel->bss->protocol, 0, "BPF_KPROBE_SYSCALL failed");
> +       ASSERT_EQ(skel->bss->fd, fd, "BPF_KRETPROBE_SYSCALL failed");
> +
> +cleanup:
> +       if (fd)

it's highly unlikely, but for FDs the check should be >= 0

> +               close(fd);
> +       test_kprobe_syscall__destroy(skel);
> +}

[...]



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux