Date: Sun, 09 Aug 2026 23:38:36 +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: 9328a7eedba1 - main - iflib: Support recoverable initialization failure Message-ID: <6a790f7c.3a865.f2736ce@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=9328a7eedba115040312bd1ea368371a0dbd0cac commit 9328a7eedba115040312bd1ea368371a0dbd0cac Author: Kevin Bowling <kbowling@FreeBSD.org> AuthorDate: 2026-08-09 07:16:56 +0000 Commit: Kevin Bowling <kbowling@FreeBSD.org> CommitDate: 2026-08-09 23:38:29 +0000 iflib: Support recoverable initialization failure The ifdi_init method cannot report an error, so iflib always marks an interface running and enables its interrupts after the callback returns. Drivers whose hardware initialization depends on an unavailable peer can only return early and leave a falsely running interface. Add iflib_init_failed() so a callback can leave the interface stopped. Also add a conditional reset request for asynchronous recovery: it is discarded if the interface is administratively down when the admin task runs, preventing a queued retry from resurrecting a stopped interface. Do not restore saved driver flags after an MTU or capability change when initialization failed. Restoring the pre-init flags would overwrite the stopped result with stale RUNNING state. Document that reset requests require the caller to schedule the admin task, that output remains blocked during recovery, and that iflib rather than the driver owns the driver flags. MFC after: 2 weeks --- share/man/man9/Makefile | 3 +++ share/man/man9/iflibdd.9 | 11 +++++++---- share/man/man9/iflibdi.9 | 35 ++++++++++++++++++++++++++++++++++ sys/net/iflib.c | 49 ++++++++++++++++++++++++++++++++++++++++++------ sys/net/iflib.h | 5 +++++ 5 files changed, 93 insertions(+), 10 deletions(-) diff --git a/share/man/man9/Makefile b/share/man/man9/Makefile index 6df531d5300c..1deb3d048f12 100644 --- a/share/man/man9/Makefile +++ b/share/man/man9/Makefile @@ -1355,8 +1355,11 @@ MLINKS+=iflibdi.9 iflib_add_int_delay_sysctl.9 \ iflibdi.9 iflib_led_create.9 \ iflibdi.9 iflib_irq_alloc.9 \ iflibdi.9 iflib_irq_alloc_generic.9 \ + iflibdi.9 iflib_init_failed.9 \ iflibdi.9 iflib_link_intr_deferred.9 \ iflibdi.9 iflib_link_state_change.9 \ + iflibdi.9 iflib_request_reset.9 \ + iflibdi.9 iflib_request_reset_if_up.9 \ iflibdi.9 iflib_rx_intr_deferred.9 \ iflibdi.9 iflib_tx_intr_deferred.9 MLINKS+=iflibtxrx.9 isc_rxd_available.9 \ diff --git a/share/man/man9/iflibdd.9 b/share/man/man9/iflibdd.9 index 67645fe5c87a..7777cbaa0022 100644 --- a/share/man/man9/iflibdd.9 +++ b/share/man/man9/iflibdd.9 @@ -1,4 +1,4 @@ -.Dd May 3, 2018 +.Dd August 8, 2026 .Dt IFLIBDD 9 .Os .Sh NAME @@ -245,9 +245,12 @@ Optional function that resumes a driver. .It Fn ifdi_init Mandatory function that will initialize and bring up the hardware. For example, it will reset the chip and enable the receiver unit. -It should mark the interface running, but not active ( -.Dv IFF_DRV_RUNNING , -.Dv ~IIF_DRV_OACTIVE ). +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 +.Fn iflib_init_failed +before returning. .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. diff --git a/share/man/man9/iflibdi.9 b/share/man/man9/iflibdi.9 index b148f74c560b..edc2c7f172f1 100644 --- a/share/man/man9/iflibdi.9 +++ b/share/man/man9/iflibdi.9 @@ -6,6 +6,7 @@ .Nd Device Independent Configuration Functions .Sh SYNOPSIS .In "ifdi_if.h" +.In "net/iflib.h" .Ss "Device Independent Functions" .Ft int .Fo iflib_device_attach @@ -80,6 +81,18 @@ .Fa "int linkstate" .Fc .Ft void +.Fo iflib_request_reset +.Fa "if_ctx_t ctx" +.Fc +.Ft void +.Fo iflib_request_reset_if_up +.Fa "if_ctx_t ctx" +.Fc +.Ft void +.Fo iflib_init_failed +.Fa "if_ctx_t ctx" +.Fc +.Ft void .Fo iflib_add_int_delay_sysctl .Fa "if_ctx_t ctx" .Fa "const char *" @@ -225,6 +238,28 @@ The following link states are currently defined: The link is up. .It Dv LINK_STATE_DOWN The link is down. +.It Fn iflib_request_reset +Request that the admin task stop and reinitialize the interface. +.It Fn iflib_request_reset_if_up +Request that the admin task stop and reinitialize the interface only if it +remains administratively up when the task runs. +This form is suitable for asynchronous recovery work that must not restart an +interface after an intervening administrative down operation. +Like +.Fn iflib_request_reset , +this function only records the request; the caller must schedule the admin +task, normally with +.Fn iflib_admin_intr_deferred . +.It Fn iflib_init_failed +Report that the current +.Fn ifdi_init +callback could not initialize the hardware. +The driver must leave the hardware stopped and call this function while the +callback holds the iflib context lock. +Iflib then leaves +.Dv IFF_DRV_RUNNING +clear and does not enable interrupts or periodic timers. +Output remains blocked so explicitly scheduled admin recovery work can run. .It Fn iflib_add_int_delay_sysctl Modifies settings to user defined values for a given set of variables. .El diff --git a/sys/net/iflib.c b/sys/net/iflib.c index 8322dbfb074d..00f4f2921dd2 100644 --- a/sys/net/iflib.c +++ b/sys/net/iflib.c @@ -337,13 +337,13 @@ typedef struct iflib_sw_tx_desc_array { #define IFC_LEGACY 0x001 #define IFC_QFLUSH 0x002 #define IFC_MULTISEG 0x004 -#define IFC_SPARE1 0x008 +#define IFC_INIT_FAILED 0x008 #define IFC_SC_ALLOCATED 0x010 #define IFC_INIT_DONE 0x020 #define IFC_PREFETCH 0x040 #define IFC_DO_RESET 0x080 #define IFC_DO_WATCHDOG 0x100 -#define IFC_SPARE0 0x200 +#define IFC_DO_RESET_IF_UP 0x200 #define IFC_SPARE2 0x400 #define IFC_IN_DETACH 0x800 @@ -2555,6 +2555,7 @@ iflib_init_locked(if_ctx_t ctx) iflib_txq_t txq; iflib_rxq_t rxq; int i, j, tx_ip_csum_flags, tx_ip6_csum_flags; + bool init_failed; if_setdrvflagbits(ifp, IFF_DRV_OACTIVE, IFF_DRV_RUNNING); IFDI_INTR_DISABLE(ctx); @@ -2598,8 +2599,16 @@ iflib_init_locked(if_ctx_t ctx) #ifdef INVARIANTS i = if_getdrvflags(ifp); #endif + STATE_LOCK(ctx); + ctx->ifc_flags &= ~IFC_INIT_FAILED; + STATE_UNLOCK(ctx); IFDI_INIT(ctx); MPASS(if_getdrvflags(ifp) == i); + STATE_LOCK(ctx); + init_failed = (ctx->ifc_flags & IFC_INIT_FAILED) != 0; + STATE_UNLOCK(ctx); + if (init_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. */ @@ -4187,15 +4196,18 @@ _task_fn_admin(void *context, int pending) if_softc_ctx_t sctx = &ctx->ifc_softc_ctx; iflib_txq_t txq; int i; - bool oactive, running, do_reset, do_watchdog, in_detach; + bool oactive, running, do_reset, do_reset_if_up, do_watchdog; + bool in_detach; STATE_LOCK(ctx); running = (if_getdrvflags(ctx->ifc_ifp) & IFF_DRV_RUNNING); oactive = (if_getdrvflags(ctx->ifc_ifp) & IFF_DRV_OACTIVE); do_reset = (ctx->ifc_flags & IFC_DO_RESET); + do_reset_if_up = (ctx->ifc_flags & IFC_DO_RESET_IF_UP); do_watchdog = (ctx->ifc_flags & IFC_DO_WATCHDOG); in_detach = (ctx->ifc_flags & IFC_IN_DETACH); - ctx->ifc_flags &= ~(IFC_DO_RESET | IFC_DO_WATCHDOG); + ctx->ifc_flags &= ~(IFC_DO_RESET | IFC_DO_RESET_IF_UP | + IFC_DO_WATCHDOG); STATE_UNLOCK(ctx); if ((!running && !oactive) && !(ctx->ifc_sctx->isc_flags & IFLIB_ADMIN_ALWAYS_RUN)) @@ -4204,6 +4216,9 @@ _task_fn_admin(void *context, int pending) return; CTX_LOCK(ctx); + if (!do_reset && do_reset_if_up && + (if_getflags(ctx->ifc_ifp) & IFF_UP) != 0) + do_reset = true; for (txq = ctx->ifc_txqs, i = 0; i < sctx->isc_ntxqsets; i++, txq++) { CALLOUT_LOCK(txq); callout_stop(&txq->ift_timer); @@ -4529,7 +4544,9 @@ iflib_if_ioctl(if_t ifp, u_long command, caddr_t data) } iflib_init_locked(ctx); STATE_LOCK(ctx); - if_setdrvflags(ifp, bits); + /* Preserve the stopped state reported by iflib_init_failed(). */ + if ((ctx->ifc_flags & IFC_INIT_FAILED) == 0) + if_setdrvflags(ifp, bits); STATE_UNLOCK(ctx); CTX_UNLOCK(ctx); break; @@ -4638,7 +4655,8 @@ iflib_if_ioctl(if_t ifp, u_long command, caddr_t data) if (bits & IFF_DRV_RUNNING && setmask & ~IFCAP_WOL) iflib_init_locked(ctx); STATE_LOCK(ctx); - if_setdrvflags(ifp, bits); + if ((ctx->ifc_flags & IFC_INIT_FAILED) == 0) + if_setdrvflags(ifp, bits); STATE_UNLOCK(ctx); CTX_UNLOCK(ctx); } @@ -7220,6 +7238,25 @@ iflib_request_reset(if_ctx_t ctx) STATE_UNLOCK(ctx); } +void +iflib_request_reset_if_up(if_ctx_t ctx) +{ + + STATE_LOCK(ctx); + ctx->ifc_flags |= IFC_DO_RESET_IF_UP; + STATE_UNLOCK(ctx); +} + +void +iflib_init_failed(if_ctx_t ctx) +{ + + sx_assert(&ctx->ifc_ctx_sx, SA_XLOCKED); + STATE_LOCK(ctx); + ctx->ifc_flags |= IFC_INIT_FAILED; + STATE_UNLOCK(ctx); +} + #ifndef __NO_STRICT_ALIGNMENT static struct mbuf * iflib_fixup_rx(struct mbuf *m) diff --git a/sys/net/iflib.h b/sys/net/iflib.h index 6e9c6e10cc5d..40080b49b0ab 100644 --- a/sys/net/iflib.h +++ b/sys/net/iflib.h @@ -428,6 +428,11 @@ if_shared_ctx_t iflib_get_sctx(if_ctx_t ctx); void iflib_set_mac(if_ctx_t ctx, uint8_t mac[ETHER_ADDR_LEN]); void iflib_request_reset(if_ctx_t ctx); +/* Defer a reset, but discard it if the interface is administratively down. */ +void iflib_request_reset_if_up(if_ctx_t ctx); + +/* Report an error from the otherwise void ifdi_init method while it runs. */ +void iflib_init_failed(if_ctx_t ctx); uint8_t iflib_in_detach(if_ctx_t ctx); uint32_t iflib_get_rx_mbuf_sz(if_ctx_t ctx);home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a790f7c.3a865.f2736ce>