netdev - Re: genphy_read_status() vs. 1000bT Pause capability

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-next>] [day] [month] [year] [list]

Date:   Tue, 28 Mar 2017 16:17:18 +1100
From:   Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>
Cc:     netdev@...r.kernel.org, Russell King <rmk+kernel@...linux.org.uk>
Subject: Re: genphy_read_status() vs. 1000bT Pause capability

On Mon, 2017-03-27 at 21:14 -0700, Florian Fainelli wrote:
> > Doesn't fix my other problem with Pause in 1000bT land. Do you know if
> > that way of reflecting the pause capability by hijacking the old
> > LPA bits is widely implemented enough that we should put it in
> > genphy_read_status() ?
> 
> Not sure I follow you here? The link partner pause capability is
> reflected in phydev->pause and phydev->asym_pause (yes, these are
> terrible names) when the link is established. 

Right. The problem is that they aren't for some gigabit links :-)

Basically, in my setup with a PHY which uses genphy_read_status()
(like most of them), I never get those advertised despite the fact
that, I *think* they are supported by the other end (even after fixing
my side of the advertising).

I added a printk inside genphy_read_status() to inspect the result
of the negociation, and this is what I read:

lpa=c1e1 lpagb=3800 adv=de1 common_adv=1e1 common_adv_gb=800

As you can see, LPA doesn't have the Pause bits. *However* it does
have bit 0x80 which can mean ADVERTISE_100HALF, but according to
our own mii.h can also mean ADVERTISE_1000XPAUSE. Similarily it 
has bit 0x100 which can mean ADVERTISE_100FULL but also can mean
ADVERTISE_1000XPSE_ASYM.

In fact we appear to have two functions to interpret them as
such inn the non-uapi mii.h:

	ethtool_adv_to_mii_adv_x
	mii_adv_to_ethtool_adv_x

However they aren't used much in the tree and not at all by the
"genphy" code.

So my question is... when we observe that we have a 1000 link
established, should we use these to "interpret" the LPA bits as
above ?

As it is, we never seem to advertise the capability because we never
decode the above (tried with 2 different PHYs, a Realtek and a
Broadcom) while my Cisco switches, I think, do support Pause.

Hence my question ... how "standard" is the re-use of the LPA bits
for these alternate meanings in 1000bT and should we update genphy
to perform that decoding ?

(I'm trying to download the 802.3 document referenced in the phy.txt
to see if it says anything about it but it's taking forever for some
reason).

Cheers,
Ben.