linux-kernel - [RFC PATCH net v3] net: phy: Fix the issue that netif always links up after resuming

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

Date:   Mon,  3 Dec 2018 17:22:29 +0900
From:   Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
To:     Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
Subject: [RFC PATCH net v3] net: phy: Fix the issue that netif always links up after resuming

Even though the link is down before entering hibernation,
there is an issue that the network interface always links up after resuming
from hibernation.

The phydev->state is PHY_READY before enabling the network interface, so
the link is down. After resuming from hibernation, the phydev->state is
forcibly set to PHY_UP in mdio_bus_phy_restore(), and the link becomes up.

This patch adds a new convenient function to check whether the PHY is in
a started state, and expects to solve the issue by changing phydev->state
to PHY_UP and calling phy_start_machine() only when the PHY is started.

Suggested-by: Heiner Kallweit <hkallweit1@...il.com>
Signed-off-by: Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
---

Changes since v2:
 - add mutex lock/unlock for changing phydev->state
 - check whether the mutex is locked in phy_is_started()
 
Changes since v1:
 - introduce a new helper function phy_is_started() and use it instead of
   checking link status
 - replace checking phydev->state with phy_is_started() in
   phy_stop_machine()

drivers/net/phy/phy.c        |  2 +-
 drivers/net/phy/phy_device.c | 12 +++++++++---
 include/linux/phy.h          | 13 +++++++++++++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 1d73ac3..f484d03 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -670,7 +670,7 @@ void phy_stop_machine(struct phy_device *phydev)
 	cancel_delayed_work_sync(&phydev->state_queue);
 
 	mutex_lock(&phydev->lock);
-	if (phydev->state > PHY_UP && phydev->state != PHY_HALTED)
+	if (phy_is_started(phydev))
 		phydev->state = PHY_UP;
 	mutex_unlock(&phydev->lock);
 }
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index ab33d17..4897d24 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -309,10 +309,16 @@ static int mdio_bus_phy_restore(struct device *dev)
 		return ret;
 
 	/* The PHY needs to renegotiate. */
-	phydev->link = 0;
-	phydev->state = PHY_UP;
+	mutex_lock(&phydev->lock);
+	if (phy_is_started(phydev)) {
+		phydev->state = PHY_UP;
+		mutex_unlock(&phydev->lock);
+		phydev->link = 0;
+		phy_start_machine(phydev);
+	} else {
+		mutex_unlock(&phydev->lock);
+	}
 
-	phy_start_machine(phydev);
 
 	return 0;
 }
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 3ea87f7..dd21537 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -898,6 +898,19 @@ static inline bool phy_is_pseudo_fixed_link(struct phy_device *phydev)
 }
 
 /**
+ * phy_is_started - Convenience function for testing whether a PHY is in
+ * a started state
+ * @phydev: the phy_device struct
+ *
+ * The caller must have taken the phy_device mutex lock.
+ */
+static inline bool phy_is_started(struct phy_device *phydev)
+{
+	WARN_ON(!mutex_is_locked(&phydev->lock));
+	return phydev->state >= PHY_UP && phydev->state != PHY_HALTED;
+}
+
+/**
  * phy_write_mmd - Convenience function for writing a register
  * on an MMD on a given PHY.
  * @phydev: The phy_device struct
-- 
2.7.4