Skip to content

Comments

pkg/bpf/collection: Temporarily don't error on unused maps#41379

Merged
joestringer merged 1 commit intocilium:mainfrom
dylandreimerink:feature/noerror-verify-unused-maps
Aug 26, 2025
Merged

pkg/bpf/collection: Temporarily don't error on unused maps#41379
joestringer merged 1 commit intocilium:mainfrom
dylandreimerink:feature/noerror-verify-unused-maps

Conversation

@dylandreimerink
Copy link
Member

@dylandreimerink dylandreimerink commented Aug 26, 2025

When we introduced the new unused map pruning logic we added a verification step which, after loading, confirms we did everything as expected. This works by reading back the loaded program instructions.

However, on GKE the process of reading back the instructions fails inside of cilium/ebpf logic. To unblock CI until the root cause can be resolved, let us not throw an error in the verification step.

See #41245

When we introduced the new unused map pruning logic we added a
verification step which, after loading, confirms we did everything
as expected. This works by reading back the loaded program instructions.

However, on GKE the process of reading back the instructions fails
inside of cilium/ebpf logic. To unblock CI until the root cause can
be resolved, let us not throw an error in the verification step.

Signed-off-by: Dylan Reimerink <[email protected]>
@dylandreimerink dylandreimerink added the release-note/misc This PR makes changes that have no direct user impact. label Aug 26, 2025
@dylandreimerink dylandreimerink requested a review from a team as a code owner August 26, 2025 09:34
@dylandreimerink
Copy link
Member Author

/test

@dylandreimerink
Copy link
Member Author

/ci-gke

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Thanks, this is a much more surgical mitigation of the immediate failure for GKE. Given that commit 059977b introduced this check and it seems the repercussions are just that Cilium may load additional maps (which is what it would have done prior to the optimization), this seems like a safe workaround for now.

@joestringer
Copy link
Member

joestringer commented Aug 26, 2025

ci-eks run: hit known issue #36428 .
gke run: no longer hits #41245, the goal of this PR. Hit other underlying failures but that's no reason to hold back this fix.

Not sure why @cilium/loader was not pulled in for review, but as I understand there's a lack of bandwidth there at the moment. Given this should allow us to mitigate reliable failures on GKE for the immediate term I think this is valuable to push in as-is.

@joestringer joestringer merged commit 31ca947 into cilium:main Aug 26, 2025
71 of 73 checks passed
dylandreimerink added a commit to dylandreimerink/cilium that referenced this pull request Nov 5, 2025
In cilium#41379 we stopped returning errors from unused map verification
because on systems with sysctls set to restrict instruction readback
the verification would always fail and block CI.

In cilium/ebpf#1858 an `ErrRestrictedKernel` error was added to
indicate this specific case.

This allows us to now differentiate between genuine unused map errors
and the restricted kernel case, and only ignore the latter. Thus we can
re-enable unused map verification errors on systems that support it.

Signed-off-by: Dylan Reimerink <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Nov 11, 2025
In #41379 we stopped returning errors from unused map verification
because on systems with sysctls set to restrict instruction readback
the verification would always fail and block CI.

In cilium/ebpf#1858 an `ErrRestrictedKernel` error was added to
indicate this specific case.

This allows us to now differentiate between genuine unused map errors
and the restricted kernel case, and only ignore the latter. Thus we can
re-enable unused map verification errors on systems that support it.

Signed-off-by: Dylan Reimerink <[email protected]>
@cilium-release-bot cilium-release-bot bot moved this to Released in cilium v1.19.0 Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note/misc This PR makes changes that have no direct user impact.

Projects

No open projects
Status: Released

Development

Successfully merging this pull request may close these issues.

2 participants