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

bpf: nodeport: add RevDNAT-based FIB lookup for reply traffic #26638

Merged

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Jul 5, 2023

When moving the RevDNAT for backend replies from bpf_lxc into bpf_host, we lost the routing based on the post-RevDNAT layout of the packet. Depending on the installed routing rules, we then end up sending the (RevDNATed) packet from the wrong egress interface.

So as part of the RevDNAT processing in to-netdev, do a FIB lookup that considers the desired source IP. And redirect the packet if it's currently on the wrong interface. We intentionally delay the actual RevDNAT rewrite until the packet has reached the correct interface, so that the packet looks as expected to components in to-netdev (in particular HostFW).

Long-term we would want

  • a FIB_DONE mark on the packet (similar to SNAT_DONE), to skip additional lookups.
  • improve the FIB lookup in from-container (for BPF Host Routing) to already consider service RevDNAT.

The initial idea was to have this check at the very top of the to-netdev program (see #26635). But we're hitting program size limits in 4.19 there, and improving that is too much of a refactor for a fix PR.

Fixes: #26166

@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 Jul 5, 2023
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Jul 5, 2023
@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 Jul 5, 2023
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann changed the title 1.14 nodeport revdnat fib v3 bpf: nodeport: add RevDNAT-based FIB lookup for reply traffic Jul 5, 2023
@julianwiedmann julianwiedmann added kind/bug This is a bug in the Cilium logic. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/loadbalancing Impacts load-balancing and Kubernetes service implementations needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jul 5, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.5 Jul 5, 2023
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test

@gentoo-root gentoo-root added this to Needs backport from main in 1.13.6 Jul 26, 2023
@gentoo-root gentoo-root removed this from Needs backport from main in 1.13.5 Jul 26, 2023
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann marked this pull request as ready for review July 26, 2023 14:17
@julianwiedmann julianwiedmann requested a review from a team as a code owner July 26, 2023 14:17
@michi-covalent michi-covalent added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Sep 9, 2023
@michi-covalent michi-covalent moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.2 Sep 9, 2023
@michi-covalent michi-covalent added this to Needs backport from main in 1.13.8 Sep 9, 2023
@michi-covalent michi-covalent removed this from Needs backport from main in 1.13.7 Sep 9, 2023
@jrajahalme jrajahalme added this to Needs backport from main in 1.13.9 Oct 17, 2023
@jrajahalme jrajahalme removed this from Needs backport from main in 1.13.8 Oct 17, 2023
@nathanjsweet nathanjsweet added this to Needs backport from main in 1.13.10 Nov 12, 2023
@nathanjsweet nathanjsweet removed this from Needs backport from main in 1.13.9 Nov 12, 2023
@nebril nebril added this to Needs backport from main in 1.13.11 Dec 11, 2023
@nebril nebril removed this from Needs backport from main in 1.13.10 Dec 11, 2023
@gentoo-root gentoo-root added this to Needs backport from main in 1.13.12 Jan 17, 2024
@gentoo-root gentoo-root removed this from Needs backport from main in 1.13.11 Jan 17, 2024
@michi-covalent michi-covalent added this to Needs backport from main in 1.13.13 Feb 13, 2024
@michi-covalent michi-covalent removed this from Needs backport from main in 1.13.12 Feb 13, 2024
@thorn3r thorn3r removed this from Needs backport from main in 1.13.13 Mar 13, 2024
@thorn3r thorn3r added this to Needs backport from main in 1.13.13 Mar 13, 2024
@thorn3r thorn3r added this to Needs backport from main in 1.13.14 Mar 13, 2024
@thorn3r thorn3r removed this from Needs backport from main in 1.13.13 Mar 13, 2024
@thorn3r thorn3r added this to Needs backport from main in 1.13.15 Mar 26, 2024
@thorn3r thorn3r removed this from Needs backport from main in 1.13.14 Mar 26, 2024
@julianwiedmann
Copy link
Member Author

No intentions to backport this into v1.13 anymore. Doing so would require #26136 and #26290.

@julianwiedmann julianwiedmann added affects/v1.13 This issue affects v1.13 branch and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Apr 2, 2024
@asauber asauber added this to Needs backport from main in 1.13.16 Apr 11, 2024
@asauber asauber removed this from Needs backport from main in 1.13.15 Apr 11, 2024
@nebril nebril added this to Needs backport from main in 1.13.17 May 10, 2024
@nebril nebril removed this from Needs backport from main in 1.13.16 May 10, 2024
@nebril nebril removed this from Needs backport from main in 1.13.17 May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/v1.13 This issue affects v1.13 branch area/loadbalancing Impacts load-balancing and Kubernetes service implementations backport/author The backport will be carried out by the author of the PR. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. kind/bug This is a bug in the Cilium logic. 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. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
1.14.2
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

DSR broken with VLANs on 1.13.3
5 participants