Re: [PATCH net-next 2/9] ice: devlink PF MSI-X max and min parameter

[David Laight <david.laight.linux@gmail.com>]

On Tue, 4 Feb 2025 07:06:00 +0100
Michal Swiatkowski <michal.swiatkowski@linux.intel.com> wrote:

> On Mon, Feb 03, 2025 at 09:48:08PM +0000, David Laight wrote:
> > On Mon,  3 Feb 2025 13:09:31 -0800
> > Tony Nguyen <anthony.l.nguyen@intel.com> wrote:
> >   
> > > From: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > > 
> > > Use generic devlink PF MSI-X parameter to allow user to change MSI-X
> > > range.
> > > 
> > > Add notes about this parameters into ice devlink documentation.
....
> > Don't those checks make it difficult to set the min and max together?
> > I think you need to create the new min/max pair and check they are
> > valid together.
> > Which probably requires one parameter with two values.
> >   
> 
> I wanted to reuse exsisting parameter. The other user of it is bnxt
> driver. In it there is a separate check for min "max" and max "max".
> It is also problematic, because min can be set to value greater than
> max (here it can happen when setting together to specific values).
> I can do a follow up to this series and change this parameter as you
> suggested. What do you think?

Changing the way a parameter is used will break API compatibility.
Perhaps you can get the generic parameter validation function to
update a 'pending' copy, and then do the final min < max check after
all the parameters have been processed before actually updating
the live limits.

The other option is just not to check whether min < max and just
document which takes precedence (and not use clamp()).

It may even be worth saving the 'live limits' as 'hi << 16 | lo' so
that then can be accessed atomically (with READ/WRITE_ONCE) to avoid
anything looking at the limits getting confused.
(Although maybe that doesn't matter here?)

	David

> 
> Thanks,
> Michal