Date: Sun, 09 Aug 2026 23:48:02 +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: 612f87c949a2 - main - igbv: Recover when the PF mailbox is unavailable Message-ID: <6a7911b2.3b0d7.4be27304@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=612f87c949a2aa3363258890bca2abaea99d3fd6 commit 612f87c949a2aa3363258890bca2abaea99d3fd6 Author: Kevin Bowling <kbowling@FreeBSD.org> AuthorDate: 2026-08-09 07:16:57 +0000 Commit: Kevin Bowling <kbowling@FreeBSD.org> CommitDate: 2026-08-09 23:45:17 +0000 igbv: Recover when the PF mailbox is unavailable A VF reset can sanitize its retained queue registers even when the PF does not complete the cooperative mailbox handshake. Keep those two states separate. Do not program or enable the rings until both queue sanitation and mailbox initialization have succeeded. Report either initialization failure to iflib so the interface remains stopped. Stopped admin and media-status passes now publish cached link-down state without polling the mailbox. While the VF remains administratively up, retry complete initialization after 250 ms, one second, four seconds, and then at a capped eight-second interval. Conditional iflib reset requests ensure an intervening administrative down cancels a queued retry. Avoid a redundant mailbox reset in the stop half of an immediate iflib reinitialization; the following init performs the required reset. Preserve the reset on an ordinary administrative stop and keep the existing bounded queue-sanitation retry policy independent from mailbox liveness recovery. --- sys/dev/e1000/e1000_vf.c | 32 +++++---- sys/dev/e1000/if_em.c | 67 ++++++++++++------ sys/dev/e1000/if_em.h | 11 +++ sys/dev/e1000/if_igbv.c | 173 +++++++++++++++++++++++++++++++++++++++++++---- 4 files changed, 232 insertions(+), 51 deletions(-) diff --git a/sys/dev/e1000/e1000_vf.c b/sys/dev/e1000/e1000_vf.c index d48a4153c1b3..2394c240d18d 100644 --- a/sys/dev/e1000/e1000_vf.c +++ b/sys/dev/e1000/e1000_vf.c @@ -275,24 +275,26 @@ static s32 e1000_reset_hw_vf(struct e1000_hw *hw) usec_delay(5); } - if (timeout) { - /* mailbox timeout can now become active */ - mbx->timeout = E1000_VF_MBX_INIT_TIMEOUT; + if (!timeout) + return -E1000_ERR_RESET; - msgbuf[0] = E1000_VF_RESET; - mbx->ops.write_posted(hw, msgbuf, 1, 0); + /* mailbox timeout can now become active */ + mbx->timeout = E1000_VF_MBX_INIT_TIMEOUT; - msec_delay(10); + msgbuf[0] = E1000_VF_RESET; + ret_val = mbx->ops.write_posted(hw, msgbuf, 1, 0); + if (ret_val) + return ret_val; - /* set our "perm_addr" based on info provided by PF */ - ret_val = mbx->ops.read_posted(hw, msgbuf, 3, 0); - if (!ret_val) { - if (msgbuf[0] == (E1000_VF_RESET | - E1000_VT_MSGTYPE_ACK)) - memcpy(hw->mac.perm_addr, addr, 6); - else - ret_val = -E1000_ERR_MAC_INIT; - } + msec_delay(10); + + /* set our "perm_addr" based on info provided by PF */ + ret_val = mbx->ops.read_posted(hw, msgbuf, 3, 0); + if (!ret_val) { + if (msgbuf[0] == (E1000_VF_RESET | E1000_VT_MSGTYPE_ACK)) + memcpy(hw->mac.perm_addr, addr, 6); + else + ret_val = -E1000_ERR_MAC_INIT; } return ret_val; diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c index d9db4677bf62..a3523315d5c7 100644 --- a/sys/dev/e1000/if_em.c +++ b/sys/dev/e1000/if_em.c @@ -1547,9 +1547,13 @@ em_if_attach_pre(if_ctx_t ctx) ** important in reading the nvm and ** mac from that. */ - if (e1000_reset_hw(hw) != E1000_SUCCESS && sc->vf_ifp) - device_printf(dev, - "PF is not ready; VF mailbox initialization deferred\n"); + error = e1000_reset_hw(hw); + if (sc->vf_ifp) { + atomic_store_rel_32(&sc->vf_mbx_ready, + error == E1000_SUCCESS); + if (error != E1000_SUCCESS) + igbv_log_reset_failure(sc, error, true); + } /* Make sure a PF has a good EEPROM before we read from it. */ if (!sc->vf_ifp && e1000_validate_nvm_checksum(hw) < 0) { @@ -1687,10 +1691,12 @@ em_if_detach(if_ctx_t ctx) INIT_DEBUGOUT("em_if_detach: begin"); igb_iov_detach(sc); - if (sc->vf_ifp) + if (sc->vf_ifp) { igbv_queue_retry_detach(sc); - if (!sc->vf_ifp) + igbv_mbx_retry_detach(sc); + } else { e1000_phy_hw_reset(&sc->hw); + } em_release_manageability(sc); em_release_hw_control(sc); @@ -1721,8 +1727,10 @@ em_if_suspend(if_ctx_t ctx) { struct e1000_softc *sc = iflib_get_softc(ctx); - if (sc->vf_ifp) + if (sc->vf_ifp) { igbv_queue_retry_stop(sc); + igbv_mbx_retry_stop(sc); + } em_release_manageability(sc); em_release_hw_control(sc); em_enable_wakeup(ctx); @@ -1807,13 +1815,12 @@ em_if_init(if_ctx_t ctx) if_softc_ctx_t scctx = sc->shared; if_t ifp = iflib_get_ifp(ctx); struct em_tx_queue *tx_que; - bool vf_mbx_ready; int i; INIT_DEBUGOUT("em_if_init: begin"); - vf_mbx_ready = !sc->vf_ifp; if (sc->vf_ifp) { igbv_queue_retry_prepare(sc); + igbv_mbx_retry_prepare(sc); sc->vf_reset_pending = true; } @@ -1842,7 +1849,7 @@ em_if_init(if_ctx_t ctx) /* Initialize the hardware */ igb_iov_reset_prepare(sc); if (sc->vf_ifp) { - vf_mbx_ready = igbv_reset(ctx); + (void)igbv_reset(ctx); em_rebase_vf_stats(sc); } else { em_reset(ctx); @@ -1851,13 +1858,18 @@ em_if_init(if_ctx_t ctx) /* * Do not program or enable rings while retained queue state * might still contain a previous VF owner's DMA address. A - * bounded callout retries initialization after iflib returns; - * the deferred admin task clears its optimistic RUNNING flag. + * bounded callout retries initialization after iflib leaves the + * failed initialization stopped. */ igbv_queue_retry_failed(ctx); return; } - if (sc->vf_ifp && vf_mbx_ready) + if (sc->vf_ifp && + atomic_load_acq_32(&sc->vf_mbx_ready) == 0) { + igbv_mbx_retry_failed(ctx); + return; + } + if (sc->vf_ifp) igbv_reconcile_mac(sc, ifp); /* Re-arm a link-up transition deferred for this reset. */ if (sc->link_state == EM_LINK_STATE_DOWN_RESET_PENDING || @@ -1904,8 +1916,7 @@ em_if_init(if_ctx_t ctx) * state. The reset detector schedules another complete init, which * replays these interface-owned lists after the handshake succeeds. */ - if (vf_mbx_ready) - em_if_multi_set(ctx); + em_if_multi_set(ctx); sc->rx_mbuf_sz = iflib_get_rx_mbuf_sz(ctx); if (sc->vf_ifp) @@ -1913,13 +1924,11 @@ em_if_init(if_ctx_t ctx) else em_initialize_receive_unit(ctx); - if (vf_mbx_ready) { - /* Set up VLAN support and filter. */ - em_setup_vlan_hw_support(ctx); + /* Set up VLAN support and filter. */ + em_setup_vlan_hw_support(ctx); - /* Don't lose promiscuous settings. */ - em_if_set_promisc_impl(ctx, if_getflags(ifp)); - } + /* Don't lose promiscuous settings. */ + em_if_set_promisc_impl(ctx, if_getflags(ifp)); atomic_readandclear_32(&sc->promisc_pending); /* Restore PF/VF pool configuration after the global reset. */ @@ -2801,21 +2810,35 @@ em_if_stop(if_ctx_t ctx) INIT_DEBUGOUT("em_if_stop: begin"); - if (sc->vf_ifp) + if (sc->vf_ifp) { igbv_queue_retry_stop(sc); + igbv_mbx_retry_stop(sc); + } /* I219 needs special flushing to avoid hangs */ if (sc->hw.mac.type >= e1000_pch_spt && sc->hw.mac.type < igb_mac_min) em_flush_desc_rings(sc); igb_iov_reset_prepare(sc); - e1000_reset_hw(&sc->hw); + if (!sc->vf_ifp || + (atomic_load_acq_32(&sc->vf_mbx_ready) != 0 && + (if_getflags(iflib_get_ifp(ctx)) & IFF_UP) == 0)) + e1000_reset_hw(&sc->hw); + if (sc->vf_ifp) + atomic_store_rel_32(&sc->vf_mbx_ready, 0); if (sc->hw.mac.type >= e1000_82544 && !sc->vf_ifp) E1000_WRITE_REG(&sc->hw, E1000_WUFC, 0); if (!sc->vf_ifp) { e1000_led_off(&sc->hw); e1000_cleanup_led(&sc->hw); + } else { + sc->link_speed = 0; + sc->link_duplex = 0; + if (sc->link_state != EM_LINK_STATE_DOWN) { + sc->link_state = EM_LINK_STATE_DOWN; + iflib_link_state_change(ctx, LINK_STATE_DOWN, 0); + } } } diff --git a/sys/dev/e1000/if_em.h b/sys/dev/e1000/if_em.h index 64c42a2c7a7e..7116ff11c371 100644 --- a/sys/dev/e1000/if_em.h +++ b/sys/dev/e1000/if_em.h @@ -672,13 +672,19 @@ struct e1000_softc { } ustats; struct callout vf_queue_retry; + struct callout vf_mbx_retry; struct timeval vf_last_queue_log; + struct timeval vf_last_mbx_log; u32 vf_queue_retry_new_epoch; u32 vf_queue_retry_pending; + u32 vf_mbx_ready; + u32 vf_mbx_retry_pending; u16 vf_ifp; u8 vf_queue_failures; + u8 vf_mbx_retry_stage; bool vf_queue_gave_up; bool vf_queue_retry_initialized; + bool vf_mbx_retry_initialized; bool vf_queues_sanitized; bool vf_reset_pending; /* A PF can retain auxiliary filters across a VF reset. */ @@ -707,12 +713,17 @@ void igbv_if_intr_disable(if_ctx_t); void igbv_if_update_admin_status(if_ctx_t); void igbv_initialize_receive_unit(if_ctx_t); void igbv_initialize_transmit_unit(if_ctx_t); +void igbv_mbx_retry_detach(struct e1000_softc *); +void igbv_mbx_retry_failed(if_ctx_t); +void igbv_mbx_retry_prepare(struct e1000_softc *); +void igbv_mbx_retry_stop(struct e1000_softc *); void igbv_queue_retry_detach(struct e1000_softc *); void igbv_queue_retry_failed(if_ctx_t); void igbv_queue_retry_prepare(struct e1000_softc *); void igbv_queue_retry_stop(struct e1000_softc *); void igbv_reconcile_mac(struct e1000_softc *, if_t); bool igbv_reset(if_ctx_t); +void igbv_log_reset_failure(struct e1000_softc *, s32, bool); void igbv_update_uc_addr_list(struct e1000_softc *, if_t); void igbv_vlan_retry_add(struct e1000_softc *, u16); void igbv_vlan_retry_clear(struct e1000_softc *, u16); diff --git a/sys/dev/e1000/if_igbv.c b/sys/dev/e1000/if_igbv.c index 566c184fb133..2d57a39f934e 100644 --- a/sys/dev/e1000/if_igbv.c +++ b/sys/dev/e1000/if_igbv.c @@ -42,10 +42,17 @@ #define IGBV_VLAN_RETRY_WINDOW (8 * SBT_1S) static const struct timeval igbv_queue_log_interval = { 2, 0 }; +static const struct timeval igbv_mbx_log_interval = { 60, 0 }; static const sbintime_t igbv_queue_retry_delay[] = { 100 * SBT_1MS, 500 * SBT_1MS, }; +static const sbintime_t igbv_mbx_retry_delay[] = { + 250 * SBT_1MS, + 1 * SBT_1S, + 4 * SBT_1S, + 8 * SBT_1S, +}; _Static_assert(nitems(igbv_queue_retry_delay) + 1 == IGBV_QUEUE_SANITIZE_ATTEMPTS, "missing queue retry delay"); @@ -72,10 +79,144 @@ igbv_queue_retry_callout(void *arg) atomic_set_32(&sc->vf_queue_retry_new_epoch, 1); return; } - iflib_request_reset(sc->ctx); + iflib_request_reset_if_up(sc->ctx); + iflib_admin_intr_deferred(sc->ctx); +} + +/* + * A missing PF can make the posted reset handshake wait for a full mailbox + * timeout. Keep that work out of stopped status paths. An administratively + * up VF retries complete initialization with an exponential delay capped at + * eight seconds, so it recovers without creating a tight mailbox poller. + */ +static void +igbv_mbx_retry_callout(void *arg) +{ + struct e1000_softc *sc; + if_t ifp; + + sc = arg; + if (atomic_readandclear_32(&sc->vf_mbx_retry_pending) == 0 || + atomic_load_acq_32(&sc->vf_mbx_ready) != 0 || + iflib_in_detach(sc->ctx)) + return; + ifp = iflib_get_ifp(sc->ctx); + if ((if_getflags(ifp) & IFF_UP) == 0) + return; + + iflib_request_reset_if_up(sc->ctx); iflib_admin_intr_deferred(sc->ctx); } +static const char * +igbv_reset_error_desc(s32 error) +{ + + switch (error) { + case -E1000_ERR_RESET: + return ("PF reset acknowledgement timed out"); + case -E1000_ERR_MAC_INIT: + return ("PF returned an invalid VF reset response"); + case -E1000_ERR_MBX: + return ("PF mailbox reset exchange failed"); + default: + return ("VF reset handshake failed"); + } +} + +void +igbv_log_reset_failure(struct e1000_softc *sc, s32 error, bool attaching) +{ + + /* Report each backoff stage, then limit the steady eight-second retry. */ + if (sc->vf_mbx_retry_stage == nitems(igbv_mbx_retry_delay) - 1 && + !ratecheck(&sc->vf_last_mbx_log, &igbv_mbx_log_interval)) + return; + device_printf(sc->dev, "%s (%d)%s\n", igbv_reset_error_desc(error), + error, attaching ? "; continuing attach" : ""); +} + +void +igbv_mbx_retry_detach(struct e1000_softc *sc) +{ + + if (!sc->vf_mbx_retry_initialized) + return; + atomic_readandclear_32(&sc->vf_mbx_retry_pending); + callout_drain(&sc->vf_mbx_retry); + sc->vf_mbx_retry_initialized = false; +} + +void +igbv_mbx_retry_prepare(struct e1000_softc *sc) +{ + + if (!sc->vf_mbx_retry_initialized) + return; + atomic_readandclear_32(&sc->vf_mbx_retry_pending); + callout_drain(&sc->vf_mbx_retry); +} + +void +igbv_mbx_retry_stop(struct e1000_softc *sc) +{ + if_t ifp; + + if (!sc->vf_mbx_retry_initialized) + return; + atomic_readandclear_32(&sc->vf_mbx_retry_pending); + callout_drain(&sc->vf_mbx_retry); + ifp = iflib_get_ifp(sc->ctx); + if ((if_getflags(ifp) & IFF_UP) == 0) + sc->vf_mbx_retry_stage = 0; +} + +void +igbv_mbx_retry_failed(if_ctx_t ctx) +{ + struct e1000_softc *sc; + if_t ifp; + sbintime_t delay; + u_int stage; + + sc = iflib_get_softc(ctx); + atomic_store_rel_32(&sc->vf_mbx_ready, 0); + sc->link_speed = 0; + sc->link_duplex = 0; + if (sc->link_state != EM_LINK_STATE_DOWN) { + sc->link_state = EM_LINK_STATE_DOWN; + iflib_link_state_change(ctx, LINK_STATE_DOWN, 0); + } + iflib_init_failed(ctx); + + ifp = iflib_get_ifp(ctx); + if (!sc->vf_mbx_retry_initialized || + (if_getflags(ifp) & IFF_UP) == 0) + return; + stage = sc->vf_mbx_retry_stage; + if (stage >= nitems(igbv_mbx_retry_delay)) + stage = nitems(igbv_mbx_retry_delay) - 1; + delay = igbv_mbx_retry_delay[stage]; + if (sc->vf_mbx_retry_stage + 1 < nitems(igbv_mbx_retry_delay)) + sc->vf_mbx_retry_stage++; + atomic_set_32(&sc->vf_mbx_retry_pending, 1); + callout_reset_sbt(&sc->vf_mbx_retry, delay, 0, + igbv_mbx_retry_callout, sc, C_PREL(1)); +} + +static void +igbv_mbx_retry_succeeded(struct e1000_softc *sc) +{ + + atomic_store_rel_32(&sc->vf_mbx_ready, 1); + atomic_readandclear_32(&sc->vf_mbx_retry_pending); + if (sc->vf_mbx_retry_initialized) + callout_stop(&sc->vf_mbx_retry); + sc->vf_mbx_retry_stage = 0; + sc->vf_last_mbx_log.tv_sec = 0; + sc->vf_last_mbx_log.tv_usec = 0; +} + void igbv_queue_retry_detach(struct e1000_softc *sc) { @@ -150,7 +291,7 @@ igbv_queue_retry_failed(if_ctx_t ctx) } iflib_link_state_change(ctx, LINK_STATE_DOWN, 0); - iflib_admin_intr_deferred(ctx); + iflib_init_failed(ctx); } void @@ -251,6 +392,8 @@ igbv_if_attach_pre(if_ctx_t ctx) sc = iflib_get_softc(ctx); callout_init(&sc->vf_queue_retry, 1); sc->vf_queue_retry_initialized = true; + callout_init(&sc->vf_mbx_retry, 1); + sc->vf_mbx_retry_initialized = true; KASSERT(sc->vf_ifp && (iflib_get_sctx(ctx)->isc_flags & IFLIB_IS_VF) != 0, @@ -303,15 +446,15 @@ igbv_if_update_admin_status(if_ctx_t ctx) dev = iflib_get_dev(ctx); KASSERT(sc->vf_ifp, ("%s called for a PF", __func__)); - /* - * iflib's init callback cannot report failure and marks the interface - * running after it returns. Complete the failed-init transition from - * this deferred task, after iflib has set its driver flags. - */ - if (!sc->vf_queues_sanitized) { - igbv_if_intr_disable(ctx); - if_setdrvflagbits(iflib_get_ifp(ctx), IFF_DRV_OACTIVE, - IFF_DRV_RUNNING); + if ((if_getdrvflags(iflib_get_ifp(ctx)) & IFF_DRV_RUNNING) == 0 || + !sc->vf_queues_sanitized || + atomic_load_acq_32(&sc->vf_mbx_ready) == 0) { + if (sc->link_state != EM_LINK_STATE_DOWN) { + sc->link_speed = 0; + sc->link_duplex = 0; + sc->link_state = EM_LINK_STATE_DOWN; + iflib_link_state_change(ctx, LINK_STATE_DOWN, 0); + } return; } @@ -485,14 +628,14 @@ igbv_reset(if_ctx_t ctx) }; error = e1000_reset_hw(hw); + atomic_store_rel_32(&sc->vf_mbx_ready, 0); sc->vf_queues_sanitized = igbv_sanitize_queues(sc); if (!sc->vf_queues_sanitized) { - e1000_check_for_link(hw); return (false); } igbv_queue_retry_succeeded(sc); if (error != E1000_SUCCESS) { - e1000_check_for_link(hw); + igbv_log_reset_failure(sc, error, false); return (false); } memset(sc->vf_vfta_stale, 0, sizeof(sc->vf_vfta_stale)); @@ -504,6 +647,7 @@ igbv_reset(if_ctx_t ctx) return (false); } e1000_check_for_link(hw); + igbv_mbx_retry_succeeded(sc); return (true); } @@ -535,7 +679,8 @@ igbv_if_intr_enable(if_ctx_t ctx) sc = iflib_get_softc(ctx); hw = &sc->hw; KASSERT(sc->vf_ifp, ("%s called for a PF", __func__)); - if (!sc->vf_queues_sanitized) + if (!sc->vf_queues_sanitized || + atomic_load_acq_32(&sc->vf_mbx_ready) == 0) return; mask = sc->que_mask | sc->link_mask;home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a7911b2.3b0d7.4be27304>