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

[netdev-bot+sashiko@kernel.org]

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.

> diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
> index 315648a770d00..a5b1e14f120f0 100755
> --- a/tools/testing/selftests/drivers/net/psp.py
> +++ b/tools/testing/selftests/drivers/net/psp.py

[ ... ]

> @@ -333,6 +335,50 @@ def assoc_version_mismatch(cfg):
>              ksft_eq(the_exception.nl_msg.error, -errno.EINVAL)
>  
>  
> +def _require_tls_ulp():
> +    with socket.create_server(("localhost", 0)) as srv, \
> +         socket.create_connection(srv.getsockname()) as s:
> +        try:
> +            s.setsockopt(socket.SOL_TCP, TCP_ULP, b"tls")
> +        except OSError as exc:
> +            raise KsftSkipEx("kTLS not available") from exc

[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