From owner-freebsd-current@FreeBSD.ORG Tue Jan 24 15:54:42 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 DD48A16A41F for ; Tue, 24 Jan 2006 15:54:42 +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 003E943D6D for ; Tue, 24 Jan 2006 15:54:38 +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 k0OFsciV067993; Tue, 24 Jan 2006 07:54:38 -0800 (PST) (envelope-from rizzo@xorpc.icir.org) Received: (from rizzo@localhost) by xorpc.icir.org (8.12.11/8.12.3/Submit) id k0OFsb7j067992; Tue, 24 Jan 2006 07:54:37 -0800 (PST) (envelope-from rizzo) Date: Tue, 24 Jan 2006 07:54:37 -0800 From: Luigi Rizzo To: current@freebsd.org Message-ID: <20060124075437.B67285@xorpc.icir.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i Cc: Subject: 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 15:54:43 -0000 approx 1 month ago there was some discussion on the usage of if_flags and if_drv_flags in the stack As an exercise, i tried to identify the places these flags are used by taking a -current snapshot, splitting if_flags into two sets (those created at init time, those modified during normal operation) called if_flags_i and if_flags_n, and renaming the flags IFF_i_* and IFF_n_* respectively. Also, to point out where the IFF_DRV_* flags are used, i have renamed them IFF_d_DRV_*. just to see how it looks, the patch that allows GENERIC to compile is at http://info.iet.unipi.it/~luigi/FreeBSD/if_flags.20060114.diff (i know it is missing netgraph, carp, vlan and atm, but covers the majority of drivers). No big suprises here but a few interesting things: - because if_drv_flags only contains RUNNING and OACTIVE, it could be manipulated by simple assignments by the drivers, instead of using bitwise ops; - ifp->if_drv_flags |= IFF_d_DRV_RUNNING; - ifp->if_drv_flags &= ~IFF_d_DRV_OACTIVE; + ifp->if_drv_flags = IFF_d_DRV_RUNNING; /* clear OACTIVE */ ... - ifp->if_drv_flags &= ~(IFF_d_DRV_RUNNING | IFF_d_DRV_OACTIVE); + ifp->if_drv_flags = 0; and so on; - the check for IFF_UP is written so often that i wonder if it wouldn't be the case to wrap it around a macro, as e.g. it is done in net80211/ieee80211_ioctl.c for something similar. - IFF_CANTCHANGE still has masks for IFF_DRV_RUNNING and IFF_DRV_OACTIVE, that are no more in if_flags so should probably be removed. - 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. - some drivers try to manipulate flags that they should not e.g. if_bge sets IFF_UP if_ed sets IFF_ALTPHYS (IFF_LINK*, but it is almost a capability here) if_fwe sets IFF_PROMISC if_ie resets IFF_UP sets IFF_UP sets IFF_ALLMULTI (commented as broken) if_plip sets IFF_UP if_re resets IFF_PROMISC sets IFF_PROMISC resets IFF_UP if_vge resets IFF_UP if_faith sets IFF_UP if_gif sets IFF_UP if_loop sets IFF_UP if_ppp resets IFF_UP if_sl resets IFF_UP sets IFF_UP if_tun sets IFF_UP if_de resets IFF_UP this can be probably fixedin many cases. - in most drivers, the logic for SIOCSIFFLAGS is very convoluted and could be cleaned up a lot. if_dc.c has a reasonable version. if_xl.c has a bad example, unfortunately copied a few times around. cheers luigi