Re: [PATCH net-next v2 4/4] selftests: net: adopt harness for flow label mgr

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
selftests: net: adopt harness for flow label mgr

This converts tools/testing/selftests/net/ipv6_flowlabel_mgr.c from the
local expect_pass/expect_fail/explain helpers to kselftest_harness.h,
splitting run_tests() into individual TEST() cases. It also removes
parse_opts(), so the two previously -l gated checks now always run.

[ ... ]

[ ... ]

 	pid = fork();
-	if (pid == -1)
-		error(1, errno, "fork");
+	ASSERT_NE(-1, pid) TH_LOG("fork failed");
 	if (!pid) {
-		expect_pass(flowlabel_get(fd, 3, IPV6_FL_S_USER, 0));
-		if (setuid(USHRT_MAX))
+		err = flowlabel_get(fd, 3, IPV6_FL_S_USER, 0);
+		ASSERT_TRUE(!err) TH_LOG("child failed to get the user-private label");
+
+		if (setuid(USHRT_MAX)) {
 			fprintf(stderr, "[INFO] skip setuid child test\n");
-		else
-			expect_fail(flowlabel_get(fd, 3, IPV6_FL_S_USER, 0));
+			exit(0);
+		}

[Severity: Medium]
Should this path report a skip rather than a pass?  In
TEST(user_private_label_share), when setuid(USHRT_MAX) fails the child
prints to stderr and calls exit(0).

The parent then runs:

	ASSERT_EQ(pid, wait(&wstatus)) TH_LOG("wait failed");
	ASSERT_TRUE(WIFEXITED(wstatus)) TH_LOG("child did not exit normally");
	ASSERT_EQ(0, WEXITSTATUS(wstatus)) TH_LOG("child reported unexpected result");

All three succeed, so the harness emits "ok user_private_label_share" even
though the check the test is named for, that an IPV6_FL_S_USER label cannot
be obtained after switching uid, never executed.

setuid(USHRT_MAX) fails when the binary runs without CAP_SETUID, or in a
user namespace where uid 65535 is unmapped, which is reachable since
ipv6_flowlabel.sh drives this through ./in_netns.sh.

Would propagating KSFT_SKIP work better here, for example the child doing
exit(KSFT_SKIP) and the parent turning that into SKIP(return, ...)?

The silent-skip logic itself predates this patch, but before the conversion
there were no per-test TAP results; now the test individually reports "ok"
instead of "# SKIP".

The raw fprintf(stderr, "[INFO] ...") is also the only diagnostic left in
the converted file that does not go through TH_LOG, which prefixes output
with "# ".

[ ... ]

+TEST(reflect_flag)
+{

[ ... ]

+	if (flowlabel_consistency_enabled())
+		SKIP(return,
+		     "net.ipv6.flowlabel_consistency must be 0 (run via ipv6_flowlabel.sh)");

This is the idiom the setuid path above could use: SKIP() sets
_metadata->exit_code = KSFT_SKIP and prints a "# SKIP" diagnostic, so an
unmet precondition is not reported as a pass.  Could both unmet-precondition
sites in this file use it consistently?

[ ... ]