Drivers that modify ifp->if_flags's IFF_ALLMULTI field

Robert Watson rwatson at FreeBSD.org
Tue Aug 9 12:24:39 GMT 2005


(maintainers or effective maintainers of the affected device drivers CC'd 
-- see below for the details, sorry about dups)

I've recently been reviewing the use of if_flags with respect to network 
stack and driver locking.  Part of that work has been to break the field 
out into two separate fields: one maintained and locked by the device 
driver (if_drv_flags), and the other maintained and locked by the network 
stack (if_flags).  So far, I've moved IFF_OACTIVE and IFF_RUNNING from 
if_flags to if_drv_flags.  This change was recently committed to 7.x, and 
will be merged to 6.x prior to 6.0.

I'm now reviewing the remainder of the flags, and hit upon IFF_ALLMULTI, 
which seems generally to be an administrative flag specificying that all 
multicast packets should be accepted at the device driver layer.  This 
flag is generally set in one of three ways:

(1) if_allmulti() is invoked by network protocols wanting to specify that
     they want to see all multicast packets, such as for multicast routing.

(2) IFF_ALLMULTI is sometimes set directly by cross-driver common link
     layer code, specifically if IN6_IS_ADDR_UNSPECIFIED(&sin6->sin6_addr)
     is matched in if_resolvemulti().

(3) Some device drivers set IFF_ALLMULTI when their multicast address
     filters overflow to indicate that all multicast traffic should and is
     being accepted, to be handled at the link or network layer.

IFF_ALLMULTI is then generally read by network device drivers in order to 
special case their multicast handling if all multicast is desired.

My feeling is that (2) and (3) are in fact bugs in device drivers and the 
IPv6 code.  Specifically:

- IFF_ALLMULTI should be set using if_allmulti(), which maintains a
   counter of consumers, which (2) bypasses.  Also, once (2) has happened,
   IFF_ALLMULTI is not disabled by the consumer.  And, it may be disabled
   by another consumer, breaking the consumer that wanted it on.  This
   should be corrected to use if_allmulti(), and ideally a symetric "now
   turn it off" should be identified.

- (3) is also a bug, as it reflects internal driver state, and will
   interfere with the administrative setting of IFF_ALLMULTI by turning it
   off even though there are consumers that want it on.  Drivers should
   maintain their forcing of the flag on or off internally.  If it is
   necesary to also expose IFF_ALLMULTI as a status flag for the device
   driver, a new flag should be introduced that is distinguished from the
   administrative state.

(3) is fairly uncommon -- most device drivers already maintain the forcing 
of the all multicast state internally in a separate flag.  The following 
device drivers do not, however:

   src/sys/dev/awi/awi.c
   src/sys/dev/gem/if_gem.c
   src/sys/dev/hme/if_hme.c
   src/sys/dev/ie/if_ie.c
   src/sys/dev/lnc/if_lnc.c
   src/sys/dev/pdq/pdq_ifsubr.c
   src/sys/dev/ray/if_ray.c
   src/sys/dev/snc/dp83932.c
   src/sys/dev/usb/if_udav.c
   src/sys/pci/if_de.c

The fix is generally pretty straight forward, and depending on the device 
driver, may or may not require adding new state to softc.

Robert N M Watson


More information about the freebsd-net mailing list