Re: [PATCH net 1/2] net/mlx5e: psp: Fix invalid access on PSP dev registration fail

[Cosmin Ratiu]

On Wed, 2026-04-22 at 09:25 +0000, Cosmin Ratiu wrote:
> On Tue, 2026-04-21 at 11:32 -0700, Jakub Kicinski wrote:
> > On Tue, 21 Apr 2026 17:34:32 +0000 Cosmin Ratiu wrote:
> > > > No, the normal thing to do is to propagate errors.
> > > > If you want to diverge from that _you_ should have a reason,
> > > > a better reason than a vague "kernel can fail".
> > > > I'd prefer for the driver to fail in an obvious way.
> > > > Which will be immediately spotted by the operator, not 2 weeks
> > > > later when 10% of the fleet is upgraded already.
> > > > The only exception I'd make is to keep devlink registered in
> > > > case the fix is to flash a different FW.  
> > > 
> > > In this case, PSP not working would be spotted on the next PSP
> > > dev-
> > > get
> > > op which produces zilch instead of working devices.
> > 
> > When you have X vendors times Y device generations times Z FW
> > versions
> > in your fleet dev-get returning nothing is not a failure. It just
> > means
> > you're running on a machine that's not capable. Best you can do to
> > spot a buggy kernel is to notice that the fraction of PSP traffic
> > is
> > decreasing over time. After significant portion of the fleet is
> > already
> > on the bad kernel.
> > 
> > > But I understand what you want. You'd like the netdevice to
> > > either
> > > be
> > > fully initialized with all supported+configured protocols or fail
> > > the
> > > open operation. No intermediate/partial states. This is a non-
> > > trivial
> > > refactor for mlx5, because mlx5_nic_enable() returns nothing.
> > > Refactoring seems possible though, its only caller is
> > > mlx5e_attach_netdev(), which returns errors. It's certainly not
> > > something that should be done for a net fix though.
> > > 
> > > I have a series pending for net-next where the PSP configuration
> > > is
> > > hooked to mlx5e_psp_set_config(). I will look into implementing
> > > what
> > > you propose there and propagate errors.
> > > 
> > > Meanwhile, do you want to take these fixes (1 and 2) or maybe
> > > just
> > > 2
> > > for net or not?
> > 
> > Can you call mlx5e_psp_cleanup() when register fails for now?
> 
> Done for the next version, currently undergoing testing.

There's a snag: priv->psp may be accessed concurrently from
mlx5e_get_stats() -> mlx5e_fold_sw_stats64() so we'd need to play
tricks with RCU and that goes beyond what a net fix should be: It's a
redesign of how priv->psp is handled in the driver. There's a risk we
are missing things, or it becomes more intrusive that what a fix should
be.

I would like to ask you: let's please not do this redesign of priv->psp
in a rush, and leave it for the net-next series I mentioned...

To reiterate, would you like to take patch 2?

Cosmin.