netdev - RE: [EXT] Re: [PATCH net] net: phylink: fix link state on phy-connect

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:   Wed, 29 Nov 2017 21:06:56 +0000
From:   Yan Markman <ymarkman@...vell.com>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
CC:     Antoine Tenart <antoine.tenart@...e-electrons.com>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "gregory.clement@...e-electrons.com" 
        <gregory.clement@...e-electrons.com>,
        "thomas.petazzoni@...e-electrons.com" 
        <thomas.petazzoni@...e-electrons.com>,
        "miquel.raynal@...e-electrons.com" <miquel.raynal@...e-electrons.com>,
        "Nadav Haklai" <nadavh@...vell.com>,
        "mw@...ihalf.com" <mw@...ihalf.com>,
        "Stefan Chulski" <stefanc@...vell.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [EXT] Re: [PATCH net] net: phylink: fix link state on phy-connect

The attached p21 patch doesn't change anything.
But another one from the mail-text is good
	void phylink_disconnect_phy(struct phylink *pl)
	+		pl->phy_state.link = false;

There still (not for my MRVL-PP2) problem:
	It is expected that on  ifconfig-down the callback
		pl->ops->mac_link_down(ndev, pl->link_an_mode);
would be called, but it isn't


-----Original Message-----
From: Russell King - ARM Linux [mailto:linux@...linux.org.uk] 
Sent: Wednesday, November 29, 2017 9:59 PM
To: Yan Markman <ymarkman@...vell.com>
Cc: Antoine Tenart <antoine.tenart@...e-electrons.com>; andrew@...n.ch; f.fainelli@...il.com; davem@...emloft.net; gregory.clement@...e-electrons.com; thomas.petazzoni@...e-electrons.com; miquel.raynal@...e-electrons.com; Nadav Haklai <nadavh@...vell.com>; mw@...ihalf.com; Stefan Chulski <stefanc@...vell.com>; netdev@...r.kernel.org; linux-kernel@...r.kernel.org
Subject: [EXT] Re: [PATCH net] net: phylink: fix link state on phy-connect

External Email

----------------------------------------------------------------------
On Wed, Nov 29, 2017 at 07:33:44PM +0000, Yan Markman wrote:
> Hi Russel
> 
> On my board I have [Marvell 88E1510] phy working with STATUS-POLLING I 
> see some inconsistencies  -- first ifconfig-up is different from furthers, no "link is down" reports.
> Please refer the behavior example below.
> My patch is a "simple solution"  -- always reset/clear Link-state-parameters before going UP.
> Possibly, more correct (but much more complicated) solution would be in the   phy state machine   and   phylink resolve modification.
> I just found that 
>     On ifconfig-down, the phy-state-machine and phylink-resolve
>     are stopped before executing before passing over full graceful down/reset state.
>     The further ifconfig-up starts with old state parameters.
> Special cases not-tested but logic 2 test-cases are:
>    remote side changes speed whilst link is Down or Disconnected. But local ifconfig-up starts with old speed.

Hi,

I think this is covered in my "phy" branch - but could probably do with further testing, specifically this patch (which I've attached):

"phylink: ensure we take the link down when phylink_stop() is called"

This takes the link down on the MAC side synchronously when phylink_stop() is called.  However, I think your case might also benefit from this patch - please test the patch referred to without this change, and let me know if you need this change to solve your problem:

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 8f43f8779317..c90ad50204b0 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -798,6 +798,7 @@ void phylink_disconnect_phy(struct phylink *pl)
 		mutex_lock(&pl->state_mutex);
 		pl->netdev->phydev = NULL;
 		pl->phydev = NULL;
+		pl->phy_state.link = false;
 		mutex_unlock(&pl->state_mutex);
 		mutex_unlock(&phy->lock);
 		flush_work(&pl->resolve);

Thanks.

--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up