-
Notifications
You must be signed in to change notification settings - Fork 1k
[WIP] Adopt specific-engine open methods (new libsinsp APIs)
#2164
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.
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
Changes from all commits
c148d31
b016d49
5f72654
207e4e8
2bd4af8
16a86fd
c74793c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -167,6 +167,9 @@ void cmdline_options::define() | |
| ("g,gvisor-config", "Parse events from gVisor using the specified configuration file. A falco-compatible configuration file can be generated with --gvisor-generate-config and can be used for both runsc and Falco.", cxxopts::value(gvisor_config), "<gvisor_config>") | ||
| ("gvisor-generate-config", "Generate a configuration file that can be used for gVisor.", cxxopts::value<std::string>(gvisor_generate_config_with_socket)->implicit_value("/tmp/gvisor.sock"), "<socket_path>") | ||
| ("gvisor-root", "gVisor root directory for storage of container state. Equivalent to runsc --root flag.", cxxopts::value(gvisor_root), "<gvisor_root>") | ||
| #endif | ||
| #ifdef HAS_MODERN_BPF | ||
| ("modern-bpf", "Use BPF modern probe to capture new events.", cxxopts::value(modern_bpf)->default_value("false")) | ||
|
Comment on lines
+171
to
+172
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to write here something about the fact that is experimental |
||
| #endif | ||
| ("i", "Print all events that are ignored by default (i.e. without the -A flag) and exit.", cxxopts::value(print_ignored_events)->default_value("false")) | ||
| #ifndef MINIMAL_BUILD | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -212,6 +212,10 @@ void falco_configuration::init(string conf_filename, const vector<string> &cmdli | |
| m_webserver_ssl_enabled = m_config->get_scalar<bool>("webserver.ssl_enabled", false); | ||
| m_webserver_ssl_certificate = m_config->get_scalar<string>("webserver.ssl_certificate", "/etc/falco/falco.pem"); | ||
|
|
||
| // we put this value in the configuration file because in this way we can change the dimension | ||
| // at every reload. | ||
| m_single_buffer_dimension = m_config->get_scalar<uint64_t>("single_buffer_dimension", 0); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should or could we be doing more to ensure end user parameter input is not bogus? This would apply to any parameter and is not specific to this PR.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For what concern the validity checks for sure we have to implement them in the final drivers since they are the end users of this info, we can put also something in Falco, BTW if we use the page number as a value there is not too much to check probably that it is greater than |
||
|
|
||
| std::list<string> syscall_event_drop_acts; | ||
| m_config->get_sequence(syscall_event_drop_acts, "syscall_event_drops.actions"); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this option affects the
syscalldata source only, I'd prefix it withsyscall_and move it close to othersyscall_options.Also, I'd simplify the name and explain in the comment what that size means and how it affects the operations (eg. the driver will create a buffer per CPU,
syscall_buffer_sizespecifies the size in bytes of each buffer...)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
completely agree with the new name!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we decide to have this param be just a multiple of page size need to adjust the
_sizeat the end and explain.[nit] More additional explanations for end users less familiar with architecture. Something like:
Falco uses a shared buffer between kernel and userspace to serve events up to userspace for subsequent processing and rules matching. With the
TBD_BUFFER_PARAM_NAMEyou can increase or decrease the default buffer size for either driver (BPF, kmod, modern BPF) wrt the system call tracing functionality. In more detail, Falco uses one buffer for every online CPU, but you generically adjust the size once with this parameter. Increasing the buffer can help reducing kernel side syscall drops while decreasing the buffer can help speed up the entire engine.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thought that just occurred to me, maybe have default value be actual default value and not 0 to be consistent with other parameters? @Andreagit97 @leogr
But now I can see an issue for kmod vs eBPF default values, hmmm ... and we should always gracefully start up Falco with default value like it's the case right now.