netdev - [PATCH net-next 3/3] net: phy: move handling latched link-down to phylib state machine

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:   Mon, 27 May 2019 20:29:45 +0200
From:   Heiner Kallweit <hkallweit1@...il.com>
To:     Russell King - ARM Linux <linux@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        David Miller <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: [PATCH net-next 3/3] net: phy: move handling latched link-down to
 phylib state machine

Especially with fibre links there may be very short link drops. And if
interrupt handling is slow we may miss such a link drop. To deal with
this we remove the double link status read from the generic link status
read functions, and call the state machine twice instead.
The flag for double-reading link status can be set by phy_mac_interrupt
from hard irq context, therefore we have to use an atomic operation.

Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
Suggested-by: Russell King - ARM Linux admin <linux@...linux.org.uk>
---
 drivers/net/phy/phy-c45.c    | 12 ------------
 drivers/net/phy/phy.c        | 11 +++++++++++
 drivers/net/phy/phy_device.c | 14 +-------------
 include/linux/phy.h          |  8 ++++++++
 4 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index b9d414578..63d9e8483 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -223,18 +223,6 @@ int genphy_c45_read_link(struct phy_device *phydev)
 		devad = __ffs(mmd_mask);
 		mmd_mask &= ~BIT(devad);
 
-		/* The link state is latched low so that momentary link
-		 * drops can be detected. Do not double-read the status
-		 * in polling mode to detect such short link drops.
-		 */
-		if (!phy_polling_mode(phydev)) {
-			val = phy_read_mmd(phydev, devad, MDIO_STAT1);
-			if (val < 0)
-				return val;
-			else if (val & MDIO_STAT1_LSTATUS)
-				continue;
-		}
-
 		val = phy_read_mmd(phydev, devad, MDIO_STAT1);
 		if (val < 0)
 			return val;
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 8030d0a97..575412ff5 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -778,6 +778,7 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
 		if (phydev->drv->handle_interrupt(phydev))
 			goto phy_err;
 	} else {
+		set_bit(PHY_LINK_DOUBLE_READ, &phydev->atomic_flags);
 		/* reschedule state queue work to run as soon as possible */
 		phy_trigger_machine(phydev);
 	}
@@ -969,6 +970,15 @@ void phy_state_machine(struct work_struct *work)
 			phydev->drv->link_change_notify(phydev);
 	}
 
+	/* link-down is latched, in order not to lose a link-up event we have
+	 * to read the link status twice
+	 */
+	if (test_and_clear_bit(PHY_LINK_DOUBLE_READ, &phydev->atomic_flags)) {
+		if (!phydev->link)
+			phy_trigger_machine(phydev);
+		return;
+	}
+
 	/* Only re-schedule a PHY state machine change if we are polling the
 	 * PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving
 	 * between states from phy_mac_interrupt().
@@ -992,6 +1002,7 @@ void phy_state_machine(struct work_struct *work)
  */
 void phy_mac_interrupt(struct phy_device *phydev)
 {
+	set_bit(PHY_LINK_DOUBLE_READ, &phydev->atomic_flags);
 	/* Trigger a state machine change */
 	phy_trigger_machine(phydev);
 }
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index dcc93a873..f2a78baa8 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1704,23 +1704,11 @@ int genphy_update_link(struct phy_device *phydev)
 {
 	int status;
 
-	/* The link state is latched low so that momentary link
-	 * drops can be detected. Do not double-read the status
-	 * in polling mode to detect such short link drops.
-	 */
-	if (!phy_polling_mode(phydev)) {
-		status = phy_read(phydev, MII_BMSR);
-		if (status < 0)
-			return status;
-		else if (status & BMSR_LSTATUS)
-			goto done;
-	}
-
 	/* Read link and autonegotiation status */
 	status = phy_read(phydev, MII_BMSR);
 	if (status < 0)
 		return status;
-done:
+
 	phydev->link = status & BMSR_LSTATUS ? 1 : 0;
 	phydev->autoneg_complete = status & BMSR_ANEGCOMPLETE ? 1 : 0;
 
diff --git a/include/linux/phy.h b/include/linux/phy.h
index f90158c67..1d1dcfa90 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -332,6 +332,10 @@ struct phy_c45_device_ids {
 	u32 device_ids[8];
 };
 
+enum phy_atomic_flags {
+	PHY_LINK_DOUBLE_READ,
+};
+
 /* phy_device: An instance of a PHY
  *
  * drv: Pointer to the driver for this PHY instance
@@ -346,6 +350,7 @@ struct phy_c45_device_ids {
  * sysfs_links: Internal boolean tracking sysfs symbolic links setup/removal.
  * loopback_enabled: Set true if this phy has been loopbacked successfully.
  * state: state of the PHY for management purposes
+ * atomic_flags: flags accessed in atomic context
  * dev_flags: Device-specific flags used by the PHY driver.
  * link_timeout: The number of timer firings to wait before the
  * giving up on the current attempt at acquiring a link
@@ -394,6 +399,9 @@ struct phy_device {
 
 	enum phy_state state;
 
+	/* flags used in atomic context */
+	unsigned long atomic_flags;
+
 	u32 dev_flags;
 
 	phy_interface_t interface;
-- 
2.21.0