On Tue, 24 Jan 2006, Luigi Rizzo wrote:
> 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_*.
I've spent several weeks over the past couple of months working through the
use of if_flags in the network stack. There are a number of complicating
issues, and I basically concluded that before we could get too much further,
we needed to catalog and normalize the use of the flags across drivers. The
mean sticking points I ran into were:
(1) Modify drivers not to directly adjust IFF_UP. In many cases, this is
simple, but in others, it is hard.
(2) Clarify semantics of driver-specific (LINKX) flags, which sometimes
reflect driver state, and sometimes control it (and sometimes both).
(3) Introduce or reuse a mutex to protect read-modify-write of if_flags by the
stack.
(4) Push if_start oactive check into device driver to allow it to lock (or
not) as it sees fit.
I have works in progress for several of these, but because there's inevitably
a snow-balling effect, I've been trickling changes in a bit as a time as I
decide what is actually the right thing to do and fix drivers to match. The
if_drv_flags breakout was all I figured was safe to get in to 6.x, but I hope
to do quite a bit more for 7.x. I have significant numbers of patches along
the above lines in p4, and will try to sort them out in the next few days.
Robert N M Watson
>
> 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
> _______________________________________________
> freebsd-current at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"
>