* [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: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
* 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
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.