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: clean up tcx bpf_links created by newer Cilium versions #31553

Merged
merged 2 commits into from Mar 27, 2024

Conversation

ti-mo
Copy link
Contributor

@ti-mo ti-mo commented Mar 22, 2024

This PR represents the backportable part of #30103 and needs to be merged and backported independently in order for the downgrade path (and tests) to work.

Remove tcx links created by Cilium 1.16 onwards

@ti-mo ti-mo requested a review from a team as a code owner March 22, 2024 09:37
@ti-mo ti-mo requested a review from rgo3 March 22, 2024 09:37
@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 Mar 22, 2024
@ti-mo ti-mo added the release-note/misc This PR makes changes that have no direct user impact. label Mar 22, 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 Mar 22, 2024
@ti-mo ti-mo added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. backport/author The backport will be carried out by the author of the PR. 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 needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Mar 22, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Mar 22, 2024
@ti-mo
Copy link
Contributor Author

ti-mo commented Mar 22, 2024

/test

@rgo3
Copy link
Contributor

rgo3 commented Mar 22, 2024

Do we need a 1.13 backport? TCX will be released with 1.16, so only 1.15 and 1.14 need the downgrade path or did we change that policy?

@ti-mo ti-mo removed the needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch label Mar 22, 2024
@borkmann
Copy link
Member

Do we need a 1.13 backport? TCX will be released with 1.16, so only 1.15 and 1.14 need the downgrade path or did we change that policy?

No, 1.13 is not needed. Technically only 1.15 needs it.

The arguments to the replaceDatapath functions are already quite numerous
and make the function signature hard to read. In preparation for future
commits, this patch aggregates almost all arguments to the function into
one option parameter.

Signed-off-by: Robin Gögge <r.goegge@isovalent.com>
@ti-mo
Copy link
Contributor Author

ti-mo commented Mar 27, 2024

/test

A follow-up commit will introduce attaching TC programs using tcx. Those
attachments cannot be overridden using netlink. If an older version of
Cilium wants to replace an TC program on a managed interface, it'll need to
remove the tcx attachment first.

This commit teaches the agent to remove leftover tcx link objects from previous
installs, before reattaching it using netlink. Note that this transition is
never seamless, since some time passes between deleting the link and attaching
the new program using netlink. However, as explained in 7a8e3c8
("loader: clean up XDP bpf_links created by newer Cilium versions"), this
downgrade path should rarely happen.

Signed-off-by: Robin Gögge <r.goegge@isovalent.com>
Co-authored-by: Timo Beckers <timo@isovalent.com>
@ti-mo
Copy link
Contributor Author

ti-mo commented Mar 27, 2024

Had to make the <device>/links removal a bit less greedy since it was also removing existing xdp links that were created right before. attachTCProgram now only removes <device>/links/cil_<prog> before trying to attach it using netlink.

@ti-mo
Copy link
Contributor Author

ti-mo commented Mar 27, 2024

/test

@ti-mo ti-mo enabled auto-merge March 27, 2024 16:19
@ti-mo ti-mo added this pull request to the merge queue Mar 27, 2024
@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 Mar 27, 2024
Merged via the queue into cilium:main with commit f2d804b Mar 27, 2024
62 checks passed
@ti-mo ti-mo deleted the tb/remove-tcx-links branch March 27, 2024 17:11
@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 Mar 28, 2024
@rgo3 rgo3 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 Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/author The backport will be carried out by the author of the PR. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants