netdev - Re: [Intel-wired-lan] [PATCH v2 iwl-net] igc: power up the PHY before the link test

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fd1e8bbe-199f-4771-ab14-086bd21d0b1d@intel.com>
Date: Sun, 28 Sep 2025 12:30:24 +0300
From: Avigail Dahan <Avigailx.dahan@...el.com>
To: "Lifshits, Vitaly" <vitaly.lifshits@...el.com>, Kohei Enju
	<enjuk@...zon.com>, <intel-wired-lan@...ts.osuosl.org>,
	<netdev@...r.kernel.org>
CC: Tony Nguyen <anthony.l.nguyen@...el.com>, Przemek Kitszel
	<przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David
 S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, "Jakub
 Kicinski" <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	<aleksandr.loktionov@...el.com>, <kohei.enju@...il.com>
Subject: Re: [Intel-wired-lan] [PATCH v2 iwl-net] igc: power up the PHY before
 the link test



On 21/09/2025 9:41, Lifshits, Vitaly wrote:
> On 9/20/2025 9:39 AM, Kohei Enju wrote:
>> The current implementation of the igc driver doesn't power up the PHY
>> before the link test in igc_ethtool_diag_test(), causing the link test
>> to always report FAIL when admin state is down and the PHY is
>> consequently powered down.
>>
>> To test the link state regardless of admin state, power up the PHY
>> before the link test in the offline test path. After the link test, the
>> original PHY state is restored by igc_reset(), so additional code which
>> explicitly restores the original state is not necessary.
>>
>> Note that this change is applied only for the offline test path. This is
>> because in the online path we shouldn't interrupt normal networking
>> operation and powering up the PHY and restoring the original state would
>> interrupt that.
>>
>> This implementation also uses igc_power_up_phy_copper() without checking
>> the media type, since igc devices are currently only copper devices and
>> the function is called in other places without checking the media type.
>>
>> Furthermore, the powering up is on a best-effort basis, that is, we
>> don't handle failures of powering up (e.g. bus error) and just let the
>> test report FAIL.
>>
>> Tested on Intel Corporation Ethernet Controller I226-V (rev 04) with
>> cable connected and link available.
>>
>> Set device down and do ethtool test.
>>    # ip link set dev enp0s5 down
>>
>> Without patch:
>>    # ethtool --test enp0s5
>>    The test result is FAIL
>>    The test extra info:
>>    Register test  (offline)         0
>>    Eeprom test    (offline)         0
>>    Interrupt test (offline)         0
>>    Loopback test  (offline)         0
>>    Link test   (on/offline)         1
>>
>> With patch:
>>    # ethtool --test enp0s5
>>    The test result is PASS
>>    The test extra info:
>>    Register test  (offline)         0
>>    Eeprom test    (offline)         0
>>    Interrupt test (offline)         0
>>    Loopback test  (offline)         0
>>    Link test   (on/offline)         0
>>
>> Fixes: f026d8ca2904 ("igc: add support to eeprom, registers and link 
>> self-tests")
>> Signed-off-by: Kohei Enju <enjuk@...zon.com>
>> ---
>> Changes:
>> v1->v2:
>>    - rephrase commit message to clarify:
>>      - applied only for offline test path
>>      - original power state is restored by igc_reset()
>>      - powering up the PHY is on a best-effort basis
>> v1: https://lore.kernel.org/intel-wired-lan/20250830170656.61496-1- 
>> enjuk@...zon.com/
>> ---
>>   drivers/net/ethernet/intel/igc/igc_ethtool.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>Tested-by: Avigail Dahan <avigailx.dahan@...el.com>