From owner-cvs-all@FreeBSD.ORG Fri Sep 16 23:06:08 2005 Return-Path: X-Original-To: cvs-all@FreeBSD.org Delivered-To: cvs-all@FreeBSD.org Received: by hub.freebsd.org (Postfix, from userid 618) id AA96916A420; Fri, 16 Sep 2005 23:06:08 +0000 (GMT) In-Reply-To: <20050916.151301.122028383.imp@bsdimp.com> from "M. Warner Losh" at "Sep 16, 2005 03:13:01 pm" To: imp@bsdimp.com (M. Warner Losh) Date: Fri, 16 Sep 2005 23:06:08 +0000 (GMT) X-Mailer: ELM [version 2.4ME+ PL54 (25)] MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-Id: <20050916230608.AA96916A420@hub.freebsd.org> From: wpaul@FreeBSD.ORG (Bill Paul) Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, ru@FreeBSD.org, cvs-all@FreeBSD.org, jhb@FreeBSD.org Subject: Re: cvs commit: src/sys/dev/re if_re.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Sep 2005 23:06:08 -0000 > In message: <20050916202207.GA22151@ip.net.ua> > Ruslan Ermilov writes: > : Hmm, I'm not very fluent in device(9) API, but I wonder what's then > : the analog of device_delete_child(sc->miibus) that the majority of > : foo_detach() methods do. I.e., will the miibus device really be > : removed? > > Yes. I believe so. > > : > How again can this happen? > : > > : tcpdump -n -i ed0 & > : kldunload if_ed > : > : Still, ed_init_locked() will instantiate many things inappropriate > : for ed_detach() context. > > How does ed_init_locked get called in ed_detach()? It looks to me > that ed_stop is called, but ed_init() isn't. Where would it be > called? BPF calls it. If someone's got a BPF listener on an interface and you detach the interface, BPF goes "oh shit, this interface is going away and I've still got my grimy paws on it -- I better clean up" and one of the things it does in the process is turn off promiscuous mode. To do that, it calls (*ifp->if_ioctl)(SIOCSIFFLAGS) to turn off IFF_PROMISC. Now, _some_ drivers will shortcut the handling of SIOCSIFFLAGS by simply calling their (*ifp->if_init)() function, since they know this routine configures the RX filter settings anyway. At this point, even though you're in the middle of detaching the interface, IFF_UP is still set, so (*ifp->if_init)() goes through the motions of initializing the chip and all the other driver machinery, which brings the interface back up again even though foo_detach() just stopped it. The main way you get in trouble with this is if your foo_init() routine launches any timers or taskqueue processing. These may be triggered after foo_detach() returns, by which time either the resources allocated for the driver have been deleted or the text pages have been invalidated, or both. You could also get in trouble if you did: foo_detach() { foo_stop(sc); free(sc->something_really_important, M_WHATEVER); ether_ifdetach(ifp); } If foo_init() depends on something_really_important, then you're hosed if foo_init() runs again. My solution in re_detach() was basically just convenient. I had no idea a bikeshed would errupt over the fact that I was clearing IFF_UP. I know IFF_UP is supposed to be an administratively changed flag, but unloading the driver _is_ an administrative action. And for crying out loud, the interface is about to be destroyed: what difference does it make? If you insist on sticking to the "no twiddling IFF_UP from inside the driver" rule, then I think the problem can be avoided by simply not being lazy in foo_ioctl() and actually having explicit code in SIOCSIFFLAGS case that turns promisc mode on and off on an IFF_PROMISC transition, and is careful not to do it unless IFF_RUNNING is set. It also needs to handle IFF_UP separately from IFF_PROMISC. I think the correct logic would look something like: if (IFF_UP was toggled up) foo_init(sc); else if (IFF_UP was toggled down) foo_stop(sc); if (IFF_PROMISC was toggled up && ifp->if_flags & IFF_RUNNING) foo_set_promisc(sc); else if (IFF_PROMISC was toggled down && ifp->if_flags & IFF_RUNNING) foo_clear_promisc(sc); Note that the ether_ifdetach->bpf->foo_ioctl interaction can occur regardless of the bus the NIC is attached to. It doesn't matter if it's PCI, pccard or usb, or whether or not it uses mii. The bus attachments can influence the thing ends up crashing though. -Bill -- ============================================================================= -Bill Paul (510) 749-2329 | Senior Engineer, Master of Unix-Fu wpaul@windriver.com | Wind River Systems ============================================================================= "Ignorance may be bliss, but delusion is ecstasy!" -Perki =============================================================================