netdev - Re: [PATCH] net: phy: phylink: fix SFP interface autodetection

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

Message-ID: <87work2kw7.fsf@tkos.co.il>
Date:   Mon, 17 Sep 2018 18:39:36 +0300
From:   Baruch Siach <baruch@...s.co.il>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        netdev@...r.kernel.org,
        Antoine Tenart <antoine.tenart@...tlin.com>,
        Gregory CLEMENT <gregory.clement@...tlin.com>
Subject: Re: [PATCH] net: phy: phylink: fix SFP interface autodetection

Hi Russell,

Russell King - ARM Linux writes:

> On Mon, Sep 17, 2018 at 05:19:57PM +0300, Baruch Siach wrote:
>> When the switching to the SFP detected link mode update the main
>> link_interface field as well. Otherwise, the link fails to come up when
>> the configured 'phy-mode' defers from the SFP detected mode.
>> 
>> This fixes 1GB SFP module link up on eth3 of the Macchiatobin board that
>> is configured in the DT to "2500base-x" phy-mode.
>
> link_interface isn't supposed to track the SFP link mode.  In any case,
> this is only used when a PHY is attached.  For a PHY on a SFP,
> phylink_connect_phy() should be using link_config.interface and not
> link_interface there.

You mean something like this?

@@ -750,7 +750,7 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 		pl->link_config.interface = pl->link_interface;
 	}
 
-	ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_interface);
+	ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_config.interface);
 	if (ret)
 		return ret;
 
This fixes a similar issue on the Armada 8040 based Clearfog GT-8K where
the SFP interface phy-mode is set to "10gbase-kr". But the Macchiatobin
PHY-less interface has phy-mode set to "2500base-x", so I hit a separate
problem:

[   49.599455] WARNING: CPU: 1 PID: 1223 at drivers/net/phy/phylink.c:741 phylink_connect_phy+0x34/0xb4
[   49.608629] CPU: 1 PID: 1223 Comm: ifconfig Not tainted 4.19.0-rc4-00006-g18cf7eb4b625-dirty #952
[   49.617537] Hardware name: Marvell 8040 MACCHIATOBin (DT)
[   49.622957] pstate: 60000005 (nZCv daif -PAN -UAO)
[   49.627767] pc : phylink_connect_phy+0x34/0xb4
[   49.632229] lr : phylink_sfp_connect_phy+0xc/0x14
[   49.636950] sp : ffff00000b273830
[   49.640276] x29: ffff00000b273830 x28: ffff80007a342df0 
[   49.645610] x27: 00000000000004b0 x26: 0000000000000004 
[   49.650945] x25: ffff80007a342940 x24: ffff80007b90cc18 
[   49.656278] x23: ffff8000799b2538 x22: ffff000008bda000 
[   49.661613] x21: ffff80007a342000 x20: ffff80007bca4000 
[   49.666947] x19: ffff80007b814a80 x18: 000000000000000a 
[   49.672282] x17: 0000000000000000 x16: 0000000000000004 
[   49.677616] x15: 0000000000000416 x14: 0000000000000400 
[   49.682951] x13: 0000000000000400 x12: 0000000000000000 
[   49.688284] x11: 0000000000000000 x10: 0000000000000000 
[   49.693618] x9 : 00000000000002c6 x8 : 0000000000000000 
[   49.698952] x7 : ffff80007ff88b40 x6 : 0000000000000001 
[   49.704286] x5 : 0000000000000003 x4 : ffff80007bca4048 
[   49.709621] x3 : 0000000000000004 x2 : 0000000000000001 
[   49.714955] x1 : ffff80007bca4000 x0 : ffff80007a337400 
[   49.720289] Call trace:
[   49.722745]  phylink_connect_phy+0x34/0xb4
[   49.726857]  phylink_sfp_connect_phy+0xc/0x14
[   49.731231]  sfp_add_phy+0x48/0x50
[   49.734647]  sfp_sm_event+0x6e0/0x86c
[   49.738323]  sfp_start+0x10/0x18
[   49.741563]  sfp_upstream_start+0x28/0x3c
[   49.745588]  phylink_start+0x120/0x164
[   49.749352]  mvpp2_start_dev+0x184/0x260
[   49.753290]  mvpp2_open+0x394/0x3e8
[   49.756793]  __dev_open+0x110/0x124
[   49.760294]  __dev_change_flags+0xe8/0x190
[   49.764406]  dev_change_flags+0x20/0x5c
[   49.768258]  devinet_ioctl+0x270/0x528
[   49.772020]  inet_ioctl+0x13c/0x2e4
[   49.775524]  sock_do_ioctl+0x4c/0x1f4
[   49.779200]  sock_ioctl+0xf4/0x360
[   49.782616]  vfs_ioctl+0x24/0x40
[   49.785855]  do_vfs_ioctl+0xa4/0x7d4
[   49.789443]  ksys_ioctl+0x44/0x74
[   49.792770]  __arm64_sys_ioctl+0x18/0x24
[   49.796708]  el0_svc_common+0x8c/0xd0
[   49.800384]  el0_svc_handler+0x3c/0x6c
[   49.804148]  el0_svc+0x8/0xc
[   49.807039] ---[ end trace 0d1e0898561fce6e ]---
[   49.811729] sfp sfp-eth3: sfp_add_phy failed: -22

This hunk fixes that one:

@@ -738,7 +738,7 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
 
 	if (WARN_ON(pl->link_an_mode == MLO_AN_FIXED ||
 		    (pl->link_an_mode == MLO_AN_INBAND &&
-		     phy_interface_mode_is_8023z(pl->link_interface))))
+		     phy_interface_mode_is_8023z(pl->link_config.interface))))
 		return -EINVAL;
 
 	if (pl->phydev)

Is that the correct fix?

Thanks,
baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@...s.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -