* [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd
@ 2022-11-23 20:08 Stanislav Fomichev
2022-11-23 20:39 ` Alexei Starovoitov
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Stanislav Fomichev @ 2022-11-23 20:08 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, Jiri Olsa
Jiri reports broken test_progs after recent commit 68f8e3d4b916
("selftests/bpf: Make sure zero-len skbs aren't redirectable").
Apparently we don't remount debugfs when we switch back networking namespace.
Let's explicitly mount /sys/kernel/debug.
0: https://lore.kernel.org/bpf/63b85917-a2ea-8e35-620c-808560910819@meta.com/T/#ma66ca9c92e99eee0a25e40f422489b26ee0171c1
Fixes: a30338840fa5 ("selftests/bpf: Move open_netns() and close_netns() into network_helpers.c")
Reported-by: Jiri Olsa <olsajiri@gmail.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/network_helpers.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
index bec15558fd93..1f37adff7632 100644
--- a/tools/testing/selftests/bpf/network_helpers.c
+++ b/tools/testing/selftests/bpf/network_helpers.c
@@ -426,6 +426,10 @@ static int setns_by_fd(int nsfd)
if (!ASSERT_OK(err, "mount /sys/fs/bpf"))
return err;
+ err = mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL);
+ if (!ASSERT_OK(err, "mount /sys/kernel/debug"))
+ return err;
+
return 0;
}
--
2.38.1.584.g0f3c55d4c2-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd
2022-11-23 20:08 [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd Stanislav Fomichev
@ 2022-11-23 20:39 ` Alexei Starovoitov
2022-11-23 22:38 ` Stanislav Fomichev
2022-11-23 20:40 ` patchwork-bot+netdevbpf
2022-11-23 21:21 ` Jiri Olsa
2 siblings, 1 reply; 7+ messages in thread
From: Alexei Starovoitov @ 2022-11-23 20:39 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Hao Luo, Jiri Olsa, Jiri Olsa
On Wed, Nov 23, 2022 at 12:08 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> Jiri reports broken test_progs after recent commit 68f8e3d4b916
> ("selftests/bpf: Make sure zero-len skbs aren't redirectable").
> Apparently we don't remount debugfs when we switch back networking namespace.
> Let's explicitly mount /sys/kernel/debug.
>
> 0: https://lore.kernel.org/bpf/63b85917-a2ea-8e35-620c-808560910819@meta.com/T/#ma66ca9c92e99eee0a25e40f422489b26ee0171c1
>
> Fixes: a30338840fa5 ("selftests/bpf: Move open_netns() and close_netns() into network_helpers.c")
> Reported-by: Jiri Olsa <olsajiri@gmail.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---
> tools/testing/selftests/bpf/network_helpers.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index bec15558fd93..1f37adff7632 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -426,6 +426,10 @@ static int setns_by_fd(int nsfd)
> if (!ASSERT_OK(err, "mount /sys/fs/bpf"))
> return err;
>
> + err = mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL);
> + if (!ASSERT_OK(err, "mount /sys/kernel/debug"))
> + return err;
> +
> return 0;
> }
Thanks.
It fixes part of it but it's still racy.
I see:
do_read:FAIL:open open /sys/fs/bpf/bpf_iter_test1 failed: No such file
or directory
I suspect it happens when iter tests are running while test_empty_skb
is cleaning the netns.
So I've added:
-void test_empty_skb(void)
+void serial_test_empty_skb(void)
-void test_xdp_do_redirect(void)
+void serial_test_xdp_do_redirect(void)
-void test_xdp_synproxy(void)
+void serial_test_xdp_synproxy(void)
to stop the bleeding and applied.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd
2022-11-23 20:08 [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd Stanislav Fomichev
2022-11-23 20:39 ` Alexei Starovoitov
@ 2022-11-23 20:40 ` patchwork-bot+netdevbpf
2022-11-23 21:21 ` Jiri Olsa
2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-11-23 20:40 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa, olsajiri
Hello:
This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Wed, 23 Nov 2022 12:08:29 -0800 you wrote:
> Jiri reports broken test_progs after recent commit 68f8e3d4b916
> ("selftests/bpf: Make sure zero-len skbs aren't redirectable").
> Apparently we don't remount debugfs when we switch back networking namespace.
> Let's explicitly mount /sys/kernel/debug.
>
> 0: https://lore.kernel.org/bpf/63b85917-a2ea-8e35-620c-808560910819@meta.com/T/#ma66ca9c92e99eee0a25e40f422489b26ee0171c1
>
> [...]
Here is the summary with links:
- [bpf-next] selftests/bpf: Mount debugfs in setns_by_fd
https://git.kernel.org/bpf/bpf-next/c/8ac88eece800
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd
2022-11-23 20:08 [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd Stanislav Fomichev
2022-11-23 20:39 ` Alexei Starovoitov
2022-11-23 20:40 ` patchwork-bot+netdevbpf
@ 2022-11-23 21:21 ` Jiri Olsa
2 siblings, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2022-11-23 21:21 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, Jiri Olsa
On Wed, Nov 23, 2022 at 12:08:29PM -0800, Stanislav Fomichev wrote:
> Jiri reports broken test_progs after recent commit 68f8e3d4b916
> ("selftests/bpf: Make sure zero-len skbs aren't redirectable").
> Apparently we don't remount debugfs when we switch back networking namespace.
> Let's explicitly mount /sys/kernel/debug.
>
> 0: https://lore.kernel.org/bpf/63b85917-a2ea-8e35-620c-808560910819@meta.com/T/#ma66ca9c92e99eee0a25e40f422489b26ee0171c1
>
> Fixes: a30338840fa5 ("selftests/bpf: Move open_netns() and close_netns() into network_helpers.c")
> Reported-by: Jiri Olsa <olsajiri@gmail.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
Acked-by: Jiri Olsa <olsajiri@gmail.com>
Tested-by: Jiri Olsa <olsajiri@gmail.com>
thanks,
jirka
> ---
> tools/testing/selftests/bpf/network_helpers.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> index bec15558fd93..1f37adff7632 100644
> --- a/tools/testing/selftests/bpf/network_helpers.c
> +++ b/tools/testing/selftests/bpf/network_helpers.c
> @@ -426,6 +426,10 @@ static int setns_by_fd(int nsfd)
> if (!ASSERT_OK(err, "mount /sys/fs/bpf"))
> return err;
>
> + err = mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL);
> + if (!ASSERT_OK(err, "mount /sys/kernel/debug"))
> + return err;
> +
> return 0;
> }
>
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd
2022-11-23 20:39 ` Alexei Starovoitov
@ 2022-11-23 22:38 ` Stanislav Fomichev
2022-11-23 23:31 ` Alexei Starovoitov
0 siblings, 1 reply; 7+ messages in thread
From: Stanislav Fomichev @ 2022-11-23 22:38 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Hao Luo, Jiri Olsa, Jiri Olsa
On Wed, Nov 23, 2022 at 12:39 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Nov 23, 2022 at 12:08 PM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > Jiri reports broken test_progs after recent commit 68f8e3d4b916
> > ("selftests/bpf: Make sure zero-len skbs aren't redirectable").
> > Apparently we don't remount debugfs when we switch back networking namespace.
> > Let's explicitly mount /sys/kernel/debug.
> >
> > 0: https://lore.kernel.org/bpf/63b85917-a2ea-8e35-620c-808560910819@meta.com/T/#ma66ca9c92e99eee0a25e40f422489b26ee0171c1
> >
> > Fixes: a30338840fa5 ("selftests/bpf: Move open_netns() and close_netns() into network_helpers.c")
> > Reported-by: Jiri Olsa <olsajiri@gmail.com>
> > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > ---
> > tools/testing/selftests/bpf/network_helpers.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> > index bec15558fd93..1f37adff7632 100644
> > --- a/tools/testing/selftests/bpf/network_helpers.c
> > +++ b/tools/testing/selftests/bpf/network_helpers.c
> > @@ -426,6 +426,10 @@ static int setns_by_fd(int nsfd)
> > if (!ASSERT_OK(err, "mount /sys/fs/bpf"))
> > return err;
> >
> > + err = mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL);
> > + if (!ASSERT_OK(err, "mount /sys/kernel/debug"))
> > + return err;
> > +
> > return 0;
> > }
>
> Thanks.
> It fixes part of it but it's still racy.
> I see:
> do_read:FAIL:open open /sys/fs/bpf/bpf_iter_test1 failed: No such file
> or directory
>
> I suspect it happens when iter tests are running while test_empty_skb
> is cleaning the netns.
>
> So I've added:
> -void test_empty_skb(void)
> +void serial_test_empty_skb(void)
> -void test_xdp_do_redirect(void)
> +void serial_test_xdp_do_redirect(void)
> -void test_xdp_synproxy(void)
> +void serial_test_xdp_synproxy(void)
>
> to stop the bleeding and applied.
Not sure I understand where the race is coming from and no luck
reproducing locally :-(
Looks like we run the tests in the forked workers, so that
unshare(mountns) shouldn't theoretically affect the rest, but
obviously I'm missing something..
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd
2022-11-23 22:38 ` Stanislav Fomichev
@ 2022-11-23 23:31 ` Alexei Starovoitov
2022-11-24 0:08 ` Stanislav Fomichev
0 siblings, 1 reply; 7+ messages in thread
From: Alexei Starovoitov @ 2022-11-23 23:31 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Hao Luo, Jiri Olsa, Jiri Olsa
On Wed, Nov 23, 2022 at 2:39 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> On Wed, Nov 23, 2022 at 12:39 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Wed, Nov 23, 2022 at 12:08 PM Stanislav Fomichev <sdf@google.com> wrote:
> > >
> > > Jiri reports broken test_progs after recent commit 68f8e3d4b916
> > > ("selftests/bpf: Make sure zero-len skbs aren't redirectable").
> > > Apparently we don't remount debugfs when we switch back networking namespace.
> > > Let's explicitly mount /sys/kernel/debug.
> > >
> > > 0: https://lore.kernel.org/bpf/63b85917-a2ea-8e35-620c-808560910819@meta.com/T/#ma66ca9c92e99eee0a25e40f422489b26ee0171c1
> > >
> > > Fixes: a30338840fa5 ("selftests/bpf: Move open_netns() and close_netns() into network_helpers.c")
> > > Reported-by: Jiri Olsa <olsajiri@gmail.com>
> > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > > ---
> > > tools/testing/selftests/bpf/network_helpers.c | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> > > index bec15558fd93..1f37adff7632 100644
> > > --- a/tools/testing/selftests/bpf/network_helpers.c
> > > +++ b/tools/testing/selftests/bpf/network_helpers.c
> > > @@ -426,6 +426,10 @@ static int setns_by_fd(int nsfd)
> > > if (!ASSERT_OK(err, "mount /sys/fs/bpf"))
> > > return err;
> > >
> > > + err = mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL);
> > > + if (!ASSERT_OK(err, "mount /sys/kernel/debug"))
> > > + return err;
> > > +
> > > return 0;
> > > }
> >
> > Thanks.
> > It fixes part of it but it's still racy.
> > I see:
> > do_read:FAIL:open open /sys/fs/bpf/bpf_iter_test1 failed: No such file
> > or directory
> >
> > I suspect it happens when iter tests are running while test_empty_skb
> > is cleaning the netns.
> >
> > So I've added:
> > -void test_empty_skb(void)
> > +void serial_test_empty_skb(void)
> > -void test_xdp_do_redirect(void)
> > +void serial_test_xdp_do_redirect(void)
> > -void test_xdp_synproxy(void)
> > +void serial_test_xdp_synproxy(void)
> >
> > to stop the bleeding and applied.
>
> Not sure I understand where the race is coming from and no luck
> reproducing locally :-(
> Looks like we run the tests in the forked workers, so that
> unshare(mountns) shouldn't theoretically affect the rest, but
> obviously I'm missing something..
I'm equally confused.
If what you're describing was the case than the bug of
not mounting debugfs wouldn't have caused issues in parallel run.
That close_netns is somehow messing with other forked processes.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd
2022-11-23 23:31 ` Alexei Starovoitov
@ 2022-11-24 0:08 ` Stanislav Fomichev
0 siblings, 0 replies; 7+ messages in thread
From: Stanislav Fomichev @ 2022-11-24 0:08 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Hao Luo, Jiri Olsa, Jiri Olsa
On Wed, Nov 23, 2022 at 3:31 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Nov 23, 2022 at 2:39 PM Stanislav Fomichev <sdf@google.com> wrote:
> >
> > On Wed, Nov 23, 2022 at 12:39 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Wed, Nov 23, 2022 at 12:08 PM Stanislav Fomichev <sdf@google.com> wrote:
> > > >
> > > > Jiri reports broken test_progs after recent commit 68f8e3d4b916
> > > > ("selftests/bpf: Make sure zero-len skbs aren't redirectable").
> > > > Apparently we don't remount debugfs when we switch back networking namespace.
> > > > Let's explicitly mount /sys/kernel/debug.
> > > >
> > > > 0: https://lore.kernel.org/bpf/63b85917-a2ea-8e35-620c-808560910819@meta.com/T/#ma66ca9c92e99eee0a25e40f422489b26ee0171c1
> > > >
> > > > Fixes: a30338840fa5 ("selftests/bpf: Move open_netns() and close_netns() into network_helpers.c")
> > > > Reported-by: Jiri Olsa <olsajiri@gmail.com>
> > > > Signed-off-by: Stanislav Fomichev <sdf@google.com>
> > > > ---
> > > > tools/testing/selftests/bpf/network_helpers.c | 4 ++++
> > > > 1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/network_helpers.c b/tools/testing/selftests/bpf/network_helpers.c
> > > > index bec15558fd93..1f37adff7632 100644
> > > > --- a/tools/testing/selftests/bpf/network_helpers.c
> > > > +++ b/tools/testing/selftests/bpf/network_helpers.c
> > > > @@ -426,6 +426,10 @@ static int setns_by_fd(int nsfd)
> > > > if (!ASSERT_OK(err, "mount /sys/fs/bpf"))
> > > > return err;
> > > >
> > > > + err = mount("debugfs", "/sys/kernel/debug", "debugfs", 0, NULL);
> > > > + if (!ASSERT_OK(err, "mount /sys/kernel/debug"))
> > > > + return err;
> > > > +
> > > > return 0;
> > > > }
> > >
> > > Thanks.
> > > It fixes part of it but it's still racy.
> > > I see:
> > > do_read:FAIL:open open /sys/fs/bpf/bpf_iter_test1 failed: No such file
> > > or directory
> > >
> > > I suspect it happens when iter tests are running while test_empty_skb
> > > is cleaning the netns.
> > >
> > > So I've added:
> > > -void test_empty_skb(void)
> > > +void serial_test_empty_skb(void)
> > > -void test_xdp_do_redirect(void)
> > > +void serial_test_xdp_do_redirect(void)
> > > -void test_xdp_synproxy(void)
> > > +void serial_test_xdp_synproxy(void)
> > >
> > > to stop the bleeding and applied.
> >
> > Not sure I understand where the race is coming from and no luck
> > reproducing locally :-(
> > Looks like we run the tests in the forked workers, so that
> > unshare(mountns) shouldn't theoretically affect the rest, but
> > obviously I'm missing something..
>
> I'm equally confused.
[..]
> If what you're describing was the case than the bug of
> not mounting debugfs wouldn't have caused issues in parallel run.
The workers are reused and aren't respawned for every test.
If some of them runs empty_skb test, its debugfs is not mounted
anymore and any other test that requires debugfs (and lands on this
worker) will be broken.
> That close_netns is somehow messing with other forked processes.
Yeah, agreed, looks like /sys/fs/bpf disappears while bpf_iter test is running.
I can try to stick open/close_netns in a bunch of places in test_progs
to see if I can trigger the issue. (or maybe have a test in parallel
that does while(1) {openns(); closens(); }
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-24 0:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-23 20:08 [PATCH bpf-next] selftests/bpf: Mount debugfs in setns_by_fd Stanislav Fomichev
2022-11-23 20:39 ` Alexei Starovoitov
2022-11-23 22:38 ` Stanislav Fomichev
2022-11-23 23:31 ` Alexei Starovoitov
2022-11-24 0:08 ` Stanislav Fomichev
2022-11-23 20:40 ` patchwork-bot+netdevbpf
2022-11-23 21:21 ` Jiri Olsa
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.