Skip to content

Conversation

ti-mo
Copy link
Contributor

@ti-mo ti-mo commented Jul 8, 2025

My take on #39462.

At present, we use ifdefs to manually determine when a map definition
should be emitted into the object file. When the object file gets loaded by
the agent, every map therein is created and populated before programs can
be loaded into the kernel.

Given the quest for clang-freedom, we want to to pre-compile all programs
and only create and populate maps for features that are enabled at runtime
based on global variables for feature flags.

This commit introduces this capability. It performs static analysis on the BPF
program, splits it into basic blocks and finds global data accesses used in
branching instructions. Then, it can predict whether a branch is always or
never taken.

By occluding unreachable parts of the program, it predicts which maps will
remain in use after the verifier performed its own dead code elimination as
part of the verification process. Before loading, unused maps are removed from
the CollectionSpec, and all of their references are poisoned with simple
immediate load instructions.

This means all maps can now be unconditionally defined, normal branches (if
statements!) can be used, and developers no longer have to worry about when
and which maps are or are not loaded under which conditions. Users don't
have to pay for features they don't use.

Automatically skip creating maps that are unused by Cilium's current configuration

@maintainer-s-little-helper
Copy link

Commit 458d50d does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jul 8, 2025
@ti-mo
Copy link
Contributor Author

ti-mo commented Jul 8, 2025

/test

@maintainer-s-little-helper
Copy link

Commit 458d50d does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@ti-mo ti-mo force-pushed the tb/dead-code-maps branch from dad2cf6 to 3d56157 Compare July 10, 2025 13:22
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jul 10, 2025
@ti-mo ti-mo added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Jul 10, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 10, 2025
@ti-mo
Copy link
Contributor Author

ti-mo commented Jul 10, 2025

/test

@ti-mo ti-mo force-pushed the tb/dead-code-maps branch from 3d56157 to d09d20e Compare July 10, 2025 15:36
@ti-mo
Copy link
Contributor Author

ti-mo commented Jul 10, 2025

/test

@ti-mo ti-mo force-pushed the tb/dead-code-maps branch from d09d20e to 1655d9e Compare July 10, 2025 18:17
@ti-mo ti-mo changed the title bpf: VariableSpec-based reachability analysis and map pruning bpf/analyze: reachability analysis for pruning unused maps Jul 10, 2025
@ti-mo
Copy link
Contributor Author

ti-mo commented Jul 10, 2025

/test

@ti-mo ti-mo marked this pull request as ready for review July 10, 2025 18:19
@ti-mo ti-mo requested review from a team as code owners July 10, 2025 18:19
@ti-mo ti-mo requested review from ldelossa and rgo3 July 10, 2025 18:19
@ti-mo ti-mo linked an issue Jul 14, 2025 that may be closed by this pull request
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.

Very nice work @tb and @dylandreimerink. Excellent comments in the code to guide readers who are less familiar or new to the code 🙇.

Special thanks to Dylan for that offline code walk, that helped a lot!

Overall looks good to me, just two questions to clarify my understanding of the tests and underlying logic.

dylandreimerink and others added 4 commits August 7, 2025 11:25
At present, we use `ifdef`s to manually determine when a map definition
should be emitted into the object file. When the object file gets loaded by
the agent, every map therein is created and populated before programs can
be loaded into the kernel.

Given the quest for clang-freedom, we want to pre-compile all programs
and only create and populate maps for features that are enabled at runtime
based on global variables for feature flags.

This commit introduces this capability. It performs static analysis on the BPF
program, splits it into basic blocks and finds global data accesses used in
branching instructions. Then, it can predict whether a branch is always or
never taken.

By occluding unreachable parts of the program, it predicts which maps will
remain in use after the verifier performed its own dead code elimination as
part of the verification process. Before loading, unused maps are removed from
the CollectionSpec, and all of their references are poisoned with simple
immediate load instructions.

This means all maps can now be unconditionally defined, normal branches (if
statements!) can be used, and developers no longer have to worry about when
and which maps are or are not loaded under which conditions. Users don't
have to pay for features they don't use.

Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Timo Beckers <timo@isovalent.com>
Co-authored-by: Timo Beckers <timo@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
Signed-off-by: Dylan Reimerink <dylan.reimerink@isovalent.com>
@dylandreimerink
Copy link
Member

/test

@dylandreimerink
Copy link
Member

@cilium/sig-datapath still looking for a review on this 🥺

@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 Aug 13, 2025
@dylandreimerink dylandreimerink added this pull request to the merge queue Aug 13, 2025
Merged via the queue into cilium:main with commit 08d36f1 Aug 13, 2025
67 of 68 checks passed
@ti-mo ti-mo deleted the tb/dead-code-maps branch September 4, 2025 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

bpf: avoid creating maps for disabled features
4 participants