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
| ||
|
Message-ID: <ZHZQG+O9HkQ+5K62@shell.armlinux.org.uk> Date: Tue, 30 May 2023 20:35:55 +0100 From: "Russell King (Oracle)" <linux@...linux.org.uk> To: Florian Fainelli <f.fainelli@...il.com> Cc: Andrew Lunn <andrew@...n.ch>, netdev <netdev@...r.kernel.org>, Heiner Kallweit <hkallweit1@...il.com>, Oleksij Rempel <linux@...pel-privat.de> Subject: Re: [RFC/RFTv3 00/24] net: ethernet: Rework EEE On Tue, May 30, 2023 at 11:31:04AM -0700, Florian Fainelli wrote: > Hi Andrew, Russell, > > On 3/30/23 17:54, Andrew Lunn wrote: > > Most MAC drivers get EEE wrong. The API to the PHY is not very > > obvious, which is probably why. Rework the API, pushing most of the > > EEE handling into phylib core, leaving the MAC drivers to just > > enable/disable support for EEE in there change_link call back, or > > phylink mac_link_up callback. > > > > MAC drivers are now expect to indicate to phylib/phylink if they > > support EEE. If not, no EEE link modes are advertised. If the MAC does > > support EEE, on phy_start()/phylink_start() EEE advertisement is > > configured. > > Thanks for doing this work, because it really is a happy mess out there. A > few questions as I have been using mvneta as the reference for fixing GENET > and its shortcomings. > > In your new patches the decision to enable EEE is purely based upon the > eee_active boolean and not eee_enabled && tx_lpi_enabled unlike what mvneta > useed to do. > > Russell, is there an use case for having eee_enabled while not having > tx_lpi_enabled? As I've been stating recently, LPI is entirely to do with the MAC, so the LPI delay and the enable boolean are about controlling the MAC end of things. I've never really understood what "eee_enabled" is supposed to be doing, since there's nowhere really to enable or disable EEE per-se. Through recent patches, phylib has since defined "eee_enabled" to mean that the advertisement is non-empty, which came as a surprise to me, but again seems rather redundant and strange. It's strange because as far as I can see from what documentation there is, "eee_enabled" is supposed to be a control that users can set independently of the advertisement, but with the phylib implementation, eee_enabled read from get_eee() as I say is defined as whether the advertisement is non-zero or not. With fibre setups, there is no EEE advertisement, so in that situation phylib's interpretation of eee_enabled via get_eee/set_eee would be very much incorrect. The only control one has is whether LPI is enabled and its timer setting. That said, the current mvneta implementation doesn't actually enable LPI for fibre... but there is a bug in that one can get the MAC to enable LPI via ethtool's set_eee() ! I think for a fibre setup, eee_enabled && tx_lpi_enabled is reasonable, given that eee_enabled is a seperate control from the advertisement which will be zero in that case. Going back to phylib, given this, things get even more "fun" if you have a dual-media PHY. As there's no EEE capability bits for 1000base-X, but a 1000base-X PCS optionally supports EEE. So, even with a zero EEE advertisement with a dual-media PHY that would only affect the copper side, and EEE may still be possible in the fibre side... which makes phylib's new interpretation of "eee_enabled" rather odd. In any case, "eee_enabled" I don't think has much meaning for the fibre case because there's definitely no control beyond what "tx_lpi_enabled" already offers. I think this is a classic case where the EEE interface has been designed solely around copper without checking what the situation is for fibre! -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists