From owner-cvs-all@FreeBSD.ORG Thu Aug 18 19:42:01 2005 Return-Path: X-Original-To: cvs-all@FreeBSD.org Delivered-To: cvs-all@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3F8A016A421; Thu, 18 Aug 2005 19:42:01 +0000 (GMT) (envelope-from jhb@FreeBSD.org) Received: from mv.twc.weather.com (mv.twc.weather.com [65.212.71.225]) by mx1.FreeBSD.org (Postfix) with ESMTP id 24EAA43D53; Thu, 18 Aug 2005 19:41:59 +0000 (GMT) (envelope-from jhb@FreeBSD.org) Received: from [10.50.40.201] (Not Verified[10.50.40.201]) by mv.twc.weather.com with NetIQ MailMarshal (v6, 0, 3, 8) id ; Thu, 18 Aug 2005 15:56:27 -0400 From: John Baldwin To: Maxim.Sobolev@portaone.com Date: Thu, 18 Aug 2005 15:38:31 -0400 User-Agent: KMail/1.8 References: <200508181429.j7IET16d038533@repoman.freebsd.org> <200508181238.05411.jhb@FreeBSD.org> <4304D6B9.8050603@portaone.com> In-Reply-To: <4304D6B9.8050603@portaone.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-6" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200508181538.32988.jhb@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@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: Thu, 18 Aug 2005 19:42:01 -0000 On Thursday 18 August 2005 02:43 pm, Maxim Sobolev wrote: > John Baldwin wrote: > > On Thursday 18 August 2005 10:29 am, Maxim Sobolev wrote: > >>sobomax 2005-08-18 14:29:01 UTC > >> > >> FreeBSD src repository > >> > >> Modified files: > >> sys/dev/re if_re.c > >> Log: > >> In re_shutdown() mark interface as down since otherwise we will panic > >> if interrupt comes in later on, which can happen in some uncommon cases. > >> > >> Another possible fix is to call re_detach() instead of re_stop(), like > >> ve(4) does, but I am not sure if the latter is really RTTD, so that > >> stick with this one-liner for now. > >> > >> PR: kern/80005 > >> Approved by: silence on -arch, no reply from selected network gurus > > > > The PR reports problems while the machine is running, not a panic during > > shutdown. I couldn't get the PR database to load the PR with the panic > > from > > Sorry, it has been a typo - in fact I was reffering to kern/85005. > > > vr(4) yesterday. It's very unclear why clearing IFF_UP makes any > > difference. Ah, perhaps re_intr() should be fixed to check > > IFF_DRV_RUNNING rather than IFF_UP? Then the re_stop() in re_shutdown() > > would be sufficient. > > Yes, this will help. However, I am not quite sure, what is the point to > do interrupt processing if interface is down? Perhaps re_intr() should > check both IFF_DRV_RUNNING and IFF_UP? IFF_DRV_RUNNING will be clear once foo_stop() is called. I think IFF_UP is the wrong flag actually as it might still be set in the shutdown case, but shutdown normally calls foo_stop() so you should be able to just check IFF_DRV_RUNNING. Note that when you down an interface, you get an ioctl that results in foo_stop() being called from foo_ioctl() and the upper layer then clears IFF_UP. > > I also think that the change to vr(4) should be reverted (way too big of > > a sledge hammer) and instead its interrupt handler can check > > IFF_DRV_RUNNING and bail if it is clear as well. > > Well, see kern/85005. IMO some generic approach should be worked out and > implemented since I think many other network drivers may be affected by > the same problem. I've still yet to see what the real panic is. For one thing, if the foo_stop method does its jobs, the ethernet hardware shouldn't be generating interrupts. The stop method should be shutting the card down (i.e. turning off the receiver and transmitter for example). Is your ethernet driver sharing an interrupt with another device and the other device is interrupting? In that case, the ethernet driver would have the same panic if you did an 'ifconfig foo0 down' and then the other device interrupted. So, I think clearing IFF_UPP in foo_shutdown() is wrong. foo_stop() should really be sufficient, and foo_intr() should be able to handle a spurious interrupt while the interface is stopped without panicing since it already needs to do so to handle the shared interrupt case. -- John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org