From owner-freebsd-net@freebsd.org Fri Dec 2 20:44:53 2016 Return-Path: Delivered-To: freebsd-net@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 96906C62A3D for ; Fri, 2 Dec 2016 20:44:53 +0000 (UTC) (envelope-from torek@torek.net) Received: from elf.torek.net (mail.torek.net [96.90.199.121]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 847D339F for ; Fri, 2 Dec 2016 20:44:52 +0000 (UTC) (envelope-from torek@torek.net) Received: from elf.torek.net (localhost [127.0.0.1]) by elf.torek.net (8.14.9/8.14.9) with ESMTP id uB2KgOkO055419 for ; Fri, 2 Dec 2016 12:42:24 -0800 (PST) (envelope-from torek@torek.net) Message-Id: <201612022042.uB2KgOkO055419@elf.torek.net> From: Chris Torek To: freebsd-net@freebsd.org Subject: mutex usage in if_bridge vs other drivers MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <55417.1480711344.1@elf.torek.net> Date: Fri, 02 Dec 2016 12:42:24 -0800 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (elf.torek.net [127.0.0.1]); Fri, 02 Dec 2016 12:42:24 -0800 (PST) X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Dec 2016 20:44:53 -0000 I'm not sure if this is really a freebsd-net topic or more generic kernel, but since I observed this by looking for possible causes of a bizarre network driver crash I'll send it here (I'm not on the freebsd-net mailing list but I'll check the archives for a bit too). When doing if_bridge SIOCSDRVSPEC commands we go through this code path: static int bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data) { [snip] BRIDGE_LOCK(sc); error = (*bc->bc_func)(sc, &args); BRIDGE_UNLOCK(sc); Note that the BRIDGE_LOCK is a plain (non-sleepable) mutex. The bc_func may be one of many functions, but two interesting ones are bridge_ioctl_add() and bridge_ioctl_del(). These are somewhat complicated, but both eventually call: bridge_mutecaps(sc); which iterates over all (newly augmented, or remaining) bridge members calling bridge_set_ifcap(sc, bif, enabled) to enable or disable capabilities like IFCAP_TSO4 and so on. This in turn sets up an SIOCSIFCAP ioctl and does: struct ifnet *ifp = bif->bif_ifp; if (ifp->if_capenable != set) { error = (*ifp->if_ioctl)(ifp, SIOCSIFCAP, (caddr_t)&ifr); ... } In the case of dev/bxe/bxe.c, this now (as of FreeBSD 10) has its own sleep (sx) lock, and it may lock that sx lock. If it does, we panic because we're holding the bridge lock. THE QUESTION: - Who is wrong, the bxe driver or the bridge code? I.e., does the bridge driver need to release its lock here, and if so, is that actually safe to do? (We might need to restart the loop over all the members if we drop the lock.) - Or if the bridge driver should retain its lock, can it use an sx lock here, to permit members to also use sx locks? Chris