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: only detach Cilium-owned XDP programs when XDP is disabled #31654

Merged
merged 1 commit into from Mar 29, 2024

Conversation

ti-mo
Copy link
Contributor

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

Currently, even when Cilium's XDP features are disabled, the Cilium agent will still attempt to detach a program attached to the legacy netlink XDP hook on managed interfaces.

This is so the agent does the right thing when a user first enables and then disables an XDP feature, where the user would expect Cilium's XDP programs to be removed. However, this is at odds with users wanting to run their own XDP programs on Cilium-managed interfaces. Even with XDP disabled, the agent will unconditionally remove any XDP programs.

This patch narrows down this behaviour by checking the name of the program attached to the legacy XDP hook before detaching it. If the kernel-provided name is not a prefix of the name expected by the agent, the program is left on the interface. Note that with XDP enabled, legacy XDP programs will always be replaced with Cilium programs.

Supersedes #31507.

Only detach Cilium-owned legacy XDP programs when XDP is disabled

Currently, even when Cilium's XDP features are disabled, the Cilium agent will
still attempt to detach a program attached to the legacy netlink XDP hook on
managed interfaces.

This is so the agent does the right thing when a user first enables and then
disables an XDP feature, where the user would expect Cilium's XDP programs to
be removed. However, this is at odds with users wanting to run their own XDP
programs on Cilium-managed interfaces. Even with XDP disabled, the agent will
unconditionally remove any XDP programs.

This patch narrows down this behaviour by checking the name of the program
attached to the legacy XDP hook before detaching it. If the kernel-provided
name is not a prefix of the name expected by the agent, the program is left
on the interface. Note that with XDP enabled, legacy XDP programs will always
be replaced with Cilium programs.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@ti-mo ti-mo requested a review from a team as a code owner March 28, 2024 12:45
@ti-mo ti-mo requested a review from lmb March 28, 2024 12:45
@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 28, 2024
@ti-mo ti-mo requested a review from rgo3 March 28, 2024 12:45
@ti-mo ti-mo added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Mar 28, 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 28, 2024
@ti-mo ti-mo added the sig/loader Impacts the loading of BPF programs into the kernel. label Mar 28, 2024
@ti-mo
Copy link
Contributor Author

ti-mo commented Mar 28, 2024

@zaheerm @dkulchinsky Please give this patch a try!

@ti-mo
Copy link
Contributor Author

ti-mo commented Mar 28, 2024

/test

@dkulchinsky
Copy link

Thank you @ti-mo! will test this today and report back.

Copy link
Contributor

@rgo3 rgo3 left a comment

Choose a reason for hiding this comment

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

Thanks! 🙏

@zaheerm
Copy link

zaheerm commented Mar 29, 2024

Thanks @ti-mo . This makes sense!

@ti-mo ti-mo added this pull request to the merge queue Mar 29, 2024
Merged via the queue into cilium:main with commit 76a659c Mar 29, 2024
63 checks passed
@ti-mo ti-mo deleted the tb/surgical-xdp-removal branch March 29, 2024 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/loader Impacts the loading of BPF programs into the kernel.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants