netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Alexander Duyck <alexander.duyck@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Joakim Zhang <qiangqing.zhang@nxp.com>,
	netdev@vger.kernel.org, Paolo Abeni <pabeni@redhat.com>
Subject: [PATCH net] net: phylink: fix suspend/resume with WoL enabled and link down
Date: Wed, 16 Apr 2025 17:16:01 +0100	[thread overview]
Message-ID: <E1u55Qf-0016RN-PA@rmk-PC.armlinux.org.uk> (raw)
In-Reply-To: <Z__URcfITnra19xy@shell.armlinux.org.uk>

When WoL is enabled, we update the software state in phylink to
indicate that the link is down, and disable the resolver from
bringing the link back up.

On resume, we attempt to bring the overall state into consistency
by calling the .mac_link_down() method, but this is wrong if the
link was already down, as phylink strictly orders the .mac_link_up()
and .mac_link_down() methods - and this would break that ordering.

Fixes: f97493657c63 ("net: phylink: add suspend/resume support")
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---

To fix the suspend/resume with link down, this is what I think we
should do. Untested at the moment.

 drivers/net/phy/phylink.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 69ca765485db..d2c59ee16ebc 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -81,6 +81,7 @@ struct phylink {
 	unsigned int pcs_state;
 
 	bool link_failed;
+	bool suspend_link_up;
 	bool major_config_failed;
 	bool mac_supports_eee_ops;
 	bool mac_supports_eee;
@@ -2545,14 +2546,16 @@ void phylink_suspend(struct phylink *pl, bool mac_wol)
 		/* Stop the resolver bringing the link up */
 		__set_bit(PHYLINK_DISABLE_MAC_WOL, &pl->phylink_disable_state);
 
-		/* Disable the carrier, to prevent transmit timeouts,
-		 * but one would hope all packets have been sent. This
-		 * also means phylink_resolve() will do nothing.
-		 */
-		if (pl->netdev)
-			netif_carrier_off(pl->netdev);
-		else
+		pl->suspend_link_up = phylink_link_is_up(pl);
+		if (pl->suspend_link_up) {
+			/* Disable the carrier, to prevent transmit timeouts,
+			 * but one would hope all packets have been sent. This
+			 * also means phylink_resolve() will do nothing.
+			 */
+			if (pl->netdev)
+				netif_carrier_off(pl->netdev);
 			pl->old_link_state = false;
+		}
 
 		/* We do not call mac_link_down() here as we want the
 		 * link to remain up to receive the WoL packets.
@@ -2603,15 +2606,18 @@ void phylink_resume(struct phylink *pl)
 	if (test_bit(PHYLINK_DISABLE_MAC_WOL, &pl->phylink_disable_state)) {
 		/* Wake-on-Lan enabled, MAC handling */
 
-		/* Call mac_link_down() so we keep the overall state balanced.
-		 * Do this under the state_mutex lock for consistency. This
-		 * will cause a "Link Down" message to be printed during
-		 * resume, which is harmless - the true link state will be
-		 * printed when we run a resolve.
-		 */
-		mutex_lock(&pl->state_mutex);
-		phylink_link_down(pl);
-		mutex_unlock(&pl->state_mutex);
+		if (pl->suspend_link_up) {
+			/* Call mac_link_down() so we keep the overall state
+			 * balanced. Do this under the state_mutex lock for
+			 * consistency. This will cause a "Link Down" message
+			 * to be printed during resume, which is harmless -
+			 * the true link state will be printed when we run a
+			 * resolve.
+			 */
+			mutex_lock(&pl->state_mutex);
+			phylink_link_down(pl);
+			mutex_unlock(&pl->state_mutex);
+		}
 
 		/* Re-apply the link parameters so that all the settings get
 		 * restored to the MAC.
-- 
2.30.2


  reply	other threads:[~2025-04-16 16:16 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-16 15:28 [net-next PATCH 0/2] net: phylink: Fix issue w/ BMC link flap Alexander Duyck
2025-04-16 15:28 ` [net-next PATCH 1/2] net: phylink: Drop unused defines for SUPPORTED/ADVERTISED_INTERFACES Alexander Duyck
2025-04-22  8:32   ` Russell King (Oracle)
2025-04-16 15:29 ` [net-next PATCH 2/2] net: phylink: Fix issues with link balancing w/ BMC present Alexander Duyck
2025-04-16 16:01   ` Russell King (Oracle)
2025-04-16 16:16     ` Russell King (Oracle) [this message]
2025-04-16 17:14       ` [PATCH net] net: phylink: fix suspend/resume with WoL enabled and link down Russell King (Oracle)
2025-04-17 14:30       ` Alexander H Duyck
2025-04-17 14:35         ` Russell King (Oracle)
2025-04-17 15:23           ` Russell King (Oracle)
2025-04-17 17:06             ` Alexander Duyck
2025-04-17 17:27               ` Russell King (Oracle)
2025-04-17 19:49                 ` Alexander Duyck
2025-04-22  9:51                   ` Russell King (Oracle)
2025-04-22 15:30                     ` Alexander Duyck
2025-04-22 16:00                       ` Andrew Lunn
2025-04-22 20:09                         ` Alexander Duyck
2025-04-23  0:00       ` patchwork-bot+netdevbpf
2025-04-16 17:12     ` [net-next PATCH 2/2] net: phylink: Fix issues with link balancing w/ BMC present Russell King (Oracle)
2025-04-16 19:03     ` Alexander Duyck
2025-04-16 19:19       ` Russell King (Oracle)
2025-04-16 20:05         ` Russell King (Oracle)
2025-04-16 22:58           ` Alexander Duyck
2025-04-19 18:11 ` [net-next PATCH 0/2] net: phylink: Fix issue w/ BMC link flap Andrew Lunn
2025-04-20 18:18   ` Alexander Duyck
2025-04-20 21:58     ` Andrew Lunn
2025-04-21 15:51       ` Alexander Duyck
2025-04-21 16:50         ` Alexander Duyck
2025-04-22  1:21           ` Jakub Kicinski
2025-04-22 13:49             ` Andrew Lunn
2025-04-22 15:28               ` Jakub Kicinski
2025-04-22 16:49                 ` Andrew Lunn
2025-04-22 17:30                   ` Russell King (Oracle)
2025-04-22 18:13                     ` Andrew Lunn
2025-04-22 18:50                       ` Russell King (Oracle)
2025-04-22 23:51                       ` Jakub Kicinski
2025-04-22 21:29                   ` Alexander Duyck
2025-04-22 22:26                     ` Andrew Lunn
2025-04-22 23:06                       ` Alexander Duyck
2025-04-23 18:38                         ` Alexander Duyck
2025-04-24 20:34                         ` Andrew Lunn
2025-04-24 23:40                           ` Alexander Duyck
2025-04-25 13:11                             ` Andrew Lunn
2025-04-25 15:41                               ` Alexander Duyck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E1u55Qf-0016RN-PA@rmk-PC.armlinux.org.uk \
    --to=rmk+kernel@armlinux.org.uk \
    --cc=alexander.duyck@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=qiangqing.zhang@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).