Skip to content

Conversation

marseel
Copy link
Contributor

@marseel marseel commented Jun 4, 2024

Once this PR is merged, a GitHub action will update the labels of these PRs:

 32588

@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.13 kind/backports This PR provides functionality previously merged into master. labels Jun 4, 2024
@marseel
Copy link
Contributor Author

marseel commented Jun 4, 2024

/test-backport-1.13

[ upstream commit 3a4c57f ]

[ Backporter's notes: switch default to false - so not enabled by
default. Switch from testing package to checkmate in unit tests. Flags
use Vp instead of vp. Minor conflicts with netlink.XfrmState* calls.
Switched from pkg/time to time. Switch from checkmate to check.v1 ]

Reduces GC CPU usage and memory allocations coming from XfrmStateList.
To ensure we have up-to-date cache, wrap all XfrmState related
functions inside cache, which is invalidated whenever XfrmState changes.

This is follow-up to #32577
While that PR averages out CPU usage over time, in large cluster 100+
nodes amount of allocations coming from netlink.XfrmStateList() is high
due to backgroundSync where we usually don't change any Xfrm states.
This becomes more and more expensive as number of nodes increases.

Added CI test to make sure that we accidentally don't add calls that
modify XFRMState without going through cache.

Also, added hidden option that allows to turn of caching.

Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
@marseel marseel force-pushed the backport_xfrm_state_cache_v1.13 branch from 39cda60 to adf9416 Compare June 4, 2024 14:33
@marseel
Copy link
Contributor Author

marseel commented Jun 4, 2024

/test-backport-1.13

@marseel marseel requested a review from pchaigno June 5, 2024 08:27
@marseel marseel marked this pull request as ready for review June 5, 2024 08:27
@marseel marseel requested a review from a team as a code owner June 5, 2024 08:27
@qmonnet

This comment was marked as outdated.

@qmonnet

This comment was marked as outdated.

@qmonnet

This comment was marked as outdated.

@marseel
Copy link
Contributor Author

marseel commented Jun 5, 2024

I was wrong Quentin, I triggered tests twice in this PR, only the first one was run in Jenkins 😢

@qmonnet
Copy link
Member

qmonnet commented Jun 5, 2024

I was wrong Quentin, I triggered tests twice in this PR, only the first one was run in Jenkins 😢

That explains why I can't find the results for the other two v1.13 backport PRs 😕. Not sure how to fix this.

@marseel
Copy link
Contributor Author

marseel commented Jun 5, 2024

/test-backport-1.13

@qmonnet
Copy link
Member

qmonnet commented Jun 5, 2024

VM provisioning failure.
/test-1.24-5.4

@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 Jun 5, 2024
@michi-covalent michi-covalent merged commit b39584f into v1.13 Jun 5, 2024
@michi-covalent michi-covalent deleted the backport_xfrm_state_cache_v1.13 branch June 5, 2024 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/backports This PR provides functionality previously merged into master. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
No open projects
Status: Released
Development

Successfully merging this pull request may close these issues.

4 participants