ipsec: Fix XFRM clean up #36031
Merged
ipsec: Fix XFRM clean up #36031
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
e90f026 to
dc4f24d
Compare
When deleting old XFRM policies (ex. when disabling IPsec), we try to skip policies not installed by Cilium, just in case someone else has the good idea of using XFRM. We do so in function isXfrmPolicyCilium with various rules. Unfortunately, our XFRM IN policies changed in f108c0c ("ipsec: Simplify XFRM IN policies") and isXfrmPolicyCilium doesn't work anymore. So we need to adapt it to recognize the new XFRM IN policy. This bug resulted in Cilium never cleaning up the XFRM IN policy, which would cause one of our IPsec integration tests to always pass. Fixes: f108c0c ("ipsec: Simplify XFRM IN policies") Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
dc4f24d to
18a91c1
Compare
|
/test |
ldelossa
reviewed
Nov 21, 2024
| @@ -890,6 +890,13 @@ func isXfrmPolicyCilium(policy netlink.XfrmPolicy) bool { | |||
| policy.Priority == linux_defaults.IPsecFwdPriority { | |||
| return true | |||
| } | |||
| // Check if its our catch-all IN policy. | |||
| if policy.Dir == netlink.XFRM_DIR_IN && len(policy.Tmpls) == 1 { | |||
There was a problem hiding this comment.
really wish the kernel would just have an ID for policies that would identify them unique 😓
There was a problem hiding this comment.
I think Louis wants an ID that we can ip x p delete/get [id]. Therefore we could hard code an ID for cilium catch-all IN policy and check or delete it by ID.
ldelossa
approved these changes
Nov 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When deleting old XFRM policies (ex. when disabling IPsec), we try to skip policies not installed by Cilium, just in case someone else has the good idea of using XFRM. We do so in function
isXfrmPolicyCiliumwith various rules.Unfortunately, our XFRM IN policies changed in f108c0c ("ipsec: Simplify XFRM IN policies") and
isXfrmPolicyCiliumdoesn't work anymore. So we need to adapt it to recognize the new XFRM IN policy.This bug resulted in Cilium never cleaning up the XFRM IN policy, which would cause one of our IPsec integration tests to always pass.
Ran it against the old
mainto show that it now fails as it should: https://github.com/cilium/cilium/actions/runs/11914957041/job/33204378536. And then rebased to catch the fix for the test.Fixes: #35831.