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

[Simon Horman]

On Tue, Jul 14, 2026 at 08:08:03PM +0000, Cosmin Ratiu wrote:
> 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.

Thanks, sorry for not noticing that discussion earlier.