netdev - Re: [PATCH net-next 2/4] net: phy: xpcs: Return error upon RX/TX fault

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]

Date:   Fri, 13 Mar 2020 14:07:31 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Jose Abreu <Jose.Abreu@...opsys.com>
Cc:     netdev@...r.kernel.org, Joao Pinto <Joao.Pinto@...opsys.com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 2/4] net: phy: xpcs: Return error upon RX/TX
 fault

On Fri, Mar 13, 2020 at 02:39:41PM +0100, Jose Abreu wrote:
> RX/TX fault status results in link errors. Return error upon these cases
> so that XPCS can be correctly resumed.

Are you sure about this?  I'm sure that I read in IEEE 802.3 that
a loss of link results in a receive fault being indicated in
status register 2.

See 49.2.14.1 describing "PCS_status" and the descriptions of MDIO
registers 3.1.2 and 3.8.10.  Basically, the link status (3.1.2) is a
latched-low version of PCS_status, and 3.8.10 is an inverted version
of this, independently latched-high.

Returning -EFAULT seems to mean that we'll soft-reset the PHY, and
reconfigure it every time we attempt to read the status whenever the
link is down.

> 
> Signed-off-by: Jose Abreu <Jose.Abreu@...opsys.com>
> 
> ---
> Cc: Jose Abreu <Jose.Abreu@...opsys.com>
> Cc: Andrew Lunn <andrew@...n.ch>
> Cc: Florian Fainelli <f.fainelli@...il.com>
> Cc: Heiner Kallweit <hkallweit1@...il.com>
> Cc: Russell King <linux@...linux.org.uk>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: netdev@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> ---
>  drivers/net/phy/mdio-xpcs.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/mdio-xpcs.c b/drivers/net/phy/mdio-xpcs.c
> index a4cbeecc6d42..23516397b982 100644
> --- a/drivers/net/phy/mdio-xpcs.c
> +++ b/drivers/net/phy/mdio-xpcs.c
> @@ -190,10 +190,14 @@ static int xpcs_read_fault(struct mdio_xpcs_args *xpcs,
>  	if (ret < 0)
>  		return ret;
>  
> -	if (ret & MDIO_STAT2_RXFAULT)
> +	if (ret & MDIO_STAT2_RXFAULT) {
>  		xpcs_warn(xpcs, state, "Receiver fault detected!\n");
> -	if (ret & MDIO_STAT2_TXFAULT)
> +		return -EFAULT;
> +	}
> +	if (ret & MDIO_STAT2_TXFAULT) {
>  		xpcs_warn(xpcs, state, "Transmitter fault detected!\n");
> +		return -EFAULT;
> +	}
>  
>  	ret = xpcs_read_vendor(xpcs, MDIO_MMD_PCS, DW_VR_XS_PCS_DIG_STS);
>  	if (ret < 0)
> -- 
> 2.7.4
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up