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

Update(driver): Introduce the BPF commands name #1545

Merged
merged 5 commits into from
Feb 7, 2024

Conversation

Rohith-Raju
Copy link
Contributor

@Rohith-Raju Rohith-Raju commented Dec 5, 2023

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 tests

Does this PR require a change in the driver versions?

What this PR does / why we need it: This PR introduces the BPF command names, so evt.arg.cmd == BPF_PROG_LOAD instead of evt.arg.cmd == 5 can be used
see: #1343

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Update(driver): Introduce the BPF commands name

Copy link

github-actions bot commented Dec 5, 2023

Please double check driver/API_VERSION file. See versioning.

/hold

@Rohith-Raju Rohith-Raju changed the title Update(bpf syscall): Introduce the BPF commands name Update(driver): Introduce the BPF commands name Dec 5, 2023
@Rohith-Raju Rohith-Raju marked this pull request as draft December 5, 2023 11:40
@incertum
Copy link
Contributor

incertum commented Dec 5, 2023

Awesome! We will start reviewing soon.

/milestone 0.15.0

@poiana poiana added this to the 0.15.0 milestone Dec 5, 2023
@Rohith-Raju Rohith-Raju force-pushed the cmd_name branch 4 times, most recently from c2eb682 to ecab2da Compare December 7, 2023 08:40
@Rohith-Raju Rohith-Raju marked this pull request as ready for review December 7, 2023 12:55
@poiana poiana requested a review from jasondellaluce December 7, 2023 12:55
Copy link
Contributor

@incertum incertum left a comment

Choose a reason for hiding this comment

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

@Rohith-Raju since the goal is to use the string representation of the bpf cmd in Falco rules, could we add libsinsp filtercheck tests? Thank you!

@incertum
Copy link
Contributor

incertum commented Feb 6, 2024

@Rohith-Raju could we get a rebase, ty! @FedeDP kind bump to double-check if your change request was addressed, ty!

@FedeDP
Copy link
Contributor

FedeDP commented Feb 7, 2024

Yes this is good to go for me !

@incertum
Copy link
Contributor

incertum commented Feb 7, 2024

Great, @Rohith-Raju we will approve once rebased, we have final agreement.

Copy link
Contributor

@incertum incertum 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 poiana added the lgtm label Feb 7, 2024
@poiana
Copy link
Contributor

poiana commented Feb 7, 2024

LGTM label has been added.

Git tree hash: 07941bd100f197c2744e33c89da9582ea4c120f8

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 Feb 7, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, incertum, Rohith-Raju

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

@FedeDP
Copy link
Contributor

FedeDP commented Feb 7, 2024

/unhold

@poiana poiana merged commit 5d811ce into falcosecurity:master Feb 7, 2024
46 checks passed
@Andreagit97 Andreagit97 modified the milestones: 0.15.0, next-driver Feb 7, 2024
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.

5 participants