[Snort-devel] The _real_ performance improvement: TestHeader was broken!

Chris Green cmg at ...402...
Thu Mar 21 17:48:03 EST 2002

cm at ...1206... (Christian Mock) writes:

>> This has been changed :-)  It will be changed further in the future
>> but it's too easy a change and too big a difference for the multiple
>> CIDR Folks to not do.
> So, after an evening of improving my patch and rewriting the whole logic
> of EvalHeader, I found the real culprit, and of course it wasn't anywhere
> near that :-)
> TestHeader() is borken -- it does an memcmp() on the IpAddrSet structures,
> but that doesn't work since even with the same data, the next pointer is
> different. So when building the RTN chains, it makes 982 chains of 987
> rules, instead of 154 chains.

You have earned yourself a Snort.org beer!

> Changing this to explicit comparisons of the relevant struct members
> fixes the problem, and brings runtime down from 2:56 to 0:25 (tcpdump
> file with 100'000 packets from a real network, config and ruleset from
> the same setup; the 2:56 is with snort 1.84 as distributed).
> Combined with the position change of port and IP checking, we get down
> to 0:20, or 11% of the original (can you tell I can get very excited
> about performance-tuning stuff? :-).
> The patch is attached as snort-testheader.diff, and I'm pretty sure it can't
> break anything.

This is a bug fix against 1.8.4 and we will be including it in the
1.9tree as well.

Looks like we're going to be doing 1.8.5 after all.  

> ----
> The other stuff I did brings the time further down to 0:18, but I'm not 
> sure it doesn't break anything.

Yeah its a bit more complex than I can approve on the 12th hour of
work :-)

> What I did was to "inline" all the port and IP address checks into EvalHeader,
> on the assumption that since this is a critical code path, the CPU and 
> compiler can be much more efficient if everything is right there in the cache
> and available for optimiziation, plus we lose the function call overhead.
> I did check that the pattern match and other functions get called the same
> number of times as with the original code, but I'm not sure what it does
> with features I'm not using, like dynamic rules etc. It also doesn't make
> the code any easier to follow.
> Given the small improvement it gives us above the two little and riskless 
> changes, I don't know if it's worth the hassle; still, I attach it as 
> snort-cm.diff.gz (includes the TestHeader patch).
> I also tried changing the recursive nature of EvalHeader to a big while loop
> (again on the assumption of lowering function call overhead), but that gains 
> nothing.

Marty played with that the other night.  The only other thing that
remains to be seen is the recusive nature of the detection plugins.
Thats another TODO item to check out for the 1.9.x tree.

> g'night,

Thank you very much.

> cm.
> -- 
> Christian Mock                          Wiedner Hauptstrasse 15
> Senior Security Engineer                1040 Wien
> CoreTEC IT Security Solutions GmbH      +43-1-5037273

Chris Green <cmg at ...402...>
Eschew obfuscation.

More information about the Snort-devel mailing list