netdev - Re: [PATCH] net: phy: at803x: the Atheros 8031 supports pause frames

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: <f34cd143-bf04-c17f-b3d6-e3077715a72a@gmail.com>
Date:   Thu, 27 Oct 2016 15:12:54 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Timur Tabi <timur@...eaurora.org>, netdev@...r.kernel.org,
        zefir.kurtisi@...atec.com, scampbel@...eaurora.org,
        alokc@...eaurora.org, shankerd@...eaurora.org, andrew@...n.ch
Subject: Re: [PATCH] net: phy: at803x: the Atheros 8031 supports pause frames

On 10/27/2016 03:05 PM, Timur Tabi wrote:
> The Atheros 8031 PHY supports the 802.3 extension for symmetric and
> asymmetric pause frames, so set that to the list of features supported
> by the phy.
> 
> Signed-off-by: Timur Tabi <timur@...eaurora.org>
> ---
> 
> Without this patch, my NIC (the Qualcomm EMAC) receives a lot of frame
> check sequence (aka CRC) errors, resulting in about 10% packet loss.

Hu? In my experience that should not come from supporting Pause frames
or not, but rather properly configuring a (RG)MII delay, but your
mileage may vary.

> Can someone help me understand why?  Because of this patch, I can't use
> the generic phy driver in phylib.  Why would a MAC controller require
> its PHY to support pause frames?

It does not, support for Pause frames is a MAC-level feature, yet,
PHYLIB (and that's been on my todo for a while now) insists on reporting
the confusing phydev->pause and phydev->asym_pause, which really is what
has been determined from auto-negotiating with your partner, as opposed
to being a supported thing or not. The PHY has absolutely not business
in that.

Your change should probably be in the Ethernet MAC driver, when you have
successfully connected to the PHY, update phydev->supported to include
SUPPORTED_Pause | SUPPORTED_Asym_pause.

> 
>  drivers/net/phy/at803x.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
> index a52b560..fb80413 100644
> --- a/drivers/net/phy/at803x.c
> +++ b/drivers/net/phy/at803x.c
> @@ -440,7 +440,8 @@ static struct phy_driver at803x_driver[] = {
>  	.get_wol		= at803x_get_wol,
>  	.suspend		= at803x_suspend,
>  	.resume			= at803x_resume,
> -	.features		= PHY_GBIT_FEATURES,
> +	.features		= PHY_GBIT_FEATURES |
> +				  SUPPORTED_Pause | SUPPORTED_Asym_Pause,
>  	.flags			= PHY_HAS_INTERRUPT,
>  	.config_aneg		= genphy_config_aneg,
>  	.read_status		= genphy_read_status,
> 


-- 
Florian