diff mbox series

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

Message ID 20211221055312.3371414-3-hengqi.chen@gmail.com (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series libbpf: Add syscall-specific variants of BPF_KPROBE/BPF_KRETPROBE | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 10 maintainers not CCed: songliubraving@fb.com netdev@vger.kernel.org shuah@kernel.org daniel@iogearbox.net yhs@fb.com john.fastabend@gmail.com linux-kselftest@vger.kernel.org kafai@fb.com ast@kernel.org kpsingh@kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next success VM_Test
bpf/vmtest-bpf-next-PR success PR summary

Commit Message

Hengqi Chen Dec. 21, 2021, 5:53 a.m. UTC 
Add tests for the newly added BPF_KPROBE_SYSCALL/BPF_KRETPROBE_SYSCALL macros.

Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
---
 .../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

--
2.30.2

Comments

Andrii Nakryiko Dec. 22, 2021, 12:34 a.m. UTC | #1 
On Mon, Dec 20, 2021 at 9:54 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> Add tests for the newly added BPF_KPROBE_SYSCALL/BPF_KRETPROBE_SYSCALL macros.
>
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> ---
>  .../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);
> +}

[...]
Andrii Nakryiko Dec. 22, 2021, 12:37 a.m. UTC | #2 
On Mon, Dec 20, 2021 at 9:54 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>
> Add tests for the newly added BPF_KPROBE_SYSCALL/BPF_KRETPROBE_SYSCALL macros.
>
> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
> ---
>  .../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"))
> +               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);
> +
> +       ASSERT_GT(fd, 0, "socket failed");
> +       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)
> +               close(fd);
> +       test_kprobe_syscall__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c b/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
> new file mode 100644
> index 000000000000..ecef9d19007c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
> @@ -0,0 +1,41 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2021 Hengqi Chen */
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include <bpf/bpf_core_read.h>
> +
> +const volatile pid_t my_pid = 0;
> +int domain = 0;
> +int type = 0;
> +int protocol = 0;
> +int fd = 0;
> +
> +SEC("kprobe/__x64_sys_socket")
> +int BPF_KPROBE_SYSCALL(socket_enter, int d, int t, int p)
> +{
> +       pid_t pid = bpf_get_current_pid_tgid() >> 32;
> +
> +       if (pid != my_pid)
> +               return 0;
> +
> +       domain = d;
> +       type = t;
> +       protocol = p;
> +       return 0;
> +}
> +
> +SEC("kretprobe/__x64_sys_socket")

oh, please also use SYS_PREFIX instead of hard-coding __x64. This is
very x86-64-specific and we have other architectures tested by
selftests, so this makes it automatically fail on non-x86_64.

If you get a chance, try also cleaning up other __x64_ uses in the
selftests as a separate patch. Thank you!

> +int BPF_KRETPROBE_SYSCALL(socket_exit, int ret)
> +{
> +       pid_t pid = bpf_get_current_pid_tgid() >> 32;
> +
> +       if (pid != my_pid)
> +               return 0;
> +
> +       fd = ret;
> +       return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.30.2
Hengqi Chen Dec. 23, 2021, 12:16 p.m. UTC | #3 
On 2021/12/22 8:37 AM, Andrii Nakryiko wrote:
> On Mon, Dec 20, 2021 at 9:54 PM Hengqi Chen <hengqi.chen@gmail.com> wrote:
>>
>> Add tests for the newly added BPF_KPROBE_SYSCALL/BPF_KRETPROBE_SYSCALL macros.
>>
>> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com>
>> ---
>>  .../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"))
>> +               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);
>> +
>> +       ASSERT_GT(fd, 0, "socket failed");
>> +       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)
>> +               close(fd);
>> +       test_kprobe_syscall__destroy(skel);
>> +}
>> diff --git a/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c b/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
>> new file mode 100644
>> index 000000000000..ecef9d19007c
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
>> @@ -0,0 +1,41 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2021 Hengqi Chen */
>> +
>> +#include "vmlinux.h"
>> +#include <bpf/bpf_helpers.h>
>> +#include <bpf/bpf_tracing.h>
>> +#include <bpf/bpf_core_read.h>
>> +
>> +const volatile pid_t my_pid = 0;
>> +int domain = 0;
>> +int type = 0;
>> +int protocol = 0;
>> +int fd = 0;
>> +
>> +SEC("kprobe/__x64_sys_socket")
>> +int BPF_KPROBE_SYSCALL(socket_enter, int d, int t, int p)
>> +{
>> +       pid_t pid = bpf_get_current_pid_tgid() >> 32;
>> +
>> +       if (pid != my_pid)
>> +               return 0;
>> +
>> +       domain = d;
>> +       type = t;
>> +       protocol = p;
>> +       return 0;
>> +}
>> +
>> +SEC("kretprobe/__x64_sys_socket")
> 
> oh, please also use SYS_PREFIX instead of hard-coding __x64. This is
> very x86-64-specific and we have other architectures tested by
> selftests, so this makes it automatically fail on non-x86_64.
> 

I just followed some existing selftests, didn't realize SYS_PREFIX.
Will update to use SYS_PREFIX.

> If you get a chance, try also cleaning up other __x64_ uses in the
> selftests as a separate patch. Thank you!
> 

OK, will do.

>> +int BPF_KRETPROBE_SYSCALL(socket_exit, int ret)
>> +{
>> +       pid_t pid = bpf_get_current_pid_tgid() >> 32;
>> +
>> +       if (pid != my_pid)
>> +               return 0;
>> +
>> +       fd = ret;
>> +       return 0;
>> +}
>> +
>> +char _license[] SEC("license") = "GPL";
>> --
>> 2.30.2
diff mbox series

Patch

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"))
+		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);
+
+	ASSERT_GT(fd, 0, "socket failed");
+	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)
+		close(fd);
+	test_kprobe_syscall__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c b/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
new file mode 100644
index 000000000000..ecef9d19007c
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_kprobe_syscall.c
@@ -0,0 +1,41 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2021 Hengqi Chen */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
+
+const volatile pid_t my_pid = 0;
+int domain = 0;
+int type = 0;
+int protocol = 0;
+int fd = 0;
+
+SEC("kprobe/__x64_sys_socket")
+int BPF_KPROBE_SYSCALL(socket_enter, int d, int t, int p)
+{
+	pid_t pid = bpf_get_current_pid_tgid() >> 32;
+
+	if (pid != my_pid)
+		return 0;
+
+	domain = d;
+	type = t;
+	protocol = p;
+	return 0;
+}
+
+SEC("kretprobe/__x64_sys_socket")
+int BPF_KRETPROBE_SYSCALL(socket_exit, int ret)
+{
+	pid_t pid = bpf_get_current_pid_tgid() >> 32;
+
+	if (pid != my_pid)
+		return 0;
+
+	fd = ret;
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";