netdev - Re: [PATCH v2] 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: <87v9j7em1r.fsf@tarshish>
Date:   Wed, 01 Jul 2020 10:23:12 +0300
From:   Baruch Siach <baruch@...s.co.il>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
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 v2] net: phy: marvell10g: support XFI rate matching mode

Hi Russell,

On Mon, Jun 29 2020, Russell King - ARM Linux admin wrote:
> On Sun, Jun 28, 2020 at 10:04:51AM +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.
>> 
>> Signed-off-by: Baruch Siach <baruch@...s.co.il>
>> ---
>> v2: Move rate matching state read to config_init (RMK)
>
> Not quite what I was after, but it'll do for now.

Thanks for you review.

> My only system which has a 3310 PHY and is configured for rate matching
> (using an XAUI interface, mode 1) seems to be sick - the 3310 no longer
> correctly negotiates on the media side (will only link at 100M/Half and
> only passes traffic in one direction), which makes any development with
> it rather difficult.  Either the media side drivers have failed or the
> magnetics.
>
> I was also hoping for some discussion, as I bought up a few points
> about the 3310's rate matching - unless you have the version with
> MACsec, the PHY expects the host side to rate limit the egress rate to
> the media rate and will _not_ send pause frames.  If you have MACsec,
> and the MACsec hardware is enabled (although may not be encrypting),
> then the PHY will send pause frames to the host as the internal buffer
> fills.

Flow control is disabled anyway in my use case (vpp).

> Then there's the whole question of what phydev->speed etc should be set
> to - the media speed or the host side link speed with the PHY, and then
> how the host side should configure itself.  At least the 88E6390x
> switch will force itself to the media side speed using that while in
> XAUI mode, resulting in a non-functioning speed.  So should the host
> side force itself to 10G whenever in something like XAUI mode?

How does the switch discover the media side speed? Is there some sort of
in-band information exchange?

> What do we do about the egress rate - ignore that statement and hope
> that the 3310 doesn't create bad packets on the wire if we fill up its
> internal buffer?

I will keep that in mind when stress testing the network. We might need
a way to set IPG on the MAC side if the MAC supports that (mvpp2 in this
case).

baruch

>>  drivers/net/phy/marvell10g.c | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>> 
>> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
>> index d4c2e62b2439..a7610eb55f30 100644
>> --- a/drivers/net/phy/marvell10g.c
>> +++ b/drivers/net/phy/marvell10g.c
>> @@ -80,6 +80,8 @@ enum {
>>  	MV_V2_PORT_CTRL		= 0xf001,
>>  	MV_V2_PORT_CTRL_SWRST	= BIT(15),
>>  	MV_V2_PORT_CTRL_PWRDOWN = BIT(11),
>> +	MV_V2_PORT_MAC_TYPE_MASK = 0x7,
>> +	MV_V2_PORT_MAC_TYPE_RATE_MATCH = 0x6,
>>  	/* Temperature control/read registers (88X3310 only) */
>>  	MV_V2_TEMP_CTRL		= 0xf08a,
>>  	MV_V2_TEMP_CTRL_MASK	= 0xc000,
>> @@ -91,6 +93,7 @@ enum {
>>  
>>  struct mv3310_priv {
>>  	u32 firmware_ver;
>> +	bool rate_match;
>>  
>>  	struct device *hwmon_dev;
>>  	char *hwmon_name;
>> @@ -458,7 +461,9 @@ static bool mv3310_has_pma_ngbaset_quirk(struct phy_device *phydev)
>>  
>>  static int mv3310_config_init(struct phy_device *phydev)
>>  {
>> +	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
>>  	int err;
>> +	int val;
>>  
>>  	/* Check that the PHY interface type is compatible */
>>  	if (phydev->interface != PHY_INTERFACE_MODE_SGMII &&
>> @@ -475,6 +480,12 @@ static int mv3310_config_init(struct phy_device *phydev)
>>  	if (err)
>>  		return err;
>>  
>> +	val = phy_read_mmd(phydev, MDIO_MMD_VEND2, MV_V2_PORT_CTRL);
>> +	if (val < 0)
>> +		return val;
>> +	priv->rate_match = ((val & MV_V2_PORT_MAC_TYPE_MASK) ==
>> +			MV_V2_PORT_MAC_TYPE_RATE_MATCH);
>> +
>>  	/* Enable EDPD mode - saving 600mW */
>>  	return mv3310_set_edpd(phydev, ETHTOOL_PHY_EDPD_DFLT_TX_MSECS);
>>  }
>> @@ -581,6 +592,17 @@ static int mv3310_aneg_done(struct phy_device *phydev)
>>  
>>  static void mv3310_update_interface(struct phy_device *phydev)
>>  {
>> +	struct mv3310_priv *priv = dev_get_drvdata(&phydev->mdio.dev);
>> +
>> +	/* In "XFI with Rate Matching" mode the PHY interface is fixed at
>> +	 * 10Gb. The PHY adapts the rate to actual wire speed with help of
>> +	 * internal 16KB buffer.
>> +	 */
>> +	if (priv->rate_match) {
>> +		phydev->interface = PHY_INTERFACE_MODE_10GBASER;
>> +		return;
>> +	}
>> +
>>  	if ((phydev->interface == PHY_INTERFACE_MODE_SGMII ||
>>  	     phydev->interface == PHY_INTERFACE_MODE_2500BASEX ||
>>  	     phydev->interface == PHY_INTERFACE_MODE_10GBASER) &&
>> -- 
>> 2.27.0
>> 
>> 


-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@...s.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -