Skip to content

NodeMapV2: Introduce a V2 of the NodeMap which includes SPI values #31431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 3, 2024

Conversation

ldelossa
Copy link
Contributor

@ldelossa ldelossa commented Mar 16, 2024

This pull request adds a new V2 addition to the NodeMap.

The NodeMap is responsible for inventorying and restoration of allocated NodeIDs which are subsequently used to correctly match XFRM policies and states given the ultimate destination of IPSec encrypted traffic.

The v2 edition of NodeMap now associates each node entry with their SPI which is also used to correctly match XFRM policies and states given the ultimate destination of IPSec encrypted traffic.

However, by providing the SPI in NodeMap we can now query the SPI value associated with a Cilium managed node. Prior to this we could only query SPI on a per-pod basis.

This ability becomes especially useful when combined with the Encrypted Overlay feature, and provides us a way to retrieve SPI when only the destination Node address is obtainable, as opposed to the destination pod's address.

This PR includes an upgrade migration which will copy v1 map entries into the v2 map and fill in the current SPI.
This migration works on the assumption that the cluster has a stable SPI and no pending key rotation is occurring during the migration (Cilium upgrade in other words).

It is best to review this PR one commit at a time.

Transition to NodeMapV2 which now includes SPI in its map values.

@ldelossa ldelossa requested review from a team as code owners March 16, 2024 17:23
@ldelossa ldelossa requested review from thorn3r and gentoo-root March 16, 2024 17:23
@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 Mar 16, 2024
@ldelossa ldelossa force-pushed the ldelossa/node-map-spi-oss branch 2 times, most recently from 7da59a0 to 13ef906 Compare March 17, 2024 00:12
@pchaigno pchaigno added the release-note/misc This PR makes changes that have no direct user impact. label Mar 17, 2024
@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 Mar 17, 2024
@pchaigno pchaigno added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. feature/ipsec Relates to Cilium's IPsec feature labels Mar 17, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Mar 17, 2024
@ldelossa ldelossa force-pushed the ldelossa/node-map-spi-oss branch from 13ef906 to 761f226 Compare March 25, 2024 13:12
@ldelossa ldelossa force-pushed the ldelossa/node-map-spi-oss branch 8 times, most recently from f80d978 to 1493c34 Compare March 26, 2024 14:11
@pchaigno pchaigno added the release-blocker/1.16 This issue will prevent the release of the next version of Cilium. label Apr 3, 2024
@pchaigno
Copy link
Member

pchaigno commented Apr 3, 2024

@ldelossa I've labeled the PR according to our Slack discussion.

@ldelossa
Copy link
Contributor Author

ldelossa commented Apr 3, 2024

@pchaigno

After some further conversation with community team, we wont be back porting this into v1.15.

@ldelossa ldelossa force-pushed the ldelossa/node-map-spi-oss branch 2 times, most recently from 7e0a51d to c42eff2 Compare April 3, 2024 16:39
@ldelossa
Copy link
Contributor Author

ldelossa commented Apr 3, 2024

/test

ldelossa added 3 commits April 3, 2024 15:54
This commit updates all references of NodeMap to NodeMapV2.

The restoration of the NodeMapV2 is updated to take account of
nodemap.NodeValueV2.

The NodeMapV2 is now loaded with SPI values on (initial) NodeUpdates,
supplying the SPI to the datapath.

Tests are updated as well.

Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
This commit adds a migration when the v1 NodeMap is detected.

The migration first opens the ENCRYPT_MAP to obtain the local SPI.

This SPI will be the same for all nodes as long as a key rotation is not
occurring during an upgrade.
Which should never occur.

The values from the V1 map is then migrated to the V2 map and the SPI
retrieved from the ENCRYPT_MAP is applied to all migrated values.

The migrated map is then unpinned from bpffs after a successful
migration.

This migration is kicked off within the NodeMap's Cell onStart hook
which should ensure it runs before the map is utilized by other Cilium
components.

Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
This commits updates the cilium-dbg command to list the SPI value of
NodeMapV2's entries.

Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
@ldelossa ldelossa force-pushed the ldelossa/node-map-spi-oss branch from c42eff2 to 88316a6 Compare April 3, 2024 19:54
@ldelossa
Copy link
Contributor Author

