netdev - Re: [PATCH 9/9] igc: add support for ethtool.set_phys

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: <ec1d55e0-f8df-4db1-a8df-22eab733e107@intel.com>
Date: Wed, 5 Jun 2024 13:33:35 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Andrew Lunn <andrew@...n.ch>, Vitaly Lifshits <vitaly.lifshits@...el.com>
CC: "Nelson, Shannon" <shannon.nelson@....com>, David Miller
	<davem@...emloft.net>, netdev <netdev@...r.kernel.org>, Jakub Kicinski
	<kuba@...nel.org>, Menachem Fogel <menachem.fogel@...el.com>, Naama Meir
	<naamax.meir@...ux.intel.com>
Subject: Re: [PATCH 9/9] igc: add support for ethtool.set_phys_id



On 6/5/2024 1:31 PM, Andrew Lunn wrote:
>> Maybe I'm misunderstanding here. Are you asking us to expose the LEDs
>> via some other interface and extend ethtool to use that interface to
>> blink LEDs?
> 
> The LEDs are already exposed:
> 
> commit ea578703b03d5d651b091c39f717dc829155b520
> Author: Kurt Kanzenbach <kurt@...utronix.de>
> Date:   Tue Feb 13 10:41:37 2024 -0800
> 
>     igc: Add support for LEDs on i225/i226
>     
>     Add support for LEDs on i225/i226. The LEDs can be controlled via sysfs
>     from user space using the netdev trigger. The LEDs are named as
>     igc-<bus><device>-<led> to be easily identified.
>     
>     Offloading link speed and activity are supported. Other modes are simulated
>     in software by using on/off. Tested on Intel i225.
>     
>     Signed-off-by: Kurt Kanzenbach <kurt@...utronix.de>
>     Reviewed-by: Andrew Lunn <andrew@...n.ch>
>     Tested-by: Naama Meir <naamax.meir@...ux.intel.com>
>     Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
>     Link: https://lore.kernel.org/r/20240213184138.1483968-1-anthony.l.nguyen@intel.com
>     Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> 
> The Linux LED subsystem knows about them.
> 
> The Linux LED subsystem also knows about the qca8k LEDs, rtl8366rb
> LEDs, mediatek-ge-soc LEDs, air_en8811h LEDs, broadcom PHY LEDs,
> Marvell PHY LEDs, dp83867 LEDs, etc. There is nothing special here
> about igc. All these should be capable of blinking.
> 
> So what i'm asking for is you add an ethtool helper, which implements
> this using the Linux LED subsystem to blink the LEDs. The same helper
> can then be used by other MAC drivers to blink their LEDs when they
> use the Linux LED subsystem.
> 
> I'm sure there are a few different ways to implement this. One could
> be to extend the existing ledtrig-netdev.c. Add a call
> 
> int netdev_trig_ethtool_phys_id(struct net_device *net_dev,
>           		       enum ethtool_phys_id_state state)
> 
> Which if passed ETHTOOL_ID_ON, ETHTOOL_ID_OFF, ETHTOOL_ID_ACTIVE,
> saves the current state and then sets the LEDs associated to the
> netdev to the requested state. If passed ETHTOOL_ID_INACTIVE it
> restores the previous state.
> 
> 	 Andrew

Makes sense, thanks for clarifying.

@Vitaly, please look into this.

Thanks,
Jake