linux-kernel - Re: [PATCH RFC net-next] net: phylink: always config mac for (delayed) phy

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]
Message-ID: <Z31CJS1YUvPGiEXs@shell.armlinux.org.uk>
Date: Tue, 7 Jan 2025 15:03:01 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Eric Woudstra <ericwouds@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Frank Wunderlich <frank-w@...lic-files.de>,
	Daniel Golle <daniel@...rotopia.org>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, netfilter-devel@...r.kernel.org,
	coreteam@...filter.org, bridge@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH RFC net-next] net: phylink: always config mac for
 (delayed) phy

On Tue, Jan 07, 2025 at 02:14:03PM +0100, Eric Woudstra wrote:
> 
> 
> On 1/7/25 1:47 PM, Russell King (Oracle) wrote:
> > Going through the log...
> > 
> > On Tue, Jan 07, 2025 at 01:36:15PM +0100, Eric Woudstra wrote:
> >> Log before this patch is applied:
> >> [root@...r3 ~]# dmesg | grep eth1
> >> [    2.515179] mtk_soc_eth 15100000.ethernet eth1: mediatek frame engine at 0xffff800082380000, irq 123
> >> [   38.271431] mtk_soc_eth 15100000.ethernet eth1: configuring for inband/2500base-x link mode
> >> [   38.279828] mtk_soc_eth 15100000.ethernet eth1: major config, requested inband/2500base-x
> >> [   38.288009] mtk_soc_eth 15100000.ethernet eth1: interface 2500base-x inband modes: pcs=01 phy=00
> >> [   38.296800] mtk_soc_eth 15100000.ethernet eth1: major config, active inband/inband,an-disabled/2500base-x
> > 
> > This is indeed without the PHY. We're using inband, although the PCS
> > mode is PHYLINK_PCS_NEG_INBAND_DISABLED, meaning inband won't be
> > used. As there is no PHY, we can't switch to MLO_AN_PHY.
> > 
> >> [   38.306362] mtk_soc_eth 15100000.ethernet eth1: phylink_mac_config: mode=inband/2500base-x/none adv=00,00000000,00008000,0000e240 pause=04
> >> [   39.220149] mtk_soc_eth 15100000.ethernet eth1:  interface 2 (mii) rate match none supports 0-3,6-7,13-14
> >> [   39.229758] mtk_soc_eth 15100000.ethernet eth1:  interface 3 (gmii) rate match none supports 0-3,5-7,13-14
> >> [   39.239420] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
> >> [   39.249173] mtk_soc_eth 15100000.ethernet eth1:  interface 22 (1000base-x) rate match none supports 5-7,13-14
> >> [   39.259080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
> >> [   39.594676] mtk_soc_eth 15100000.ethernet eth1: PHY i2c:sfp-1:11 uses interfaces 4,23, validating 4,23
> > 
> > The PHY comes along...
> > 
> >> [   39.603992] mtk_soc_eth 15100000.ethernet eth1:  interface 4 (sgmii) rate match none supports 0-3,5-7,13-14
> >> [   39.650080] mtk_soc_eth 15100000.ethernet eth1:  interface 23 (2500base-x) rate match none supports 6-7,13-14,47
> >> [   39.660266] mtk_soc_eth 15100000.ethernet eth1: PHY [i2c:sfp-1:11] driver [RTL8221B-VB-CG 2.5Gbps PHY (C45)] (irq=POLL)
> >> [   39.671037] mtk_soc_eth 15100000.ethernet eth1: phy: 2500base-x setting supported 00,00000000,00008000,000060ef advertising 00,00000000,00008000,000060ef
> >> [   39.684761] mtk_soc_eth 15100000.ethernet eth1: requesting link mode inband/2500base-x with support 00,00000000,00008000,000060ef
> > 
> > We decide to use MLO_AN_INBAND and 2500base-X, which we're already using.
> > 
> >> [   40.380076] mtk_soc_eth 15100000.ethernet eth1: phy link down 2500base-x/Unknown/Unknown/none/off
> >> [   40.397090] brlan: port 5(eth1) entered blocking state
> >> [   40.402223] brlan: port 5(eth1) entered disabled state
> >> [   40.407437] mtk_soc_eth 15100000.ethernet eth1: entered allmulticast mode
> >> [   40.414400] mtk_soc_eth 15100000.ethernet eth1: entered promiscuous mode
> >> [   44.500077] mtk_soc_eth 15100000.ethernet eth1: phy link up 2500base-x/2.5Gbps/Full/none/off
> >> [   44.508528] mtk_soc_eth 15100000.ethernet eth1: No phy led trigger registered for speed(2500)
> > 
> > ... but we don't see link-up reported by the PCS after the PHY comes
> > up. Why is that - I think that needs investigation before we proceed
> > to patch the issue, because that suggests the PCS isn't seeing
> > valid 2500base-X from the PHY.
> > 
> 
> I think it is because pl->act_link_an_mode stays at MLO_AN_INBAND, but
> it needs to be set to MLO_AN_PHY, so that only the phy determines the
> link state:
> 
> phylink_resolve() {
>     ...
> 	} else if (pl->act_link_an_mode == MLO_AN_PHY) {
> 		link_state = pl->phy_state;
>     ...
> }

Please see my reply to Daniel. The PCS should still be capable of
reporting whether its link is up or down irrespective of whether
in-band status is being used or not.

While it is correct that PHY mode needs to be used here, your report
has pointed out that the driver is not reporting the PCS link state
correctly when in-band is disabled.

Given that the current state of affairs has revealed this other bug,
I would like that addressed first while there is a trivial test case
here.

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