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:   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

Powered by Openwall GNU/*/Linux Powered by OpenVZ