From owner-svn-src-head@FreeBSD.ORG Thu Dec 1 15:11:47 2011 Return-Path: Delivered-To: svn-src-head@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7F4A61065672; Thu, 1 Dec 2011 15:11:47 +0000 (UTC) (envelope-from lstewart@freebsd.org) Received: from lauren.room52.net (lauren.room52.net [210.50.193.198]) by mx1.freebsd.org (Postfix) with ESMTP id 178558FC0C; Thu, 1 Dec 2011 15:11:46 +0000 (UTC) Received: from lstewart1.loshell.room52.net (ppp59-167-184-191.static.internode.on.net [59.167.184.191]) by lauren.room52.net (Postfix) with ESMTPSA id 84DA97E820; Fri, 2 Dec 2011 02:11:44 +1100 (EST) Message-ID: <4ED79930.60307@freebsd.org> Date: Fri, 02 Dec 2011 02:11:44 +1100 From: Lawrence Stewart User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:7.0.1) Gecko/20111016 Thunderbird/7.0.1 MIME-Version: 1.0 To: Jung-uk Kim References: <201111210417.pAL4HOdi023556@svn.freebsd.org> <4ED4D89D.9000904@freebsd.org> <4ED5049E.60307@freebsd.org> <201111291309.20419.jkim@FreeBSD.org> In-Reply-To: <201111291309.20419.jkim@FreeBSD.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=0.0 required=5.0 tests=UNPARSEABLE_RELAY autolearn=unavailable version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on lauren.room52.net 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 15:11:47 -0000 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_intact >> 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_intactabi_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(). - 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