| 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
| ||
|
Message-ID: <130bb780-0dc1-3819-8f6d-f2daf4d9ece9@huawei.com>
Date: Thu, 31 Mar 2022 21:57:06 +0800
From: "huangguangbin (A)" <huangguangbin2@...wei.com>
To: Andrew Lunn <andrew@...n.ch>
CC: <davem@...emloft.net>, <kuba@...nel.org>,
<o.rempel@...gutronix.de>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <lipeng321@...wei.com>,
<chenhao288@...ilicon.com>
Subject: Re: [PATCH] net: phy: genphy_loopback: fix loopback failed when speed
is unknown
On 2022/3/31 20:23, Andrew Lunn wrote:
> On Thu, Mar 31, 2022 at 07:48:19PM +0800, Guangbin Huang wrote:
>> If phy link status is down because link partner goes down, the phy speed
>> will be updated to SPEED_UNKNOWN when autoneg on with general phy driver.
>> If test loopback in this case, the phy speed will be set to 10M. However,
>> the speed of mac may not be 10M, it causes loopback test failed.
>>
>> To fix this problem, if speed is SPEED_UNKNOWN, don't configure link speed.
>
> I don't think this explanation is correct.
>
> If speed is UNKNOWN, ctl is just going to have BMCR_LOOPBACK set. That
> is very similar to what you are doing. The code then waits for the
> link to establish. This is where i guess your problem is. Are you
> seeing ETIMEDOUT? Does the link not establish?
>
> Thanks
> Andrew
> .
>
Hi Andrew
This problem is not timeout, I have print return value of phy_read_poll_timeout()
and it is 0.
In this case, as speed and duplex both are unknown, ctl is just set to 0x4000.
However, the follow code sets mask to ~0 for function phy_modify():
int genphy_loopback(struct phy_device *phydev, bool enable)
{
if (enable) {
...
phy_modify(phydev, MII_BMCR, ~0, ctl);
...
}
so all other bits of BMCR will be cleared and just set bit 14, I use phy trace to
prove that:
$ cat /sys/kernel/debug/tracing/trace
# tracer: nop
#
# entries-in-buffer/entries-written: 923/923 #P:128
#
# _-----=> irqs-off/BH-disabled
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / _-=> migrate-disable
# |||| / delay
# TASK-PID CPU# ||||| TIMESTAMP FUNCTION
# | | | ||||| | |
kworker/u257:2-694 [015] ..... 209.263912: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
kworker/u257:2-694 [015] ..... 209.263951: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
kworker/u257:2-694 [015] ..... 209.263990: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
kworker/u257:2-694 [015] ..... 209.264028: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x09 val:0x0200
kworker/u257:2-694 [015] ..... 209.264067: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x0a val:0x0000
ethtool-1148 [007] ..... 209.665693: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
ethtool-1148 [007] ..... 209.665706: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x1840
ethtool-1148 [007] ..... 210.588139: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1840
ethtool-1148 [007] ..... 210.588152: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x1040
ethtool-1148 [007] ..... 210.615900: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x1040
ethtool-1148 [007] ..... 210.615912: mdio_access: mii-0000:bd:00.1 write phy:0x03 reg:0x00 val:0x4000 //here just set bit 14
ethtool-1148 [007] ..... 210.620952: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.625992: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.631034: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.636075: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.641116: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.646159: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.651215: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.656256: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.661296: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.666338: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x7989
ethtool-1148 [007] ..... 210.671378: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x798d
ethtool-1148 [007] ..... 210.679016: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x4000
ethtool-1148 [007] ..... 210.679053: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x798d
ethtool-1148 [007] ..... 210.679091: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x09 val:0x0200
ethtool-1148 [007] ..... 210.679129: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x0a val:0x0000
ethtool-1148 [007] ..... 210.695902: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x00 val:0x4000
ethtool-1148 [007] ..... 210.695939: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x01 val:0x798d
ethtool-1148 [007] ..... 210.695977: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x09 val:0x0200
ethtool-1148 [007] ..... 210.696014: mdio_access: mii-0000:bd:00.1 read phy:0x03 reg:0x0a val:0x0000
So phy speed will be set to 10M in this case, if previous speed of device before going
down is 10M, loopback test is pass. Only previous speed is 100M or 1000M, loopback test is failed.
Thanks
Guangbin
.
Powered by blists - more mailing lists