netdev - Re: [PATCH net-next] net: dsa: mark parsed interface mode for legacy switch drivers

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] [day] [month] [year] [list]

Date: Fri, 18 Aug 2023 19:49:44 +0300
From: Vladimir Oltean <olteanv@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Linus Walleij <linus.walleij@...aro.org>,
	Heiner Kallweit <hkallweit1@...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] net: dsa: mark parsed interface mode for legacy
 switch drivers

On Fri, Aug 18, 2023 at 05:21:05PM +0300, Vladimir Oltean wrote:
> On Fri, Aug 18, 2023 at 02:10:21PM +0100, Russell King (Oracle) wrote:
> > > > The second patch is my patch adding a phylink_get_caps method for
> > > > Realtek drivers - should that allow all "rgmii" interface types,
> > > > or do we want to just allow "rgmii" to encourage the use of the
> > > > [tr]x-internal-delay-ps properties?
> > > 
> > > Same opinion as above. As long as it's understood that the RTL8366RB
> > > MAC, like any other MAC, shouldn't be acting upon the phy-mode when
> > > adding delays, let's just accept all 4 variants, with future support to
> > > be added for [rt]x-internal-delay-ps if there turn out to be
> > > configurable MAC-side delays present.
> > 
> > Yes, I think you're right, because we could have the situation where
> > the CPU side is adding the delays, and the DSA side is not, which
> > should be described in DT as:
> > 
> > 	phy-mode = "rgmii-id";
> > 
> > on the DSA side, and e.g.:
> > 
> > 	phy-mode = "rgmii";
> > 	rx-internal-delay-ps = <2000>;
> > 	tx-internal-delay-ps = <2000>;
> > 
> > on the CPU side. Yes?
> 
> Yes, this is the situation I was thinking of, where the DSA CPU port
> would have "rgmii-id" to denote that the remote side has added the
> delays.
> 
> At least, that would be the intuitive way for me to describe things
> according to our definitions from the documentation.
> 
> (open question: if those remote delays are added through pinctrl-gmii
> rather than through the MAC OF node, would that count towards DSA's
> phy-mode, to make it rgmii-id, or not?)
> 
> Though I agree that I can't see the exact phy-mode breaking anything
> with a fixed link, given this interpretation, even if it's "wrong".

I haven't fully digested this, but would it make sense to say:
"for fixed links, only phy-mode 'rgmii' should be used, since the remote
side is not known, and thus, it is also not known whether it has set up
clock skews in any direction"?

One possible advantage would be that it would make people think a bit
more whether they should add code that applies MAC-side delays in
fixed-link mode based on the phy-mode. If we had that extra recommendation
documented somewhere centrally, doing that wouldn't make a lot of sense.