Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(userspace/libsinsp): let plugins parse events before eventually filtering them out through inspector global filter #2182

Merged
merged 2 commits into from
Dec 4, 2024
Merged

fix(userspace/libsinsp): let plugins parse events before eventually filtering them out through inspector global filter #2182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b581f9
fix(userspace/libsinsp): let plugins parse events before eventually f…
FedeDP Dec 4, 2024
b162571
chore(userspace/libsinsp): move `sinsp_evt_filter` definition to sins…
FedeDP Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 1 addition & 68 deletions userspace/libsinsp/parsers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,52 +73,7 @@ void sinsp_parser::set_track_connection_status(bool enabled) {
// PROCESSING ENTRY POINT
///////////////////////////////////////////////////////////////////////////////
void sinsp_parser::process_event(sinsp_evt *evt) {
uint16_t etype = evt->get_scap_evt()->type;
bool is_live = m_inspector->is_live() || m_inspector->is_syscall_plugin();

//
// Cleanup the event-related state
//
reset(evt);

//
// When debug mode is not enabled, filter out events about itself
//
if(is_live && !m_inspector->is_debug_enabled()) {
if(evt->get_tid() == m_inspector->m_self_pid && etype != PPME_SCHEDSWITCH_1_E &&
etype != PPME_SCHEDSWITCH_6_E && etype != PPME_DROP_E && etype != PPME_DROP_X &&
etype != PPME_SCAPEVENT_E && etype != PPME_PROCINFO_E && etype != PPME_CPU_HOTPLUG_E &&
m_inspector->m_self_pid) {
evt->set_filtered_out(true);
return;
}
}

//
// Filtering
//
bool do_filter_later = false;

if(m_inspector->m_filter) {
ppm_event_flags eflags = evt->get_info_flags();

if(eflags & EF_MODIFIES_STATE) {
do_filter_later = true;
} else {
if(m_inspector->run_filters_on_evt(evt) == false) {
if(evt->get_tinfo() != NULL) {
if(!(eflags & EF_SKIPPARSERESET || etype == PPME_SCHEDSWITCH_6_E)) {
evt->get_tinfo()->set_lastevent_type(PPM_EVENT_MAX);
}
}

evt->set_filtered_out(true);
return;
}
}
}

evt->set_filtered_out(false);
const uint16_t etype = evt->get_scap_evt()->type;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the parser is only responsible for the big switch case (ie: the actual parsing).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the code below has been moved to a RAII oriented structure under sinsp.h: struct sinsp_evt_filter.


//
// Route the event to the proper function
Expand Down Expand Up @@ -451,28 +406,6 @@ void sinsp_parser::process_event(sinsp_evt *evt) {
break;
}

//
// With some state-changing events like clone, execve and open, we do the
// filtering after having updated the state
//
if(do_filter_later) {
if(!m_inspector->run_filters_on_evt(evt)) {
evt->set_filtered_out(true);
return;
}
evt->set_filtered_out(false);
}
//
// Offline captures can produce events with the SCAP_DF_STATE_ONLY. They are
// supposed to go through the engine, but they must be filtered out before
// reaching the user.
//
if(m_inspector->is_capture()) {
if(evt->get_dump_flags() & SCAP_DF_STATE_ONLY) {
evt->set_filtered_out(true);
}
}

// Check to see if the name changed as a side-effect of
// parsing this event. Try to avoid the overhead of a string
// compare for every event.
Expand Down
2 changes: 1 addition & 1 deletion userspace/libsinsp/parsers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class sinsp_parser {
void process_event(sinsp_evt* evt);
void event_cleanup(sinsp_evt* evt);

bool reset(sinsp_evt* evt);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it is now called by sinsp, moved to public.

void erase_fd(erase_fd_params* params);

//
Expand All @@ -57,7 +58,6 @@ class sinsp_parser {
//
// Helpers
//
bool reset(sinsp_evt* evt);
inline void store_event(sinsp_evt* evt);

//
Expand Down
110 changes: 98 additions & 12 deletions userspace/libsinsp/sinsp.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,79 @@
*/
#define DEFAULT_ASYNC_EVENT_QUEUE_SIZE 1000

// Small sinsp event filter wrapper logic
// that uses RAII to eventually filter out events.
struct sinsp_evt_filter {
bool do_filter_later;
sinsp_evt* evt;

explicit sinsp_evt_filter(sinsp_evt* e): do_filter_later(false), evt(e) {
//
// Filtering
//
const auto inspector = evt->get_inspector();
const bool is_live = inspector->is_live() || inspector->is_syscall_plugin();
const uint16_t etype = evt->get_type();

evt->set_filtered_out(false);

//
// When debug mode is not enabled, filter out events about itself
//
if(is_live && !inspector->is_debug_enabled()) {
if(evt->get_tid() == inspector->m_self_pid && etype != PPME_SCHEDSWITCH_1_E &&
etype != PPME_SCHEDSWITCH_6_E && etype != PPME_DROP_E && etype != PPME_DROP_X &&
etype != PPME_SCAPEVENT_E && etype != PPME_PROCINFO_E &&
etype != PPME_CPU_HOTPLUG_E && inspector->m_self_pid) {
evt->set_filtered_out(true);
return;

Check warning on line 84 in userspace/libsinsp/sinsp.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/sinsp.cpp#L83-L84 

Added lines #L83 - L84 were not covered by tests
}
}

if(inspector->m_filter) {
ppm_event_flags eflags = evt->get_info_flags();
if(eflags & EF_MODIFIES_STATE) {
do_filter_later = true;
} else {
if(!inspector->run_filters_on_evt(evt)) {
if(evt->get_tinfo() != NULL) {
if(!(eflags & EF_SKIPPARSERESET || etype == PPME_SCHEDSWITCH_6_E)) {
evt->get_tinfo()->set_lastevent_type(PPM_EVENT_MAX);
}
}
evt->set_filtered_out(true);
}
}
}
}

~sinsp_evt_filter() {
//
// With some state-changing events like clone, execve and open, we do the
// filtering after having updated the state
//
const auto inspector = evt->get_inspector();
if(do_filter_later) {
if(!inspector->run_filters_on_evt(evt)) {
evt->set_filtered_out(true);
return;
}
evt->set_filtered_out(false);
}

//
// Offline captures can produce events with the SCAP_DF_STATE_ONLY. They are
// supposed to go through the engine, but they must be filtered out before
// reaching the user.
//
if(inspector->is_capture()) {
if(evt->get_dump_flags() & SCAP_DF_STATE_ONLY) {
evt->set_filtered_out(true);

Check warning on line 126 in userspace/libsinsp/sinsp.cpp

View check run for this annotation

Codecov / codecov/patch

userspace/libsinsp/sinsp.cpp#L126 

Added line #L126 was not covered by tests
}
}
}
};

int32_t on_new_entry_from_proc(void* context,
char* error,
int64_t tid,
Expand Down Expand Up @@ -1216,19 +1289,32 @@
}

//
// Run the state engine
// Cleanup the event-related state
//
m_parser->process_event(evt);
m_parser->reset(evt);

// run plugin-implemented parsers
// note: we run the parsers even if the event has been filtered out,
// because we have no guarantee that the plugin parsers will not use a given
// event for state updates. Sinsp understands this through the
// EF_MODIFIES_STATE flag, which however is only relevant in the context of
// the internal implementation of libsinsp.
for(auto& pp : m_plugin_parsers) {
// todo(jason): should we log parsing errors here?
pp.process_event(evt, m_event_sources);
// Since evt_filter object below uses RAII, create a new scope.
{
// Object that uses RAII to enable event filtered out flag
sinsp_evt_filter evt_filter(evt);

if(!evt->is_filtered_out()) {
//
// Run the state engine
//
m_parser->process_event(evt);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep old behavior: only process event by sinsp if the event has not been filtered out.

}

// run plugin-implemented parsers
// note: we run the parsers even if the event has been filtered out,
// because we have no guarantee that the plugin parsers will not use a given
// event for state updates. Sinsp understands this through the
// EF_MODIFIES_STATE flag, which however is only relevant in the context of
// the internal implementation of libsinsp.
for(auto& pp : m_plugin_parsers) {
// todo(jason): should we log parsing errors here?
pp.process_event(evt, m_event_sources);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For plugins, as stated by the comment, we cannot know in advance if an event will modify its state; therefore always call the plugin parsers.

}
}

// Finally set output evt;
Expand Down Expand Up @@ -1536,7 +1622,7 @@
return m_filterstring;
}

bool sinsp::run_filters_on_evt(sinsp_evt* evt) {
bool sinsp::run_filters_on_evt(sinsp_evt* evt) const {
//
// First run the global filter, if there is one.
//
Expand Down
2 changes: 1 addition & 1 deletion userspace/libsinsp/sinsp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ class SINSP_PUBLIC sinsp : public capture_stats_source {
return m_internal_flt_ast;
}

bool run_filters_on_evt(sinsp_evt* evt);
bool run_filters_on_evt(sinsp_evt* evt) const;

/*!
\brief This method can be used to specify a function to collect the library
Expand Down
Loading