From owner-cvs-src@FreeBSD.ORG Thu Dec 9 21:02:59 2004 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0361416A4CE; Thu, 9 Dec 2004 21:02:59 +0000 (GMT) Received: from ebb.errno.com (ebb.errno.com [66.127.85.87]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8E6DB43D5F; Thu, 9 Dec 2004 21:02:58 +0000 (GMT) (envelope-from sam@errno.com) Received: from [66.127.85.91] ([66.127.85.91]) (authenticated bits=0) by ebb.errno.com (8.12.9/8.12.6) with ESMTP id iB9L2vWi032139 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 9 Dec 2004 13:02:58 -0800 (PST) (envelope-from sam@errno.com) Message-ID: <41B8BF81.2000701@errno.com> Date: Thu, 09 Dec 2004 13:11:29 -0800 From: Sam Leffler User-Agent: Mozilla Thunderbird 1.0RC1 (X11/20041208) X-Accept-Language: en-us, en MIME-Version: 1.0 To: Andre Oppermann References: <200412091641.iB9GflnD067866@repoman.freebsd.org> <20041209174457.GA82542@freefall.freebsd.org> <41B893D3.59939DEA@freebsd.org> In-Reply-To: <41B893D3.59939DEA@freebsd.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed 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-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Dec 2004 21:02:59 -0000 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. Sam