From nobody Fri Jul 28 09:44:33 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4RC2m15FBXz4p7Lc; Fri, 28 Jul 2023 09:44:33 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4RC2m14qKMz41lV; Fri, 28 Jul 2023 09:44:33 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1690537473; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2mgp8o1zBJwPfLw8rJItC3aQvy3mzv6oS1LrZB/jGxM=; b=I7R404pf2Fv9RFrUS3RPwDfxoMmRPFtcIi4Sm90IwIyXyWY/py0II8sOxq4TxI4WRf9w08 gomhksGI7scrCi+EPFHtmRPAXhYS4S1tW3TcGqQ5NPNnQHp6q+OWacAp6J1UBGfxk+L8Ex D42992RYLOulIXHZEbbXQyn4q1IIpanli2IZ5rXa/QGEskLEEODp1h/dbM29RWgWTWkbuQ U32rb5L/PD7Oyht1CudmIKxrG+0jUeiUTGnl3RUmjeYPoi+z3Crgr6ixckY96qtb/BY7i6 ewyth4y0AQGkNBAyL2EPnDjZbd28l3oCm1F1soKtPi8fRLIpw6MLPWCveKUy4g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1690537473; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=2mgp8o1zBJwPfLw8rJItC3aQvy3mzv6oS1LrZB/jGxM=; b=TM6XfIghn5BdRrCA8AcbSUpqIvdUenPb0EWtB81Y4r33LbP5lCpetd93zxCHRlFAHzEPXO U8yC4mOdbKaxCnDtn015ERgTmIfca3OHv0IsTSUflUXwG9bajOngygxo0lKincIk5Xp4i2 H7QGHrz+qLCdWITXBgDTiXE2UlMAh2rtC+Sp9BXrI7qbus6tWF/OqCeM2L3QYk8ODdx3tb SQZWboQ0Cyv2H4NgnnyCpL+EqQeFzUMa1+d7BdNTK5WerLKdpkwQDnlErXyqxOkxMT4joo N39JkJCz+XUViZmbwlW9A87kmKUuitefwXVR2lewHCkeLWwCjw71fftSRebCLA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1690537473; a=rsa-sha256; cv=none; b=nGkh4u5q3sZ/i25IMXEMyM6pUsgp6AJMBL9tlz+2NX0Mx5B8fVNwOQQRhAVzcmH8L3UHAX CO4ufsLLd2eO9KSKuYIRX9tnDwrFEqCeLA7trbzBR6wLXpPffW7XseHSmui8SbshEokK+o Gw4+XP2AIyRW5OtVpj+oBRYlazKKE4jJWBL9g6iaQNh+HXqtPkq+x1TkrURr+k8fnsTxYN 4frriXBghSLmvl61N4vb56ceSTz+yk5hDbxV6pq7HA09gEi0Athn4LdQbFI8kdbZwZxDvL ARYZmYE2JkeH+KlvePSwUT9Vu3Rjz7YHAIQqy31PlUbBUuO7QBE7YQRUa0UnBw== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4RC2m13tXKzmmd; Fri, 28 Jul 2023 09:44:33 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 36S9iXVu029494; Fri, 28 Jul 2023 09:44:33 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 36S9iXnJ029493; Fri, 28 Jul 2023 09:44:33 GMT (envelope-from git) Date: Fri, 28 Jul 2023 09:44:33 GMT Message-Id: <202307280944.36S9iXnJ029493@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 680ad06f90bf - main - mroute: avoid calling if_allmulti with the lock held List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 680ad06f90bf3c9728e1352475915547665b0b96 Auto-Submitted: auto-generated The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=680ad06f90bf3c9728e1352475915547665b0b96 commit 680ad06f90bf3c9728e1352475915547665b0b96 Author: Kristof Provost AuthorDate: 2023-07-27 06:03:25 +0000 Commit: Kristof Provost CommitDate: 2023-07-28 09:32:39 +0000 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 --- sys/netinet/ip_mroute.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/sys/netinet/ip_mroute.c b/sys/netinet/ip_mroute.c index ac5e1c5f09c6..124a8cd394cb 100644 --- a/sys/netinet/ip_mroute.c +++ b/sys/netinet/ip_mroute.c @@ -317,7 +317,7 @@ static void bw_upcalls_send(void); static int del_bw_upcall(struct bw_upcall *); static int del_mfc(struct mfcctl2 *); static int del_vif(vifi_t); -static int del_vif_locked(vifi_t, struct ifnet **); +static int del_vif_locked(vifi_t, struct ifnet **, struct ifnet **); static void expire_bw_upcalls_send(void *); static void expire_mfc(struct mfc *); static void expire_upcalls(void *); @@ -618,7 +618,7 @@ if_detached_event(void *arg __unused, struct ifnet *ifp) { vifi_t vifi; u_long i, vifi_cnt = 0; - struct ifnet *free_ptr; + struct ifnet *free_ptr, *multi_leave; MRW_WLOCK(); @@ -635,6 +635,7 @@ if_detached_event(void *arg __unused, struct ifnet *ifp) * 3. Expire any matching multicast forwarding cache entries. * 4. Free vif state. This should disable ALLMULTI on the interface. */ +restart: for (vifi = 0; vifi < V_numvifs; vifi++) { if (V_viftable[vifi].v_ifp != ifp) continue; @@ -647,9 +648,15 @@ if_detached_event(void *arg __unused, struct ifnet *ifp) } } } - del_vif_locked(vifi, &free_ptr); + del_vif_locked(vifi, &multi_leave, &free_ptr); if (free_ptr != NULL) vifi_cnt++; + if (multi_leave) { + MRW_WUNLOCK(); + if_allmulti(multi_leave, 0); + MRW_WLOCK(); + goto restart; + } } MRW_WUNLOCK(); @@ -998,11 +1005,12 @@ add_vif(struct vifctl *vifcp) * Delete a vif from the vif table */ static int -del_vif_locked(vifi_t vifi, struct ifnet **ifp_free) +del_vif_locked(vifi_t vifi, struct ifnet **ifp_multi_leave, struct ifnet **ifp_free) { struct vif *vifp; *ifp_free = NULL; + *ifp_multi_leave = NULL; MRW_WLOCK_ASSERT(); @@ -1015,7 +1023,7 @@ del_vif_locked(vifi_t vifi, struct ifnet **ifp_free) } if (!(vifp->v_flags & (VIFF_TUNNEL | VIFF_REGISTER))) - if_allmulti(vifp->v_ifp, 0); + *ifp_multi_leave = vifp->v_ifp; if (vifp->v_flags & VIFF_REGISTER) { V_reg_vif_num = VIFI_INVALID; @@ -1045,14 +1053,17 @@ static int del_vif(vifi_t vifi) { int cc; - struct ifnet *free_ptr; + struct ifnet *free_ptr, *multi_leave; MRW_WLOCK(); - cc = del_vif_locked(vifi, &free_ptr); + cc = del_vif_locked(vifi, &multi_leave, &free_ptr); MRW_WUNLOCK(); - if (free_ptr) + if (multi_leave) + if_allmulti(multi_leave, 0); + if (free_ptr) { if_free(free_ptr); + } return cc; }