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.

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

bpf: fib: delay smac selection until fib_do_redirect() has picked the oif #26290

Merged
merged 1 commit into from Jun 16, 2023
Merged

bpf: fib: delay smac selection until fib_do_redirect() has picked the oif #26290

merged 1 commit into from Jun 16, 2023

Conversation

julianwiedmann
Copy link
Member

fib_do_redirect() potentially takes the oif from the fib_params. But we currently don't consider this when selecting the smac.

Fix this by delaying the smac selection until we actually need it.

Fixes: 5fff05d ("bpf,fib: introduce fib_do_redirect function")

@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. release-note/misc This PR makes changes that have no direct user impact. needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Jun 16, 2023
@julianwiedmann julianwiedmann requested a review from a team as a code owner June 16, 2023 06:02
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.5 Jun 16, 2023
borkmann
borkmann approved these changes Jun 16, 2023
View reviewed changes
Copy link
Member

@borkmann borkmann left a comment

Choose a reason for hiding this comment

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

nice catch!

@borkmann borkmann added the release-blocker/1.14 This issue will prevent the release of the next version of Cilium. label Jun 16, 2023
@julianwiedmann
bpf: fib: delay smac selection until fib_do_redirect() has picked the…
2118930 
… oif

fib_do_redirect() potentially takes the `oif` from the fib_params. But
we currently don't consider this when selecting the smac.

Fix this by delaying the smac selection until we actually need it.

Fixes: 5fff05d ("bpf,fib: introduce fib_do_redirect function")
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
Copy link
Member Author

/test

@borkmann borkmann added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 16, 2023
@julianwiedmann julianwiedmann merged commit 7eb2801 into cilium:main Jun 16, 2023
62 checks passed
@julianwiedmann julianwiedmann deleted the 1.14-bpf-fib-lookup branch June 16, 2023 10:45
@nbusseneau
Copy link
Member

@julianwiedmann Backporting the changes in this PR to v1.13 yields conflicts as they depend on changes made in #23884, which was not backported to v1.13. I'm not competent enough in BPF to fully understand what's going on, can you double-check if it's OK to backport #23884 to v1.13?

Note: #26136 from @ldelossa is hitting the exact same dependency issue.

This was referenced Jun 22, 2023
Merged
Merged
@julianwiedmann
Copy link
Member Author

@julianwiedmann Backporting the changes in this PR to v1.13 yields conflicts as they depend on changes made in #23884, which was not backported to v1.13. I'm not competent enough in BPF to fully understand what's going on, can you double-check if it's OK to backport #23884 to v1.13?

Note: #26136 from @ldelossa is hitting the exact same dependency issue.

This PR is merely a fix-up for #26136. So I'll leave the backport details to @ldelossa 🙏

@tklauser tklauser added the backport/author The backport will be carried out by the author of the PR. label Jun 26, 2023
@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
@nebril nebril added this to Needs backport from main in 1.13.7 Aug 10, 2023
@nebril nebril removed this from Needs backport from main in 1.13.6 Aug 10, 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 removed this from Needs backport from main in 1.13.9 Nov 12, 2023
@nathanjsweet nathanjsweet added this to Needs backport from main in 1.13.10 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 julianwiedmann mentioned this pull request Apr 2, 2024
Merged
@julianwiedmann julianwiedmann removed the needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch label 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
Reviewers

@borkmann borkmann borkmann approved these changes

@ldelossa ldelossa Awaiting requested review from ldelossa

Assignees
No one assigned
Labels
backport/author The backport will be carried out by the author of the PR. 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-blocker/1.14 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. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@julianwiedmann @nbusseneau @borkmann @tklauser