feat: Support IPv6 for delegated IPAM by kadevu · Pull Request #38249 · cilium/cilium

kadevu · 2025-03-18T02:47:36Z

For the scenario where there are multiple interfaces on a Node and delegated IPAM is used to determine the IP and the uplink interface of a Pod. The "interfaces" field can be used by IPAM plugin to pass uplink interface info to Cilium. And then Cilium can setup host routes for the Pod according to the uplink interface info.

The "interfaces" field is allowed to specify the related host interface according to the CNI spec and is already widely used by existing plugins, such as bridge and ptp plugin. It's actually not used by any IPAM plugin for now, but it's reasonable and conforms to the original definition of the field.

Motivations/Benefits of the PR for the native routing mode:

More cloud(public or private) envs with multiple uplink can be easily ingregrated with Cilium without involving any cloud specific logic. A common way is provided for the case: "uplink interface" support.
Cilium keeps full control of the Pod traffic. Cilium choose the implement of the "routing" for egress traffic, through Linux routes or eBPF forwarding rules or any other methods.

The IPv4 support for this was done as part of this PR #34779

This PR is for IPv6 support

These changes were initially authored by @caorui-io as part of this PR #36656 but had been inactive for a couple of months.

Support IPv6 for delegated IPAM

maintainer-s-little-helper · 2025-03-18T02:47:39Z

Commit 53ec516 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 · 2025-03-18T03:23:53Z

Commits 53ec516, f4effcd do 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

gandro

Thanks for the PR! I think the change high-level makes sense to me, I'll review it a bit more in detail later. A few things that stood out to me from a high-level review perspective:

Please use commit descriptions to provide some context what this change does and why it is needed (i.e. what problems it solves). While I have a guess what this is used for (a delegated IPAM plugin wants to install ENI-style per-device IP rules for IPv6), this should be part of the commit description.
Squash all your commits into one and make sure sign off the commits. See https://docs.cilium.io/en/stable/contributing/development/dev_setup/#making-changes
Let's not introduce new dependencies where not strictly necessary

Thanks!

plugins/cilium-cni/cmd/interface.go

kadevu · 2025-03-18T17:55:44Z

Thanks for the PR! I think the change high-level makes sense to me, I'll review it a bit more in detail later. A few things that stood out to me from a high-level review perspective:

Please use commit descriptions to provide some context what this change does and why it is needed (i.e. what problems it solves). While I have a guess what this is used for (a delegated IPAM plugin wants to install ENI-style per-device IP rules for IPv6), this should be part of the commit description.

Squash all your commits into one and make sure sign off the commits. See https://docs.cilium.io/en/stable/contributing/development/dev_setup/#making-changes

Let's not introduce new dependencies where not strictly necessary

Thanks!

Thanks @gandro for your comments. I will update the PRs with the info that you have asked for.

kadevu · 2025-03-19T03:44:24Z

Thanks for the PR! I think the change high-level makes sense to me, I'll review it a bit more in detail later. A few things that stood out to me from a high-level review perspective:

Please use commit descriptions to provide some context what this change does and why it is needed (i.e. what problems it solves). While I have a guess what this is used for (a delegated IPAM plugin wants to install ENI-style per-device IP rules for IPv6), this should be part of the commit description.

Squash all your commits into one and make sure sign off the commits. See https://docs.cilium.io/en/stable/contributing/development/dev_setup/#making-changes

Let's not introduce new dependencies where not strictly necessary

Thanks!

Thanks @gandro for your comments. I will update the PRs with the info that you have asked for.

I will wait for your review and make the fixes in a commit

pkg/ipam/types.go

gandro

Thanks for the changes! High-level, they look good to me now. A few minor issues with the implementation that need to be addressed

pkg/datapath/linux/routing/info.go

pkg/datapath/linux/routing/routing.go

plugins/cilium-cni/cmd/cmd.go

plugins/cilium-cni/cmd/interface.go

gandro

Awesome work! One last thing that I noticed, then I think this is good to go.

pkg/endpoint/endpoint.go

gandro

Thanks!

gandro · 2025-03-25T15:28:30Z

/test

kadevu · 2025-03-25T20:00:42Z

/test

gandro · 2025-03-26T08:12:53Z

Please note that pushing to the branch invalidates CI results, therefore I recommend only updating the branch if necessary. I'll retrigger CI (and will take a look at failing tests, we currently have some flakes which are likely unrelated to the PR) Only organization members can trigger CI.

gandro · 2025-03-26T08:12:58Z

/test

kadevu · 2025-03-26T13:19:19Z

Please note that pushing to the branch invalidates CI results, therefore I recommend only updating the branch if necessary. I'll retrigger CI (and will take a look at failing tests, we currently have some flakes which are likely unrelated to the PR) Only organization members can trigger CI.

