Netdev Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH bpf-next] selftests/bpf: fix spurious test failures in core_retro selftest
@ 2020-07-31 20:49 Andrii Nakryiko
  2020-07-31 21:55 ` John Fastabend
  0 siblings, 1 reply; 3+ messages in thread
From: Andrii Nakryiko @ 2020-07-31 20:49 UTC (permalink / raw)
  To: bpf, netdev, ast, daniel; +Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

core_retro selftest uses BPF program that's triggered on sys_enter
system-wide, but has no protection from some unrelated process doing syscall
while selftest is running. This leads to occasional test failures with
unexpected PIDs being returned. Fix that by filtering out all processes that
are not test_progs process.

Fixes: fcda189a5133 ("selftests/bpf: Add test relying only on CO-RE and no recent kernel features")
Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 tools/testing/selftests/bpf/prog_tests/core_retro.c |  8 ++++++--
 tools/testing/selftests/bpf/progs/test_core_retro.c | 13 +++++++++++++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/core_retro.c b/tools/testing/selftests/bpf/prog_tests/core_retro.c
index 78e30d3a23d5..6acb0e94d4d7 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_retro.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_retro.c
@@ -6,7 +6,7 @@
 
 void test_core_retro(void)
 {
-	int err, zero = 0, res, duration = 0;
+	int err, zero = 0, res, duration = 0, my_pid = getpid();
 	struct test_core_retro *skel;
 
 	/* load program */
@@ -14,6 +14,10 @@ void test_core_retro(void)
 	if (CHECK(!skel, "skel_load", "skeleton open/load failed\n"))
 		goto out_close;
 
+	err = bpf_map_update_elem(bpf_map__fd(skel->maps.exp_tgid_map), &zero, &my_pid, 0);
+	if (CHECK(err, "map_update", "failed to set expected PID: %d\n", errno))
+		goto out_close;
+
 	/* attach probe */
 	err = test_core_retro__attach(skel);
 	if (CHECK(err, "attach_kprobe", "err %d\n", err))
@@ -26,7 +30,7 @@ void test_core_retro(void)
 	if (CHECK(err, "map_lookup", "failed to lookup result: %d\n", errno))
 		goto out_close;
 
-	CHECK(res != getpid(), "pid_check", "got %d != exp %d\n", res, getpid());
+	CHECK(res != my_pid, "pid_check", "got %d != exp %d\n", res, my_pid);
 
 out_close:
 	test_core_retro__destroy(skel);
diff --git a/tools/testing/selftests/bpf/progs/test_core_retro.c b/tools/testing/selftests/bpf/progs/test_core_retro.c
index 75c60c3c29cf..20861ec2f674 100644
--- a/tools/testing/selftests/bpf/progs/test_core_retro.c
+++ b/tools/testing/selftests/bpf/progs/test_core_retro.c
@@ -8,6 +8,13 @@ struct task_struct {
 	int tgid;
 } __attribute__((preserve_access_index));
 
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, int);
+} exp_tgid_map SEC(".maps");
+
 struct {
 	__uint(type, BPF_MAP_TYPE_ARRAY);
 	__uint(max_entries, 1);
@@ -21,6 +28,12 @@ int handle_sys_enter(void *ctx)
 	struct task_struct *task = (void *)bpf_get_current_task();
 	int tgid = BPF_CORE_READ(task, tgid);
 	int zero = 0;
+	int real_tgid = bpf_get_current_pid_tgid() >> 32;
+	int *exp_tgid = bpf_map_lookup_elem(&exp_tgid_map, &zero);
+
+	/* only pass through sys_enters from test process */
+	if (!exp_tgid || *exp_tgid != real_tgid)
+		return 0;
 
 	bpf_map_update_elem(&results, &zero, &tgid, 0);
 
-- 
2.24.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH bpf-next] selftests/bpf: fix spurious test failures in core_retro selftest
  2020-07-31 20:49 [PATCH bpf-next] selftests/bpf: fix spurious test failures in core_retro selftest Andrii Nakryiko
@ 2020-07-31 21:55 ` John Fastabend
  2020-08-02  6:20   ` Alexei Starovoitov
  0 siblings, 1 reply; 3+ messages in thread
From: John Fastabend @ 2020-07-31 21:55 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, netdev, ast, daniel
  Cc: andrii.nakryiko, kernel-team, Andrii Nakryiko

Andrii Nakryiko wrote:
> core_retro selftest uses BPF program that's triggered on sys_enter
> system-wide, but has no protection from some unrelated process doing syscall
> while selftest is running. This leads to occasional test failures with
> unexpected PIDs being returned. Fix that by filtering out all processes that
> are not test_progs process.
> 
> Fixes: fcda189a5133 ("selftests/bpf: Add test relying only on CO-RE and no recent kernel features")
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ---

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: fix spurious test failures in core_retro selftest
  2020-07-31 21:55 ` John Fastabend
@ 2020-08-02  6:20   ` Alexei Starovoitov
  0 siblings, 0 replies; 3+ messages in thread
From: Alexei Starovoitov @ 2020-08-02  6:20 UTC (permalink / raw)
  To: John Fastabend
  Cc: Andrii Nakryiko, bpf, Network Development, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Kernel Team

On Fri, Jul 31, 2020 at 2:55 PM John Fastabend <john.fastabend@gmail.com> wrote:
>
> Andrii Nakryiko wrote:
> > core_retro selftest uses BPF program that's triggered on sys_enter
> > system-wide, but has no protection from some unrelated process doing syscall
> > while selftest is running. This leads to occasional test failures with
> > unexpected PIDs being returned. Fix that by filtering out all processes that
> > are not test_progs process.
> >
> > Fixes: fcda189a5133 ("selftests/bpf: Add test relying only on CO-RE and no recent kernel features")
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
>
> Acked-by: John Fastabend <john.fastabend@gmail.com>

Applied. Thanks

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-31 20:49 [PATCH bpf-next] selftests/bpf: fix spurious test failures in core_retro selftest Andrii Nakryiko
2020-07-31 21:55 ` John Fastabend
2020-08-02  6:20   ` Alexei Starovoitov

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git