Date: Sat, 25 Jul 2026 21:04:26 +0000 From: Kevin Bowling <kbowling@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 0bd6a167c156 - main - e1000: Defer link-up notification until after TSO reset Message-ID: <6a6524da.41983.8c6cef9@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by kbowling: URL: https://cgit.FreeBSD.org/src/commit/?id=0bd6a167c1561f01c227b1c428a3d8adf0e38833 commit 0bd6a167c1561f01c227b1c428a3d8adf0e38833 Author: Kevin Bowling <kbowling@FreeBSD.org> AuthorDate: 2026-07-25 21:00:10 +0000 Commit: Kevin Bowling <kbowling@FreeBSD.org> CommitDate: 2026-07-25 21:00:10 +0000 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") MFC after: 1 week --- sys/dev/e1000/if_em.c | 79 +++++++++++++++++++++++++++++++++++++++------------ sys/dev/e1000/if_em.h | 10 ++++++- 2 files changed, 70 insertions(+), 19 deletions(-) diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c index 3118b27e8122..5547c4ec8614 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; @@ -1999,7 +2003,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; } @@ -2219,7 +2224,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 */ @@ -2262,7 +2267,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 */ @@ -2279,7 +2290,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 && @@ -2299,17 +2310,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); @@ -2758,7 +2785,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; @@ -4342,6 +4371,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 && @@ -4351,20 +4382,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 4c80c7696952..56c28597fa22 100644 --- a/sys/dev/e1000/if_em.h +++ b/sys/dev/e1000/if_em.h @@ -469,6 +469,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; @@ -530,7 +538,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;home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a6524da.41983.8c6cef9>