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

bpf, ipcache: unconditionally assume support for LPM trie maps #24258

Merged
merged 2 commits into from Mar 13, 2023
Merged

bpf, ipcache: unconditionally assume support for LPM trie maps #24258

merged 2 commits into from Mar 13, 2023

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Mar 9, 2023

LPM trie maps are available since kernel version 4.11 [1], [2]. For Cilium v1.14, the minimum required kernel version is 4.19.57 [3]. Thus we can now unconditionally assume support for that map type.

[1] https://github.com/iovisor/bcc/blob/master/docs/kernel-versions.md#map-types
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b95a5c4db09bc7c253636cb84dc9b12c577fd5a0
[3] https://docs.cilium.io/en/latest/operations/system_requirements/#base-requirements

For #22116

@tklauser tklauser added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Mar 9, 2023
@tklauser tklauser force-pushed the pr/tklauser/bpf-assume-lpm-trie branch 5 times, most recently from 104f019 to 0e3aae3 Compare March 9, 2023 10:38
@tklauser
bpf, ipcache: unconditionally assume support for LPM trie maps
d85a54d 
LPM trie maps are available since kernel version 4.11 [1], [2]. For Cilium v1.14,
the minimum required kernel version is 4.19.57 [3]. Thus we can now
unconditionally assume support for that map type.

[1] https://github.com/iovisor/bcc/blob/master/docs/kernel-versions.md#map-types
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b95a5c4db09bc7c253636cb84dc9b12c577fd5a0
[3] https://docs.cilium.io/en/latest/operations/system_requirements/#base-requirements

For #22116

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser
bpf: unconditionally assume map name support
b6c67ba 
For Cilium v1.14, the minimum required kernel version is 4.19.57 [1].
Thus we can now unconditionally assume support for map names which are
available in version >= 4.15.

[1] https://docs.cilium.io/en/latest/operations/system_requirements/#base-requirements

For #22116

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser force-pushed the pr/tklauser/bpf-assume-lpm-trie branch from 0e3aae3 to b6c67ba Compare March 9, 2023 10:45
@tklauser
Copy link
Member Author

tklauser commented Mar 9, 2023

/test

@tklauser tklauser marked this pull request as ready for review March 9, 2023 12:57
@tklauser tklauser requested review from a team as code owners March 9, 2023 12:57
@tklauser
Copy link
Member Author

tklauser commented Mar 9, 2023

The checkpatch failure is about the use of _Pragma here: https://github.com/cilium/cilium/pull/24258/files#diff-2538528be946afcdb57421de47717b0debdfb88f66755a1c3d88d4c2c3bee340R267. The LPM_LOOKUP_FN was using this previously and AFAIK there is no other way to unroll a loop within a macro. This PR just moves the macro as it's only used by BPF tests now. Looks to me like this failure can be ignored.

rgo3
rgo3 approved these changes Mar 10, 2023
View reviewed changes
Copy link
Contributor

@rgo3 rgo3 left a comment

Choose a reason for hiding this comment

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

Not related to the LPM trie maps but one of these cleanup PRs of 4.8/4.9 stuff should probably increase the versions in the general kernel version requirement checks:

cilium/pkg/datapath/linux/requirements.go

Line 28 in 7dbc63a

minKernelVer = "4.8.0"
If it doesn't belong here I can make a separate PR for it.

@tklauser
Copy link
Member Author

Not related to the LPM trie maps but one of these cleanup PRs of 4.8/4.9 stuff should probably increase the versions in the general kernel version requirement checks:

cilium/pkg/datapath/linux/requirements.go

Line 28 in 7dbc63a

minKernelVer = "4.8.0"

If it doesn't belong here I can make a separate PR for it.

Good point. I've noticed this as well when working on this PR but decided to leave it for a separate PR where the specifics of which versions exactly should be required could be discussed. I've added an item to the checklist in #22116 for that. Please feel free to open a PR to address that.

@borkmann
Copy link
Member

Good point. I've noticed this as well when working on this PR but decided to leave it for a separate PR where the specifics of which versions exactly should be required could be discussed. I've added an item to the checklist in #22116 for that. Please feel free to open a PR to address that.

Also clang in there should be reworked.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Mar 13, 2023
@tklauser tklauser merged commit 9152558 into master Mar 13, 2023
@tklauser tklauser deleted the pr/tklauser/bpf-assume-lpm-trie branch March 13, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@borkmann borkmann borkmann approved these changes

@christarazi christarazi christarazi approved these changes

@aspsk aspsk aspsk approved these changes

@rgo3 rgo3 rgo3 approved these changes

Assignees
No one assigned
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tklauser @borkmann @christarazi @aspsk @rgo3