e1000: Defer link-up notification until after TSO reset

em_automask_tso() changes the enabled TSO capabilities when the link
moves between 10/100 and 1000 Mb/s.  A running interface must be
reinitialized to apply the new capability set.  Do not publish
LINK_STATE_UP until the requested iflib reset has completed.

Replace link_active with an explicit state machine that distinguishes
the physical link, its publication to iflib, and an outstanding reset
barrier.  Preserve that barrier across a link flap with
DOWN_RESET_PENDING, and only publish DOWN if UP was previously
published.

Only request a reset for a running interface or for an initialization
while the interface is administratively up.  In other states the next
initialization will apply the capability changes, avoiding a reset
request that iflib's admin task could discard.

Reviewed by:	Faraz Vahedi <kfv@kfv.io>
Fixes:	2ddf24f8f525 ("e1000: Automask TSO on lem(4)/em(4) 10/100 Ethernet")

(cherry picked from commit 0bd6a167c1561f01c227b1c428a3d8adf0e38833)

2 files changed, 70 insertions, 19 deletions

diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c
index 9e7f4fb230e2..238d089c9018 100644
--- a/sys/dev/e1000/if_em.c
+++ b/sys/dev/e1000/if_em.c
@@ -1562,6 +1562,10 @@ em_if_init(if_ctx_t ctx)
 
 	/* Initialize the hardware */
 	em_reset(ctx);
+	/* Re-arm a link-up transition deferred for this reset. */
+	if (sc->link_state == EM_LINK_STATE_DOWN_RESET_PENDING ||
+	    sc->link_state == EM_LINK_STATE_UP_RESET_PENDING)
+		sc->link_state = EM_LINK_STATE_DOWN;
 	em_if_update_admin_status(ctx);
 
 	for (i = 0, tx_que = sc->tx_queues; i < sc->tx_num_queues;
@@ -2000,7 +2004,8 @@ em_if_media_status(if_ctx_t ctx, struct ifmediareq *ifmr)
 	ifmr->ifm_status = IFM_AVALID;
 	ifmr->ifm_active = IFM_ETHER;
 
-	if (!sc->link_active) {
+	if (sc->link_state == EM_LINK_STATE_DOWN ||
+	    sc->link_state == EM_LINK_STATE_DOWN_RESET_PENDING) {
 		return;
 	}
 
@@ -2220,7 +2225,7 @@ em_if_update_admin_status(if_ctx_t ctx)
 	struct e1000_hw *hw = &sc->hw;
 	device_t dev = iflib_get_dev(ctx);
 	u32 link_check, thstat, ctrl;
-	bool automasked = false;
+	bool reset_requested = false;
 
 	link_check = thstat = ctrl = 0;
 	/* Get the cached link value or read phy for real */
@@ -2263,7 +2268,13 @@ em_if_update_admin_status(if_ctx_t ctx)
 	}
 
 	/* Now check for a transition */
-	if (link_check && (sc->link_active == 0)) {
+	if (link_check &&
+	    (sc->link_state == EM_LINK_STATE_DOWN ||
+	    sc->link_state == EM_LINK_STATE_DOWN_RESET_PENDING)) {
+		bool reset_pending;
+
+		reset_pending =
+		    sc->link_state == EM_LINK_STATE_DOWN_RESET_PENDING;
 		e1000_get_speed_and_duplex(hw, &sc->link_speed,
 		    &sc->link_duplex);
 		/* Check if we must disable SPEED_MODE bit on PCI-E */
@@ -2280,7 +2291,7 @@ em_if_update_admin_status(if_ctx_t ctx)
 			    sc->link_speed,
 			    ((sc->link_duplex == FULL_DUPLEX) ?
 			    "Full Duplex" : "Half Duplex"));
-		sc->link_active = 1;
+		sc->link_state = EM_LINK_STATE_UP;
 		sc->smartspeed = 0;
 		if ((ctrl & E1000_CTRL_EXT_LINK_MODE_MASK) ==
 		    E1000_CTRL_EXT_LINK_MODE_GMII &&
@@ -2300,17 +2311,33 @@ em_if_update_admin_status(if_ctx_t ctx)
 		}
 		/* Only do TSO on gigabit for older chips due to errata */
 		if (hw->mac.type < igb_mac_min)
-			automasked = em_automask_tso(ctx);
+			reset_requested = em_automask_tso(ctx);
 
-		/* Automasking resets the interface so don't mark it up yet */
-		if (!automasked)
+		if (reset_pending || reset_requested) {
+			/*
+			 * The PHY is up, but publish it only after the TSO
+			 * capability-change reset.
+			 */
+			sc->link_state = EM_LINK_STATE_UP_RESET_PENDING;
+		} else {
 			iflib_link_state_change(ctx, LINK_STATE_UP,
 			    IF_Mbps(sc->link_speed));
-	} else if (!link_check && (sc->link_active == 1)) {
+		}
+	} else if (!link_check &&
+	    (sc->link_state == EM_LINK_STATE_UP ||
+	    sc->link_state == EM_LINK_STATE_UP_RESET_PENDING)) {
+		bool link_was_published;
+		bool reset_pending;
+
+		link_was_published = sc->link_state == EM_LINK_STATE_UP;
+		reset_pending =
+		    sc->link_state == EM_LINK_STATE_UP_RESET_PENDING;
 		sc->link_speed = 0;
 		sc->link_duplex = 0;
-		sc->link_active = 0;
-		iflib_link_state_change(ctx, LINK_STATE_DOWN, 0);
+		sc->link_state = reset_pending ?
+		    EM_LINK_STATE_DOWN_RESET_PENDING : EM_LINK_STATE_DOWN;
+		if (link_was_published)
+			iflib_link_state_change(ctx, LINK_STATE_DOWN, 0);
 	}
 	em_update_stats_counters(sc);
 
@@ -2759,7 +2786,9 @@ lem_smartspeed(struct e1000_softc *sc)
 {
 	u16 phy_tmp;
 
-	if (sc->link_active || (sc->hw.phy.type != e1000_phy_igp) ||
+	if (sc->link_state == EM_LINK_STATE_UP ||
+	    sc->link_state == EM_LINK_STATE_UP_RESET_PENDING ||
+	    (sc->hw.phy.type != e1000_phy_igp) ||
 	    sc->hw.mac.autoneg == 0 ||
 	    (sc->hw.phy.autoneg_advertised & ADVERTISE_1000_FULL) == 0)
 		return;
@@ -4360,6 +4389,8 @@ em_automask_tso(if_ctx_t ctx)
 	struct e1000_softc *sc = iflib_get_softc(ctx);
 	if_softc_ctx_t scctx = iflib_get_softc_ctx(ctx);
 	if_t ifp = iflib_get_ifp(ctx);
+	bool reset_needed;
+	int drvflags;
 
 	if (!em_unsupported_tso && sc->link_speed &&
 	    sc->link_speed != SPEED_1000 &&
@@ -4369,20 +4400,32 @@ em_automask_tso(if_ctx_t ctx)
 		sc->tso_automasked = scctx->isc_capenable & IFCAP_TSO;
 		scctx->isc_capenable &= ~IFCAP_TSO;
 		if_setcapenablebit(ifp, 0, IFCAP_TSO);
-		/* iflib_init_locked handles ifnet hwassistbits */
-		iflib_request_reset(ctx);
-		return true;
 	} else if (sc->link_speed == SPEED_1000 && sc->tso_automasked) {
 		device_printf(sc->dev, "Re-enabling TSO for GbE.\n");
 		scctx->isc_capenable |= sc->tso_automasked;
 		if_setcapenablebit(ifp, sc->tso_automasked, 0);
 		sc->tso_automasked = 0;
-		/* iflib_init_locked handles ifnet hwassistbits */
-		iflib_request_reset(ctx);
-		return true;
+	} else {
+		return (false);
 	}
 
-	return false;
+	/*
+	 * Reset a running interface, or one being initialized while
+	 * administratively up.  OACTIVE remains set after iflib_stop(), so
+	 * it alone cannot distinguish initialization from an interface that
+	 * is down.  In other states, the next initialization will apply the
+	 * updated capabilities.
+	 */
+	drvflags = if_getdrvflags(ifp);
+	reset_needed = (drvflags & IFF_DRV_RUNNING) != 0 ||
+	    ((drvflags & IFF_DRV_OACTIVE) != 0 &&
+	    (if_getflags(ifp) & IFF_UP) != 0);
+	if (!reset_needed)
+		return (false);
+
+	/* iflib_init_locked handles ifnet hwassistbits */
+	iflib_request_reset(ctx);
+	return (true);
 }
 
 /*
diff --git a/sys/dev/e1000/if_em.h b/sys/dev/e1000/if_em.h
index 582e8d9c6327..59d3acca33c9 100644
--- a/sys/dev/e1000/if_em.h
+++ b/sys/dev/e1000/if_em.h
@@ -471,6 +471,14 @@ struct em_rx_queue {
 	struct if_irq		que_irq;
 };  
 
+/* Driver-observed link state and its publication barrier. */
+enum em_link_state {
+	EM_LINK_STATE_DOWN = 0,
+	EM_LINK_STATE_DOWN_RESET_PENDING,
+	EM_LINK_STATE_UP,
+	EM_LINK_STATE_UP_RESET_PENDING,
+};
+
 /* Our softc structure */
 struct e1000_softc {
 	struct e1000_hw		hw;
@@ -532,7 +540,7 @@ struct e1000_softc {
 	u32			shadow_vfta[EM_VFTA_SIZE];
 
 	/* Info about the interface */
-	u16			link_active;
+	enum em_link_state	link_state;
 	u16			fc;
 	u16			link_speed;
 	u16			link_duplex;