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

Cosmin Ratiu
2026-04-22T09:25:19Z

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.

Cosmin.