Check rulesets first #1126
Check rulesets first #1126
Conversation
Change how rulesets are organized in sinsp_evttype_filter. Previously
the primary index into the set of filters was by event type. For filters
matching an event type, each would be considered to see if they were
relevant for a given ruleset, and if so, the filter would be run.
This ends up being inefficient for the use case of iterating over
rulesets, as you end up traversing the list of filters for a given event
type N times for N rulesets.
Instead, add a new object ruleset_filters which holds the set of filters
for a given ruleset. The lists of filters indexed by evttype/syscall
id move from the main object to the ruleset_filters object.
Also, get rid of the notion of "catchall" filters. If a filter really
should apply for all evttypes/syscalls, it just gets included in every
m_filter_by_{evttype,syscall} list. Catchall filters are highly
discouraged and don't exist for the default falco ruleset, so it's ok to
not have a special case for it. This ends up significantly simplifying
::run(), as you can iterate over a single list instead of two and don't
need to keep track of any order value for a given filter.
While running some tests using valgrind, I noticed that a value from a node that was removed due to a shorter path matching a value wasn't being deleted.
Previously, a filter would be added to a rulset when enable was set to true, but nothing was done if enable was set to false. Fix this by actually removing the filter via a new method rulset_filters::remove_filter.
|
Could you take a look at the addl commit from today that properly handles removing filters for a tag/regex? |
|
Looks good. On a more general note, why can't we switch the wrappers evttype/syscallID vectors to sets? There would be no need to do a loop of all event types/syscalls every time we need to use them. |
|
Right, all the filters ever added are in m_filters, and copies of those pointers are spread amongst the rulesets. I think m_filter_by_syscall/m_filter_by_evttype need to be vectors so you can efficiently find the right list of filters in sinsp_evttype_filter::ruleset_filters::run. If you mean evttypes_for_ruleset()/syscalls_for_ruleset(), you're right, those could be sets, but it requires changing both falco and the agent to use it properly. So I'm ok with also making that change, but let me do it separately so I can get this merged. I filed falcosecurity/falco#369 to track that. |
Previously, rules were enabled by default. Some performance improvements in draios/sysdig#1126 broke this, requiring that each rule is explicitly enabled or disabled for a given ruleset. So if enabled is true, explicitly enable the rule for the default ruleset.
* Proactively enable rules instead of only disabling Previously, rules were enabled by default. Some performance improvements in draios/sysdig#1126 broke this, requiring that each rule is explicitly enabled or disabled for a given ruleset. So if enabled is true, explicitly enable the rule for the default ruleset. * Get rid of shadowed res variable. It was used both for the inspector loop and the falco result.
Improve performance for the common use case of iterating over a bunch of rulesets and calling sinsp_evttype_filter::run() for each, by grouping filters by ruleset first. Check the first commit for details on the change.