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: <6514e6a9-3b4d-48ba-b895-a12c5beff820@lunn.ch>
Date: Tue, 16 Apr 2024 00:03:49 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Wojciech Drewek <wojciech.drewek@...el.com>
Cc: pabeni@...hat.com, netdev@...r.kernel.org, edumazet@...gle.com,
	marcin.szycik@...ux.intel.com, anthony.l.nguyen@...el.com,
	idosch@...dia.com, kuba@...nel.org,
	intel-wired-lan@...ts.osuosl.org, przemyslaw.kitszel@...el.com
Subject: Re: [Intel-wired-lan] [PATCH net-next 0/3] ethtool: Max power support

On Fri, Apr 12, 2024 at 03:21:24PM +0200, Wojciech Drewek wrote:
> 
> 
> On 09.04.2024 15:39, Andrew Lunn wrote:
> >> This is something my current design supports I think. Using
> >> ETHTOOL_A_MODULE_MAX_POWER_SET user can get what cage supports
> >> and change it.
> >  
> >> This could be done using ethtool_module_power_mode_policy I think.
> > 
> > All these 'I think' don't give me a warm fuzzy feeling this is a well
> > thought out and designed uAPI.
> > 
> > I assume you have ethtool patches for your new netlink attributes. So
> > show us the real usage. Start with an SFP in its default lower power
> > mode. Show us the commands to display the current status. Allocate it
> > more power, tell the module it can use more power, and then show us
> > the status after the change has been made.
> 
> Ok, but do we really need an API to switch the module between high/low power mode?

Probably not. But you need to document that the API you are adding is
also expected to talk to the module and tell it to use more/less
power.

> Regarding the current status and what module supports, there is -m option:
> $ ethtool -m ens801f0np0
>         Identifier                                : 0x0d (QSFP+)
>         Extended identifier                       : 0x00
>         Extended identifier description           : 1.5W max. Power consumption
>         Extended identifier description           : No CDR in TX, No CDR in RX
>         Extended identifier description           : High Power Class (> 3.5 W) not enabled

So you can make this part of your commit message. Show this. Invoke
your new ethtool option, then show this again with the module
reporting a higher power consumption. The reduce the power using
ethtool and show the power consumption has reduced.

Also, in the ethtool-netlink.rst file, clearly document what the API
is doing, so that somebody else can implement it for another device.

Please also document hotplug behaviour. Say I use your new API to
increase the power to 3.5W. I then eject the module. Does the
available power automatically get put back into the pool? When i
reinsert the module, it will be in low power class, and i need to
issue the ethtool command again to increase its power?

   Andrew