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

Message-ID: <88fc6454-01dc-4907-9b9b-702c9464c9ec@intel.com>
Date: Thu, 18 Apr 2024 13:48:25 +0200
From: Wojciech Drewek <wojciech.drewek@...el.com>
To: Andrew Lunn <andrew@...n.ch>
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 16.04.2024 00:03, Andrew Lunn wrote:
> 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?

Ok, I'll answer all of those questions in the documentation included
in the 2nd version of the patchset.

> 
>    Andrew
>