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

examples: fentry - add comments to illustrate difference with tcprtt #615

Merged
merged 1 commit into from Mar 30, 2022

Conversation

ti-mo
Copy link
Collaborator

@ti-mo ti-mo commented Mar 30, 2022

The tcprtt example was added recently, which relies on CO-RE information to work across different kernel versions. fentry, on the other hand, will break if structs change in the running kernel, so document this fact.

Replaced unused fields with padding and addressed some nits.

The tcprtt example was added recently, which relies on CO-RE information
to work across different kernel versions. fentry, on the other hand, will
break if structs change in the running kernel, so document this fact.

Replaced unused fields with padding and addressed some nits.

Signed-off-by: Timo Beckers <timo@isovalent.com>
*
* Also note that BTF-enabled programs like fentry, fexit, fmod_ret, tp_btf,
* lsm, etc. declared using the BPF_PROG macro can read kernel memory without
* needing to call bpf_probe_read*().
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't this be BPF_CORE_READ*()?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

BPF_CORE_READ and friends emit a CO-RE relocation and call bpf_probe_read_*() behind the scenes, but that's not what is meant here specifically. This comment is just about the memory accesses in the kernel, not the CO-RE part.

@ti-mo ti-mo merged commit 403bb27 into cilium:master Mar 30, 2022
@ti-mo ti-mo deleted the tb/document-fentry branch March 30, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants