cvs commit: src/sys/netinet ip_fw_pfil.c

Gleb Smirnoff glebius at freebsd.org
Thu Dec 9 13:25:23 PST 2004


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.

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.

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.

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'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

P.S. If you and Chris insist on reverting, I will do revert, but you didn't
made my opinion.

-- 
Totus tuus, Glebius.
GLEBIUS-RIPN GLEB-RIPE


More information about the cvs-src mailing list