git: fb8fcd4bff36 - main - iflib: Track queue datapath lifecycle
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 11 Sep 2026 19:30:53 UTC
The branch main has been updated by kbowling:
URL: https://cgit.FreeBSD.org/src/commit/?id=fb8fcd4bff36d620d00fc20903379fb9ee844c57
commit fb8fcd4bff36d620d00fc20903379fb9ee844c57
Author: Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2026-08-26 09:21:55 +0000
Commit: Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2026-09-11 19:27:45 +0000
iflib: Track queue datapath lifecycle
Track whether iflib queue mappings may still be accessed by the
device. Keep the state private to iflib and conservative: an unknown
or failed device must pass through IFDI_STOP() before mappings are
reused or released, while a device known to be stopped need not
receive another hardware stop.
Enter the starting state before IFDI_INIT(), publish running only
after receive buffers and framework state are ready, and stop hardware
if receive-buffer setup fails after driver initialization.
Do not initialize an administratively-down interface merely because
its MTU, capabilities, VLAN configuration, or media changed. Preserve
successful retries for an administratively-up interface whose previous
initialization failed.
This state describes ownership of iflib datapath mappings only. It
deliberately makes no claim about firmware queues, administrative DMA,
PCI power state, or whether a driver can safely elide a hardware reset.
Reviewed by: iflib (gallatin)
MFC after: 2 weeks
Sponsored by: BBOX.io
Differential Revision: https://reviews.freebsd.org/D59328
---
share/man/man9/iflibdd.9 | 14 ++++--
sys/net/iflib.c | 127 +++++++++++++++++++++++++++++++++++------------
2 files changed, 106 insertions(+), 35 deletions(-)
diff --git a/share/man/man9/iflibdd.9 b/share/man/man9/iflibdd.9
index 7777cbaa0022..d42ad6d2872e 100644
--- a/share/man/man9/iflibdd.9
+++ b/share/man/man9/iflibdd.9
@@ -248,12 +248,20 @@ For example, it will reset the chip and enable the receiver unit.
Iflib marks the interface running after the callback returns successfully;
the driver must not modify the driver flags itself.
If initialization cannot complete, the driver must leave the hardware stopped
-and call
+or in a state in which
+.Fn ifdi_stop
+can safely quiesce it, and call
.Fn iflib_init_failed
before returning.
+Iflib treats this report conservatively and invokes the driver stop method
+before retrying initialization or releasing queue mappings.
.It Fn ifdi_stop
-Mandatory function that should disable all traffic on the interface by issuing
-a global reset on the MAC and deallocating the TX and RX buffers.
+Mandatory function that performs a terminal stop and disables all traffic on
+the interface.
+The driver must stop DMA before returning; iflib releases the TX and RX
+buffers after the callback.
+The method commonly issues a global reset and may also put the PHY or device
+into its administratively-down state.
.It Fn ifdi_multi_set
Programs the interfaces multicast addresses
.It Fn ifdi_media_status
diff --git a/sys/net/iflib.c b/sys/net/iflib.c
index cc74fbcca607..d4c6b02762fe 100644
--- a/sys/net/iflib.c
+++ b/sys/net/iflib.c
@@ -142,6 +142,27 @@ typedef struct iflib_fl *iflib_fl_t;
struct iflib_ctx;
+/*
+ * This state describes access to queue mappings owned by iflib. It does not
+ * describe driver-owned administrative DMA or the PCI function's power state.
+ * Normal transitions are serialized by ifc_ctx_sx.
+ *
+ * Only STOPPED establishes that the device can no longer access the mappings;
+ * a failed initialization can leave queues active. IFF_UP separately records
+ * administrative intent. Existing datapath users still use IFF_DRV_RUNNING,
+ * but clearing that flag does not establish quiescence: the watchdog clears
+ * it before the admin task stops the hardware. Use this state for lifecycle
+ * decisions under ifc_ctx_sx, not as an unlocked datapath admission check.
+ */
+enum iflib_datapath_state {
+ IFLIB_DP_UNKNOWN = 0,
+ IFLIB_DP_STOPPED,
+ IFLIB_DP_FAILED,
+ IFLIB_DP_STARTING,
+ IFLIB_DP_RUNNING,
+ IFLIB_DP_STOPPING,
+};
+
static void iru_init(if_rxd_update_t iru, iflib_rxq_t rxq, uint8_t flid);
static void iflib_timer(void *arg);
static void iflib_tqg_detach(if_ctx_t ctx);
@@ -176,6 +197,7 @@ struct iflib_ctx {
iflib_rxq_t ifc_rxqs;
uint32_t ifc_if_flags;
uint32_t ifc_flags;
+ enum iflib_datapath_state ifc_datapath_state;
uint32_t ifc_max_fl_buf_size;
uint32_t ifc_rx_mbuf_sz;
@@ -2587,6 +2609,11 @@ iflib_init_locked(if_ctx_t ctx)
int i, j, tx_ip_csum_flags, tx_ip6_csum_flags;
bool init_failed;
+ sx_assert(&ctx->ifc_ctx_sx, SA_XLOCKED);
+ KASSERT(ctx->ifc_datapath_state == IFLIB_DP_STOPPED,
+ ("iflib init from datapath state %d", ctx->ifc_datapath_state));
+ ctx->ifc_datapath_state = IFLIB_DP_STARTING;
+
if_setdrvflagbits(ifp, IFF_DRV_OACTIVE, IFF_DRV_RUNNING);
IFDI_INTR_DISABLE(ctx);
@@ -2637,8 +2664,15 @@ iflib_init_locked(if_ctx_t ctx)
STATE_LOCK(ctx);
init_failed = (ctx->ifc_flags & IFC_INIT_FAILED) != 0;
STATE_UNLOCK(ctx);
- if (init_failed)
+ if (init_failed) {
+ /*
+ * IFDI_INIT failed, but that alone does not prove that the
+ * driver stopped every queue or fenced DMA. Force the next
+ * lifecycle transition through the driver's stop method.
+ */
+ ctx->ifc_datapath_state = IFLIB_DP_FAILED;
return;
+ }
for (i = 0, rxq = ctx->ifc_rxqs; i < scctx->isc_nrxqsets; i++, rxq++) {
if (iflib_netmap_rxq_init(ctx, rxq) > 0) {
/* This rxq is in netmap mode. Skip normal init. */
@@ -2649,11 +2683,16 @@ iflib_init_locked(if_ctx_t ctx)
device_printf(ctx->ifc_dev,
"setting up free list %d failed - "
"check cluster settings\n", j);
- goto done;
+ /*
+ * IFDI_INIT has started the hardware. Stop it before
+ * releasing partially populated receive mappings.
+ */
+ iflib_init_failed(ctx);
+ iflib_stop(ctx);
+ return;
}
}
}
-done:
if_setdrvflagbits(ctx->ifc_ifp, IFF_DRV_RUNNING, IFF_DRV_OACTIVE);
IFDI_INTR_ENABLE(ctx);
txq = ctx->ifc_txqs;
@@ -2663,16 +2702,20 @@ done:
/* Re-enable txsync/rxsync. */
netmap_enable_all_rings(ifp);
+ ctx->ifc_datapath_state = IFLIB_DP_RUNNING;
}
static int
iflib_media_change(if_t ifp)
{
if_ctx_t ctx = if_getsoftc(ifp);
+ bool restart;
int err;
CTX_LOCK(ctx);
- if ((err = IFDI_MEDIA_CHANGE(ctx)) == 0)
+ restart = (if_getflags(ifp) & IFF_UP) != 0 ||
+ ctx->ifc_datapath_state == IFLIB_DP_RUNNING;
+ if ((err = IFDI_MEDIA_CHANGE(ctx)) == 0 && restart)
iflib_if_init_locked(ctx);
CTX_UNLOCK(ctx);
return (err);
@@ -2712,15 +2755,25 @@ iflib_stop(if_ctx_t ctx)
if_shared_ctx_t sctx = ctx->ifc_sctx;
iflib_dma_info_t di;
iflib_fl_t fl;
+ bool stop_hardware;
int i, j;
+ sx_assert(&ctx->ifc_ctx_sx, SA_XLOCKED);
+ KASSERT(ctx->ifc_datapath_state != IFLIB_DP_STOPPING,
+ ("recursive iflib stop"));
+ stop_hardware = ctx->ifc_datapath_state != IFLIB_DP_STOPPED;
+
/* Tell the stack that the interface is no longer active */
if_setdrvflagbits(ctx->ifc_ifp, IFF_DRV_OACTIVE, IFF_DRV_RUNNING);
- IFDI_INTR_DISABLE(ctx);
- DELAY(1000);
- IFDI_STOP(ctx);
- DELAY(1000);
+ if (stop_hardware) {
+ ctx->ifc_datapath_state = IFLIB_DP_STOPPING;
+ IFDI_INTR_DISABLE(ctx);
+ DELAY(1000);
+ IFDI_STOP(ctx);
+ DELAY(1000);
+ ctx->ifc_datapath_state = IFLIB_DP_STOPPED;
+ }
/*
* Stop any pending txsync/rxsync and prevent new ones
@@ -4344,7 +4397,8 @@ iflib_sysctl_int_delay(SYSCTL_HANDLER_ARGS)
static void
iflib_if_init_locked(if_ctx_t ctx)
{
- iflib_stop(ctx);
+ if (ctx->ifc_datapath_state != IFLIB_DP_STOPPED)
+ iflib_stop(ctx);
iflib_init_locked(ctx);
}
@@ -4553,8 +4607,8 @@ iflib_if_ioctl(if_t ifp, u_long command, caddr_t data)
#if defined(INET) || defined(INET6)
struct ifaddr *ifa = (struct ifaddr *)data;
#endif
- bool avoid_reset = false;
- int err = 0, reinit = 0, bits;
+ bool avoid_reset = false, restart;
+ int err = 0, reinit = 0;
switch (command) {
case SIOCSIFADDR:
@@ -4587,9 +4641,11 @@ iflib_if_ioctl(if_t ifp, u_long command, caddr_t data)
CTX_UNLOCK(ctx);
break;
}
- bits = if_getdrvflags(ifp);
- /* stop the driver and free any clusters before proceeding */
- iflib_stop(ctx);
+ restart = ctx->ifc_datapath_state == IFLIB_DP_RUNNING ||
+ (if_getflags(ifp) & IFF_UP) != 0;
+ /* Quiesce a datapath whose stopped state is not established. */
+ if (ctx->ifc_datapath_state != IFLIB_DP_STOPPED)
+ iflib_stop(ctx);
if ((err = IFDI_MTU_SET(ctx, ifr->ifr_mtu)) == 0) {
STATE_LOCK(ctx);
@@ -4600,12 +4656,8 @@ iflib_if_ioctl(if_t ifp, u_long command, caddr_t data)
STATE_UNLOCK(ctx);
err = if_setmtu(ifp, ifr->ifr_mtu);
}
- iflib_init_locked(ctx);
- STATE_LOCK(ctx);
- /* Preserve the stopped state reported by iflib_init_failed(). */
- if ((ctx->ifc_flags & IFC_INIT_FAILED) == 0)
- if_setdrvflags(ifp, bits);
- STATE_UNLOCK(ctx);
+ if (restart)
+ iflib_init_locked(ctx);
CTX_UNLOCK(ctx);
break;
case SIOCSIFFLAGS:
@@ -4620,7 +4672,8 @@ iflib_if_ioctl(if_t ifp, u_long command, caddr_t data)
}
} else
reinit = 1;
- } else if (if_getdrvflags(ifp) & IFF_DRV_RUNNING) {
+ } else if (ctx->ifc_datapath_state != IFLIB_DP_STOPPED) {
+ /* Stop partially initialized hardware as well as running queues. */
iflib_stop(ctx);
}
ctx->ifc_if_flags = if_getflags(ifp);
@@ -4703,19 +4756,17 @@ iflib_if_ioctl(if_t ifp, u_long command, caddr_t data)
*/
if (setmask) {
CTX_LOCK(ctx);
- bits = if_getdrvflags(ifp);
- if (bits & IFF_DRV_RUNNING && setmask & ~IFCAP_WOL)
+ restart = (setmask & ~IFCAP_WOL) != 0 &&
+ (ctx->ifc_datapath_state == IFLIB_DP_RUNNING ||
+ (if_getflags(ifp) & IFF_UP) != 0);
+ if (restart)
iflib_stop(ctx);
STATE_LOCK(ctx);
if_togglecapenable(ifp, setmask);
ctx->ifc_softc_ctx.isc_capenable ^= setmask;
STATE_UNLOCK(ctx);
- if (bits & IFF_DRV_RUNNING && setmask & ~IFCAP_WOL)
+ if (restart)
iflib_init_locked(ctx);
- STATE_LOCK(ctx);
- if ((ctx->ifc_flags & IFC_INIT_FAILED) == 0)
- if_setdrvflags(ifp, bits);
- STATE_UNLOCK(ctx);
CTX_UNLOCK(ctx);
}
if_vlancap(ifp);
@@ -4773,6 +4824,7 @@ static void
iflib_vlan_register(void *arg, if_t ifp, uint16_t vtag)
{
if_ctx_t ctx = if_getsoftc(ifp);
+ bool restart;
if ((void *)ctx != arg)
return;
@@ -4784,12 +4836,15 @@ iflib_vlan_register(void *arg, if_t ifp, uint16_t vtag)
return;
CTX_LOCK(ctx);
+ restart = IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_VLAN_CONFIG) &&
+ ((if_getflags(ifp) & IFF_UP) != 0 ||
+ ctx->ifc_datapath_state == IFLIB_DP_RUNNING);
/* Driver may need all untagged packets to be flushed */
- if (IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_VLAN_CONFIG))
+ if (restart)
iflib_stop(ctx);
IFDI_VLAN_REGISTER(ctx, vtag);
/* Re-init to load the changes, if required */
- if (IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_VLAN_CONFIG))
+ if (restart)
iflib_init_locked(ctx);
CTX_UNLOCK(ctx);
}
@@ -4798,6 +4853,7 @@ static void
iflib_vlan_unregister(void *arg, if_t ifp, uint16_t vtag)
{
if_ctx_t ctx = if_getsoftc(ifp);
+ bool restart;
if ((void *)ctx != arg)
return;
@@ -4806,12 +4862,15 @@ iflib_vlan_unregister(void *arg, if_t ifp, uint16_t vtag)
return;
CTX_LOCK(ctx);
+ restart = IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_VLAN_CONFIG) &&
+ ((if_getflags(ifp) & IFF_UP) != 0 ||
+ ctx->ifc_datapath_state == IFLIB_DP_RUNNING);
/* Driver may need all tagged packets to be flushed */
- if (IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_VLAN_CONFIG))
+ if (restart)
iflib_stop(ctx);
IFDI_VLAN_UNREGISTER(ctx, vtag);
/* Re-init to load the changes, if required */
- if (IFDI_NEEDS_RESTART(ctx, IFLIB_RESTART_VLAN_CONFIG))
+ if (restart)
iflib_init_locked(ctx);
CTX_UNLOCK(ctx);
}
@@ -5302,6 +5361,7 @@ iflib_device_register(device_t dev, void *sc, if_shared_ctx_t sctx, if_ctx_t *ct
intr_allocated = false;
queues_allocated = false;
ctx = malloc(sizeof(*ctx), M_IFLIB, M_WAITOK | M_ZERO);
+ ctx->ifc_datapath_state = IFLIB_DP_UNKNOWN;
if (sc == NULL) {
sc = malloc(sctx->isc_driver->size, M_IFLIB, M_WAITOK | M_ZERO);
@@ -7443,6 +7503,9 @@ iflib_init_failed(if_ctx_t ctx)
{
sx_assert(&ctx->ifc_ctx_sx, SA_XLOCKED);
+ KASSERT(ctx->ifc_datapath_state == IFLIB_DP_STARTING,
+ ("iflib_init_failed outside IFDI_INIT, state %d",
+ ctx->ifc_datapath_state));
STATE_LOCK(ctx);
ctx->ifc_flags |= IFC_INIT_FAILED;
STATE_UNLOCK(ctx);