netdev - Re: [PATCH net-next 3/6] net: dsa: add support for retrieving the interface mode

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: <20220723192655.46de7cae@thinkpad>
Date:   Sat, 23 Jul 2022 19:26:55 +0200
From:   Marek Behún <kabel@...nel.org>
To:     Vladimir Oltean <olteanv@...il.com>,
        "Russell King (Oracle)" <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Alvin __ipraga <alsi@...g-olufsen.dk>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Daniel Scally <djrscally@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        DENG Qingfang <dqfext@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        George McCollister <george.mccollister@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Landen Chao <Landen.Chao@...iatek.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        linux-acpi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org,
        Matthias Brugger <matthias.bgg@...il.com>,
        netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Sean Wang <sean.wang@...iatek.com>,
        UNGLinuxDriver@...rochip.com,
        Vivien Didelot <vivien.didelot@...il.com>,
        Woojung Huh <woojung.huh@...rochip.com>
Subject: Re: [PATCH net-next 3/6] net: dsa: add support for retrieving the
 interface mode

On Sat, 23 Jul 2022 01:39:32 +0300
Vladimir Oltean <olteanv@...il.com> wrote:

> You mean in general, or with the firmware description you posted above?
> Because the Lynx PCS does the best it can (considering it does this from
> a function that returns void) to complain that you shouldn't put
> MLO_AN_INBAND for 2500base-x.
> 
> static void lynx_pcs_link_up_2500basex(struct mdio_device *pcs,
> 				       unsigned int mode,
> 				       int speed, int duplex)
> {
> 	u16 if_mode = 0;
> 
> 	if (mode == MLO_AN_INBAND) {
> 		dev_err(&pcs->dev, "AN not supported for 2500BaseX\n");
> 		return;
> 	}
> 
> I noticed just earlier today that I made a blunder while upstreaming some
> riser cards for some LS1028A-QDS development boards, and I did just that
> (left 2500base-x with in-band-status). But the system just errors out.
> I need to boot a board and fix that up. They're just NXP development
> systems so not a big issue. Otherwise I'm not aware of what you're
> talking about.
> 
> > However, both will request link status from the PCS side and use that
> > to determine whether the link is up, and use the parameters that the
> > PCS code returns for the link. Since 2500base-X can only operate at
> > 2.5G, PCS code always reports SPEED_2500, and as half duplex is
> > virtually never supported above 1G, DUPLEX_FULL.  
> 
> If you're saying this just because Lynx implements pcs_get_state for
> 2500base-x, it's extremely likely that this simply originates from
> vsc9959_pcs_link_state_2500basex(), which was deleted in ocelot in
> commit 588d05504d2d ("net: dsa: ocelot: use the Lynx PCS helpers in
> Felix and Seville"), and it was always dead code. It wasn't the only
> dead code, remember commit b4c2354537b4 ("net: dsa: felix: delete
> .phylink_mac_an_restart code").
> 
> Since the Lynx PCS prints error messages in inband/2500base-x mode,
> and so did Felix/Ocelot before the code became common, I'm pretty sure
> no one relies on this mode.

Does Lynx PCS support 1000base-x with AN? Because if so, it may be
possible to somehow hack working AN for 2500base-x, as I managed it for
88E6393X in the commit I mentioned (by configuring 1000base-x and then
hacking the PHY speed to 2.5x).

Anyway, I am now looking at the standards, and it seems that all the X
and R have K variant: 1000base-kx, 2500base-kx, 5gbase-kr and
10gbase-kr. These modes have mandatory clause 73 autonegotiation.

So either we need to add these as different modes of the
phy_interface_t type, or we need to differentiate whether clause 37 or
clause 73 AN should be used by another property.

But since 1000base-x supports clause 37 and 1000base-kx clause 73, the
one property that we have, managed="in-band-status" is not enough, if
we keep calling both modes '1000base-x'.

So maybe we really need to add K variants as separate
PHY_INTERFACE_MODED_ constants. That way we can keep assuming clause 37
for 2500base-x, and try to implement it for as much drivers as
possible, by hacking it up...

And I still don't understand this clause 73 AN at all. For example, if
one PHY supports only up to 2.5g speeds, will it complete AN with
another PHY that supports up to 10g speeds, if the second PHY will
(maybe?) try at higher frequency?

Marek