Date: Tue, 24 Jan 2006 07:54:37 -0800 From: Luigi Rizzo <rizzo@icir.org> To: current@freebsd.org Subject: if_flags usage etc. Message-ID: <20060124075437.B67285@xorpc.icir.org>
next in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20060124075437.B67285>