Thread (16 messages) flat view 16 messages, 6 authors, 2017-01-09

Re: [PATCH v2] phy state machine: failsafe leave invalid RUNNING state

From: David Miller <davem@davemloft.net>
Date: 2017-01-08 23:16:02

From: Zefir Kurtisi <redacted>
Date: Fri,  6 Jan 2017 12:14:48 +0100
While in RUNNING state, phy_state_machine() checks for link changes by
comparing phydev->link before and after calling phy_read_status().
This works as long as it is guaranteed that phydev->link is never
changed outside the phy_state_machine().

If in some setups this happens, it causes the state machine to miss
a link loss and remain RUNNING despite phydev->link being 0.

This has been observed running a dsa setup with a process continuously
polling the link states over ethtool each second (SNMPD RFC-1213
agent). Disconnecting the link on a phy followed by a ETHTOOL_GSET
causes dsa_slave_get_settings() / dsa_slave_get_link_ksettings() to
call phy_read_status() and with that modify the link status - and
with that bricking the phy state machine.

This patch adds a fail-safe check while in RUNNING, which causes to
move to CHANGELINK when the link is gone and we are still RUNNING.

Signed-off-by: Zefir Kurtisi <redacted>
---
Changes to v1:
* fix kbuild test robot error: use phydev_err instead of dev_warn
  (adapt to changed struct phy_device after 4.4.21)
Florian and Andrew, please provide some feedback on this.

Thank you.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help