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

loader: insert policy programs before attaching bpf entrypoints, enforce missed tail calls in ci-e2e #30248

Merged
merged 2 commits into from
Jan 17, 2024

Conversation

ti-mo
Copy link
Contributor

@ti-mo ti-mo commented Jan 15, 2024

Since the early days of Cilium, we would miss tail calls in CI and not really monitor when or how these happened. Many improvements have been made in recent months to make these a thing of the past, but they were still occurring with host-fw enabled.

After the most recent round of fixes, most of these errors disappeared, but one bugbear remained. This PR addresses the final outstanding error in ordering prog array insertions:

loader: install an ELF's policy programs before attaching tc/xdp hooks

See code comments for a detailed description of the problem. This commit
installs policy programs before attaching tc/xdp hooks since doing things
in the wrong order means dropping tail calls when handling traffic if the
policy programs aren't inserted.

Finally, strictly enforce no missed tail calls in all ci-e2e scenarios.

Fixes: #29476

Fix all packet drops due to missed tail calls, enable zero tolerance for these errors in CI

@ti-mo ti-mo added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/ci This PR makes changes to the CI. labels Jan 15, 2024
@ti-mo
Copy link
Contributor Author

ti-mo commented Jan 15, 2024

/test

@ti-mo ti-mo force-pushed the tb/enforce-missed-tail-calls branch 2 times, most recently from 2acc01e to e88fc22 Compare January 15, 2024 11:53
@ti-mo
Copy link
Contributor Author

ti-mo commented Jan 15, 2024

/test

See code comments for a detailed description of the problem. This commit
installs policy programs before attaching tc/xdp hooks since doing things
in the wrong order means dropping tail calls when handling traffic if the
policy programs aren't inserted.

Signed-off-by: Timo Beckers <timo@isovalent.com>
Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo force-pushed the tb/enforce-missed-tail-calls branch from e88fc22 to 251be03 Compare January 16, 2024 15:22
@ti-mo
Copy link
Contributor Author

ti-mo commented Jan 16, 2024

/test

@ti-mo ti-mo marked this pull request as ready for review January 17, 2024 13:31
@ti-mo ti-mo requested review from a team as code owners January 17, 2024 13:31
@ti-mo ti-mo requested review from rgo3 and brlbil January 17, 2024 13:31
@ti-mo ti-mo added needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch backport/author The backport will be carried out by the author of the PR. labels Jan 17, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in v1.15.0-rc.1 Jan 17, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.6 Jan 17, 2024
@ti-mo ti-mo added the needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch label Jan 17, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.13.11 Jan 17, 2024
@ti-mo
Copy link
Contributor Author

ti-mo commented Jan 17, 2024

This did 8 consecutive successful rounds of ci-e2e, so I'm calling it. No more missing tail calls (for now)! Marking this as ready for review.

(Aside from hitting #30266 on a few occasions, of course.)

@ti-mo ti-mo changed the title conformance-e2e: enforce no missed tail calls occurring during tests loader: insert policy programs before attaching bpf entrypoints, enforce missed tail calls in ci-e2e Jan 17, 2024
@ti-mo ti-mo added release-note/bug This PR fixes an issue in a previous release of Cilium. sig/loader Impacts the loading of BPF programs into the kernel. and removed release-note/ci This PR makes changes to the CI. labels Jan 17, 2024
@ti-mo ti-mo added this pull request to the merge queue Jan 17, 2024
Merged via the queue into main with commit f28817b Jan 17, 2024
208 checks passed
@ti-mo ti-mo deleted the tb/enforce-missed-tail-calls branch January 17, 2024 14:56
@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
@gentoo-root gentoo-root added this to Needs backport from main in 1.14.7 Jan 17, 2024
@gentoo-root gentoo-root removed this from Needs backport from main in 1.14.6 Jan 17, 2024
@ti-mo ti-mo added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Jan 18, 2024
@ti-mo ti-mo added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jan 18, 2024
@ti-mo ti-mo added backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Jan 18, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.15 in v1.15.0-rc.1 Jan 18, 2024
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.13 The backport for Cilium 1.13.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. backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Jan 23, 2024
@giorio94 giorio94 added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. labels Jan 29, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.15 to Backport done to v1.15 in v1.15.0-rc.1 Jan 29, 2024
@aanm aanm moved this from Needs backport from main to Backport done to v1.14 in 1.14.7 Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow backport/author The backport will be carried out by the author of the PR. backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. 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. sig/loader Impacts the loading of BPF programs into the kernel.
Projects
No open projects
1.13.12
Needs backport from main
1.14.7
Backport done to v1.14
v1.15.0-rc.1
Backport done to v1.15
Development

Successfully merging this pull request may close these issues.

CI: Cilium E2E Upgrade (ci-e2e-upgrade) fails in no-missed-tail-calls
4 participants