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

[Jakub Kicinski]

On Tue, 21 Apr 2026 12:29:13 +0000 Cosmin Ratiu wrote:
> > Sure but why are you leaving the priv->psp struct in place and
> > whatever
> > FS init has been done? IOW if you really want PSP init to not block
> > probe why is mlx5e_psp_register() a void function rather than
> > mlx5e_psp_init() ? Ignoring errors from psp_dev_create()
> > makes no sense to me - what are you protecting from?
> > kmalloc(GFP_KERNEL)
> > failing?  
> 
> priv->psp and steering at the time of mlx5e_psp_register() is inert
> without the PSP device. Cleaning it on psp_dev_create() failure would
> be weird, it's cleaned up anyway on netdev teardown. The fact that only
> memory allocations can fail inside psp_dev_create() is irrelevant here.
> psp_dev_create() failing shouldn't bring down the whole netdevice, so
> logging a message and continuing is ok (which is what is also done for
> macsec and ktls).

This is a misguided cargo cult. Or something motivated by OOT
compatibility. Alex D sometimes tries to do the same thing with Meta
drivers. I don't get it. Of course we want the device to be operational
if some *device* init fails. The compatibility matrix with all device
generations and fw versions could justify that. But continuing init
when a single-page kmalloc failed is pure silliness.

> mlx5e_psp_register() is void because it's called from
> mlx5e_nic_enable() which can't fail, so it really can't do much other
> than complain to dmesg.
> 
> But while thinking about this, I suppose we could change the entire PSP
> initialization to happen at the time of the current
> mlx5e_psp_register(), and that would simplify the number of states.
> I will do that in the next planned PSP series for net-next.
> 
> Meanwhile, could you please take the 2nd patch and leave this one out?
> It should apply with no conflicts by itself.
> 
> Or you would like to see a separate submission with the 2nd patch
> alone?

Please resubmit.