Skip to content

bpf: Ensure BPF host routing works with tunnel #35098

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 28, 2024
Merged

bpf: Ensure BPF host routing works with tunnel #35098

merged 1 commit into from
Oct 28, 2024

Conversation

jschwinger233
Copy link
Member

@jschwinger233 jschwinger233 commented Sep 30, 2024 •
edited
Loading

Fixes: #35023

Fix an issue where pod-to-world traffic goes up stack when BPF host routing is enabled with tunnel.

@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 Sep 30, 2024
@jschwinger233
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member

@jschwinger233 I'm thinking that https://github.com/cilium/cilium/actions/runs/11101570329/job/30849111216 is what Jarno wanted to fix in #33014. Now that we actually use BPF Host Routing, the to-ingress traffic no longer passes through the stack (where it magically gets routed), but just exits through the default external interface.

@julianwiedmann
Copy link
Member

Was looking at this once more - uff, we even do it right for IPv6. That's probably the first time that IPv6 is correct but IPv4 is buggy ...

@julianwiedmann
Copy link
Member

@jschwinger233 I'm thinking that https://github.com/cilium/cilium/actions/runs/11101570329/job/30849111216 is what Jarno wanted to fix in #33014. Now that we actually use BPF Host Routing, the to-ingress traffic no longer passes through the stack (where it magically gets routed), but just exits through the default external interface.

Probably worth re-testing on-top of #35143 :)

@julianwiedmann
Copy link
Member

/test

sayboras added a commit that referenced this pull request Oct 27, 2024
@sayboras
gha: Remove hostLegacyRouting in clustermesh
b914a36 
Relates: #35098
Fixes: #31653

Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras
Copy link
Member

Thanks a lot both, I can see that this change fixes the issue mentioned in #31653 tested in #35418

@sayboras sayboras added needs-backport/1.14 needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch release-note/bug This PR fixes an issue in a previous release of Cilium. labels Oct 27, 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 Oct 27, 2024
@sayboras sayboras mentioned this pull request Oct 27, 2024
Merged
sayboras
sayboras approved these changes Oct 27, 2024
View reviewed changes
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

😲

@julianwiedmann
Copy link
Member

I think we'll need a backport of #35143 first, so let's set this to author for now.

@julianwiedmann julianwiedmann added backport/author The backport will be carried out by the author of the PR. area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. labels Oct 27, 2024
@jschwinger233
bpf: Ensure BPF host routing works with tunnel
16ea3cc 
When BPF host routing is enabled with tunnel, encap_and_redirect_lxc()
returns DROP_NO_TUNNEL_ENDPOINT for pod-to-world traffic, which then
goes up stack instead of being fib_redirected. This patch ensures
to-world traffic follow the expected path.

Please note that we handled this correctly for IPv6, so only IPv4 is
being amended.

Fixes: cilium#35023

Signed-off-by: gray <gray.liang@isovalent.com>
@jschwinger233 jschwinger233 changed the title bpf: fib_redirect when DROP_NO_TUNNEL_ENDPOINT bpf: Ensure BPF host routing works with tunnel Oct 27, 2024
@jschwinger233
Copy link
Member Author

/test

@jschwinger233 jschwinger233 marked this pull request as ready for review October 27, 2024 14:15
@jschwinger233 jschwinger233 requested a review from a team as a code owner October 27, 2024 14:15
@jschwinger233 jschwinger233 requested a review from jibi October 27, 2024 14:15
julianwiedmann
julianwiedmann approved these changes Oct 28, 2024
View reviewed changes
Copy link
Member

@julianwiedmann julianwiedmann 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! That goto maze is getting a bit much, but let's clean it up afterwards.

@julianwiedmann julianwiedmann added this pull request to the merge queue Oct 28, 2024
Merged via the queue into cilium:main with commit 1adcc15 Oct 28, 2024
68 checks passed
sayboras added a commit that referenced this pull request Oct 28, 2024
@sayboras
gha: Remove hostLegacyRouting in clustermesh
3bfa5c6 
Relates: #35098
Fixes: #31653

Signed-off-by: Tam Mach <tam.mach@cilium.io>
github-merge-queue bot pushed a commit that referenced this pull request Oct 28, 2024
@sayboras
gha: Remove hostLegacyRouting in clustermesh
78e8f73 
Relates: #35098
Fixes: #31653

Signed-off-by: Tam Mach <tam.mach@cilium.io>
@julianwiedmann julianwiedmann mentioned this pull request Oct 29, 2024
Merged
@jschwinger233 jschwinger233 mentioned this pull request Oct 30, 2024
Open
@julianwiedmann
Copy link
Member

Thank you! That goto maze is getting a bit much, but let's clean it up afterwards.

#35691

@julianwiedmann julianwiedmann added affects/v1.14 This issue affects v1.14 branch affects/v1.15 This issue affects v1.15 branch backport/author The backport will be carried out by the author of the PR. and removed needs-backport/1.14 backport/author The backport will be carried out by the author of the PR. labels Nov 5, 2024
@julianwiedmann
Copy link
Member

Switching back to author backport, as a draft attempt in #35861 reliably fails in the Clustermesh CI workflow.

@julianwiedmann julianwiedmann removed the backport/author The backport will be carried out by the author of the PR. label Nov 25, 2024
@julianwiedmann
Copy link
Member

Switching back to author backport, as a draft attempt in #35861 reliably fails in the Clustermesh CI workflow.

False alarm, should be good now.

@julianwiedmann julianwiedmann mentioned this pull request Nov 29, 2024
Merged
1 task
@julianwiedmann julianwiedmann added backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. and removed needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Nov 29, 2024
@github-actions github-actions bot added backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. and removed backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. labels Nov 29, 2024
sayboras added a commit that referenced this pull request Dec 4, 2024
@sayboras
gha: Remove hostLegacyRouting in clustermesh
d9bc2e8 
[ upstream commit 78e8f73 ]

Relates: #35098
Fixes: #31653

Signed-off-by: Tam Mach <tam.mach@cilium.io>
github-merge-queue bot pushed a commit that referenced this pull request Dec 9, 2024
@sayboras
gha: Remove hostLegacyRouting in clustermesh
962f2e5 
[ upstream commit 78e8f73 ]

Relates: #35098
Fixes: #31653

Signed-off-by: Tam Mach <tam.mach@cilium.io>
@cilium-release-bot cilium-release-bot bot mentioned this pull request Dec 12, 2024
Merged
@julianwiedmann julianwiedmann mentioned this pull request Dec 20, 2024
Closed
3 tasks
PhilipSchmid added a commit to isovalent/terraform-aws-talos that referenced this pull request Jan 6, 2025
@PhilipSchmid
conformance: Added bpf-hostlegacyrouting option
ff76a34 
Added a configurable bpf-hostlegacyrouting option to the conformance
tests as Cilium 1.16.5+ introduced a breaking change for Talos
'forwardKubeDNSToHost'. See cilium/cilium#35098
for more details.

Signed-off-by: Philip Schmid <phisch@cisco.com>
@PhilipSchmid PhilipSchmid mentioned this pull request Jan 6, 2025
Merged
PhilipSchmid added a commit to isovalent/terraform-aws-talos that referenced this pull request Jan 6, 2025
@PhilipSchmid
conformance: Added bpf-hostlegacyrouting option
2faca6e 
Added a configurable bpf-hostlegacyrouting option to the conformance
tests as Cilium 1.16.5+ introduced a breaking change for Talos
'forwardKubeDNSToHost'. See cilium/cilium#35098
for more details.

Signed-off-by: Philip Schmid <phisch@cisco.com>
@jschwinger233 jschwinger233 mentioned this pull request Jan 7, 2025
Merged
PhilipSchmid added a commit to isovalent/terraform-aws-talos that referenced this pull request Jan 7, 2025
@PhilipSchmid
conformance: Added bpf-hostlegacyrouting option
c26c947 
Added a configurable bpf-hostlegacyrouting option to the conformance
tests as Cilium 1.16.5+ introduced a breaking change for Talos
'forwardKubeDNSToHost'. See cilium/cilium#35098
for more details.

Signed-off-by: Philip Schmid <phisch@cisco.com>
@rsafonseca rsafonseca mentioned this pull request Mar 18, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@borkmann borkmann borkmann approved these changes

@sayboras sayboras sayboras approved these changes

@julianwiedmann julianwiedmann julianwiedmann approved these changes

@jibi jibi Awaiting requested review from jibi jibi is a code owner automatically assigned from cilium/sig-datapath

Assignees
No one assigned
Labels
affects/v1.14 This issue affects v1.14 branch affects/v1.15 This issue affects v1.15 branch area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

IPv4 Pod to world traffic going up to stack when BPF host routing is used with tunnel
4 participants
@jschwinger233 @julianwiedmann @sayboras @borkmann