Date: Thu, 09 Dec 2004 14:00:01 -0800 From: Sam Leffler <sam@errno.com> To: Gleb Smirnoff <glebius@FreeBSD.org> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/netinet ip_fw_pfil.c Message-ID: <41B8CAE1.6040703@errno.com> In-Reply-To: <20041209212512.GA946@cell.sick.ru> References: <200412091641.iB9GflnD067866@repoman.freebsd.org> <20041209174457.GA82542@freefall.freebsd.org> <41B893D3.59939DEA@freebsd.org> <41B8BF81.2000701@errno.com> <20041209212512.GA946@cell.sick.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
Gleb Smirnoff wrote: > On Thu, Dec 09, 2004 at 01:11:29PM -0800, Sam Leffler wrote: > S> Chris and I had a discussion a while back about not adding code that > S> checks for the presence of optional functionality. Using tags was > S> intended to eliminate checks like this. Doing this sort of stuff can > S> cause an unmaintainable/unreadable mess. Unless you can show there is a > > I don't see reduced readability. I don't see reduced maintainability. Sigh. > > S> noticeable performance gain from doing this I think it's a bad idea. > S> Remember that m_tag_find already inlines the check for whether any tags > S> are present or not before doing the lookup. > > S> If tag lookup cost becomes an important consideration in writing code > S> then we need to address that basic functionality. > > Searching a list is always more expensive than comparing a pointer value. And your point? > > There may exist a tag from divert, from altq and any future tag. It is > obvious that this change gives performance gain, and it is obvious that > this gain is not noticeable. You are adding cost to the most common use case in order to satisfy your preferred operating conditions. The overhead for optional features must be weighed against the cost to users that do not need/use these features. I suspect the vast majority of users do not use altq, divert, or ipfw, pf, whatever. I'm sure someone will chime in and explain that everyone should run a packet filter on their machine (that Darren's schtick ? :)) but my point is that it is not obvious this is a win. > > Now I'm working on interaction between netgraph and ipfw. I'm going > to add a very similar tag lookup here. The vast majority of hosts > running ipfw do not use dummynet or netgraph facility. Why would they > look in every packet for tags that would never appear in any packet? I'm glad your working on this stuff but please don't lose sight that netgraph and ipfw are optional components and their operation should not weigh down the operation of a system w/o them. If the cost is minimal I've got no issue. But since you refuse to measure the cost and simply state "it's obviously no cost" I'm going to complain. I understand this is one memory compare, but adding one here, one there, and eventually it does add up. > > I'd like to remind you that in this thread you were against removal > of the very similar check before doing tag lookup: > > http://freebsd.rambler.ru/bsdmail/freebsd-current_2004/msg22234.html True, but there are differences. The post you cite was about vlans and checking every packet that passes through a driver. That case was significantly more performance critical than yours. > > P.S. If you and Chris insist on reverting, I will do revert, but you didn't > made my opinion. > I never asked to revert; I am stating that I think this is a bad direction to go in. I removed all such checks a while back when I converted a lot of the old goo to use tags and at that time I MEASURED the overehead associated with the changes. Sam
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?41B8CAE1.6040703>