git: 468384c8bd11 - main - ice: Fix SR-IOV VF resource cleanup
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 17 Sep 2026 08:27:05 UTC
The branch main has been updated by kbowling:
URL: https://cgit.FreeBSD.org/src/commit/?id=468384c8bd1130a880fd87ac5c2d182123f9f6ec
commit 468384c8bd1130a880fd87ac5c2d182123f9f6ec
Author: Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2026-08-18 08:48:36 +0000
Commit: Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2026-09-17 08:26:18 +0000
ice: Fix SR-IOV VF resource cleanup
ice_iov_uninit() freed each VF interrupt-map array without returning the
reserved indices to the device interrupt resource manager. Repeated VF
create and destroy cycles therefore exhausted the PF interrupt map even
though no VFs remained.
Return the interrupt allocation before freeing its map. Also split
software-only VSI release from hardware teardown so failures before
ice_initialize_vsi() do not issue invalid RSS, scheduler, and Free VSI
commands for an object firmware has never seen.
Keep a VF disabled until all of its resources and hardware state have
been created successfully. Clear the enabled state before teardown and
after any failed add so asynchronous mailbox processing cannot use a
partial or freed VSI. Consume VFLR status for inactive VF slots without
trying to reset a nonexistent VSI.
Track whether firmware currently owns each VSI and clear that ownership
after resets. Teardown can then skip AdminQ commands for VSIs which
were not rebuilt. Remove every VSI switch filter before firmware
teardown, matching Linux and preventing filter-list leaks across
create and destroy cycles. This also applies to the PF VSI detach path.
Validated on an E810-XXV with two consecutive create and destroy cycles
of 128 four-queue VFs. A 16-queue VF could then be created. Two
oversized configurations each failed, cleaned back to zero VFs without
invalid firmware teardown commands, and were each followed by a
successful 16-queue VF creation.
MFC after: 2 weeks
Sponsored by: BBOX.io
Differential Revision: https://reviews.freebsd.org/D58908
---
sys/dev/ice/ice_iov.c | 28 ++++++++++++---
sys/dev/ice/ice_lib.c | 85 ++++++++++++++++++++++++++++++++--------------
sys/dev/ice/ice_lib.h | 2 ++
sys/dev/ice/if_ice_iflib.c | 6 ++++
4 files changed, 91 insertions(+), 30 deletions(-)
diff --git a/sys/dev/ice/ice_iov.c b/sys/dev/ice/ice_iov.c
index 0d53f8f14253..869fe43a9411 100644
--- a/sys/dev/ice/ice_iov.c
+++ b/sys/dev/ice/ice_iov.c
@@ -227,7 +227,7 @@ ice_iov_add_vf(struct ice_softc *sc, uint16_t vfnum, const nvlist_t *params)
int i;
vf = ice_iov_get_vf(sc, vfnum);
- vf->vf_flags = VF_FLAG_ENABLED;
+ vf->vf_flags = 0;
/* This VF needs at least one VSI */
vsi = ice_alloc_vsi(sc, ICE_VSI_VF);
@@ -385,6 +385,7 @@ ice_iov_add_vf(struct ice_softc *sc, uint16_t vfnum, const nvlist_t *params)
goto release_imap;
}
+ atomic_set_32(&vf->vf_flags, VF_FLAG_ENABLED);
ice_iov_ready_vf(sc, vf);
return (0);
@@ -408,8 +409,12 @@ free_txqs:
free(vsi->tx_queues, M_ICE);
vsi->tx_queues = NULL;
release_vsi:
- ice_release_vsi(vsi);
+ if (vsi->hw_vsi_created)
+ ice_release_vsi(vsi);
+ else
+ ice_release_vsi_resources(vsi);
vf->vsi = NULL;
+ atomic_store_rel_32(&vf->vf_flags, 0);
return (error);
}
@@ -426,10 +431,13 @@ ice_iov_uninit(struct ice_softc *sc)
/* Release per-VF resources */
for (int i = 0; i < sc->num_vfs; i++) {
vf = &sc->vfs[i];
+ atomic_store_rel_32(&vf->vf_flags, 0);
vsi = vf->vsi;
/* Free VF interrupt reservation */
if (vf->vf_imap) {
+ ice_resmgr_release_map(&sc->dev_imgr, vf->vf_imap,
+ vf->num_irq_vectors);
free(vf->vf_imap, M_ICE);
vf->vf_imap = NULL;
}
@@ -457,7 +465,10 @@ ice_iov_uninit(struct ice_softc *sc)
vsi->rx_queues = NULL;
}
- ice_release_vsi(vsi);
+ if (vsi->hw_vsi_created)
+ ice_release_vsi(vsi);
+ else
+ ice_release_vsi_resources(vsi);
vf->vsi = NULL;
}
@@ -489,8 +500,17 @@ ice_iov_handle_vflr(struct ice_softc *sc)
reg_idx = (hw->func_caps.vf_base_id + vf->vf_num) / 32;
bit_idx = (hw->func_caps.vf_base_id + vf->vf_num) % 32;
reg = rd32(hw, GLGEN_VFLRSTAT(reg_idx));
- if (reg & BIT(bit_idx))
+ if ((reg & BIT(bit_idx)) == 0)
+ continue;
+ if ((atomic_load_acq_32(&vf->vf_flags) &
+ VF_FLAG_ENABLED) != 0 && vf->vsi != NULL) {
ice_reset_vf(sc, vf, false);
+ continue;
+ }
+
+ /* Consume reset events for inactive or incompletely added VFs. */
+ wr32(hw, GLGEN_VFLRSTAT(reg_idx), BIT(bit_idx));
+ ice_flush(hw);
}
}
diff --git a/sys/dev/ice/ice_lib.c b/sys/dev/ice/ice_lib.c
index 6af0d2e550d2..b92b55208b78 100644
--- a/sys/dev/ice/ice_lib.c
+++ b/sys/dev/ice/ice_lib.c
@@ -777,6 +777,7 @@ ice_initialize_vsi(struct ice_vsi *vsi)
ice_aq_str(hw->adminq.sq_last_status));
return (EIO);
}
+ vsi->hw_vsi_created = true;
vsi->info = ctx.info;
/* Initialize VSI with just 1 TC to start */
@@ -816,6 +817,8 @@ ice_deinit_vsi(struct ice_vsi *vsi)
/* Assert that the VSI pointer matches in the list */
MPASS(vsi == sc->all_vsi[vsi->idx]);
+ if (!vsi->hw_vsi_created)
+ return;
ctx.info = vsi->info;
@@ -837,9 +840,32 @@ ice_deinit_vsi(struct ice_vsi *vsi)
"Free VSI %u AQ call failed, err %s aq_err %s\n",
vsi->idx, ice_status_str(status),
ice_aq_str(hw->adminq.sq_last_status));
+ } else {
+ vsi->hw_vsi_created = false;
}
}
+/*
+ * Release the queue maps and storage owned by a VSI. Callers must remove
+ * the VSI sysctl context before reaching this helper.
+ */
+static void
+ice_free_vsi_resources(struct ice_vsi *vsi)
+{
+ struct ice_softc *sc = vsi->sc;
+ int idx = vsi->idx;
+
+ /* Assert that the VSI pointer matches in the list */
+ MPASS(vsi == sc->all_vsi[idx]);
+
+ ice_free_vsi_qmaps(vsi);
+
+ if (vsi->dynamic)
+ free(sc->all_vsi[idx], M_ICE);
+
+ sc->all_vsi[idx] = NULL;
+}
+
/**
* ice_release_vsi - Release resources associated with a VSI
* @vsi: the VSI to release
@@ -851,35 +877,39 @@ ice_deinit_vsi(struct ice_vsi *vsi)
void
ice_release_vsi(struct ice_vsi *vsi)
{
- struct ice_softc *sc = vsi->sc;
- int idx = vsi->idx;
-
- /* Assert that the VSI pointer matches in the list */
- MPASS(vsi == sc->all_vsi[idx]);
+ MPASS(vsi == vsi->sc->all_vsi[vsi->idx]);
/* Cleanup RSS configuration */
- if (ice_is_bit_set(sc->feat_en, ICE_FEATURE_RSS))
+ if (ice_is_bit_set(vsi->sc->feat_en, ICE_FEATURE_RSS))
ice_clean_vsi_rss_cfg(vsi);
+ /* Drain sysctl handlers before invalidating the hardware VSI. */
ice_del_vsi_sysctl_ctx(vsi);
- /* Remove the configured mirror rule, if it exists */
- ice_remove_vsi_mirroring(vsi);
-
- /*
- * If we unload the driver after a reset fails, we do not need to do
- * this step.
- */
- if (!ice_test_state(&sc->state, ICE_STATE_RESET_FAILED))
+ /* Do not issue firmware commands for a missing VSI or failed device. */
+ if (vsi->hw_vsi_created &&
+ !ice_test_state(&vsi->sc->state, ICE_STATE_RESET_FAILED)) {
+ ice_remove_vsi_mirroring(vsi);
+ ice_remove_vsi_fltr(&vsi->sc->hw, vsi->idx);
ice_deinit_vsi(vsi);
-
- ice_free_vsi_qmaps(vsi);
-
- if (vsi->dynamic) {
- free(sc->all_vsi[idx], M_ICE);
}
- sc->all_vsi[idx] = NULL;
+ ice_free_vsi_resources(vsi);
+}
+
+/**
+ * ice_release_vsi_resources - Release software resources for a VSI
+ * @vsi: the VSI to release
+ *
+ * Release resources allocated by ice_alloc_vsi() without issuing firmware
+ * commands. This is used when setup fails before ice_initialize_vsi() has
+ * attempted to create the VSI in hardware.
+ */
+void
+ice_release_vsi_resources(struct ice_vsi *vsi)
+{
+ ice_del_vsi_sysctl_ctx(vsi);
+ ice_free_vsi_resources(vsi);
}
/**
@@ -7908,13 +7938,16 @@ ice_clean_vsi_rss_cfg(struct ice_vsi *vsi)
device_t dev = sc->dev;
int status;
- status = ice_rem_vsi_rss_cfg(hw, vsi->idx);
- if (status)
- device_printf(dev,
- "Failed to remove RSS configuration for VSI %d, err %s\n",
- vsi->idx, ice_status_str(status));
+ if (vsi->hw_vsi_created &&
+ !ice_test_state(&sc->state, ICE_STATE_RESET_FAILED)) {
+ status = ice_rem_vsi_rss_cfg(hw, vsi->idx);
+ if (status)
+ device_printf(dev,
+ "Failed to remove RSS configuration for VSI %d, err %s\n",
+ vsi->idx, ice_status_str(status));
+ }
- /* Remove this VSI from the RSS list */
+ /* Remove software tracking even if the hardware VSI no longer exists. */
ice_rem_vsi_rss_list(hw, vsi->idx);
}
diff --git a/sys/dev/ice/ice_lib.h b/sys/dev/ice/ice_lib.h
index 2562a9e2476f..6c6f93b97228 100644
--- a/sys/dev/ice/ice_lib.h
+++ b/sys/dev/ice/ice_lib.h
@@ -558,6 +558,7 @@ struct ice_vsi {
struct ice_softc *sc;
bool dynamic; /* if true, dynamically allocated */
+ bool hw_vsi_created; /* firmware owns a VSI for this handle */
enum ice_vsi_type type; /* type of this VSI */
u16 idx; /* software index to sc->all_vsi[] */
@@ -935,6 +936,7 @@ int ice_map_bar(device_t dev, struct ice_bar_info *bar, int bar_num);
void ice_free_bar(device_t dev, struct ice_bar_info *bar);
void ice_set_ctrlq_len(struct ice_hw *hw);
void ice_release_vsi(struct ice_vsi *vsi);
+void ice_release_vsi_resources(struct ice_vsi *vsi);
struct ice_vsi *ice_alloc_vsi(struct ice_softc *sc, enum ice_vsi_type type);
void ice_alloc_vsi_qmap(struct ice_vsi *vsi, const int max_tx_queues,
const int max_rx_queues);
diff --git a/sys/dev/ice/if_ice_iflib.c b/sys/dev/ice/if_ice_iflib.c
index 7f6bd1e1e0a3..344e308597b1 100644
--- a/sys/dev/ice/if_ice_iflib.c
+++ b/sys/dev/ice/if_ice_iflib.c
@@ -2696,11 +2696,17 @@ ice_rebuild(struct ice_softc *sc)
enum ice_ddp_state pkg_state;
int status;
int err;
+ int i;
sc->rebuild_ticks = ticks;
/* If we're rebuilding, then a reset has succeeded. */
ice_clear_state(&sc->state, ICE_STATE_RESET_FAILED);
+ /* The reset discarded every firmware VSI before reconstruction. */
+ for (i = 0; i < sc->num_available_vsi; i++) {
+ if (sc->all_vsi[i] != NULL)
+ sc->all_vsi[i]->hw_vsi_created = false;
+ }
/*
* If the firmware is in recovery mode, only restore the limited