Date: Thu, 07 Mar 2002 21:59:51 -0800 From: Terry Lambert <tlambert2@mindspring.com> To: Bill Fumerola <billf@mu.org> Cc: Julian Elischer <julian@elischer.org>, green@freebsd.org, net@freebsd.org, hackers@freebsd.org Subject: Re: in_pcblookup_hash() called multiple times Message-ID: <3C885357.931A4E43@mindspring.com> References: <20020307092848.GX803@elvis.mu.org> <Pine.BSF.4.21.0203070351020.41354-100000@InterJet.elischer.org> <20020308033724.GZ803@elvis.mu.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Bill Fumerola wrote:
> On Thu, Mar 07, 2002 at 03:51:41AM -0800, Julian Elischer wrote:
> > what would be even nicer is if ipfw found the cached entry and passed it
> > back to ip_input so it didn't need to :-)
>
> i think this entire idea of cacheing it in ip_input() is a bad idea, no
> offense terry.
No, the idea is to cache it when it's looked up, and pass it
arounf from ip_input.
In the modified firewall code, I have:
P = in_pcblookup_hash(&udbinfo, src_ip,
src_port, dst_ip, dst_port, 1,
NULL);
if( inpp)
*inpp = P;
inside the loop. THis could just as easily be:
if (inpp) {
if (*inpp) {
P = *inpp;
} else {
P = *inpp = in_pcblookup_hash(&udbinfo, src_ip,
src_port, dst_ip, dst_port, 1,
NULL);
}
} else {
P = *inpp = in_pcblookup_hash(&udbinfo, src_ip,
src_port, dst_ip, dst_port, 1,
NULL);
}
etc., which would cache the lookup.
Also, the ip_fw_chk is not necessarily the first thing that
gets called that needs to do an in_pcblookup_hash.
> first, having a uid or gid rule in your ipfw ruleset (or even running
> ipfw) certainly isn't the common case. we're now going to bloat the
> ip_fw_chk() calls protocol handler calls to add an arguement that 99%
> of time is going to be NULL.
That's an extra push and pop. When a UDP or TCP packet
is handed off to the upper level code, the cost compared
to an additional in_pcblookup_hash call is very, very tiny.
If you want to get technical, the same sort of lookup is
used in the ip_flow.c code. If ipforwarding is enabled,
and there is at least one flow active, then the lookup will
happen, as well.
> then you have to bloat the protocol handlers to check if
> that pointer, that is NULL 99% of the time, isn't NULL.
For the TCP and UDP cases, this is true... if the firewall
code was not invoked. That's an extra push/pop and compare.
> i just don't think that the gain in the edge case justifies the slowdown
> in the common case. the first person to say 'sysctl' or '#ifdef' gets
> to drink from the fire hose.
8-).
Actually, you should ask John Lemon about this; he has
splicing code. Splicing needs to do the same sort of
lookup. I don't know if Cisco will let him commit it,
but if so, that's three cases.
> second, the real travesty is that if you have N rules in ipfw that
> reference a uid or gid, you will do a pcb lookup on the same packet for
> each of those N rules until you match. the worst case scenario of having
> to do a lookup for each uid rule is what the charts in my previous post
> measure.
That's cacheable (per the above).
> terry asked in his post: "NB: Is there a particular reason Bill's changes
> haven't simply been committed?", the reason is that my cache of the pcb
> and uid was done in my compiledipfw code, not the mainline ipfw. its
> really not a straight port because of somethings that compiledipfw keeps
> state on before generating code (skiptos, mostly); ipfw would have to
> be more careful then the generated code needs to be.
I think that you could cache the lookup the first time
through the loop, in any case, or, even better, before
the loop, if the loop is going to be hit (the list of
rules is non-empty).
> if brian feldman (the author of the ipfw uid/gid code) doesn't fix this
> out of embarassment first, i'll backport my cache into the main ipfw
> code.
I think it's useful to push the cached value up through
the stack. In a firewall case, or in a splice case, if
you can get the code out of Lemon, or in the ip_flow case
for the IP fast forwarding, if the hash is unified properly,
we're talking about 1/2 to 1/4 of the overhead for the lookup.
Actually, I'd use a global if I thought I could get away
with it... ;^)... but the overhead of pushing another pointer
and popping it later is really low, compared to the alternative.
-- Terry
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?3C885357.931A4E43>