From owner-freebsd-current@FreeBSD.ORG Tue Aug 28 07:26:12 2007 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BAE8516A417; Tue, 28 Aug 2007 07:26:12 +0000 (UTC) (envelope-from bms@FreeBSD.org) Received: from out2.smtp.messagingengine.com (out2.smtp.messagingengine.com [66.111.4.26]) by mx1.freebsd.org (Postfix) with ESMTP id 9277A13C46E; Tue, 28 Aug 2007 07:26:12 +0000 (UTC) (envelope-from bms@FreeBSD.org) Received: from compute1.internal (compute1.internal [10.202.2.41]) by out1.messagingengine.com (Postfix) with ESMTP id 2013026FAF; Tue, 28 Aug 2007 03:08:37 -0400 (EDT) Received: from heartbeat1.messagingengine.com ([10.202.2.160]) by compute1.internal (MEProxy); Tue, 28 Aug 2007 03:08:37 -0400 X-Sasl-enc: xYtP/J06CBDUDJzZiCMQb6s4bCUipJFwunujlvTRN+hi 1188284916 Received: from empiric.lon.incunabulum.net (82-35-112-254.cable.ubr07.dals.blueyonder.co.uk [82.35.112.254]) by mail.messagingengine.com (Postfix) with ESMTP id 8A6721314; Tue, 28 Aug 2007 03:08:36 -0400 (EDT) Message-ID: <46D3C9F3.2010802@FreeBSD.org> Date: Tue, 28 Aug 2007 08:08:35 +0100 From: "Bruce M. Simpson" User-Agent: Thunderbird 2.0.0.4 (X11/20070630) MIME-Version: 1.0 To: Andrew Thompson References: <20070828040026.GB42201@heff.fud.org.nz> In-Reply-To: <20070828040026.GB42201@heff.fud.org.nz> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: FreeBSD Current Subject: Re: multicast packets from bpf X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Aug 2007 07:26:12 -0000 Seems reasonable, both patches are syntactically sane. There are arguments in favour of both changes. I favour the first approach, however, it may make more sense to put the logic into bpf_movein() as it already builds a sockaddr based on the header data provided to bpf during a write. For the first patch: I previously fixed tapwrite() to check injected frames in the same way, as this was causing a problem with my own use of if_bridge. There is no way that I see for bpf to be able to tell if a frame is link-layer multicast or not, and checking at that layer does introduce a little pollution. Ethernet is the most common case so it could be argued that's OK, as we have ethernet-specific fields in struct mbuf now. Your change is the parallel change in the bpfwrite path to what I have in the tapwrite path. The second patch: Conceptually similar to the loopback check in ip_output() for multicast. we wind up doing this check elsewhere, in particular netgraph. It is a relatively cheap check although it does involve changing the flags on a potentially read-only mbuf chain, which is bending the rules a bit (the stack often needs to change stuff in m_pkthdr even if the clusters are read-only). BTW this patch looks like it touches the paths which would need to be changed if IGMP snooping were to be implemented for if_bridge. regards, BMS