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

refactor: eliminate variables by BPF_CORE_READ_INTO #214

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

Forsworns
Copy link
Contributor

@Forsworns Forsworns commented Jul 1, 2022

The BPF_CORE_READ_INTO can help reduce unnecessary variables and simplify the logic

Signed-off-by: Peihao Yang [email protected]

@Forsworns Forsworns requested a review from a team as a code owner July 1, 2022 08:26
@Forsworns Forsworns requested a review from kaworu July 1, 2022 08:26
@Forsworns Forsworns marked this pull request as draft July 1, 2022 08:26
@Forsworns Forsworns changed the title refactor: eliminate unnecessary temperary variables by BPF_CORE_READ_… refactor: eliminate variables by BPF_CORE_READ_INTO Jul 1, 2022
@Forsworns Forsworns marked this pull request as ready for review July 1, 2022 08:45
@kaworu kaworu added kind/enhancement This improves or streamlines existing functionality area/bpf This is related to BPF code labels Jul 1, 2022
*/
static long (*bpf_probe_read)(void *dst, __u32 size,
const void *unsafe_ptr) = (void *)4;

Copy link
Member

Choose a reason for hiding this comment

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

Why do we need that?

Copy link
Contributor Author

@Forsworns Forsworns Jul 7, 2022

Choose a reason for hiding this comment

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

because currently, probe_read is defined in https://github.com/cilium/tetragon/blob/HEAD/bpf/include/api.h#L206, so it can work without this definition.

However, BPF_CORE_READ_INTO in fact calls bpf_core_read, which is defined in

https://github.com/cilium/tetragon/blob/HEAD/bpf/libbpf/bpf_core_read.h#L117

by bpf_probe_read.

bpf_probe_read is not defined. So we have to supplement its definition, if we want to use BPF_CORE_READ_INTO.

@kaworu kaworu removed their request for review July 12, 2022 14:37
@Forsworns Forsworns marked this pull request as draft August 8, 2022 10:47
The BPF_CORE_READ_INTO can help reduce unnecessary variables and simplify the logic

Signed-off-by: Peihao Yang <[email protected]>
@Forsworns
Copy link
Contributor Author

Rebase since the definition of bpf_probe_read has been added before :)

@Forsworns Forsworns marked this pull request as ready for review August 8, 2022 11:13
@kkourt kkourt merged commit ecfe462 into cilium:main Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bpf This is related to BPF code kind/enhancement This improves or streamlines existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants