Skip to content

Conversation

suchit07-git
Copy link
Contributor

@suchit07-git suchit07-git commented Jul 23, 2025

WriteEndpoint() previously updated all BPF map keys for an endpoint without handling partial failures during the update process. This commit fixes the issue by tracking successfully written keys, and rolling them back if any subsequent update fails. This ensures the LXC map remains consistent and avoids stale partial entries.
Fixes #40829

@suchit07-git suchit07-git requested a review from a team as a code owner July 23, 2025 10:58
@suchit07-git suchit07-git requested a review from ldelossa July 23, 2025 10:58
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 23, 2025
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jul 23, 2025
@suchit07-git suchit07-git reopened this Jul 27, 2025
@suchit07-git suchit07-git reopened this Jul 27, 2025
@ldelossa
Copy link
Contributor

ldelossa commented Jul 30, 2025

@suchit07-git does an issue exist for this? If not could you create one?

I think the change is reasonable, however I'm curious if we need a more generic method of doing this, for updating maps. I'd like to raise an issue to determine how wide spread this issue may be, and if it needs to be addressed globally, and not just here.

@suchit07-git
Copy link
Contributor Author

suchit07-git commented Jul 30, 2025

@suchit07-git does an issue exist for this? If not could you create one?

@ldelossa I couldn't find one. So I'll create one. I should create it specific to this issue, correct?

I think the change is reasonable, however I'm curious if we need a more generic method of doing this, for updating maps. I'd like to raise an issue to determine how wide spread this issue may be, and if it needs to be addressed globally, and not just here.

I'm not sure actually. I think if there are similar issues in other places then we can create a generic solution.

@ldelossa
Copy link
Contributor

@suchit07-git

Yup, let's open an issue specific to this.

I'm curious to pull in some others to see if this is more of a wide spread issue.

@suchit07-git
Copy link
Contributor Author

suchit07-git commented Jul 30, 2025

@ldelossa I've created an issue as per your suggestion

Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

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

Changes look good :)

Nice find, and thanks for the fix.

@ldelossa
Copy link
Contributor

/test

@ldelossa ldelossa added release-note/minor This PR changes functionality that users may find relevant to operating Cilium. release-note/bug This PR fixes an issue in a previous release of Cilium. and removed release-note/minor This PR changes functionality that users may find relevant to operating Cilium. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jul 31, 2025
@ldelossa
Copy link
Contributor

@suchit07-git

Looks like quite a few test failures here. Can you rebase this PR against HEAD and force push the changes? This will just ensure any test fixes are applied to your branch, I'll rerun tests after.

WriteEndpoint previously updated all BPF map keys for an endpoint without handling partial failures during the update process. This commit fixes the issue by tracking successfully written keys, and  rolling them back if any subsequent update fails. This ensures the LXC map  remains consistent and avoids stale partial entries.

Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
@suchit07-git suchit07-git force-pushed the pr/lxcmap-rollback-on-update-failure branch from 1eb2872 to 32cb3c2 Compare July 31, 2025 13:55
@suchit07-git
Copy link
Contributor Author

@ldelossa I've rebased it. Could you please check now?

@ldelossa
Copy link
Contributor

/test

@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 Jul 31, 2025
@ldelossa ldelossa added this pull request to the merge queue Jul 31, 2025
Merged via the queue into cilium:main with commit cd33deb Jul 31, 2025
68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WriteEndpoint() doesn't handle partial BPF map update failures, leaving stale entries
2 participants