git: 9d59ca793f70 - main - iflib: Do not hold the ifnet lock across registration

From: Kevin Bowling <kbowling_at_FreeBSD.org>
Date: Wed, 02 Sep 2026 19:57:08 UTC
The branch main has been updated by kbowling:

URL: https://cgit.FreeBSD.org/src/commit/?id=9d59ca793f7097575351d6de2fd61431233b5ed9

commit 9d59ca793f7097575351d6de2fd61431233b5ed9
Author:     Kevin Bowling <kbowling@FreeBSD.org>
AuthorDate: 2026-08-31 23:24:49 +0000
Commit:     Kevin Bowling <kbowling@FreeBSD.org>
CommitDate: 2026-09-02 19:56:14 +0000

    iflib: Do not hold the ifnet lock across registration
    
    iflib_device_register() acquired IFNET_WLOCK to preserve lock order
    when ether_ifattach() was called with the context lock held.  The context
    lock is now released around ether_ifattach(), making registration-wide
    ifnet serialization unnecessary.
    
    Keeping IFNET_WLOCK across driver attachment also allows synchronous
    interface event handlers to recurse on it.  The rtnetlink interface-group
    dump does so through if_foreach_group() while handling the interface
    attachment event.
    
    Remove the outer lock and the corresponding failure-path unlock and
    relock transitions.  Continue to drop the context lock around
    ether_ifattach() and taskqueue drains, and preserve context-lock coverage
    for driver attach and detach.
    
    Validated under WITNESS on 82576 and I226 controllers.  Multiple VF
    attach and detach cycles, netmap control operations, and every iflib
    registration failure injection point completed without lock or cleanup
    errors.
    
    PR:             298121
    Reported by:    glebius, netchild, Yuichiro NAITO <naito.yuichiro@gmail.com>
    Reviewed by:    gallatin, glebius
    Fixes:          e0e12405285b ("netmap: fix LOR in iflib_netmap_register")
    Fixes:          2f8f892ca344 ("rtnetlink: Add FreeBSD-specific IFLAF_GROUP support")
    Fixes:          90e7dbe5e2ca ("iflib: Add registration failure injection points")
    MFC after:      2 weeks
    Sponsored by:   BBOX.io
    Differential Revision:  https://reviews.freebsd.org/D59294
---
 sys/net/iflib.c | 23 +++++------------------
 1 file changed, 5 insertions(+), 18 deletions(-)

diff --git a/sys/net/iflib.c b/sys/net/iflib.c
index 34d53d648a7c..83a518462100 100644
--- a/sys/net/iflib.c
+++ b/sys/net/iflib.c
@@ -5311,7 +5311,6 @@ iflib_device_register(device_t dev, void *sc, if_shared_ctx_t sctx, if_ctx_t *ct
 #endif
 	}
 	iflib_reset_qvalues(ctx);
-	IFNET_WLOCK();
 	CTX_LOCK(ctx);
 	IFLIB_REGISTER_FAIL_POINT(dev, register_before_attach_pre, err,
 	    fail_cleanup);
@@ -5544,7 +5543,6 @@ iflib_device_register(device_t dev, void *sc, if_shared_ctx_t sctx, if_ctx_t *ct
 	iflib_add_pfil(ctx);
 	ctx->ifc_flags |= IFC_INIT_DONE;
 	CTX_UNLOCK(ctx);
-	IFNET_WUNLOCK();
 
 	/* Create led(4) devices if the driver defined the method */
 	kobj_desc = &ifdi_led_func_desc;
@@ -5559,9 +5557,8 @@ fail_detach:
 	STATE_LOCK(ctx);
 	ctx->ifc_flags |= IFC_IN_DETACH;
 	STATE_UNLOCK(ctx);
-	/* Tasks may need either lock; ether_ifdetach() takes ifnet_detach_sx. */
+	/* Tasks may need the context lock; ether_ifdetach() may sleep. */
 	CTX_UNLOCK(ctx);
-	IFNET_WUNLOCK();
 	taskqueue_drain_all(ctx->ifc_tq);
 #ifdef PCI_IOV
 	/*
@@ -5578,7 +5575,6 @@ fail_detach:
 	}
 #endif
 	ether_ifdetach(ctx->ifc_ifp);
-	IFNET_WLOCK();
 	CTX_LOCK(ctx);
 	goto fail_cleanup_detaching;
 
@@ -5599,14 +5595,12 @@ fail_cleanup_detaching:
 
 	if (ctx->ifc_tq != NULL) {
 		/*
-		 * Drain without holding the ifnet or context locks so configuration
-		 * tasks can run to completion.  On fail_detach a second drain also
-		 * catches tasks queued during the first drain.
+		 * Drain without holding the context lock so configuration tasks can
+		 * run to completion.  On fail_detach a second drain also catches
+		 * tasks queued during the first drain.
 		 */
 		CTX_UNLOCK(ctx);
-		IFNET_WUNLOCK();
 		taskqueue_drain_all(ctx->ifc_tq);
-		IFNET_WLOCK();
 		CTX_LOCK(ctx);
 	}
 
@@ -5619,18 +5613,12 @@ fail_cleanup_detaching:
 	/*
 	 * A successful IFDI_ATTACH_PRE must be matched by IFDI_DETACH, even
 	 * when registration fails before queue allocation.  Match
-	 * iflib_device_deregister by detaching before taskqueue_free, and avoid
-	 * holding IFNET_WLOCK across driver detach (LinuxKPI workqueue drain).
+	 * iflib_device_deregister by detaching before taskqueue_free.
 	 */
 	if (attach_pre_succeeded) {
-		IFNET_WUNLOCK();
 		IFDI_DETACH(ctx);
 		if (queues_allocated)
 			IFDI_QUEUES_FREE(ctx);
-		/* Reacquire the global lock before the context lock. */
-		CTX_UNLOCK(ctx);
-		IFNET_WLOCK();
-		CTX_LOCK(ctx);
 	}
 	if (ctx->ifc_tq != NULL) {
 		taskqueue_free(ctx->ifc_tq);
@@ -5640,7 +5628,6 @@ fail_cleanup_detaching:
 		iflib_free_intr_mem(ctx);
 
 	CTX_UNLOCK(ctx);
-	IFNET_WUNLOCK();
 	iflib_deregister(ctx);
 	device_set_softc(ctx->ifc_dev, NULL);
 	if (ctx->ifc_flags & IFC_SC_ALLOCATED)