Re: [PATCH net-next 15/15] net/mlx5e: psp: Report PSP dev registration errors

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
--- Patch [15]: [PATCH net-next 15/15] net/mlx5e: psp: Report PSP dev registration errors ---
commit 76afe7de4d1e708e7c01d267ef1fb9cf946de937
Author: Cosmin Ratiu <cratiu@xxxxxxxxxx>

net/mlx5e: psp: Report PSP dev registration errors

This commit updates mlx5e_psp_register() to return an error code instead
of void when psp_dev_create() fails. The caller, mlx5e_nic_enable(), is
also updated to check this return value and abort if PSP registration fails.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -6199,7 +6199,9 @@ static int mlx5e_nic_enable(struct mlx5e_priv *priv)
>  
>  	mlx5e_fs_init_l2_addr(priv->fs, netdev);
>  	mlx5e_ipsec_init(priv);
> -	mlx5e_psp_register(priv);
> +	err = mlx5e_psp_register(priv);
> +	if (err)
> +		goto out_ipsec_cleanup;

[Severity: High]
Could propagating this error cause the entire network interface to fail if
the optional PSP offload fails to register?

If psp_dev_create() fails (e.g., due to memory pressure or ID exhaustion
in xa_alloc_cyclic), mlx5e_nic_enable() will now abort, resulting in a
complete loss of networking capability for the device.

Notice how the code immediately following this handles MACsec initialization
failures by logging the error and allowing the NIC to continue operating:

	err = mlx5e_macsec_init(priv);
	if (err)
		mlx5_core_err(mdev, "MACsec initialization failed, %d\n", err);

Should PSP registration failure fall back gracefully and allow the core NIC
to function without the offload, similar to how MACsec is handled?

>  
>  	err = mlx5e_macsec_init(priv);
>  	if (err)
>  		mlx5_core_err(mdev, "MACsec initialization failed, %d\n", err);