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: <cfe3d910-adee-a3bf-96e2-ce1c10109e58@seco.com>
Date:   Thu, 18 Aug 2022 13:28:19 -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 1:12 PM, Vladimir Oltean wrote:
> On Thu, Aug 18, 2022 at 01:03:54PM -0400, Sean Anderson wrote:
>> Well, I suppose the real reason is that this will cause a merge conflict
>> (or lack of one), since this series introduces phylink_interface_max_speed
>> in patch 7, which is supposed to contain all the phy modes. So depending on
>> what gets merged first, the other series will have to be modified and resent.
>> 
>> To be honest, I had expected that trivial patches like that would have been
>> applied and merged already.
> 
> There's nothing trivial about this patch.

Perhaps "limited in scope and mostly independent" is better, then.

> 1000Base-KX is not a phy-mode
> in exactly the same way that 1000Base-T isn't, either. 

It has different AN from 1000BASE-X (c73 vs c37), and doesn't support half
duplex. This is something the serdes and PCS have to care about. Unfortunately,
we don't have a separate PCS_INTERFACE_MODE so these things become
PHY_INTERFACE_MODEs.

> If you want to
> bring PHY_INTERFACE_MODE_10GKR as a "yes, but" counterexample, it was
> later clarified that 10gbase-r was what was actually meant in that case,
> and we keep 10gbase-kr as phy-mode only for compatibility with some
> device trees.

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'd suggest resolving the merge conflict without 1000Base-KX and
> splitting off a separate discussion about this topic. Otherwise it will
> unnecessarily detract from PAUSE-based rate adaptation.

Well, no one is using it yet, so hopefully it will not be a problem...

--Sean