netdev - Re: [PATCH] net: phy: Make sure PHY_RESUMING state change is always processed

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]

Date:	Sat, 16 May 2015 17:16:02 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	f.fainelli@...il.com
Cc:	tim.beale@...iedtelesis.co.nz, netdev@...r.kernel.org
Subject: Re: [PATCH] net: phy: Make sure PHY_RESUMING state change is
 always processed

From: Florian Fainelli <f.fainelli@...il.com>
Date: Fri, 15 May 2015 14:25:49 -0700

> On 12/05/15 18:55, Tim Beale wrote:
>> If phy_start_aneg() was called while the phydev is in the PHY_RESUMING
>> state, then its state would immediately transition to PHY_AN (or
>> PHY_FORCING). This meant the phy_state_machine() never processed the
>> PHY_RESUMING state change, which meant interrupts weren't enabled for the
>> PHY. If the PHY used low-power mode (i.e. using BMCR_PDOWN), then the
>> physical link wouldn't get powered up again.
>> 
>> There seems no point for phy_start_aneg() to make the PHY_RESUMING -->
>> PHY_AN transition, as the state machine will do this anyway. I'm not sure
>> about the case where autoneg is disabled, as my patch will change
>> behaviour so that the PHY goes to PHY_NOLINK instead of PHY_FORCING. An
>> alternative solution would be to move the phy_config_interrupt() and
>> phy_resume() work out of the state machine and into phy_start().
> 
> Could you prepare a patch which does that? I do not have a setup where
> the PHY IRQ is a dedicated interrupt line, but I might be able to test
> something with a hack.
> 
>> 
>> The background behind this: we're running linux v3.16.7 and from user-space
>> we want to enable the eth port (i.e. do a SIOCSIFFLAGS ioctl with the
>> IFF_UP flag) and immediately afterward set the interface's speed/duplex.
>> Enabling the interface calls .ndo_open() then phy_start() and the PHY
>> transitions PHY_HALTED --> PHY_RESUMING. Setting the speed/duplex ends up
>> calling phy_ethtool_sset(), which calls phy_start_aneg() (meanwhile the
>> phy_state_machine() hasn't processed the PHY_RESUMING state change yet).
>> 
>> Signed-off-by: Tim Beale <tim.beale@...iedtelesis.co.nz>
> 
> Reviewed-by: Florian Fainelli <f.fainelli@...il.com>

Applied, thanks everyone.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html