netdev - Re: Convention regarding SGMII in-band autonegotiation

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]

Date:   Tue, 4 Apr 2023 08:31:12 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Daniel Golle <daniel@...rotopia.org>, netdev@...r.kernel.org,
        Vladimir Oltean <vladimir.oltean@....com>,
        "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
        Alexander 'lynxis' Couzens <lynxis@...0.eu>,
        Chukun Pan <amadeus@....edu.cn>
Cc:     John Crispin <john@...ozen.org>
Subject: Re: Convention regarding SGMII in-band autonegotiation

On 04.04.2023 02:29, Daniel Golle wrote:
> Hi!
> 
> I've been dealing with several SGMII TP PHYs, some of them with support
> for 2500Base-T, by switching to 2500Base-X interface mode (or by using
> rate-adaptation to 2500Base-X or proprietary HiSGMII).
> 
> Dealing with such PHYs in MAC-follow-PHY-rate-mode (ie. not enabling
> rate-adaptation which is worth avoiding imho) I've noticed that the
> current behavior of PHY and MAC drivers in the kernel is not as
> consistent as I assumed it would be.
> 
> Background:
>>>From Russell's comments and the experiments carried out together with
> Frank Wunderlich for the MediaTek SGMII PCS found in mtk_eth_soc I
> understood that in general in-band autonegotiation should always be
> switched off unless phylink_autoneg_inband(mode) returns true, ie.
> mostly in case 'managed = "in-band-status";' is set in device tree,
> which is generally the case for SFP cages or PHYs which are not
> accessible via MDIO.
> 
> As of today this is what pcs-mtk-lynxi is now doing as this behavior
> was inherited from the implementation previously found at
> drivers/net/ethernet/mediatek/mtk_sgmii.c.
> 
> Hence, with MLO_AN_PHY we are expecting both MAC and PHY to *not* use
> in-band autonegotiation. It is not needed as we have out-of-band status
> using MDIO and maybe even an interrupt to communicate the link status
> between the two. Correct so far?
> 
> I've also previously worked around this using Vladimir Oltean's patch
> series introducing sync'ing and validation of in-band-an modes between
> MAC and PHY -- however, this turns out to be overkill in case the
> above is true and given there is a way to always switch off in-band-an
> on both, the MAC and the PHY.
> 
> Or should PHY drivers setup in-band AN according to
> pl->config->ovr_an_inband...?
> 
> Also note that the current behavior of PHY drivers is that consistent:
> 
>  * drivers/net/phy/mxl-gpy.c
>    This goes through great lengths to switch on inband-an when initially
>    coming up in SGMII mode, then switches is off when switching to
>    2500Base-X mode and after that **never switches it on again**. This
>    is obviously not correct and the driver can be greatly reduced if
>    dropping all that (non-)broken logic.
>    Would a patch like [1] this be acceptable?
> 
>  * drivers/net/phy/realtek.c
>    The driver simply doesn't do anything about in-band-an and hence looks
>    innocent. However, all RTL8226* and RTL8221* PHYs enable in-band-an
>    by default in SGMII mode after reset. As many vendors use rate-adapter-
>    mode, this only surfaces if not using the rate-adapter and having the
>    MAC follow the PHY mode according to speed, as we do using [2] and [3].
> 
These PHY's are supported as internal PHY's in RTL8125 MAC/PHY chips
where the MAC/PHY communication is handled chip-internally.
Other use cases are not officially supported (yet), also due to lack of
public datasheets.

>    SGMII in-band AN can be switched off using a magic sequence carried
>    out on undocumented registers [5].
> 
>    Would patches [2], [3], [4], [5] be acceptable?
> 
Ideas from the patches can be re-used. Some patches itself are not ready
for mainline (replace magic numbers with proper constants (as far as
documented by Realtek), inappropriate use of phy_modify_mmd_changed,
read_status() being wrong place for updating interface mode).

> 
> Thank you for your advise!
> 
> 
> Daniel
> 
> [1]: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob_plain;f=target/linux/mediatek/patches-5.15/731-net-phy-hack-mxl-gpy-disable-sgmii-an.patch;hb=HEAD
> [2]: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob_plain;f=target/linux/generic/pending-5.15/721-net-phy-realtek-rtl8221-allow-to-configure-SERDES-mo.patch;hb=HEAD
> [3]: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob_plain;f=target/linux/generic/pending-5.15/722-net-phy-realtek-support-switching-between-SGMII-and-.patch;hb=HEAD
> [4]: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob_plain;f=target/linux/generic/pending-5.15/724-net-phy-realtek-use-genphy_soft_reset-for-2.5G-PHYs.patch;hb=HEAD
> [5]: https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob_plain;f=target/linux/generic/pending-5.15/725-net-phy-realtek-disable-SGMII-in-band-AN-for-2-5G-PHYs.patch;hb=HEAD