From owner-freebsd-net@FreeBSD.ORG Tue Aug 9 12:24:39 2005 Return-Path: X-Original-To: net@FreeBSD.org Delivered-To: freebsd-net@FreeBSD.ORG Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 5865116A41F; Tue, 9 Aug 2005 12:24:39 +0000 (GMT) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [204.156.12.53]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0401643D48; Tue, 9 Aug 2005 12:24:38 +0000 (GMT) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [204.156.12.50]) by cyrus.watson.org (Postfix) with ESMTP id 333CE46B55; Tue, 9 Aug 2005 08:24:38 -0400 (EDT) Date: Tue, 9 Aug 2005 13:27:51 +0100 (BST) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: net@FreeBSD.org Message-ID: <20050809131102.H84992@fledge.watson.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Mailman-Approved-At: Tue, 09 Aug 2005 15:09:40 +0000 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 X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Aug 2005 12:24:39 -0000 (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