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: <202e55e2-be5f-4c7a-955e-fd726963c19c@lunn.ch>
Date: Wed, 5 Jun 2024 22:31:10 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: "Nelson, Shannon" <shannon.nelson@....com>,
	David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>,
	Jakub Kicinski <kuba@...nel.org>,
	Vitaly Lifshits <vitaly.lifshits@...el.com>,
	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

> 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