Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
LINUX_KERNEL_VERSION
is set only when loading with libbpf (we are using github.com/cilium/ebpf to load the prog).We could ask / contribute to cilium/ebpf to add the support for it (should be relatively trivial, see https://github.com/libbpf/libbpf/blob/v0.5.0/src/libbpf.c#L6804). /cc @ti-mo @lmb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LINUX_KERNEL_VERSION is just to extract a running kernel version or a name that matches one of Kconfig’s keys. I do not think it has something with libbpf.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version is set during the loadtime. Check https://github.com/libbpf/libbpf/blob/v0.5.0/src/libbpf.c#L6804 to see how it's set in libbpf. Currently, cilium/ebpf doesn't support it.
Also, you can check it by adding into the code:
and then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@brb thanks for you explanation. I got the point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Please let me know if you are going to work on adding
LINUX_KERNEL_VERSION
support to cilium/ebpf.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, apparenly this depends on parsing
/proc/config.gz
: https://nakryiko.com/posts/bpf-core-reference-guide/#linux-kernel-version. I don't thinkcilium/ebpf
has support for Kconfig externs yet.We have kernel version detection, though: https://github.com/cilium/ebpf/blob/1eb0e981058a2d5335965c6dc450a6442ad0377e/internal/version.go#L90-L99. I would prefer to use this for
LINUX_KERNEL_VERSION
over parsing kconfig.gz in the mean time.@lmb any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on config.gz for
CONFIG_
parsing. WhileLINUX_KERNEL_VERSION
- on theuname
syscall.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, we're open to implementing the
LINUX_KERNEL_VERSION
extern using the existing kernel version detection we have in the lib. @zhangbo1882 would you be willing to contribute this as a feature?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ti-mo , I am very pleased to do that. Let me check cilium/ebpf code first.