Re: [PATCH net-next 13/15] net/mlx5e: psp: Make PSP steering config dynamic

[Simon Horman <horms@kernel.org>]

On Tue, Jul 14, 2026 at 08:05:39PM +0000, Cosmin Ratiu wrote:
> On Tue, 2026-07-14 at 18:30 +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 [13]: [PATCH net-next 13/15] net/mlx5e: psp: Make PSP
> > steering config dynamic ---
> > commit 133d9a32cbf9c1b939ae4ca8473c41624c313951
> > Author: Cosmin Ratiu <cratiu@nvidia.com>
> > 
> > net/mlx5e: psp: Make PSP steering config dynamic
> > 
> > This commit changes PSP steering tables to only be created when PSP
> > configuration is enabled on a device, instead of at device
> > initialization
> > time. This avoids imposing steering overhead on all UDP packets when
> > PSP
> > is not being used.
> > 
> > > diff --git
> > > a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
> > > b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
> > > index b526b3898c22e..3f212e46fc2f9 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
> > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/en_accel.h
> > > @@ -220,18 +220,7 @@ static inline void
> > > mlx5e_accel_tx_finish(struct mlx5e_txqsq *sq,
> > >  
> > >  static inline int mlx5e_accel_init_rx(struct mlx5e_priv *priv)
> > >  {
> > > -	int err;
> > > -
> > > -	err = mlx5_accel_psp_fs_init_rx_tables(priv);
> > > -	if (err)
> > > -		goto out;
> > > -
> > > -	err = mlx5e_ktls_init_rx(priv);
> > > -	if (err)
> > > -		mlx5_accel_psp_fs_cleanup_rx_tables(priv);
> > > -
> > > -out:
> > > -	return err;
> > > +	return mlx5e_ktls_init_rx(priv);
> > >  }
> > >  
> > >  static inline void mlx5e_accel_cleanup_rx(struct mlx5e_priv *priv)
> > > @@ -242,12 +231,6 @@ static inline void
> > > mlx5e_accel_cleanup_rx(struct mlx5e_priv *priv)
> > >  
> > >  static inline int mlx5e_accel_init_tx(struct mlx5e_priv *priv)
> > >  {
> > > -	int err;
> > > -
> > > -	err = mlx5_accel_psp_fs_init_tx_tables(priv);
> > > -	if (err)
> > > -		return err;
> > > -
> > >  	return mlx5e_ktls_init_tx(priv);
> > >  }
> > 
> > [Severity: Critical]
> > Does removing the unconditional table creation here prevent PSP
> > tables
> > from being restored on device attach or resume?
> > 
> > Since mlx5_accel_psp_fs_cleanup_tx_tables() is still called during
> > device
> > detach or suspend, the PSP steering tables are destroyed. However,
> > because
> > mlx5e_accel_init_tx() no longer recreates them, the tables appear to
> > remain
> > missing after a suspend/resume or detach/attach cycle.
> > 
> > Because software psp_assoc structures remain bound to active sockets,
> > could
> > this leave the device in a state where the hardware TX steering table
> > (which
> > contains the CRYPTO_ENCRYPT action) is missing?
> > 
> > If so, would this cause egress traffic to bypass encryption and be
> > transmitted in plaintext?
> 
> When the device is suspended, 1. all TX keys are released and 2.
> steering rules are removed. 3. psp state is completely freed, including
> the configuration intention.
> 
> When the device is resumed, psp is reinitialized to a blank state.
> 
> Before these changes, psp was also reset to a blank state (no keys, but
> with steering rules readded). This also broke existing PSP connections.
> 
> I guess fixing this behavior during suspend is outside the scope of
> this series. I don't like it, we should do something about it, but not
> in this series.

Thanks, point taken.
I think we can leave this problem for another time.