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

Jump to bottom

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

Merged
merged 5 commits into from
Feb 7, 2024
Merged

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

merged 5 commits into from
Feb 7, 2024

Conversation

Rohith-Raju
Copy link
Contributor

@Rohith-Raju Rohith-Raju commented Dec 5, 2023 •
edited
Loading

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

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2023 •
edited
Loading

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
incertum
incertum reviewed Dec 9, 2023
View reviewed changes
Copy link
Contributor

@incertum incertum left a comment •
edited
Loading

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 incertum mentioned this pull request Dec 9, 2023
Closed
@Rohith-Raju Rohith-Raju force-pushed the cmd_name branch 3 times, most recently from 2400bb0 to f6db1df Compare December 12, 2023 08:17
@poiana poiana requested a review from incertum January 23, 2024 08:56
@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.

@Rohith-Raju
update(bpf): introduce the BPF commands name
da49823 
Signed-off-by: rohith-raju <rohithraju488@gmail.com>
@Rohith-Raju
update: address review requests
c16ba20 
Signed-off-by: RohithRaju <rohithraju488@gmail.com>
@Rohith-Raju
update: use unsigned long
38b40a4 
Signed-off-by: RohithRaju <rohithraju488@gmail.com>
@Rohith-Raju
update: made changes as per review request
d98fb8a 
Signed-off-by: RohithRaju <rohithraju488@gmail.com>
@Rohith-Raju
"Revert: made changes as per review request"
aa9abc5 
This reverts commit 41208ce.

Signed-off-by: RohithRaju <rohithraju488@gmail.com>
incertum
incertum approved these changes Feb 7, 2024
View reviewed changes
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

FedeDP
FedeDP approved these changes Feb 7, 2024
View reviewed changes
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
@incertum incertum mentioned this pull request Mar 7, 2024
Open
3 tasks
@LucaGuerra LucaGuerra mentioned this pull request Aug 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FedeDP FedeDP FedeDP approved these changes

@incertum incertum incertum approved these changes

@hbrueckner hbrueckner Awaiting requested review from hbrueckner

@jasondellaluce jasondellaluce Awaiting requested review from jasondellaluce

Assignees

@FedeDP FedeDP

@incertum incertum

Labels
approved area/driver-bpf area/driver-kmod area/driver-modern-bpf area/tests dco-signoff: yes kind/feature New feature or request lgtm release-note size/L
Projects
None yet
Milestone
7.1.0+driver
Development

Successfully merging this pull request may close these issues.
5 participants
@Rohith-Raju @incertum @poiana @FedeDP @Andreagit97