netdev - Re: [Intel-wired-lan] [PATCH net-next 0/3] ethtool: Max power support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <dc601a7c-7bb7-4857-8991-43357b15ed5a@lunn.ch>
Date: Tue, 2 Apr 2024 16:46:54 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Wojciech Drewek <wojciech.drewek@...el.com>
Cc: netdev@...r.kernel.org, idosch@...dia.com, edumazet@...gle.com,
	marcin.szycik@...ux.intel.com, anthony.l.nguyen@...el.com,
	kuba@...nel.org, intel-wired-lan@...ts.osuosl.org,
	pabeni@...hat.com, przemyslaw.kitszel@...el.com
Subject: Re: [Intel-wired-lan] [PATCH net-next 0/3] ethtool: Max power support

On Tue, Apr 02, 2024 at 01:38:59PM +0200, Wojciech Drewek wrote:
> 
> 
> On 30.03.2024 22:57, Andrew Lunn wrote:
> > On Fri, Mar 29, 2024 at 10:23:18AM +0100, Wojciech Drewek wrote:
> >> Some ethernet modules use nonstandard power levels [1]. Extend ethtool
> >> module implementation to support new attributes that will allow user
> >> to change maximum power. Rename structures and functions to be more
> >> generic. Introduce an example of the new API in ice driver.
> >>
> >> Ethtool examples:
> >> $ ethtool --show-module enp1s0f0np0
> >> Module parameters for enp1s0f0np0:
> >> power-min-allowed: 1000 mW
> >> power-max-allowed: 3000 mW
> >> power-max-set: 1500 mW
> >>
> >> $ ethtool --set-module enp1s0f0np0 power-max-set 4000
> > 
> > We have had a device tree property for a long time:
> > 
> >   maximum-power-milliwatt:
> >     minimum: 1000
> >     default: 1000
> >     description:
> >       Maximum module power consumption Specifies the maximum power consumption
> >       allowable by a module in the slot, in milli-Watts. Presently, modules can
> >       be up to 1W, 1.5W or 2W.
> > 
> > Could you flip the name around to be consistent with DT?
> 
> Yea, I'm open to any name suggestion although I don't like the unit in the parameter name :) 

That is a DT thing. Helps make the units of an ABI obvious. However,
milliwatts is pretty standard with the kernel of user APIs, e.g. all
hwmon calls use milliwatts.

> >> minimum-power-allowed: 1000 mW
> >> maximum-power-allowed: 3000 mW
> >> maximum-power-set: 1500 mW
> > 
> > Also, what does minimum-power-allowed actually tell us? Do you imagine
> > it will ever be below 1W because of bad board design? Do you have a
> > bad board design which does not allow 1W?
> 
> Yes. in case of QSFP we don't support 1W, 1.5W is the minimum.

So if i plug in a 1W QSFP device, it will let the magic smoke out
because it is force fed 1.5W?

Looking at
https://www.optcore.net/wp-content/uploads/2017/04/QSFP-MSA.pdf table
7 it indicates different power budget classifications. Power level 1
is a Maximum power of 1.5W. So does your parameter represent this?  It
is the minimum maximum power? And your other parameter is the maximum
maximum power?

I agree with Jakub here, there needs to be documentation added
explaining in detail what these parameters mean, and ideally,
references to the specification.

Does

$ ethtool --set-module enp1s0f0np0 power-max-set 4000

actually talk to the SFP module and tell it the maximum power it can
consume. So in this case, it is not the cage, but the module?

Or is it talking to some entity which is managing the overall power
consumption of a number of cages, and asking it to allocate a maximum
of 4W to this cage. It might return an error message saying there is
no power budget left?

Or is it doing both?

Sorry to be picky, but at some point, somebody is going to want to
implement this in the Linux SFP driver, and we want a consistent
implementation cross different implementations.

	Andrew