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
| ||
|
Date: Tue, 24 Apr 2012 14:05:58 -0500 From: Andy Fleming <afleming@...il.com> To: Jonas Gorski <jonas.gorski@...il.com> Cc: mbizon@...ebox.fr, Andy Fleming <afleming@...escale.com>, netdev@...r.kernel.org, Florian Fainelli <florian@...nwrt.org>, Eric Dumazet <eric.dumazet@...il.com>, "David S. Miller" <davem@...emloft.net> Subject: Re: [PATCH] NET: bcm63xx_enet: move phy_(dis)connect into probe/remove On Sun, Apr 22, 2012 at 6:31 AM, Jonas Gorski <jonas.gorski@...il.com> wrote: > On 19 April 2012 18:17, Maxime Bizon <mbizon@...ebox.fr> wrote: >> >> On Thu, 2012-04-19 at 16:52 +0200, Jonas Gorski wrote: >> >>> Yes, but none of the ethtool functions cause register writes in the >>> priv->has_phy = true case when in PHY_READY or PHY_HALTED state. All >>> they do is modify the phy_device's settings. >> >> unless I'm mistaken: >> >> phy_ethtool_sset() => phy_start_aneg() >> >> will kick the state machine even when state is PHY_READY > > Hmm. I see what you mean. I wonder if it is intended that you can do > that without having phy_start() called first. > > @Andy, can you perhaps shed some light on this? How are ethernet > drivers supposed to behave/when should they call > phy_connect()/phy_start()? Currently most drivers call phy_connect() > in their _probe(), and phy_start() in _open(), so many seem to have > the issue that the phy state machine is in PHY_READY after _probe(), > and can be kicked into running through ethtool even if the interface > is down. > > This problem goes away after the first ifup/ifdown cycle, since the > phy state machine is then in PHY_HALTED, which gets properly caught in > phy_start_aneg(). > > To me it looks like phy_start_aneg() should check for some more > states, as it currently would also overwrite a PHY_STARTING or > PHY_PENDING state, which looks definitely wrong to me. Ugh, it looks like much has gone wrong with the state machine (possibly from when I wrote it). I'm thinking that what we should probably do is eliminate PHY_UP and PHY_READY, and have the PHY come up to PHY_HALTED. For some reason, PHY_RESUMING always enables interrupts, even if phydev->irq is in POLL mode, so that should be fixed. Other than that, it looks like PHY_RESUMING should work in the place of PHY_UP, and PHY_HALTED should be the same as PHY_READY... Andy -- 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
Powered by blists - more mailing lists