[PATCH iwl-net] e1000e: fix flow control stuck off after link partner reset

[PATCH iwl-net] e1000e: fix flow control stuck off after link partner reset

[Craig I. Hagan]

On an I219 (pch_spt+) copper with jumbo MTU enabled, a link partner
reset (e.g. a switch reboot) can leave the PHY's pause advertisement
bits dropped. Nothing re-asserts them on link-up, so every renegotiation
resolves flow control to none. rx_crc_errors climb and traffic stalls until
the interface is manually reset. Observed on I219-LM (8086:15b7) at
MTU 9000 behind a switch that was power-cycled.

In the watchdog, detect a completed autonegotiation that resolved
to no flow control despite pause being requested and the pause bits
missing from the advertisement, and schedule reset_task to rebuild
the advertisement (as ethtool -r does). Remediation is limited to three
attempts, 30s apart, until the next link down.

Remediate via the existing reset path rather than re-asserting the
advertisement on link-up; a root-cause rework can follow in net-next.

Fixes: 79849ebc0e06 ("e1000e: initial support for i219")
Cc: stable@vger.kernel.org
Link: https://github.com/systemd/systemd/issues/25925
Assisted-by: LLM
Signed-off-by: Craig I. Hagan <hagan@amazon.com>
---
 drivers/net/ethernet/intel/e1000e/e1000.h  |  11 ++
 drivers/net/ethernet/intel/e1000e/netdev.c | 132 +++++++++++++++++++++
 2 files changed, 143 insertions(+)

