From owner-freebsd-current@FreeBSD.ORG Tue Jan 24 19:31:01 2006 Return-Path: X-Original-To: current@freebsd.org Delivered-To: freebsd-current@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 642FA16A41F for ; Tue, 24 Jan 2006 19:31:01 +0000 (GMT) (envelope-from rizzo@icir.org) Received: from xorpc.icir.org (xorpc.icir.org [192.150.187.68]) by mx1.FreeBSD.org (Postfix) with ESMTP id CE1CA43D49 for ; Tue, 24 Jan 2006 19:31:00 +0000 (GMT) (envelope-from rizzo@icir.org) Received: from xorpc.icir.org (localhost [127.0.0.1]) by xorpc.icir.org (8.12.11/8.12.11) with ESMTP id k0OJUpAd091377; Tue, 24 Jan 2006 11:30:51 -0800 (PST) (envelope-from rizzo@xorpc.icir.org) Received: (from rizzo@localhost) by xorpc.icir.org (8.12.11/8.12.3/Submit) id k0OJUptJ091376; Tue, 24 Jan 2006 11:30:51 -0800 (PST) (envelope-from rizzo) Date: Tue, 24 Jan 2006 11:30:50 -0800 From: Luigi Rizzo To: Sam Leffler Message-ID: <20060124113050.A76608@xorpc.icir.org> References: <20060124075437.B67285@xorpc.icir.org> <43D67C6E.7020403@errno.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: <43D67C6E.7020403@errno.com>; from sam@errno.com on Tue, Jan 24, 2006 at 11:13:50AM -0800 Cc: current@freebsd.org Subject: Re: if_flags usage etc. X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Jan 2006 19:31:01 -0000 On Tue, Jan 24, 2006 at 11:13:50AM -0800, Sam Leffler wrote: > Luigi Rizzo wrote: ... > > - very few places outside drivers look at IFF_DRV_RUNNING. Basically. > > net/if_ethersubr.c, netinet/ip_fastfwd.c , net80211/ieee80211_ioctl.c > > in all cases checking that both IFF_UP and IFF_RUNNING are set. > > I am not sure why we need both. > > IFF_UP is set by administrative choice and is different from IFF_RUNNING > which means the device/driver is operational. We'd need to look at the > cases where both are checked; it's likely the decision making is > muddled. I know when I did the virtual ap stuff that the distinction > between the two became very important so I believe it is good to keep both. ok, i have no intention to kill either of them, am just unsure on the usage patterns, especially because IFF_RUNNING is set/cleared by the driver and the network layer looks at it without any locking, so it may well see stale information, at which point i wonder why look at IFF_RUNNING at all, and not just rely on IFF_UP being reset one way or another upon errors. It happens already (e.g. i see my "iwi" card clearing IFF_UP when the association is lost), i just haven't figured out how. ... > This list looks very incomplete; I'm aware of many drivers that alter i know, i am trying to build a more complete list compiling modules. > IFF_UP incorrectly. One reason drivers touch IFF_UP is because if_init > has no return value so drivers use IFF_UP to indicate whether or not the > device initialization was successful. I held off changing the type > signature for this method but think it's time to change it in HEAD. which raises the issue - what constraints do we have on changing things here ? E.g. the splitting of flags, and more in general the struct ifnet, which has lot of stuff that should be reorganized. I don't think there is an issue with binary-only drivers but there is certainly one in sharing code with the other BSD's... > Another area where there is major confusion is in handling ioctls that > change driver/device state. The basic model appears to be that an upper > layer changes state then calls the device if_init method to push state > into the hardware. However this puts the onus on the driver to identify > what has changed if it wants to optimize this work; otherwise it has to right... in fact the 'optimized' drivers keep a shadow copy of the interesting flags (in many case IFF_PROMISC) and the non-optimized ones just reinit the device on all changes. cheers luigi