PatternDB: fixed fastpath of empty patterndb

[juhaszviktor]

If no ruleset is present, patterndb wont try to find any matching pattern.

Signed-off-by: Peter Gyorko peter.gyorko@balabit.com
Signed-off-by: Laszlo Budai Laszlo.Budai@balabit.com
Signed-off-by: Juhász Viktor viktor.juhasz@balabit.com

[bazsi]

It would be nicer if we got a bit more details wrt the patch, as clearly the existing code bypassed patterndb processing even now.

Also, this information about whether we loaded anything could probably be checked without the lock being held, which was probably the motivation for this patch.

Now, with this patch we take the lock, always. So what's the point here?

[bazsi]

I would reopen this ticket if I could, but seems I can't because its already merged. Let's hope this discussion Will not be forgotten.

[lbudai]

Hmm... it was ported from PE and it should have to solve a performance issue for those cases when the patterndb is empty.
But you are right: the commit message is not the best and maybe the solution is not a solution :-)
We can revert the patch if it is necessary.

[bazsi]

I think the intention was to avoid patterndb processing in case there were no ruleset tag in the XML. It was probably performance motivated (in SSB?) where db-parser() is always in the configuration, often with an empty xml.

The lock is needed if we dereference the pointer itself, as that can be replaced independently by another thread, but for the NULL check we don't need it. (as the worst thing that can happen is that we don't apply patterndb to a message where we could. Although the reverse might also happen (e.g. the ruleset pointer is already non-NULL but it contents didn't make it to the RAM yet). a g_atomic_pointer_get/set would solve that though.

Hmm, the lock is more correct than the original code so maybe we should leave it as is. I am sure performance gots a bit worse with this but if that turns out to be the case, we could always play with atomics and memory barriers.