linux-kernel - Re: [PATCH v3 02/11] net: phy: Add 1000BASE-KX 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: <b858932a-3e34-7365-f64b-63decfe83b41@seco.com>
Date:   Thu, 18 Aug 2022 16:40:37 -0400
From:   Sean Anderson <sean.anderson@...o.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Alexandru Marginean <alexandru.marginean@....com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 02/11] net: phy: Add 1000BASE-KX interface mode

On 8/18/22 3:51 PM, Vladimir Oltean wrote:
> On Thu, Aug 18, 2022 at 01:28:19PM -0400, Sean Anderson wrote:
>> That's not what's documented:
>> 
>> > ``PHY_INTERFACE_MODE_10GBASER``
>> >     This is the IEEE 802.3 Clause 49 defined 10GBASE-R protocol used with
>> >     various different mediums. Please refer to the IEEE standard for a
>> >     definition of this.
>> > 
>> >     Note: 10GBASE-R is just one protocol that can be used with XFI and SFI.
>> >     XFI and SFI permit multiple protocols over a single SERDES lane, and
>> >     also defines the electrical characteristics of the signals with a host
>> >     compliance board plugged into the host XFP/SFP connector. Therefore,
>> >     XFI and SFI are not PHY interface types in their own right.
>> > 
>> > ``PHY_INTERFACE_MODE_10GKR``
>> >     This is the IEEE 802.3 Clause 49 defined 10GBASE-R with Clause 73
>> >     autonegotiation. Please refer to the IEEE standard for further
>> >     information.
>> > 
>> >     Note: due to legacy usage, some 10GBASE-R usage incorrectly makes
>> >     use of this definition.
>> 
>> so indeed you get a new phy interface mode when you add c73 AN. The
>> clarification only applies to *incorrect* usage.
> 
> I challenge you to the following thought experiment. Open clause 73 from
> IEEE 802.3, and see what is actually exchanged through auto-negotiation.
> You'll discover that the *use* of the 10GBase-KR operating mode is
> *established* through clause 73 AN (the Technology Ability field).
> 
> So what sense does it make to define 10GBase-KR as "10Base-R with clause 73 AN"
> as the document you've quoted does?None whatsoever. The K in KR stands
> for bacKplane, and typical of this type of PMD are the signaling and
> link training procedures described in the previous clause, 72.
> Clause 73 AN is not something that is a property of 10GBase-KR, but
> something that exists outside of it.

You should send a patch; this document is Documentation/networking/phy.rst

> So if clause 73 *establishes* the use of 10GBase-KR (or 1000Base-KX or
> others) through autonegotiation, then what sense does it have to put
> phy-mode = "1000base-kx" in the device tree? Does it mean "use C73 AN",
> or "don't use it, I already know what operating mode I want to use"?
> 
> If it means "use C73 AN", then what advertisement do you use for the
> Technology Ability field? There's a priority resolution function for
> C73, just like there is one for C28/C40 for the twisted pair medium (aka
> that thing that allows you to fall back to the highest supported common
> link speed). So why would you populate just one bit in Technology
> Ability based on DT, if you can potentially support multiple operating
> modes? And why would you even create your advertisement based on the
> device tree, for that matter? Twisted pair PHYs don't do this.

The problem is that our current model looks something like

1. MAC <--               A              --> phy (ethernet) --> B <-- far end
2. MAC <-> "PCS" <-> phy (serdes) --> C <-- phy (ethernet) --> B <-- far end
3.                                --> C <-- transciever    --> B <-- far end
4.                                -->           D                <-- far end

Where 1 is the traditional MAC+phy architecture, 2 is a MAC connected to
a phy over a serial link, 3 is a MAC connected to an optical
transcievber, and 4 is a backplane connection. A is the phy interface
mode, and B is the ethtool link mode. C is also the "phy interface
mode", except that sometimes it is highly-dependent on the link mode
(e.g. 1000BASE-X) and sometimes it is not (SGMII). The problem is case
4. Here, there isn't really a phy interface mode; just a link mode.

Consider the serdes driver. It has to know how to configure itself.
Sometimes this will be the phy mode (cases 2 and 3), and sometimes it
will be the link mode (case 4). In particular, for a link mode like
1000BASE-SX, it should be configured for 1000BASE-X. But for
1000BASE-KX, it has to be configured for 1000BASE-KX. I suppose the
right thing to do here is rework the phy subsystem to use link modes and
not phy modes for phy_mode_ext, since it seems like there is a
1000BASE-X link mode. But what should be passed to mac_prepare and
mac_select_pcs?

As another example, consider the AQR113C. It supports the following
(abbreviated) interfaces on the MAC side:

- 10GBASE-KR
- 1000BASE-KX
- 10GBASE-R
- 1000BASE-X
- USXGMII
- SGMII

This example of what phy-mode = "1000base-kx" would imply. I would
expect that selecting -KX over -X would change the electrical settings
to comply with clause 70 (instead of the SFP spec).

--Sean