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

[Cosmin Ratiu]

On Tue, 2026-07-14 at 18:31 +0100, Simon Horman wrote:
> 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);

This is an intentional change, as discussed in this thread with Jakub:

https://lore.kernel.org/netdev/20260417050201.192070-1-tariqt@xxxxxxxxxx/t/#u

We carefully chose to fail device init on the very unlikely event PSP
dev creation fails.

Cosmin.