Skip site navigation (1)Skip section navigation (2)
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>