netdev - Re: [PATCH v4 net-next 3/8] net: phy: bcm84881: move the in-band capability check where it belongs

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y4YSMV3y6kA1VJ5F@lunn.ch>
Date:   Tue, 29 Nov 2022 15:07:45 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        UNGLinuxDriver@...rochip.com,
        bcm-kernel-feedback-list@...adcom.com,
        Madalin Bucur <madalin.bucur@....nxp.com>,
        Camelia Groza <camelia.groza@....com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Maxim Kochetkov <fido_max@...ox.ru>,
        Sean Anderson <sean.anderson@...o.com>,
        Antoine Tenart <atenart@...nel.org>,
        Michael Walle <michael@...le.cc>,
        Raag Jadav <raagjadav@...il.com>,
        Siddharth Vadapalli <s-vadapalli@...com>,
        Ong Boon Leong <boon.leong.ong@...el.com>,
        Colin Foster <colin.foster@...advantage.com>,
        Marek Behun <marek.behun@....cz>
Subject: Re: [PATCH v4 net-next 3/8] net: phy: bcm84881: move the in-band
 capability check where it belongs

> > However, I've just tripped over some information in the 88E1111
> > manual which states that in SGMII mode, if bypass mode is used, then
> > the PHY will apparently renegotiate on the copper side advertising
> > 1000baseT HD and FD only, no pause. So I checked what my link partner
> > is seeing, and it was seeing the original advertisement.
> > 
> > So I then triggered a renegotiate from the partner, and it now shows
> > only 1000baseT/Half 1000baseT/Full being advertised by the 88E1111.
> > Reading the advertisement register, it still contains 0x0d41, which
> > shows pause modes, 100FD, 10FD - so the advertisement register doesn't
> > reflect what was actually adfertised in this case.
> > 
> > Also, presumably, based on this observation, it will only renegotiate
> > if the copper side hadn't resolved to gigabit. If correct, what this
> > means is that when operating in SGMII mode, the the PHY becomes
> > gigabit-only if bypass mode gets used.
> > 
> > Given this behaviour, the fact that it switches to gigabit only when
> > the SGMII side enters bypass mode, I think we should _positively_ be
> > disabling inband bypass in SGMII mode. This change in advertisement
> > is not what phylib would expect, and I suspect could lead to surprises
> > e.g. if phylib was told to advertise non-gigabit speeds only.

Sorry, i've not been reading this thread.

This behaviour is not very nice. So i agree, it should be avoided if
possible.

> > Thanks for testing. So that means m88e1111_config_init_sgmii() will not
> > enable in-band if it was previously disabled. So we need to check the
> > fiber ANENABLE bit and unconditionally return PHY_AN_INBAND_OFF if it is
> > clear before evaluating anything else.
> > 
> > Also, given this behaviour of bypass mode, it seems it would only make
> > sense if the PHY were operating in 1000base-X mode, which we don't do
> > with SFPs, so maybe it makes no sense to support the ON_TIMEOUT as an
> > option right now - and as I say above, maybe we should be focing the
> > AN bypass allow bit to be clear in SGMII mode.
> > 
> > I think maybe Andrew needs to be involved in that last bit though.

As you say, 1000Base-X does not make much sense for a copper
SFP. SGMII does odd things, so just not supporting ON_TIMEOUT seems
reasonable.

	Andrew