8000 ebpf: non non-core. building files. by rafaeldtinoco · Pull Request #3015 · aquasecurity/tracee · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

ebpf: non non-core. building files. #3015

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

Merged
merged 6 commits into from
May 1, 2023
Merged

ebpf: non non-core. building files. #3015

merged 6 commits into from
May 1, 2023

Conversation

rafaeldtinoco
Copy link
Contributor
@rafaeldtinoco rafaeldtinoco commented Apr 24, 2023

I want to split the eBPF code into multiple eBPF objects and include the logic to load them depending on the events being traced, for example. But I think as an initial step we should drop non CORE, clean up the code a bit for readability and styling, and do that soon/later.

I'll try to enforce a code styling through some formatter (as we have many developers and each one seems to be doing a different style currently, even with clang-format trying to enforce some rules).

I have removed all NON CORE related code (AFAICT) and changed Makefiles, Dockerfiles and Workflows appropriately. Let's test here and then with our internal E2E tests to see how things go.

NOTE: I'll have to test the "snapshot" process after this is merged (I don't think there will be a problem) AND the "release" process during the next release.

This PR might impact other PRs touching eBPF code, but, still, easy to rebase as there are no logic changes, just positional/styling changes mostly.


commit 829adfe (HEAD -> drop-noncore, rafaeldtinoco/drop-noncore)
Author: Rafael David Tinoco rafaeldtinoco@gmail.com
Date: Wed Apr 26 01:02:57 2023

ebpf: re-organize headers and styling

After dropping the non CORE eBPF code, there is finally an opportunity
for a few things to finally happen:

1. Adjust headers recursion so clangd is capable of parsing types.

2. Adjust code styling and comments to something closer to kernel style:

  - Line wrapping at col 80. Accept up to col 102 for a few logics.
  - Not so descriptive variable names (although encouraged/accepted).

  > Styling should be enforced by a formatter (like clang-format) but
  > it does not take care of all peculiarities. To be done soon.

3. Split headers (for now) into:

  - kernel related headers (types, macros, constants)
  - tracee related headers (ebpf macros, internal type/structs, ...)

  > Just a trigger for splitting the code into multiple eBPF objects
  > with multiple header files, per object, and a logic to load
  > eBPF objects on demand (and depending on events being traced).

commit 239768b (test)
Author: Rafael David Tinoco rafaeldtinoco@gmail.com
Date: Tue Apr 25 23:24:11 2023

ebpf: non non-core. remove non-core ebpf code.

commit 380a1e1
Author: Rafael David Tinoco rafaeldtinoco@gmail.com
Date: Mon Apr 24 17:41:39 2023

ebpf: non non-core. workflows.

commit f7da045
Author: Rafael David Tinoco rafaeldtinoco@gmail.com
Date: Mon Apr 24 11:40:10 2023

ebpf: non non-co
8000
re. building files.

commit 467da62
Author: Rafael David Tinoco rafaeldtinoco@gmail.com
Date: Mon Apr 24 18:28:51 2023

packaging: update releases

@rafaeldtinoco

This comment was marked as outdated.

@rafaeldtinoco rafaeldtinoco marked this pull request as ready for review April 26, 2023 22:23
Copy link
Collaborator
@yanivagman yanivagman left a comment

Choose a reason for hiding this comment

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

Let's avoid renaming files in this PR (done in the last commit).
I disagree with some of these renamings (e.g. common->include and splitting kernel and tracee), yet I think we should merge the other commits.
We can discuss further changes in the next milestone.

@yanivagman
Copy link
Collaborator

Please, let's defer the last commit to another PR

@rafaeldtinoco
Copy link
Contributor Author

Okay I think this is "ready" to be merged IMO (unless there are other issues to be addressed that I couldn't spot). Let me know WYT. Thank you.

8000

Copy link
Collaborator
@yanivagman yanivagman left a comment

Choose a reason for hiding this comment

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

Mostly header inclusion doubts.
I believe we can merge after these nit fixes

Current eBPF source headers have several problems:

- some of them use local headers "", some the search path <>
- clangd wasn't able to index headers correctly

Follow: #2359 for TODOs

NO CHANGES BUT POSITIONAL AND INCLUDE FIXES.
@rafaeldtinoco rafaeldtinoco requested a review from yanivagman May 1, 2023 17:05
Copy link
Collaborator
@yanivagman yanivagman left a comment

Choose a reason for hiding this comment

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

LGTM!
Bye bye non-core! You served us well

@rafaeldtinoco rafaeldtinoco merged commit 2b39c0d into aquasecurity:main May 1, 2023
@rafaeldtinoco rafaeldtinoco deleted the drop-noncore branch May 1, 2023 18:53
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.

eBPF CO-RE improvements: remove non CO-RE source code
3 participants
0