Skip to content

datapath: Avoid using link-local IPv6 address for direct routing.#37839

Merged
ti-mo merged 1 commit intocilium:mainfrom
sypakine:link-local
Feb 28, 2025
Merged

datapath: Avoid using link-local IPv6 address for direct routing.#37839
ti-mo merged 1 commit intocilium:mainfrom
sypakine:link-local

Conversation

@sypakine
Copy link
Copy Markdown
Contributor

@sypakine sypakine commented Feb 25, 2025

Fixes: #36752

This fixes an issue where link-local IPv6 address ends up being used for direct routing. Using link-local address results in packets being dropped during FIB lookup with Nodeport services.

When no global IPv6 address is encountered, fallback to a link-local address.

datapath: Prefer IPv6 global address to link-local for direct routing.

@sypakine sypakine requested a review from a team as a code owner February 25, 2025 02:42
@sypakine sypakine requested a review from rgo3 February 25, 2025 02:42
@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 Feb 25, 2025
@sypakine
Copy link
Copy Markdown
Contributor Author

sypakine commented Feb 25, 2025

/test

@sypakine
Copy link
Copy Markdown
Contributor Author

sypakine commented Feb 25, 2025

Hey @ti-mo, was this what you had in mind for testing? or were you thinking an e2e test?

@sypakine
Copy link
Copy Markdown
Contributor Author

sypakine commented Feb 25, 2025

/test

@ti-mo
Copy link
Copy Markdown
Contributor

ti-mo commented Feb 26, 2025

@sypakine Thanks and sorry, maybe I should've been more clear: this will create even more work for whomever lifts this logic out of WriteNodeConfig, since that will break the test you just added.

I would've probably written a small helper encapsulating the for _, addr := range drd.Addrs { loop with the loopback skip applied and written some tests for that. That way, the next person simply needs to move the function call, reducing it to a mechanical refactor.

@sypakine
datapath: Avoid using link-local IPv6 address for direct routing.
6dcbad0 
This fixes an issue where link-local IPv6 address ends up being used for direct routing. Using link-local address results in packets being dropped during FIB lookup with Nodeport services.

When no global IPv6 address is encountered, fallback to a link-local address.

Fixes: cilium#36752

Signed-off-by: Mark St John <markstjohn@google.com>
@sypakine
Copy link
Copy Markdown
Contributor Author

sypakine commented Feb 26, 2025

Thanks for the clarification -- done.

ti-mo
ti-mo approved these changes Feb 27, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@ti-mo ti-mo left a comment

Choose a reason for hiding this comment

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

Thank you!

@ti-mo
Copy link
Copy Markdown
Contributor

ti-mo commented Feb 27, 2025

/test

@ti-mo ti-mo mentioned this pull request Feb 27, 2025
Closed
@ti-mo ti-mo added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Feb 27, 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 Feb 27, 2025
@ti-mo ti-mo enabled auto-merge February 27, 2025 15:58
rgo3
rgo3 approved these changes Feb 28, 2025
View reviewed changes
Copy link
Copy Markdown
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.

Thanks!

@ti-mo ti-mo added this pull request to the merge queue Feb 28, 2025
@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 Feb 28, 2025
Merged via the queue into cilium:main with commit 61ece9b Feb 28, 2025
63 checks passed
@julianwiedmann
Copy link
Copy Markdown
Member

julianwiedmann commented May 13, 2025

@sypakine 👋 given this fix is labeled as release-note/bug, did you consider whether it also needs a backport to v1.17? Or is the severity low enough that it can be skipped?

@sypakine
Copy link
Copy Markdown
Contributor Author

sypakine commented May 27, 2025

@sypakine 👋 given this fix is labeled as release-note/bug, did you consider whether it also needs a backport to v1.17? Or is the severity low enough that it can be skipped?

The severity and probability appear low enough that a backport to 1.16 is not necessary.

@julianwiedmann julianwiedmann added the severity/low Impact is limited, only affecting unusual configs, or all configs with low impact in prod. label May 27, 2025
@Bigdelle Bigdelle mentioned this pull request Mar 24, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ti-mo ti-mo ti-mo approved these changes

@rgo3 rgo3 rgo3 approved these changes

Assignees

No one assigned

Labels

ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium. severity/low Impact is limited, only affecting unusual configs, or all configs with low impact in prod.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Connection to Nodeport service failing on a dual-stack GKE cluster with Ubuntu 24.04 (picks link-local address for IPV6_DIRECT_ROUTING)

4 participants

@sypakine @ti-mo @julianwiedmann @rgo3