linux-kernel - Re: [PATCH net-next v1 4/4] net: phy: Always read EEE LPA in genphy_c45_ethtool_get

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]

Message-ID: <aApO59e6I6uLaw2P@shell.armlinux.org.uk>
Date: Thu, 24 Apr 2025 15:47:03 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>
Cc: Oleksij Rempel <o.rempel@...gutronix.de>,
	Woojung Huh <woojung.huh@...rochip.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Heiner Kallweit <hkallweit1@...il.com>, kernel@...gutronix.de,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	UNGLinuxDriver@...rochip.com, Simon Horman <horms@...nel.org>,
	Maxime Chevallier <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH net-next v1 4/4] net: phy: Always read EEE LPA in
 genphy_c45_ethtool_get_eee()

On Thu, Apr 24, 2025 at 04:34:27PM +0200, Andrew Lunn wrote:
> On Thu, Apr 24, 2025 at 02:16:01PM +0100, Russell King (Oracle) wrote:
> > However, I've no objection to reading the LPA EEE state and
> > reporting it.
> 
> What happens with normal link mode LPA when autoneg is disabled? I
> guess they are not reported because the PHY is not even listening for
> the autoneg pulses. We could be inconsistent between normal LPA and
> LPA EEE, but is that a good idea?

With autoneg state, that controls whether the various pages get
exchanged or not - which includes the EEE capabilties. This is the
big hammer for anything that is negotiated.

With EEE, as long as autoneg in the main config is true, the PHY will
exchange the EEE capability pages if it supports them. Our eee_enabled
is purely just a software switch, there's nothing that corresponds to it
in hardware, unlike autoneg which has a bit in BMCR.

We implement eee_enabled by clearing the advertisement in the hardware
but accepting (and remembering) the advertisement from userspace
unmodified.

The two things are entirely different in hardware.

Since:

 ethtool --set-eee eee off

Will use ETHTOOL_GEEE, modify eee_enabled to be false (via
do_generic_set), and then use ETHTOOL_SEEE to write it back, the
old advertisement will be passed back to the kernel in this case.

If we don't preserve the advertisement, then:

 ethtool --set-eee eee off

will clear the advertisement, and then:

 ethtool --set-eee eee on

will set eee_enabled true but we'll have an empty advertisement. Not
ideal.

If we think about forcing it for an empty advertisement to e.g. fully
populated, then:

 ethtool --set-eee eee on advertise 0

will surprisingly not end up with an empty advertisement.

So, I don't think it's realistic to come up with a way that --set-eee
behaves the same way as -s because of the way ethtool has been
implemented.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!