Re: [PATCH net v5 2/2] selftests: net: add TLS 1.2 hardware offload tests

[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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] On offload NICs that only support AES-GCM-128, the
  tls12_aes256 variant of test_tls_offload reports FAIL when it should…

--

selftests: net: add TLS 1.2 hardware offload tests

This adds a two-node kTLS hardware offload selftest. A C helper installs
fixed AES-GCM keys on both ends, and the peer echoes data back to the DUT.
It checks that HW offload is used in each enabled DUT direction, covers
AES-GCM-128/256, IPv4/IPv6 and several payload sizes, and adds handshake
and alert record-type regression cases for the push_pending_record fix.

> diff --git a/tools/testing/selftests/drivers/net/hw/tls_hw_offload.py b/tools/testing/selftests/drivers/net/hw/tls_hw_offload.py
> new file mode 100755
> index 0000000000000..e720ce6547610
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/tls_hw_offload.py
> @@ -0,0 +1,321 @@

[ ... ]

> +def check_tls_support(cfg):
> +    """Require kTLS on both hosts and at least one DUT HW direction."""

[ ... ]

> +    try:
> +        features = cmd(f"ethtool -k {cfg.ifname}").stdout
> +    except CmdExitFailure as e:
> +        raise KsftSkipEx(f"Cannot determine TLS HW offload support: {e}") from e
> +    cfg.tls_hw_directions = {
> +        direction for direction in ('Tx', 'Rx')
> +        if f'tls-hw-{direction.lower()}-offload: on' in features
> +    }
> +    if not cfg.tls_hw_directions:
> +        raise KsftSkipEx("Device has no enabled TLS HW offload direction")

[Severity: Medium]
On NICs whose offload only supports AES-GCM-128, will the tls12_aes256
variant of test_tls_offload() report FAIL instead of SKIP?

cfg.tls_hw_directions comes from the tls-hw-tx-offload and
tls-hw-rx-offload feature flags. Those flags don't depend on the cipher.
run_tls_test() then requires HW in those directions for every cipher.

Several in-tree drivers that advertise these features reject AES-GCM-256
in tls_dev_add():

drivers/net/ethernet/netronome/nfp/crypto/tls.c:nfp_net_cipher_supported() {
	switch (cipher_type) {
	case TLS_CIPHER_AES_GCM_128:
		...
		break;
	default:
		return false;
	}
	...
}

The other drivers do the same:

- fun_ktls_add() in funeth returns -EOPNOTSUPP for any cipher other
  than AES-GCM-128.
- chcr_ktls_save_keys() returns -EINVAL for non-128 ciphers.
- mlx5 only supports AES-256 when MLX5_CAP_TLS(tls_1_2_aes_gcm_256)
  is set.

When tls_dev_add() fails, do_tls_setsockopt_conf() falls back to SW
without returning an error:

net/tls/tls_main.c:do_tls_setsockopt_conf() {
	...
		rc = tls_set_device_offload(sk);
		conf = TLS_HW;
		if (!rc) {
			...
		} else {
			rc = tls_set_sw_offload(sk, 1,
						update ? crypto_info : NULL);
	...
}

The helper's setsockopt() calls succeed and the echo passes. Only
TlsTxSw/TlsRxSw go up. check_path() then fails with "HW offload not
engaged (Device=0, Sw=1)".

The HW_NIC_COUNTERS comment lists nfp, cxgb4 and funeth, so this would
hit hardware the test means to cover.

Could the AES-256 variant skip, or accept SW, when the DUT driver doesn't
offload that cipher?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261102213011.4191020-1-rjethwani%40purestorage.com