netdev - Re: [PATCH net-next] net: phy: avoid kernel warning dump when stopping an errored PHY

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]

Message-ID: <fd08a80d-c70b-4943-8cca-b038f54f8eaa@lunn.ch>
Date: Mon, 4 Sep 2023 15:43:00 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jijie Shao <shaojijie@...wei.com>
Cc: f.fainelli@...il.com, davem@...emloft.net, edumazet@...gle.com,
	hkallweit1@...il.com, kuba@...nel.org, netdev@...r.kernel.org,
	pabeni@...hat.com, rmk+kernel@...linux.org.uk,
	"shenjian15@...wei.com" <shenjian15@...wei.com>,
	"liuyonglong@...wei.com" <liuyonglong@...wei.com>,
	wangjie125@...wei.com, chenhao418@...wei.com,
	Hao Lan <lanhao@...wei.com>,
	"wangpeiyang1@...wei.com" <wangpeiyang1@...wei.com>
Subject: Re: [PATCH net-next] net: phy: avoid kernel warning dump when
 stopping an errored PHY

On Mon, Sep 04, 2023 at 05:50:32PM +0800, Jijie Shao wrote:
> Hi all,
> We encountered an issue when resetting our netdevice recently, it seems
> related to this patch.
> 
> During our process, we stop phy first and call phy_start() later.
> phy_check_link_status returns error because it read mdio failed. The
> reason why it happened is that the cmdq is unusable when we reset and we
> can't access to mdio.

At what point in the flow below do you apply the reset which stops
access to the MDIO bus? Ideally you want to do phy_stop(), then apply
the reset, get the hardware working again, and then do a phy_start().

> 
> The process and logs are showed as followed:
> Process:
> reset process       |    phy_state_machine           |  phy_state
> ==========================================================================
>                     | mutex_lock(&phydev->lock);     | PHY_RUNNING
>                     | ...                            |
>                     | case PHY_RUNNING:              |
>                     | err = phy_check_link_status()  | PHY_RUNNING
>                     | ...                            |
>                     | mutex_unlock(&phydev->lock)    | PHY_RUNNING
>  phy_stop()         |                                |
>    ...              |                                |
>    mutex_lock()     |                                | PHY_RUNNING
>    ...              |                                |
>    phydev->state =  |                                |
>      PHY_HALTED;    |                                |  PHY_HALTED
>    ...              |                                |
>    mutex_unlock()   |                                |  PHY_HALTED
>                     | phy_error_precise():           |
>                     |   mutex_lock(&phydev->lock);   | PHY_HALTED
>                     |   phydev->state = PHY_ERROR;   | PHY_ERROR
>                     |   mutex_unlock(&phydev->lock); | PHY_ERROR
>                     |                                |
> phy_start()         |                                |  PHY_ERROR
>   ...               |                                |


	Andrew