From owner-svn-src-head@FreeBSD.ORG Thu Dec 1 16:43:35 2011 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id D60CC106566B; Thu, 1 Dec 2011 16:43:34 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: Lawrence Stewart Date: Thu, 1 Dec 2011 11:43:25 -0500 User-Agent: KMail/1.6.2 References: <201111210417.pAL4HOdi023556@svn.freebsd.org> <201111291309.20419.jkim@FreeBSD.org> <4ED79930.60307@freebsd.org> In-Reply-To: <4ED79930.60307@freebsd.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201112011143.27707.jkim@FreeBSD.org> Cc: src-committers@freebsd.org, John Baldwin , svn-src-all@freebsd.org, Julien Ridoux , Ben Kaduk , Benjamin Kaduk , svn-src-head@freebsd.org Subject: Re: svn commit: r227778 - head/sys/net X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Dec 2011 16:43:35 -0000 On Thursday 01 December 2011 10:11 am, Lawrence Stewart wrote: > On 11/30/11 05:09, Jung-uk Kim wrote: > > On Tuesday 29 November 2011 11:13 am, Lawrence Stewart wrote: > >> On 11/30/11 00:05, Lawrence Stewart wrote: > >>> On 11/28/11 14:59, Benjamin Kaduk wrote: > >>>> On Wed, 23 Nov 2011, Lawrence Stewart wrote: > >>>>> On 11/23/11 17:42, Julien Ridoux wrote: > >> > >> [snip] > >> > >>>>>> What is your favourite option? > >>>>> > >>>>> FreeBSD parlance is to ask what colour you would like to > >>>>> paint the bikeshed ;) > >>>>> > >>>>> As I've never experienced the pain John refers to, I'll defer > >>>>> to the wisdom of others on whether the proposed patch will > >>>>> create pain down the road. I think it's ok, but if consensus > >>>>> is 8bytes per packet isn't going to break the bank, I guess > >>>>> we just go for it - but I guess I am cautious about this > >>>>> route as we can push a lot of packets per second through the > >>>>> stack. > >>>> > >>>> Since other people seem to be keeping quiet, I'll add that I'm > >>>> in favor of just always adding the 8 bytes per packet. > >>> > >>> Julien and I discussed this at length today, and agree that for > >>> head, we'll add the new bh_ffcounter member to the BPF header > >>> unconditionally. > >>> > >>> Thanks to you and John for the input. > >>> > >>> I'm going to revert r227778 in order to start form a clean > >>> slate, and add two separate patches. One will reintegrate > >>> FFCLOCK support with BPF without breaking the ABI. A follow up > >>> patch will bump the ffclock version and add the bh_ffcounter to > >>> the bpf header (after the timestamp member). Then a final patch > >>> will bump __FreeBSD_version and add a note to UPDATING about > >>> recompiling to get kernel/world in sync, which should seal the > >>> deal. > >> > >> Here's the first of the patches: > >> > >> http://people.freebsd.org/~lstewart/patches/misc/ffclock_bpf_int > >>act abi_10.x.r228130.patch > > > > I only glanced at it but it looks very close to what I wanted to > > suggest. > > Final candidate patch is at: > > http://people.freebsd.org/~lstewart/patches/misc/ffclock_bpf_intact >abi_10.x.r228180.patch > > Assuming it passes the "make tinderbox" build I'm currently running > and no further input is received from interested parties, I plan to > commit it in ~10 hours. > > Changes since the r228130 patch I sent previously: > > - The new flags in bpf.h are added unconditionally so that they can > always be referenced at compile time and a decision made at runtime > as to whether a flag will be set or not. Using one of the new flags > when the kernel doesn't have FFCLOCK compiled in results in the > flag being ignored. An app should check for the existence of the > "ffclock" kernel feature or the "kern.sysclock" sysctl tree before > attempting to use the flags. > > - This patch will hopefully be MFCed at some point, so I added a > CTASSERT to bpf.c to ensure that the ABI of structs bpf_hdr32, > bpf_hdr and bpf_xhdr remains intact when FFCLOCK is enabled and the > union of a ffcounter and struct timeval32/timeval/bpf_ts is > switched in. > > - bpf_gettime() more comprehensively covers all the possible cases > of flag combination and does sensible things for each case (even > though some cases are rather silly). > > - The snippet of code at the beginning of catchpacket() that was > manipulating the struct bintime derived from bpf_gettime() was > gross and has been removed in favour of selecting the right > {get}bin{up}time() function call in bpf_gettime(). I did that to reduce branching. Since you are introducing more branches, it warrants a function pointer now. Jung-uk Kim > - bpf.4 man page updated > > - Proposed commit message at top of patch updated > > > If you'd like to look it over closely but can't do it in the next > 10 hours or so, let me know and I'll postpone committing. > > Cheers, > Lawrence