Skip to content

Commit f10efe9

Browse files
committed
mroute: avoid calling if_allmulti with the lock held
Avoid locking issues when if_allmulti() calls the driver's if_ioctl, because that may acquire sleepable locks (while we hold a non-sleepable rwlock). Fortunately there's no pressing need to hold the mroute lock while we do this, so we can postpone the call slightly, until after we've released the lock. This avoids the following WITNESS warning (with iflib drivers): lock order reversal: (sleepable after non-sleepable) 1st 0xffffffff82f64960 IPv4 multicast forwarding (IPv4 multicast forwarding, rw) @ /usr/src/sys/netinet/ip_mroute.c:1050 2nd 0xfffff8000480f180 iflib ctx lock (iflib ctx lock, sx) @ /usr/src/sys/net/iflib.c:4525 lock order IPv4 multicast forwarding -> iflib ctx lock attempted at: #0 0xffffffff80bbd6ce at witness_checkorder+0xbbe #1 0xffffffff80b56d10 at _sx_xlock+0x60 #2 0xffffffff80c9ce5c at iflib_if_ioctl+0x2dc #3 0xffffffff80c7c395 at if_setflag+0xe5 #4 0xffffffff82f60a0e at del_vif_locked+0x9e #5 0xffffffff82f5f0d5 at X_ip_mrouter_set+0x265 #6 0xffffffff80bfd402 at sosetopt+0xc2 #7 0xffffffff80c02105 at kern_setsockopt+0xa5 #8 0xffffffff80c02054 at sys_setsockopt+0x24 #9 0xffffffff81046be8 at amd64_syscall+0x138 #10 0xffffffff8101930b at fast_syscall_common+0xf8 See also: https://redmine.pfsense.org/issues/12079 Reviewed by: mjg Sponsored by: Rubicon Communications, LLC ("Netgate") Differential Revision: https://reviews.freebsd.org/D41209 (cherry picked from commit 680ad06)
1 parent 0b72d3e commit f10efe9

File tree

1 file changed

+19
-8
lines changed

1 file changed

+19
-8
lines changed

sys/netinet/ip_mroute.c

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ static void bw_upcalls_send(void);
317317
static int del_bw_upcall(struct bw_upcall *);
318318
static int del_mfc(struct mfcctl2 *);
319319
static int del_vif(vifi_t);
320-
static int del_vif_locked(vifi_t, struct ifnet **);
320+
static int del_vif_locked(vifi_t, struct ifnet **, struct ifnet **);
321321
static void expire_bw_upcalls_send(void *);
322322
static void expire_mfc(struct mfc *);
323323
static void expire_upcalls(void *);
@@ -618,7 +618,7 @@ if_detached_event(void *arg __unused, struct ifnet *ifp)
618618
{
619619
vifi_t vifi;
620620
u_long i, vifi_cnt = 0;
621-
struct ifnet *free_ptr;
621+
struct ifnet *free_ptr, *multi_leave;
622622

623623
MRW_WLOCK();
624624

@@ -635,6 +635,7 @@ if_detached_event(void *arg __unused, struct ifnet *ifp)
635635
* 3. Expire any matching multicast forwarding cache entries.
636636
* 4. Free vif state. This should disable ALLMULTI on the interface.
637637
*/
638+
restart:
638639
for (vifi = 0; vifi < V_numvifs; vifi++) {
639640
if (V_viftable[vifi].v_ifp != ifp)
640641
continue;
@@ -647,9 +648,15 @@ if_detached_event(void *arg __unused, struct ifnet *ifp)
647648
}
648649
}
649650
}
650-
del_vif_locked(vifi, &free_ptr);
651+
del_vif_locked(vifi, &multi_leave, &free_ptr);
651652
if (free_ptr != NULL)
652653
vifi_cnt++;
654+
if (multi_leave) {
655+
MRW_WUNLOCK();
656+
if_allmulti(multi_leave, 0);
657+
MRW_WLOCK();
658+
goto restart;
659+
}
653660
}
654661

655662
MRW_WUNLOCK();
@@ -998,11 +1005,12 @@ add_vif(struct vifctl *vifcp)
9981005
* Delete a vif from the vif table
9991006
*/
10001007
static int
1001-
del_vif_locked(vifi_t vifi, struct ifnet **ifp_free)
1008+
del_vif_locked(vifi_t vifi, struct ifnet **ifp_multi_leave, struct ifnet **ifp_free)
10021009
{
10031010
struct vif *vifp;
10041011

10051012
*ifp_free = NULL;
1013+
*ifp_multi_leave = NULL;
10061014

10071015
MRW_WLOCK_ASSERT();
10081016

@@ -1015,7 +1023,7 @@ del_vif_locked(vifi_t vifi, struct ifnet **ifp_free)
10151023
}
10161024

10171025
if (!(vifp->v_flags & (VIFF_TUNNEL | VIFF_REGISTER)))
1018-
if_allmulti(vifp->v_ifp, 0);
1026+
*ifp_multi_leave = vifp->v_ifp;
10191027

10201028
if (vifp->v_flags & VIFF_REGISTER) {
10211029
V_reg_vif_num = VIFI_INVALID;
@@ -1045,14 +1053,17 @@ static int
10451053
del_vif(vifi_t vifi)
10461054
{
10471055
int cc;
1048-
struct ifnet *free_ptr;
1056+
struct ifnet *free_ptr, *multi_leave;
10491057

10501058
MRW_WLOCK();
1051-
cc = del_vif_locked(vifi, &free_ptr);
1059+
cc = del_vif_locked(vifi, &multi_leave, &free_ptr);
10521060
MRW_WUNLOCK();
10531061

1054-
if (free_ptr)
1062+
if (multi_leave)
1063+
if_allmulti(multi_leave, 0);
1064+
if (free_ptr) {
10551065
if_free(free_ptr);
1066+
}
10561067

10571068
return cc;
10581069
}

0 commit comments

Comments
 (0)