netdev - Re: [Intel-wired-lan] [PATCH v1 iwl-net] igc: power up PHY before 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <5d795b34-1ad7-4250-b7fd-0b8558e30b5e@intel.com>
Date: Sun, 7 Sep 2025 16:51:53 +0300
From: "Lifshits, Vitaly" <vitaly.lifshits@...el.com>
To: Kohei Enju <enjuk@...zon.com>
CC: <andrew+netdev@...n.ch>, <anthony.l.nguyen@...el.com>,
	<davem@...emloft.net>, <edumazet@...gle.com>,
	<intel-wired-lan@...ts.osuosl.org>, <kohei.enju@...il.com>,
	<kuba@...nel.org>, <netdev@...r.kernel.org>, <pabeni@...hat.com>,
	<przemyslaw.kitszel@...el.com>
Subject: Re: [Intel-wired-lan] [PATCH v1 iwl-net] igc: power up PHY before
 link test

On 9/6/2025 6:49 AM, Kohei Enju wrote:
> On Mon, 1 Sep 2025 07:36:21 +0300, Lifshits, Vitaly wrote:
> 
>>
>>> ---
>>>    drivers/net/ethernet/intel/igc/igc_ethtool.c | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>>> index f3e7218ba6f3..ca93629b1d3a 100644
>>> --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
>>> +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>>> @@ -2094,6 +2094,9 @@ static void igc_ethtool_diag_test(struct net_device *netdev,
>>>    		netdev_info(adapter->netdev, "Offline testing starting");
>>>    		set_bit(__IGC_TESTING, &adapter->state);
>>>    
>>> +		/* power up PHY for link test */
>>> +		igc_power_up_phy_copper(&adapter->hw);
>>
>> I suggest moving this to igc_link_test functionn igc_diags.c as it
>> relates only to the link test.
> 
> Thank you for taking a look, Lifshits.
> 
> Now I'm considering changing only offline test path, not including
> online test path.
> This is because I think online test path should not trigger any
> interruption and power down/up PHY may cause disruption.
> 
> So, I forgo the online path and my intention for this patch is to power
> up PHY state only in offline test path.
> I think introducing igc_power_up_phy_copper() in igc_link_test()
> needs careful consideration in online test path.

Yes, I see that now.
Then it's ok to leave it as is.

Regarding the question whether to wrap igc_power_up_phy_copper with an 
if (hw->phy.media_type == igc_media_type_copper), I think that it's not 
necessary. First of all, igc devices are only copper devices. Secondly, 
in the other place where we call this function (in igc_main), we don't 
check the media type, therefore I suggest being consistent with the 
already existing code and leaving it as it is now.

> 
>>
>>> +
>>>    		/* Link test performed before hardware reset so autoneg doesn't
>>>    		 * interfere with test result
>>>    		 */
>>
>>
>> Thank you for this patch.