From owner-freebsd-current@FreeBSD.ORG Tue Aug 28 10:31: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 ED1C716A419 for ; Tue, 28 Aug 2007 10:31:12 +0000 (UTC) (envelope-from thompsa@FreeBSD.org) Received: from heff.fud.org.nz (203-109-251-39.static.bliink.ihug.co.nz [203.109.251.39]) by mx1.freebsd.org (Postfix) with ESMTP id 83FFB13C49D for ; Tue, 28 Aug 2007 10:31:12 +0000 (UTC) (envelope-from thompsa@FreeBSD.org) Received: by heff.fud.org.nz (Postfix, from userid 1001) id 997E81CC58; Tue, 28 Aug 2007 22:31:10 +1200 (NZST) Date: Tue, 28 Aug 2007 22:31:10 +1200 From: Andrew Thompson To: "Bruce M. Simpson" Message-ID: <20070828103110.GA43049@heff.fud.org.nz> References: <20070828040026.GB42201@heff.fud.org.nz> <46D3C9F3.2010802@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <46D3C9F3.2010802@FreeBSD.org> User-Agent: Mutt/1.5.13 (2006-08-11) 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 10:31:13 -0000 On Tue, Aug 28, 2007 at 08:08:35AM +0100, Bruce M. Simpson wrote: > 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. I had originally started to put it there but realised that I need a pointer to the ifnet to read if_broadcastaddr, I didnt think it was worth changing the function parameters when the check can also just go in bpfwrite. I dont mind moving it if its the more correct place to put it. > 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. Is the tapwrite patch still needed? The mbuf is passed to ether_input which should do the right thing. > 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). I could go with this but it seems wrong to be passed a mbuf which has incorrect flags, there may be other places in the stack that look for M_*CAST that also have quirks. Andrew