netdev - Re: [PATCH v2 net 3/4] net: dsa: mv88e6xxx: Never force link on in-band managed MACs

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: <87zfk974br.fsf@waldekranz.com>
Date: Thu, 02 Jan 2025 14:06:32 +0100
From: Tobias Waldekranz <tobias@...dekranz.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: davem@...emloft.net, kuba@...nel.org, andrew@...n.ch,
 f.fainelli@...il.com, olteanv@...il.com, netdev@...r.kernel.org,
 chris.packham@...iedtelesis.co.nz, pabeni@...hat.com, marek.behun@....cz
Subject: Re: [PATCH v2 net 3/4] net: dsa: mv88e6xxx: Never force link on
 in-band managed MACs

On tor, jan 02, 2025 at 10:31, "Russell King (Oracle)" <linux@...linux.org.uk> wrote:
> On Thu, Dec 19, 2024 at 01:30:42PM +0100, Tobias Waldekranz wrote:
>> NOTE: This issue was addressed in the referenced commit, but a
>> conservative approach was chosen, where only 6095, 6097 and 6185 got
>> the fix.
>> 
>> Before the referenced commit, in the following setup, when the PHY
>> detected loss of link on the MDI, mv88e6xxx would force the MAC
>> down. If the MDI-side link was then re-established later on, there was
>> no longer any MII link over which the PHY could communicate that
>> information back to the MAC.
>> 
>>         .-SGMII/USXGMII
>>         |
>> .-----. v .-----.   .--------------.
>> | MAC +---+ PHY +---+ MDI (Cu/SFP) |
>> '-----'   '-----'   '--------------'
>> 
>> Since this a generic problem on all MACs connected to a SERDES - which
>> is the only time when in-band-status is used - move all chips to a
>> common mv88e6xxx_port_sync_link() implementation which avoids forcing
>> links on _all_ in-band managed ports.
>> 
>> Fixes: 4efe76629036 ("net: dsa: mv88e6xxx: Don't force link when using in-band-status")
>> Signed-off-by: Tobias Waldekranz <tobias@...dekranz.com>
>
> I'm feeling uneasy about this change.
>
> The history of the patch you refer to is - original v1:
>
> https://lore.kernel.org/r/20201013021858.20530-2-chris.packham@alliedtelesis.co.nz
>
> When v3 was submitted, it was unchanged:
>
> https://lore.kernel.org/r/20201020034558.19438-2-chris.packham@alliedtelesis.co.nz
>
> Both of these applied the in-band-status thing to all Marvell DSA
> switches, but as Marek states here:
>
> https://lore.kernel.org/r/20201020165115.3ecfd601@nic.cz

Thanks for that context!

> doing so breaks last least one Marvell DSA switch (88E6390). Hence why
> this approach is taken, rather than not forcing the link status on all
> DSA switches.
>
> Your patch appears to be reverting us back to what was effectively in
> Chris' v1 patch from back then, so I don't think we can accept this
> change. Sorry.

Before I abandon this broader fix, maybe you can help me understand
something:

If a user explicitly selects `managed = "in-band-status"`, why would we
ever interpret that as "let's force the MAC's settings according to what
the PHY says"? Is that not what `managed = "auto"` is for?

Could Marek's issue not stem from the fact that his DT stated that the
MAC would pick up the link information in-band, when in fact it was
unable to do so?

Looking at the functional spec for the 6390, it only seems to support
in-band-status when SGMII is used. So would it not be more accurate to
require `managed = "auto"` on boards with 6390 SERDES lanes connected to
an SFP cage, if hot-swap support between SGMII/1000BASE-X/2500BASE-X
transceivers is needed?

I.e. users requiring maximum flexibility can still have it, while
someone with a fixed connection to an SGMII PHY can still get the
benefit of using in-band signaling.