netdev - Re: [PATCH] net: phy: marvell10g: support XFI rate matching 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: <20200625175012.GL1551@shell.armlinux.org.uk>
Date:   Thu, 25 Jun 2020 18:50:12 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Baruch Siach <baruch@...s.co.il>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Shmuel Hazan <sh@...s.co.il>
Subject: Re: [PATCH] net: phy: marvell10g: support XFI rate matching mode

On Thu, Jun 25, 2020 at 08:19:21PM +0300, Baruch Siach wrote:
> When the hardware MACTYPE hardware configuration pins are set to "XFI
> with Rate Matching" the PHY interface operate at fixed 10Gbps speed. The
> MAC buffer packets in both directions to match various wire speeds.
> 
> Read the MAC Type field in the Port Control register, and set the MAC
> interface speed accordingly.

Rate matching brings with it a whole host of issues, not just the
interface type, but also the phydev->speed, which is commonly used
to program the MAC.

Rate matching is also used with the unsupported 3310 PHY on the ZII
devel rev C board, and there we need the PHY to also report a speed
of 10G as well as the interface type correctly.  The whole thing
gets quite yucky when you have a 10baseT link on a 3310 PHY with
the host interface running with 10GBASE-R but the MAC programmed for
10Mbps.

The approach I hacked up was to split the current link_state into
media_state and mac_state, and then do this:

+       /* If the PHY supports rate-matching, it will report slower speeds
+        * for these fixed-speed interface modes. Force the MAC side to
+        * full speed.
+        */
+       if (mac_state.interface == PHY_INTERFACE_MODE_XAUI ||
+           mac_state.interface == PHY_INTERFACE_MODE_RXAUI ||
+           mac_state.interface == PHY_INTERFACE_MODE_10GBASER) {
+               mac_state.speed = SPEED_10000;
+               mac_state.duplex = DUPLEX_FULL;
+       }

which is really a dirty hack.  I'd need to re-read the switch
documentation and review what we're doing to check whether such a
thing is really necessary, or whether merely using 10GBASE-R but
programming the MAC to 10Mbps (e.g.) is actually acceptable.

What I'm basically saying is, there could be way more to this than
just setting the interface mode.

We also /should/ be setting the 3310's interface mode according to
how the PHY is configured - but that is slightly complicated by the
various different modes presented by different variants of the PHY
(the P variant vs the non-P variant.)  I seem to remember that
disambiguating them requires looking at the revision bits in the
PHY ID registers, and I've been debating whether we should not be
using the standard mask when matching them.

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