Re: [PATCH net v2 2/2] selftests: drv-net: psp: test PSP and TCP ULP mutual exclusion

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] In tools/testing/selftests/drivers/net/psp.py, the new helper
  `_require_tls_ulp()` (lines 338-344) wraps…

--

selftests: drv-net: psp: test PSP and TCP ULP mutual exclusion

This adds two psp.py cases that check PSP associations and TCP ULPs are
mutually exclusive in both orders, plus a _require_tls_ulp() helper that
skips when kTLS is unavailable, and enables CONFIG_TLS=y in the
drivers/net selftest config.

[ ... ]

[Severity: Medium]

Should this handler be narrowed to the errnos that actually mean "no
kTLS"?  As written, every OSError from the TCP_ULP setsockopt turns into
a skip, including -EINVAL, which is exactly the errno the kernel side of
this series returns:

net/ipv4/tcp_ulp.c:__tcp_set_ulp() {
	...
	err = -EINVAL;
	if (sk_has_decrypt_user(sk))
		goto out_err;
	...
}

The probe socket here is a plain loopback TCP connection with no PSP
association, so the only legitimate reasons to skip are the "ULP unknown
/ TLS not built" errnos, ENOENT from __tcp_ulp_find_autoload() and
EOPNOTSUPP.

If sk_has_decrypt_user() ever returns true for an ordinary TCP socket -
and the parent commit anticipates growth there ("As other users of
skb->decrypted come along, they can be added to sk_has_decrypt_user()") -
the probe fails with EINVAL and the helper reports a skip.

Would restricting the skip to exc.errno in (errno.ENOENT,
errno.EOPNOTSUPP) and letting other errnos propagate keep the assertions
below load-bearing?

+
+
+def assoc_psp_ulp_exclusive(cfg):
+    """ Test that a TCP ULP cannot be attached to a PSP socket """
+    _init_psp_dev(cfg)
+    _require_tls_ulp()

[Severity: Medium]

Since _require_tls_ulp() is the first gate in both assoc_psp_ulp_exclusive()
and assoc_ulp_psp_exclusive(), does an over-broad kernel-side
sk_has_decrypt_user() end up disabling both directions of the
mutual-exclusion check at once?  The result would be two skips in CI rather
than a failure for the case these tests were added to catch.

+
+    with _make_clr_conn(cfg) as s:
+        try:
+            cfg.pspnl.rx_assoc({"version": 0,
+                                "dev-id": cfg.psp_dev_id,
+                                "sock-fd": s.fileno()})
+            with ksft_raises(OSError) as cm:
+                s.setsockopt(socket.SOL_TCP, TCP_ULP, b"tls")
+            ksft_eq(cm.exception.errno, errno.EINVAL)
+        finally:
+            _close_conn(cfg, s)
+
+
+def assoc_ulp_psp_exclusive(cfg):
+    """ Test that a PSP assoc cannot be added to a socket with a TCP ULP """
+    _init_psp_dev(cfg)
+    _require_tls_ulp()

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915-psp-ktls-fix-v2-0-0eedc3b148ec%40gmail.com