netdev - Re: [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2)

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Z3bYZZu8Ip8it0RZ@shell.armlinux.org.uk>
Date: Thu, 2 Jan 2025 18:18:13 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Florian Fainelli <florian.fainelli@...adcom.com>,
	Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
	UNGLinuxDriver@...rochip.com,
	Woojung Huh <woojung.huh@...rochip.com>
Subject: Re: [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2)

On Thu, Dec 12, 2024 at 09:48:53PM +0200, Vladimir Oltean wrote:
> On Tue, Dec 10, 2024 at 02:25:44PM +0000, Russell King (Oracle) wrote:
> > This is part 2 of the DSA EEE cleanups, and is being sent out becaues it
> > is relevant for the review of part 1, but would make part 1 too large.
> > 
> > Patch 1 removes the useless setting of tx_lpi parameters in the
> > ksz driver.
> > 
> > Patch 2 removes the DSA core code that calls the get_mac_eee() operation.
> > This needs to be done before removing the implementations because doing
> > otherwise would cause dsa_user_get_eee() to return -EOPNOTSUPP.
> > 
> > Patches 3..6 remove the trivial get_mac_eee() implementations from DSA
> > drivers.
> > 
> > Patch 7 finally removes the get_mac_eee() method from struct
> > dsa_switch_ops.
> 
> I appreciate the splitting of the get_mac_eee() removal into multiple
> patches per driver and 2 for the DSA framework. It should help BSP
> backporters which target only a subset of DSA drivers. Monolithic
> patches are harder to digest, and may have trivial context conflicts due
> to unrelated changes.
> 
> The set looks good, please don't forget to also update the documentation.

Sorry, but which documentation are you referring to?

$ grep get_mac_eee Documentation/networking/ drivers/net include/ net/ -r
$ 

No references to get_mac_eee() anywhere (except in the mt7530 driver
which I've missed - patches now included, will be in v2.)

Even looking for "et_mac_eee" in case of Documentation using
[sg]et_mac_eee() reveals nothing.

So, I don't think there's any documentation that these patches change.

There is this in Documentation/networking/dsa/dsa.rst:

- ``get_eee``: ethtool function which is used to query a switch port EEE settings,
  this function should return the EEE state of the switch port MAC controller
  and data-processing logic as well as query the PHY for its currently configured
  EEE settings

First, realise that my patch set actually changes nothing - all these
implementations were entirely useless because everything they have been
doing has been overwritten by phylib since phylib's EEE support was
changed. Therefore, the above as wrong both before and after this
patch set.

Second, when phylink managed EEE gets merged, the above becomes true
again.

Think of the current phylib behaviour as a user visible regression
(because that's exactly what it is), and this patch set is part of a
move towards resolving that regression.

IMHO, phylib managed EEE support should never have been merged - not
in the way it was. There should have been either a way to transition
drivers to it without causing all this breakage, or there should have
been a commitment to getting this fixed in a timely manner (in the
same cycle that it was merged.)

What we have right now is a total trainwreck.

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