[PATCH bpf-next] selftests/bpf: ensure proper root namespace cleanup when test fail

[PATCH bpf-next] selftests/bpf: ensure proper root namespace cleanup when test fail

[Alexis Lothoré (eBPF Foundation)]

serial_test_flow_dissector_namespace manipulates both the root net
namespace and a dedicated non-root net namespace. If for some reason a
program attach on root namespace succeeds while it was expected to
fail, the unexpected program will remain attached to the root namespace,
possibly affecting other runs or even other tests in the same run.

Fix undesired test failure side effect by explicitly detaching programs
on failing tests expecting attach to fail. As a side effect of this
change, do not test errno value if the tested operation do not fail.

Fixes: 284ed00a59dd ("selftests/bpf: migrate flow_dissector namespace exclusivity test")
Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
---
This small fix addresses an issue discovered while trying to add a new
test in my recently merged work on flow_dissector migration. This new
test is still only present in bpf-next, hence this fix does not target
the bpf tree but the bpf-next tree.
---
 tools/testing/selftests/bpf/prog_tests/flow_dissector.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
index 8e6e483fead3f71f21e2223c707c6d4fb548a61e..08bae13248c4a8ab0bfa356a34b2738964d97f4c 100644
--- a/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
+++ b/tools/testing/selftests/bpf/prog_tests/flow_dissector.c
@@ -525,11 +525,14 @@ void serial_test_flow_dissector_namespace(void)
 	ns = open_netns(TEST_NS);
 	if (!ASSERT_OK_PTR(ns, "enter non-root net namespace"))
 		goto out_clean_ns;
-
 	err = bpf_prog_attach(prog_fd, 0, BPF_FLOW_DISSECTOR, 0);
+	if (!ASSERT_ERR(err,
+			"refuse new flow dissector in non-root net namespace"))
+		bpf_prog_detach2(prog_fd, 0, BPF_FLOW_DISSECTOR);
+	else
+		ASSERT_EQ(errno, EEXIST,
+			  "refused because of already attached prog");
 	close_netns(ns);
-	ASSERT_ERR(err, "refuse new flow dissector in non-root net namespace");
-	ASSERT_EQ(errno, EEXIST, "refused because of already attached prog");
 
 	/* If no flow dissector is attached to the root namespace, we must
 	 * be able to attach one to a non-root net namespace
@@ -545,8 +548,11 @@ void serial_test_flow_dissector_namespace(void)
 	 * a flow dissector to root namespace must fail
 	 */
 	err = bpf_prog_attach(prog_fd, 0, BPF_FLOW_DISSECTOR, 0);
-	ASSERT_ERR(err, "refuse new flow dissector on root namespace");
-	ASSERT_EQ(errno, EEXIST, "refused because of already attached prog");
+	if (!ASSERT_ERR(err, "refuse new flow dissector on root namespace"))
+		bpf_prog_detach2(prog_fd, 0, BPF_FLOW_DISSECTOR);
+	else
+		ASSERT_EQ(errno, EEXIST,
+			  "refused because of already attached prog");
 
 	ns = open_netns(TEST_NS);
 	bpf_prog_detach2(prog_fd, 0, BPF_FLOW_DISSECTOR);

---
base-commit: 04e7b00083a120d60511443d900a5cc10dbed263
change-id: 20241128-small_flow_test_fix-0c53624a3c4c

Best regards,
-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH bpf-next] selftests/bpf: ensure proper root namespace cleanup when test fail

[Stanislav Fomichev]

On 11/28, Alexis Lothoré (eBPF Foundation) wrote:
> serial_test_flow_dissector_namespace manipulates both the root net
> namespace and a dedicated non-root net namespace. If for some reason a
> program attach on root namespace succeeds while it was expected to
> fail, the unexpected program will remain attached to the root namespace,
> possibly affecting other runs or even other tests in the same run.
> 
> Fix undesired test failure side effect by explicitly detaching programs
> on failing tests expecting attach to fail. As a side effect of this
> change, do not test errno value if the tested operation do not fail.
> 
> Fixes: 284ed00a59dd ("selftests/bpf: migrate flow_dissector namespace exclusivity test")
> Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>

Acked-by: Stanislav Fomichev <sdf@fomichev.me>

Re: [PATCH bpf-next] selftests/bpf: ensure proper root namespace cleanup when test fail

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Thu, 28 Nov 2024 15:38:43 +0100 you wrote:
> serial_test_flow_dissector_namespace manipulates both the root net
> namespace and a dedicated non-root net namespace. If for some reason a
> program attach on root namespace succeeds while it was expected to
> fail, the unexpected program will remain attached to the root namespace,
> possibly affecting other runs or even other tests in the same run.
> 
> Fix undesired test failure side effect by explicitly detaching programs
> on failing tests expecting attach to fail. As a side effect of this
> change, do not test errno value if the tested operation do not fail.
> 
> [...]

Here is the summary with links:
  - [bpf-next] selftests/bpf: ensure proper root namespace cleanup when test fail
    https://git.kernel.org/bpf/bpf-next/c/c721d8f8b196

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html