netdev - Re: [PATCH net-next v2] net: dsa: mv88e6060: add phylink_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: <20230810171100.dvnsjgjo67hax4ld@skbuf>
Date: Thu, 10 Aug 2023 20:11:00 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	Sergei Antonov <saproj@...il.com>,
	Florian Fainelli <f.fainelli@...il.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 v2] net: dsa: mv88e6060: add phylink_get_caps
 implementation

On Thu, Aug 10, 2023 at 05:52:41PM +0100, Russell King (Oracle) wrote:
> I wonder whether we have any implementation using SNI mode. I couldn't
> find anything in the in-kernel dts files for this driver, the only
> dts we have is one that was posted on-list recently, and that was using
> MII at 100Mbps:
> 
> https://lore.kernel.org/r/CABikg9zfGVEJsWf7eq=K5oKQozt86LLn-rzMaVmycekXkQEa8Q@mail.gmail.com
> 
> No one would be able to specify "sni" in their dts, so maybe for the
> sake of simplicity, we shouldn't detect whether it's in SNI mode, and
> just use MII, and limit the speed to just 10Mbps?

Based on the fact that "marvell,mv88e6060" is in
dsa_switches_apply_workarounds[], it is technically possible that there
exist boards which use the SNI mode but have no phy-mode and other
phylink properties on the CPU port, and thus they work fine while
skipping phylink. Of course, "possible" != "real".

What I would like is to not discourage the board user to set phy-mode =
"sni" in the device tree if that's what PortMode in the Port Status
Register says that the port is strapped for. I'm afraid that by
intentionally ignoring that bit and putting PHY_INTERFACE_MODE_REVMII in
supported_interfaces, we're kind of suggesting to that person that this
is what is correct, as that is the only thing that would work without
modifying the kernel?

Maybe if we don't want to introduce PHY_INTERFACE_MODE_SNI for fear of a
lack of real users, we could at least detect PortMode=0, and not
populate supported_interfaces, leading to an intentional validation
failure and a comment above that check, stating that phy-mode = "sni" is
not yet implemented?