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.

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

new: configure syscall buffer dimension from Falco #2214

Merged
merged 5 commits into from
Sep 27, 2022

Conversation

Andreagit97
Copy link
Member

@Andreagit97 Andreagit97 commented Sep 20, 2022

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

What this PR does / why we need it:

As discussed in various places (#2164 (comment), #2164 (comment), #2208) this PR adds the possibility to change the syscall buffer dimension through the Falco configuration file.

The approach used is the one described here #2208 (comment). Users will have an array of indexes that map to precise buffer dimensions in bytes. You can find more information in the falco.yaml file, but just for simplicity I report them here:

# --- [Description]
#
# This is an index that controls the dimension of the syscall buffers.
# The syscall buffer is the shared space between Falco and its drivers where all the syscall events
# are stored.
# Falco uses a syscall buffer for every online CPU, and all these buffers share the same dimension.
# So this parameter allows you to control the size of all the buffers!
#
# --- [Usage]
#
# You can choose between different indexes: from `1` to `10` (`0` is reserved for future uses).
# Every index corresponds to a dimension in bytes:
#
# [(*), 1 MB, 2 MB, 4 MB, 8 MB, 16 MB, 32 MB, 64 MB, 128 MB, 256 MB, 512 MB]
#   ^    ^     ^     ^     ^     ^      ^      ^       ^       ^       ^
#   |    |     |     |     |     |      |      |       |       |       |
#   0    1     2     3     4     5      6      7       8       9       10
#
# As you can see the `0` index is reserved, while the index `1` corresponds to
# `1 MB` and so on.
#
# These dimensions in bytes derive from the fact that the buffer size must be:
# (1) a power of 2.
# (2) a multiple of your system_page_dimension.
# (3) greater than `2 * (system_page_dimension)`.
#
# According to these constraints is possible that sometimes you cannot use all the indexes, let's consider an
# example to better understand it:
# If you have a `page_size` of 1 MB the first available buffer size is 4 MB because 2 MB is exactly
# `2 * (system_page_size)` -> `2 * 1 MB`, but this is not enough we need more than `2 * (system_page_size)`!
# So from this example is clear that if you have a page size of 1 MB the first index that you can use is `3`.
#
# Please note: this is a very extreme case just to let you understand the mechanism, usually the page size is something
# like 4 KB so you have no problem at all and you can use all the indexes (from `1` to `10`).
#
# To check your system page size use the Falco `--page-size` command line option. The output on a system with a page
# size of 4096 Bytes (4 KB) should be the following:
#
# "Your system page size is: 4096 bytes."
#
# --- [Suggestions]
#
# Before the introduction of this param the buffer size was fixed to 8 MB (so index `4`, as you can see
# in the default value below).
# You can increase the buffer size when you face syscall drops. A size of 16 MB (so index `5`) can reduce
# syscall drops in production-heavy systems without noticeable impact. Very large buffers however could
# slow down the entire machine.
# On the other side you can try to reduce the buffer size to speed up the system, but this could
# increase the number of syscall drops!
# As a final remark consider that the buffer size is mapped twice in the process' virtual memory so a buffer of 8 MB
# will result in a 16 MB area in the process virtual memory.
# Please pay attention when you use this parameter and change it only if the default size doesn't fit your use case.

syscall_buf_size_preset: 4

I added also a new command line option --page-size to let the user easily obtain the page size of their system

If you have any suggestions I'm all ears :)

Which issue(s) this PR fixes:

Fixes #2208

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new: configure syscall buffer dimension from Falco

@Andreagit97
Copy link
Member Author

Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

Left a comment; otherwise this LGTM!

Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

This looks great.

I've just left some suggestions (nitpicking 😸)

userspace/falco/app_cmdline_options.cpp Outdated Show resolved Hide resolved
falco.yaml Outdated Show resolved Hide resolved
falco.yaml Outdated Show resolved Hide resolved
FedeDP
FedeDP previously approved these changes Sep 20, 2022
Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Sep 20, 2022

LGTM label has been added.

Git tree hash: 5c95ce7172deeca86625ee937bbb3f6d92d7354a

Andreagit97 and others added 5 commits September 24, 2022 16:41
Signed-off-by: Andrea Terzolo <[email protected]>
Co-authored-by: Federico Di Pierro <[email protected]>
Signed-off-by: Andrea Terzolo <[email protected]>
Co-authored-by: Leonardo Grasso <[email protected]>
Co-authored-by: Melissa Kilby <[email protected]>
improve also some logs for `m_syscall_buf_size_preset` configuration errors

Signed-off-by: Andrea Terzolo <[email protected]>
Co-authored-by: Jason Dellaluce <[email protected]>
@Andreagit97
Copy link
Member Author

Andreagit97 commented Sep 24, 2022

Rebased on actual master just to have this fix #2215
I've also fixed the case in which we are not able to get the page system size in the last commit (fix(syscall_buffer): set dimension if page size not available)

@jasondellaluce
Copy link
Contributor

/milestone 0.33.0

@poiana poiana added this to the 0.33.0 milestone Sep 27, 2022
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

😍

@poiana poiana added the lgtm label Sep 27, 2022
@poiana
Copy link
Contributor

poiana commented Sep 27, 2022

LGTM label has been added.

Git tree hash: 450a6f8a38ed518bcd8fc9921aa2ee755470b0e0

Copy link
Contributor

@FedeDP FedeDP left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Sep 27, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, leogr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [Andreagit97,FedeDP,leogr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add an option to pick the buffer dimension
6 participants