diff --git a/drivers/net/ethernet/intel/e1000e/e1000.h b/drivers/net/ethernet/intel/e1000e/e1000.h
index 63ebe0037..4353519d4 100644
--- a/drivers/net/ethernet/intel/e1000e/e1000.h
+++ b/drivers/net/ethernet/intel/e1000e/e1000.h
@@ -210,6 +210,17 @@ struct e1000_adapter {
 	/* track device up/down/testing state */
 	unsigned long state;
 
+	/* Flow-control advertisement-latch auto-recovery.
+	 * These MUST persist across the reinit that remediation triggers
+	 * (the reinit generates its own carrier-up), so they are NOT reset
+	 * on link-up. The holdoff and per-episode attempt budget prevent a
+	 * reset/flap storm against a partner that is still booting or that
+	 * genuinely never advertises pause.
+	 */
+	unsigned long fclatch_last_fix;	/* jiffies of last remediation, 0=none */
+	u8 fclatch_attempts;		/* attempts in the current link episode */
+	bool fclatch_gave_up;		/* terminal for episode; re-armed on link-down */
+
 	/* Interrupt Throttle Rate */
 	u32 itr;
 	u32 itr_setting;
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 844f31ab3..28bd33335 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -38,6 +38,15 @@ static int debug = -1;
 module_param(debug, int, 0);
 MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
 
+/* Flow-control advertisement-latch auto-recovery tunables. Fixed constants
+ * rather than user knobs: the values only need to be safe, not tunable.
+ *   MAX_ATTEMPTS : autoneg-restart attempts per link episode before giving up
+ *   HOLDOFF      : minimum spacing between attempts; longer than a switch's
+ *                  autoneg settle so we do not fight a still-booting partner
+ */
+#define E1000E_FCLATCH_MAX_ATTEMPTS	3	/* autoneg-restarts per link episode */
+#define E1000E_FCLATCH_HOLDOFF		(30 * HZ)	/* min spacing between attempts */
+
 static const struct e1000_info *e1000_info_tbl[] = {
 	[board_82571]		= &e1000_82571_info,
 	[board_82572]		= &e1000_82572_info,
@@ -5217,6 +5226,113 @@ static void e1000_watchdog(struct timer_list *t)
 	/* TODO: make this use queue_delayed_work() */
 }
 
+/**
+ * e1000e_check_fc_latch - re-assert a dropped pause advertisement
+ * @adapter: board private structure
+ *
+ * On the I219/PCH family at jumbo MTU, an upstream switch reboot can bring the
+ * link back with the PHY's pause-advertisement bits dropped and flow control
+ * resolved to none, even when a pause mode was requested. The driver does not
+ * re-assert the advertisement on a plain link-up (e1000_phy_setup_autoneg()
+ * runs only during a full setup pass), so every later renegotiation resolves
+ * to none and the link does not recover on its own.
+ *
+ * Detect that state from a fresh PHY read and schedule a full reconfigure (the
+ * reset_task path, as ethtool -r uses) to rebuild the advertisement. reset_task
+ * bounces the link and re-enters this check, so the attempt count is kept
+ * across the bounce rather than reset on link-up. Remediation stops after three
+ * tries per link episode; the give-up state is re-armed only on a real
+ * link-down (peer/cable change), so a partner that never advertises pause is
+ * reset three times and then left alone until the link is re-established --
+ * it cannot flap indefinitely.
+ */
+static void e1000e_check_fc_latch(struct e1000_adapter *adapter)
+{
+	struct e1000_hw *hw = &adapter->hw;
+	struct net_device *netdev = adapter->netdev;
+	u16 bmsr = 0, adv = 0, expected;
+	unsigned long now;
+	int rv;
+
+	/* Skip while down/resetting or with a reset already pending. */
+	if (!netif_carrier_ok(netdev) ||
+	    test_bit(__E1000_DOWN, &adapter->state) ||
+	    test_bit(__E1000_RESETTING, &adapter->state) ||
+	    (adapter->flags & FLAG_RESTART_NOW))
+		return;
+
+	/* Limited to I219/PCH copper at jumbo MTU, where the behavior is seen
+	 * and the RX-side impact matters. Act only when a pause mode was
+	 * requested but autoneg resolved none. Pause is only meaningful on a
+	 * full-duplex link, so a half-duplex result legitimately carries no
+	 * pause bits and must not be treated as the fault.
+	 */
+	if (hw->phy.media_type != e1000_media_type_copper ||
+	    hw->mac.type < e1000_pch_spt ||		/* pch_spt+ (I219 and later) */
+	    netdev->mtu <= ETH_DATA_LEN ||		/* jumbo only */
+	    adapter->link_duplex != FULL_DUPLEX ||
+	    !hw->mac.autoneg || !adapter->fc_autoneg ||
+	    hw->fc.current_mode != e1000_fc_none)
+		return;
+	if (hw->fc.requested_mode != e1000_fc_full &&
+	    hw->fc.requested_mode != e1000_fc_rx_pause &&
+	    hw->fc.requested_mode != e1000_fc_tx_pause)
+		return;
+
+	/* Fresh, self-contained reads; act only on a good, autoneg-complete read. */
+	rv = e1e_rphy(hw, MII_BMSR, &bmsr);
+	if (!rv)
+		rv = e1e_rphy(hw, MII_ADVERTISE, &adv);
+	if (rv || !(bmsr & BMSR_ANEGCOMPLETE))
+		return;
+
+	/* Pause bits we should be advertising for the requested mode (mirrors
+	 * e1000_phy_setup_autoneg): full/rx use CAP|ASYM, tx uses ASYM only.
+	 */
+	if (hw->fc.requested_mode == e1000_fc_tx_pause)
+		expected = ADVERTISE_PAUSE_ASYM;
+	else
+		expected = ADVERTISE_PAUSE_CAP | ADVERTISE_PAUSE_ASYM;
+
+	/* The advertisement should carry these bits; missing them while a pause
+	 * mode was requested is the failure this recovers from.
+	 */
+	if ((adv & expected) == expected)
+		return;
+
+	now = jiffies;
+
+	/* Give-up is terminal for this link episode; it is re-armed only on a
+	 * real link-down (see the watchdog link-down path), i.e. when the peer
+	 * or cabling actually changes -- not on a timer, which cannot tell a
+	 * recovered switch from the same never-pause partner 60s later.
+	 */
+	if (adapter->fclatch_gave_up)
+		return;
+	if (adapter->fclatch_last_fix &&
+	    time_before(now, adapter->fclatch_last_fix + E1000E_FCLATCH_HOLDOFF))
+		return;				/* within holdoff */
+
+	if (adapter->fclatch_attempts >= E1000E_FCLATCH_MAX_ATTEMPTS) {
+		adapter->fclatch_gave_up = true;
+		e_warn("flow-control advertisement latch persists after %u autoneg-restart attempts (adv=0x%04x, requested fc=%u); giving up until the link goes down and back up\n",
+		       adapter->fclatch_attempts, adv, hw->fc.requested_mode);
+		return;
+	}
+
+	adapter->fclatch_attempts++;
+	adapter->fclatch_last_fix = now;
+	e_warn("flow-control advertisement latch detected (adv=0x%04x missing pause for requested fc=%u); forcing autoneg restart (attempt %u/%d)\n",
+	       adv, hw->fc.requested_mode, adapter->fclatch_attempts,
+	       E1000E_FCLATCH_MAX_ATTEMPTS);
+	/* Request the reset via FLAG_RESTART_NOW; the watchdog schedules
+	 * reset_task on this flag just below the call site. The flag also
+	 * tells reset_task this is intentional, suppressing its "Reset
+	 * adapter unexpectedly" warning + register dump.
+	 */
+	adapter->flags |= FLAG_RESTART_NOW;
+}
+
 static void e1000_watchdog_task(struct work_struct *work)
 {
 	struct e1000_adapter *adapter = container_of(work,
@@ -5353,6 +5469,17 @@ static void e1000_watchdog_task(struct work_struct *work)
 			netdev_info(netdev, "NIC Link is Down\n");
 			netif_carrier_off(netdev);
 			netif_stop_queue(netdev);
+
+			/* A real link-down ends the current fc-latch episode:
+			 * re-arm remediation so the next link-up (e.g. the
+			 * switch finishing its reboot) gets a fresh set of
+			 * attempts. Our own reset_task bounce does not reach
+			 * here (it goes through __E1000_DOWN/RESETTING), so
+			 * this does not defeat the per-episode attempt cap.
+			 */
+			adapter->fclatch_attempts = 0;
+			adapter->fclatch_gave_up = false;
+			adapter->fclatch_last_fix = 0;
 			if (!test_bit(__E1000_DOWN, &adapter->state))
 				mod_timer(&adapter->phy_info_timer,
 					  round_jiffies(jiffies + 2 * HZ));
@@ -5384,6 +5511,11 @@ static void e1000_watchdog_task(struct work_struct *work)
 	adapter->gotc_old = adapter->stats.gotc;
 	spin_unlock(&adapter->stats64_lock);
 
+	/* Auto-recover from a latched pause advertisement (I219/PCH,
+	 * jumbo) after an upstream switch reboot; see e1000e_check_fc_latch().
+	 */
+	e1000e_check_fc_latch(adapter);
+
 	/* If the link is lost the controller stops DMA, but
 	 * if there is queued Tx work it cannot be done.  So
 	 * reset the controller to flush the Tx packet buffers.
-- 
2.47.3

Re: [PATCH iwl-net] e1000e: fix flow control stuck off after link partner reset

[Matt Vollrath]

On 9/23/26 19:58, Craig I. Hagan wrote:
> On an I219 (pch_spt+) copper with jumbo MTU enabled, a link partner
> reset (e.g. a switch reboot) can leave the PHY's pause advertisement
> bits dropped. Nothing re-asserts them on link-up, so every renegotiation
> resolves flow control to none. rx_crc_errors climb and traffic stalls until
> the interface is manually reset. Observed on I219-LM (8086:15b7) at
> MTU 9000 behind a switch that was power-cycled.
> 
> In the watchdog, detect a completed autonegotiation that resolved
> to no flow control despite pause being requested and the pause bits
> missing from the advertisement, and schedule reset_task to rebuild
> the advertisement (as ethtool -r does). Remediation is limited to three
> attempts, 30s apart, until the next link down.
> 
> Remediate via the existing reset path rather than re-asserting the
> advertisement on link-up; a root-cause rework can follow in net-next.
> 
> Fixes: 79849ebc0e06 ("e1000e: initial support for i219")
> Cc: stable@vger.kernel.org
> Link: https://github.com/systemd/systemd/issues/25925
> Assisted-by: LLM
> Signed-off-by: Craig I. Hagan <hagan@amazon.com>
> ---
I was able to repro this on an I219-LM laptop by link cycling with MEI
runtime PM enabled while the NIC runtime PM was disabled.

At 1500 MTU flow control still doesn't end up in the right state, but
traffic otherwise flows normally. I don't think traffic is stalling
because flow control is off, I think it's stalling because
e1000_lv_jumbo_workaround_ich8lan didn't run after the PHY reset.

I didn't try your patch, but there's a four line fix. Where the watchdog
resets just the PHY when the MAC exits DMoff state, set FLAG_RESTART_NOW
instead of resetting the PHY, then goto link_up. This works for me at
both 1500 and 9000 MTU.

If runtime PM were enabled on your NIC it would reset anyway and avoid
this error.

See:

https://lore.kernel.org/netdev/20190628224932.3389-16-jeffrey.t.kirsher@intel.com/