Date: Thu, 09 Dec 2004 13:11:29 -0800 From: Sam Leffler <sam@errno.com> To: Andre Oppermann <andre@FreeBSD.org> Cc: src-committers@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet ip_fw_pfil.c Message-ID: <41B8BF81.2000701@errno.com> In-Reply-To: <41B893D3.59939DEA@freebsd.org> References: <200412091641.iB9GflnD067866@repoman.freebsd.org> <20041209174457.GA82542@freefall.freebsd.org> <41B893D3.59939DEA@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?41B8BF81.2000701>