Date: Tue, 9 Aug 2005 13:27:51 +0100 (BST) From: Robert Watson <rwatson@FreeBSD.org> To: net@FreeBSD.org Cc: sobomax@FreeBSD.org, jhb@FreeBSD.org, onoe@FreeBSD.org, joerg@FreeBSD.org, marius@FreeBSD.org, mdodd@FreeBSD.org, nyan@FreeBSD.org, imp@FreeBSD.org, yongari@FreeBSD.org Subject: Drivers that modify ifp->if_flags's IFF_ALLMULTI field Message-ID: <20050809131102.H84992@fledge.watson.org>
next in thread | raw e-mail | index | archive | help
(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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050809131102.H84992>