Sure @gandro. There were a couple of tests failing in pkg/datapath/linux/routing/routing_test.go so I fixed them and pushed the changes.

pkg/datapath/linux/node.go

plugins/cilium-cni/cmd/cmd.go

daemon/cmd/ipam.go

gandro · 2025-03-31T15:37:01Z

/test

joamaki · 2025-04-01T13:42:44Z

/test

gandro · 2025-04-02T08:29:50Z

I think the multi-pool CI check is failing because the branch is outdated: https://github.com/cilium/cilium/actions/runs/14197243933/job/39775304335

I'll rebase the branch and re-run CI

Signed-off-by: Rui Cao <[email protected]> Removed reference to v6 in RoutingInfo and AllocationResult Signed-off-by: Kaushik Vuligonda <[email protected]>

gandro · 2025-04-02T08:30:19Z

/test

pippolo84

Thanks!

gandro · 2025-04-02T10:41:06Z

/test

kadevu requested review from a team as code owners March 18, 2025 02:47

kadevu requested review from joamaki and marseel March 18, 2025 02:47

maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 18, 2025

kadevu requested a review from pippolo84 March 18, 2025 02:47

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 18, 2025

kadevu requested a review from gandro March 18, 2025 02:47

github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Mar 18, 2025

gandro reviewed Mar 18, 2025

View reviewed changes

plugins/cilium-cni/cmd/interface.go Outdated Show resolved Hide resolved

kadevu changed the title ~~Ipam uplink v6~~ Support IPv6 for delegated IPAM Mar 18, 2025

gandro requested changes Mar 19, 2025

View reviewed changes

pkg/ipam/types.go Outdated Show resolved Hide resolved

kadevu force-pushed the ipam-uplink-v6 branch from f4effcd to 9f2d951 Compare March 20, 2025 19:07

maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Mar 20, 2025

kadevu force-pushed the ipam-uplink-v6 branch 2 times, most recently from be17226 to 49036b6 Compare March 20, 2025 20:58

kadevu changed the title ~~Support IPv6 for delegated IPAM~~ feat: Support IPv6 for delegated IPAM Mar 21, 2025

gandro reviewed Mar 24, 2025

View reviewed changes

kadevu force-pushed the ipam-uplink-v6 branch from 49036b6 to e7e3f21 Compare March 24, 2025 15:16

kadevu requested review from a team as code owners March 24, 2025 15:16

gandro removed request for a team March 25, 2025 08:35

gandro reviewed Mar 25, 2025

View reviewed changes

pkg/endpoint/endpoint.go Outdated Show resolved Hide resolved

gandro approved these changes Mar 25, 2025

View reviewed changes

joamaki requested changes Mar 26, 2025

View reviewed changes

pkg/datapath/linux/node.go Outdated Show resolved Hide resolved

kadevu commented Mar 26, 2025

View reviewed changes

plugins/cilium-cni/cmd/cmd.go Show resolved Hide resolved

gandro approved these changes Mar 31, 2025

View reviewed changes

daemon/cmd/ipam.go Outdated Show resolved Hide resolved

joamaki approved these changes Apr 1, 2025

View reviewed changes

marseel approved these changes Apr 1, 2025

View reviewed changes

Support IPv6 for delegated IPAM

522ce61

Signed-off-by: Rui Cao <[email protected]> Removed reference to v6 in RoutingInfo and AllocationResult Signed-off-by: Kaushik Vuligonda <[email protected]>

pippolo84 approved these changes Apr 2, 2025

View reviewed changes

NihaNallappagari mentioned this pull request Sep 9, 2025

Fix: Update ipv6 default route with interface index #41598

Merged

Conversation

kadevu commented Mar 18, 2025 • edited by joestringer Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maintainer-s-little-helper bot commented Mar 18, 2025

Uh oh!

maintainer-s-little-helper bot commented Mar 18, 2025

Uh oh!

gandro left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kadevu commented Mar 18, 2025

Uh oh!

kadevu commented Mar 19, 2025

Uh oh!

Uh oh!

gandro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gandro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gandro left a comment

Choose a reason for hiding this comment

Uh oh!

gandro commented Mar 25, 2025

Uh oh!

kadevu commented Mar 25, 2025

Uh oh!

gandro commented Mar 26, 2025

Uh oh!

gandro commented Mar 26, 2025

Uh oh!

kadevu commented Mar 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gandro commented Mar 31, 2025

Uh oh!

joamaki commented Apr 1, 2025

Uh oh!

gandro commented Apr 2, 2025

Uh oh!

gandro commented Apr 2, 2025

Uh oh!

pippolo84 left a comment

Choose a reason for hiding this comment

Uh oh!

gandro commented Apr 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

kadevu commented Mar 18, 2025 •

edited by joestringer

Loading

gandro left a comment •

edited

Loading