netdev - Re: [PATCH net-next] net: genet: Fixup EEE

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: <843316df-162f-4551-97ec-8e30dc054b41@lunn.ch>
Date: Thu, 11 Apr 2024 16:35:04 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: Florian Fainelli <florian.fainelli@...adcom.com>,
	Doug Berger <opendmb@...il.com>,
	Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: genet: Fixup EEE

On Thu, Apr 11, 2024 at 07:17:36AM +0200, Oleksij Rempel wrote:
> Hi Florian,
> 
> On Wed, Apr 10, 2024 at 10:48:26AM -0700, Florian Fainelli wrote:
> 
> > I am seeing a functional difference with and without your patch however, and
> > also, there appears to be something wrong within the bcmgenet driver after
> > PHYLIB having absorbed the EEE configuration. Both cases we start on boot
> > with:
> > 
> > # ethtool --show-eee eth0
> > EEE settings for eth0:
> >         EEE status: disabled
> >         Tx LPI: disabled
> >         Supported EEE link modes:  100baseT/Full
> >                                    1000baseT/Full
> >         Advertised EEE link modes:  100baseT/Full
> >                                     1000baseT/Full
> >         Link partner advertised EEE link modes:  100baseT/Full
> >                                                  1000baseT/Full
> > 
> > I would expect the EEE status to be enabled, that's how I remember it
> > before.
> 
> Yes, current default kernel implementation is to use EEE if available.

We do however seem to be inconsistent in this example. EEE seems to be
disabled, yet it is advertising? Or is it showing what we would
advertise, when EEE is enabled?

> > Now, with your patch, once I turn on EEE with:
> > 
> > # ethtool --set-eee eth0 eee on
> > # ethtool --show-eee eth0
> > EEE settings for eth0:
> >         EEE status: enabled - active
> >         Tx LPI: disabled
> >         Supported EEE link modes:  100baseT/Full
> >                                    1000baseT/Full
> >         Advertised EEE link modes:  100baseT/Full
> >                                     1000baseT/Full
> >         Link partner advertised EEE link modes:  100baseT/Full
> >                                                  1000baseT/Full
> > #
> > 
> > there is no change to the EEE_CTRL register to set the EEE_EN, this only
> > happens when doing:
> > 
> > # ethtool --set-eee eth0 eee on tx-lpi on
> > 
> > which is consistent with the patch, but I don't think this is quite correct
> > as I remembered that "eee on" meant enable EEE for the RX path, and "tx-lpi
> > on" meant enable EEE for the TX path?
> 
> Yes. More precisely, with "eee on" we allow the PHY to advertise EEE
> link modes. On link_up, if both sides are agreed to use EEE, MAC is
> configured to process LPI opcodes from the PHY and send LPI opcodes to
> the PHY if "tx-lpi on" was configured too. tx-lpi will not be enabled in
> case of "eee off".

Florian seems to be suggesting the RX and TX path could have different
configurations? RX EEE could be enabled but TX EEE disabled? That i
don't understand, in terms of auto-neg. auto-neg is for the link as a
whole, it does not appear to allow different results for each
direction. Does tx-lpi only make sense when EEE is forced, not
auto-neg'ed?

	Andrew