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(driver): unify support for io_uring syscall family between drivers #844

Merged
merged 6 commits into from
Jan 31, 2023

Conversation

Andreagit97
Copy link
Member

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libpman

/area tests

Does this PR require a change in the driver versions?

No

What this PR does / why we need it:

On one side this PR supports 3 missing syscalls in the modern bpf probe #723:

  • io_uring_enter
  • io_uring_register
  • io_uring_setup

On the other side it unifies the behavior of this syscall between our 3 drivers:

  • file descriptor managed on 64 bit
  • send event even when the pointer to struct io_uring_params is invalid

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

new(driver): unify support for `io_uring` syscall family between drivers

@FedeDP
Copy link
Contributor

FedeDP commented Jan 26, 2023

Wow great job Andrea! Gonna give a look at this asap!
/milestone next-driver

@poiana poiana added this to the next-driver milestone Jan 26, 2023
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.

We are only testing syscall failures here, right?
Left a comment, but the PR lgtm!

driver/ppm_fillers.c Outdated Show resolved Hide resolved
@Andreagit97
Copy link
Member Author

We are only testing syscall failures here, right?

Yes, collected data don't change if the syscall is successful or fails, so testing the failure case without adding other entropy in the testing framework seems the right way to go :)

Andreagit97 and others added 6 commits January 31, 2023 11:59
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: Hendrik Brueckner <[email protected]>
Signed-off-by: Andrea Terzolo <[email protected]>
Co-authored-by: Hendrik Brueckner <[email protected]>
@Andreagit97
Copy link
Member Author

Rebased and added 3 commits to address review comments :)

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.

Thank you!
/approve

@poiana
Copy link
Contributor

poiana commented Jan 31, 2023

LGTM label has been added.

Git tree hash: 128a1a6ef32bd1150154aacbf8d3e99e7b4c0d73

Copy link
Contributor

@hbrueckner hbrueckner left a comment

Choose a reason for hiding this comment

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

Thanks a lot @Andreagit97 !
/approve

@poiana
Copy link
Contributor

poiana commented Jan 31, 2023

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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

@poiana poiana merged commit 6dfd64b into falcosecurity:master Jan 31, 2023
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.

4 participants