From owner-cvs-all@FreeBSD.ORG Fri Dec 10 00:14:29 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 BBBA316A4CE for ; Fri, 10 Dec 2004 00:14:29 +0000 (GMT) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id CEE6243D66 for ; Fri, 10 Dec 2004 00:14:28 +0000 (GMT) (envelope-from andre@freebsd.org) Received: (qmail 90004 invoked from network); 10 Dec 2004 00:04:10 -0000 Received: from unknown (HELO freebsd.org) ([62.48.0.53]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 10 Dec 2004 00:04:10 -0000 Message-ID: <41B8EA66.4875E2D9@freebsd.org> Date: Fri, 10 Dec 2004 01:14:30 +0100 From: Andre Oppermann X-Mailer: Mozilla 4.8 [en] (Windows NT 5.0; U) X-Accept-Language: en MIME-Version: 1.0 To: Sam Leffler References: <200412091641.iB9GflnD067866@repoman.freebsd.org> <20041209174457.GA82542@freefall.freebsd.org> <41B893D3.59939DEA@freebsd.org> <41B8BF81.2000701@errno.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit cc: cvs-all@FreeBSD.org cc: cvs-src@FreeBSD.org cc: Gleb Smirnoff cc: "Christian S.J. Peron" cc: src-committers@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet ip_fw_pfil.c 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, 10 Dec 2004 00:14:29 -0000 Sam Leffler wrote: > > Andre Oppermann wrote: > > "Christian S.J. Peron" wrote: > > > >>On Thu, Dec 09, 2004 at 04:41:47PM +0000, Gleb Smirnoff wrote: > >> > >>>glebius 2004-12-09 16:41:47 UTC > >>> > >>> FreeBSD src repository > >>> > >>> Modified files: > >>> sys/netinet ip_fw_pfil.c > >>> Log: > >>> Check that DUMMYNET_LOADED before seeking dummynet m_tag. > >> > >>I think Sam had some reservations about doing this before, We had some > >>discussions and in the end it was pretty much concluded that since > >>tags are rarely present, and m_tag_locate is only called if tags are present, > >>adding this check unconditionally added a memory write and a compare > >>for every packet. > >> > >>This change may be a mistake unless you can prove some significant > >>performance gain. > > > > > > Checking for DUMMYNET_LOADED is a simple pointer compare to NULL and doesn't > > add a memory write. Not a big difference for sure but not hurting either. > > > > Chris and I had a discussion a while back about not adding code that > checks for the presence of optional functionality. Using tags was > intended to eliminate checks like this. Doing this sort of stuff can > cause an unmaintainable/unreadable mess. Unless you can show there is a > noticeable performance gain from doing this I think it's a bad idea. > Remember that m_tag_find already inlines the check for whether any tags > are present or not before doing the lookup. > > If tag lookup cost becomes an important consideration in writing code > then we need to address that basic functionality. Actually this is a good argument and reasoning and I buy into it. Not that is matter this much in this case but having nice and clean code wins big over time. I've spent and am still spending too much time cleaning up old BSD PDP-11 "optimizations" and other shortcuts. -- Andre