Skip to content
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

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

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
@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 8 times, most recently from f80d978 to 1493c34 Compare March 26, 2024 14:11
This commit introduces a V2 of the NodeMap.

This V2 map grows a "SPI" field of size u8.

This will allow each node within the node map to also be associated with
an IPSec SPI.

For at least two release cycles we will maintain the V1 map and mirror
writes from V2 into V1 map.

This allows a user to 'downgrade' mid-patch release without needing a
migration.

Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
This commits introduces a V2 of the NODE_MAP eBPF map which now
associates an IPSec SPI with each node.

The SPI associated with the IPSec of a remote node can now be retrieved
by querying the NODE_MAP_V2 eBPF map.

All eBPF unit tests are updated accordingly.

Signed-off-by: Louis DeLosSantos <louis.delos@isovalent.com>
@ldelossa ldelossa marked this pull request as ready for review March 28, 2024 18:17
@ldelossa
Copy link
Contributor Author

@pchaigno

Latest push now performs "mirroring" of MapV2 Update and Delete.

Tests are updated to ensure we

  1. Don't remove the V1 Map after initial migration
  2. Mirror writes from MapV2 into MapV1

I think the initial reviews from others can still stand, these new updates are solely focusing on new eBPF map manipulation.

@ldelossa ldelossa requested a review from pchaigno March 28, 2024 18:38
@harsimran-pabla
Copy link
Contributor

Hi @ldelossa 👋 Is this binary file pushed by mistake ?
pkg/bpf/testdata/unreachable-tailcall.o

@ldelossa
Copy link
Contributor Author

@harsimran-pabla Nope, see:
#31431 (comment)

@harsimran-pabla
Copy link
Contributor

@harsimran-pabla Nope, see: #31431 (comment)

I should have read the comments :)

@pchaigno pchaigno added needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch release-blocker/1.16 This issue will prevent the release of the next version of Cilium. labels Apr 3, 2024
pkg/maps/nodemap/node_map_v2.go Outdated Show resolved Hide resolved
@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 removed the needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch label Apr 3, 2024
@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

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
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
62 checks passed
@ldelossa ldelossa deleted the ldelossa/node-map-spi-oss branch April 3, 2024 21:32
Comment on lines +195 to +199
struct node_value {
__u16 id;
__u8 spi;
__u8 pad;
};
Copy link
Member

Choose a reason for hiding this comment

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

Could you please also add this to the alignchecker? 🙏

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.

None yet

9 participants