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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z6MV7KY81S+/bGGY@mev-dev.igk.intel.com>
Date: Wed, 5 Feb 2025 08:40:28 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Tony Nguyen <anthony.l.nguyen@...el.com>, davem@...emloft.net,
	kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com,
	andrew+netdev@...n.ch, netdev@...r.kernel.org,
	sridhar.samudrala@...el.com, jacob.e.keller@...el.com,
	pio.raczynski@...il.com, konrad.knitter@...el.com,
	marcin.szycik@...el.com, nex.sw.ncis.nat.hpm.dev@...el.com,
	przemyslaw.kitszel@...el.com, jiri@...nulli.us, horms@...nel.org,
	David.Laight@...lab.com, pmenzel@...gen.mpg.de, mschmidt@...hat.com,
	tatyana.e.nikolova@...el.com, Jason Gunthorpe <jgg@...pe.ca>,
	Leon Romanovsky <leon@...nel.org>, linux-rdma@...r.kernel.org,
	corbet@....net, linux-doc@...r.kernel.org
Subject: Re: [PATCH net-next 2/9] ice: devlink PF MSI-X max and min parameter

On Tue, Feb 04, 2025 at 06:41:21PM +0000, David Laight wrote:
> On Tue, 4 Feb 2025 07:06:00 +0100
> Michal Swiatkowski <michal.swiatkowski@...ux.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@...el.com> wrote:
> > >   
> > > > From: Michal Swiatkowski <michal.swiatkowski@...ux.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

Right, I though it is better to have any additional validation for min >
max cases, but it looks like it is more problematic. I can drop it to
algin with the bnxt solution.

Thanks,
Michal

> 
> > 
> > Thanks,
> > Michal