ldelossa commented Apr 3, 2024

/test

@ldelossa ldelossa added this pull request to the merge queue Apr 3, 2024
Merged via the queue into cilium:main with commit dc3caa7 Apr 3, 2024
@ldelossa ldelossa deleted the ldelossa/node-map-spi-oss branch April 3, 2024 21:32
@julianwiedmann
Copy link
Member

@thorn3r

LGTM! In the case of a downgrade, a new nodemap v1 would be created with new entries, right?

A downgrade PR will be opened again v1.15 stable to address this situation after this is merged.

@ldelossa did that v1.15 downgrade toleration happen after all, or is it still todo?

@julianwiedmann
Copy link
Member

@thorn3r

LGTM! In the case of a downgrade, a new nodemap v1 would be created with new entries, right?

A downgrade PR will be opened again v1.15 stable to address this situation after this is merged.

@ldelossa did that v1.15 downgrade toleration happen after all, or is it still todo?

Ah, I think that's now fully handled by the "mirror write" in nodeMapV2's Update() routine, which ensures the v1 map is kept in-sync and can be used after a downgrade to v1.15. So right now a downgrade would just "leak" the unused v2 map?

julianwiedmann added a commit to julianwiedmann/cilium that referenced this pull request Mar 18, 2025
cilium#31431 introduced v2 of the nodemap,
and as of v1.16 the datapath only uses this v2 map. Hence it's safe to
remove the agent logic that was still populating the v1 map.

Fixes: cilium#34670
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
julianwiedmann added a commit to julianwiedmann/cilium that referenced this pull request Mar 18, 2025
cilium#31431 introduced v2 of the nodemap,
and as of v1.16 the datapath only uses this v2 map. Hence it's safe to
remove the agent logic that was still populating the v1 map.

Fixes: cilium#34670
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
julianwiedmann added a commit to julianwiedmann/cilium that referenced this pull request Mar 26, 2025
cilium#31431 introduced v2 of the nodemap,
and as of v1.16 the datapath only uses this v2 map. Hence it's safe to
remove the agent logic that was still populating the v1 map.

Fixes: cilium#34670
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
julianwiedmann added a commit to julianwiedmann/cilium that referenced this pull request May 28, 2025
cilium#31431 introduced v2 of the nodemap,
and as of v1.16 the datapath only uses this v2 map. Hence it's safe to
remove the agent logic that was still populating the v1 map.

Fixes: cilium#34670
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
julianwiedmann added a commit to julianwiedmann/cilium that referenced this pull request May 28, 2025
cilium#31431 introduced v2 of the nodemap,
and as of v1.16 the datapath only uses this v2 map. Hence it's safe to
remove the agent logic that was still populating the v1 map.

Fixes: cilium#34670
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
julianwiedmann added a commit to julianwiedmann/cilium that referenced this pull request May 28, 2025
cilium#31431 introduced v2 of the nodemap,
and as of v1.16 the datapath only uses this v2 map. Hence it's safe to
remove the agent logic that was still populating the v1 map.

Fixes: cilium#34670
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
julianwiedmann added a commit to julianwiedmann/cilium that referenced this pull request May 28, 2025
cilium#31431 introduced v2 of the nodemap,
and as of v1.16 the datapath only uses this v2 map. Hence it's safe to
remove the agent logic that was still populating the v1 map.

Fixes: cilium#34670
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
github-merge-queue bot pushed a commit that referenced this pull request May 28, 2025
#31431 introduced v2 of the nodemap,
and as of v1.16 the datapath only uses this v2 map. Hence it's safe to
remove the agent logic that was still populating the v1 map.

Fixes: #34670
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/ipsec Relates to Cilium's IPsec feature release-blocker/1.16 This issue will prevent the release of the next version of Cilium. release-note/misc This PR makes changes that have no direct user impact.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

9 participants