From owner-cvs-all@FreeBSD.ORG Fri Aug 27 17:04:58 2004 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 1CE6216A4D0 for ; Fri, 27 Aug 2004 17:04:58 +0000 (GMT) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id 401A143D49 for ; Fri, 27 Aug 2004 17:04:57 +0000 (GMT) (envelope-from andre@freebsd.org) Received: (qmail 58868 invoked from network); 27 Aug 2004 17:03:34 -0000 Received: from unknown (HELO freebsd.org) ([62.48.0.53]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 27 Aug 2004 17:03:34 -0000 Message-ID: <412F69BC.51FDDA2E@freebsd.org> Date: Fri, 27 Aug 2004 19:05:00 +0200 From: Andre Oppermann X-Mailer: Mozilla 4.8 [en] (Windows NT 5.0; U) X-Accept-Language: en MIME-Version: 1.0 To: Max Laier References: <200408271516.i7RFGO8L061926@repoman.freebsd.org> <200408271812.18748.max@love2party.net> <412F6108.8C380C17@freebsd.org> <200408271834.24506.max@love2party.net> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/share/man/man4 ipfirewall.4 src/share/man/man9 pfil.9 src/sys/alpha/conf GENERIC src/sys/amd64/conf GENERIC src/sys/conf NOTES files options src/sys/i386/conf GENERIC src/sys/ia64/conf GENERIC SKI src/sys/modules/bridge Makefile ... X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Aug 2004 17:04:58 -0000 Max Laier wrote: > > On Friday 27 August 2004 18:27, Andre Oppermann wrote: > > Max Laier wrote: > > > On Friday 27 August 2004 17:16, Andre Oppermann wrote: > > > > andre 2004-08-27 15:16:24 UTC > > > > > > > > FreeBSD src repository > > > > > > > > Modified files: > > > > share/man/man4 ipfirewall.4 > > > > share/man/man9 pfil.9 > > > > sys/alpha/conf GENERIC > > > > sys/amd64/conf GENERIC > > > > sys/conf NOTES files options > > > > sys/i386/conf GENERIC > > > > sys/ia64/conf GENERIC SKI > > > > sys/modules/bridge Makefile > > > > sys/net bridge.c > > > > sys/netinet ip_fastfwd.c ip_fw_pfil.c ip_input.c > > > > ip_output.c ip_var.h > > > > sys/netinet6 ip6_forward.c ip6_input.c ip6_output.c > > > > ip6_var.h > > > > sys/pc98/conf GENERIC > > > > sys/powerpc/conf GENERIC > > > > sys/sparc64/conf GENERIC > > > > . UPDATING > > > > Log: > > > > Always compile PFIL_HOOKS into the kernel and remove the associated > > > > kernel compile option. All FreeBSD packet filters now use the > > > > PFIL_HOOKS API and thus it becomes a standard part of the network > > > > stack. > > > > > > > > If no hooks are connected the entire packet filter hooks section and > > > > related activities are jumped over. This removes any performance > > > > impact if no hooks are active. > > > > > > Great!!! > > > > > > Maybe we should hide: > > > if (inet_pfil_hook.ph_busy_count == -1) > > > behind a macro in case we modify the locking for pfil_hooks in the > > > future. I am thinking of something like: > > > if (PFIL_IS_EMPTY(&inet_pfil_hook)) > > > > Checking for (inet_pfil_hook.ph_busy_count == -1) is the official to see if > > there are any hooks connected. I don't think we need to abstract this in a > > macro. > > Well, having written the locking there I can tell you that ph_busy_count is > really an *internal* value of the locking and should not be used directly. At > least as long as we want to be able to change the locking at a later point. > > Right now pfil uses a handrolled sleep lock (as the default sx(9) lock is not > very suitable or fast) but this might change in the future. Using > ph_busy_count globaly will make that change more difficult. > > Moreover, I find PFIL_IS_EMPTY much easier to understand. Ok, you convinced me. I'm going to make it macro called PFIL_IS_HOOKED. This seems to be a little bit more descriptive than IS_EMPTY. IS_EMPTY is easy to confuse with some kind of packet queue or so (which PFIL_HOOKS isn't). -- Andre