Skip to content

[v1.17] bpf: host: don't detect WG traffic in from-netdev@cilium_wg0 #38233

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
Merged
merged 1 commit into from
May 21, 2025
Merged

[v1.17] bpf: host: don't detect WG traffic in from-netdev@cilium_wg0 #38233

merged 1 commit into from
May 21, 2025

Conversation

julianwiedmann
Copy link
Member

When from-netdev is attached to cilium_wg0 and processes the payload of a WireGuard packet, there is no need to check whether this payload is a Cilium-originating WireGuard packet. We don't do double-encryption.

Hence limit the WG-detection block to IS_BPF_HOST.

@julianwiedmann julianwiedmann added area/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. kind/complexity-issue Relates to BPF complexity or program size issues feature/wireguard Relates to Cilium's Wireguard feature needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch labels Mar 17, 2025
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

(IS_BPF_HOST makes no sense, this would also need the THIS_INTERFACE_IFINDEX == WG_IFINDEX check)

smagnani96
smagnani96 requested changes Mar 17, 2025
View reviewed changes
Copy link
Contributor

@smagnani96 smagnani96 left a comment

Choose a reason for hiding this comment

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

As also you were mentioning, we'd need to check for THIS_INTERFACE_IFINDEX in the code. Unfortunately, this will always be compiled as IS_BPF_HOST in v1.17 and before.

@github-actions GitHub Actions
Copy link

This pull request has been automatically marked as stale because it
has not had recent activity. It will be closed if no further activity
occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. label Apr 26, 2025
@julianwiedmann julianwiedmann changed the base branch from main to v1.17 May 6, 2025 10:30
@julianwiedmann
Copy link
Member Author

(re-targeted this to v1.17, as main no longer uses from-netdev on cilium_wg)

@julianwiedmann julianwiedmann changed the title bpf: host: don't detect WG traffic in from-netdev@cilium_wg0 [v1.17] bpf: host: don't detect WG traffic in from-netdev@cilium_wg0 May 6, 2025
@julianwiedmann julianwiedmann removed needs-backport/1.17 This PR / issue needs backporting to the v1.17 branch stale The stale bot thinks this issue is old. Add "pinned" label to prevent this from becoming stale. labels May 6, 2025
@julianwiedmann
bpf: host: don't detect WG traffic in from-netdev@cilium_wg0
f34b32d 
When from-netdev is attached to cilium_wg0 and processes the payload of a
WireGuard packet, there is no need to check whether this payload is a
Cilium-originating WireGuard packet. We don't do double-encryption.

Hence exclude the WG-detection block when attached to cilium_wg0.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann requested a review from smagnani96 May 20, 2025 11:35
smagnani96
smagnani96 approved these changes May 20, 2025
View reviewed changes
Copy link
Contributor

@smagnani96 smagnani96 left a comment

Choose a reason for hiding this comment

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

Right, we should skip this logic when this program's attached to the wireguard ingress hook, thanks!

@julianwiedmann julianwiedmann marked this pull request as ready for review May 20, 2025 17:11
@julianwiedmann julianwiedmann requested a review from a team as a code owner May 20, 2025 17:11
@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 May 20, 2025
@julianwiedmann julianwiedmann added this pull request to the merge queue May 21, 2025
Merged via the queue into cilium:v1.17 with commit f96b77c May 21, 2025
63 checks passed
@julianwiedmann julianwiedmann deleted the 1.18-bpf-host-wg branch May 21, 2025 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YutaroHayakawa YutaroHayakawa YutaroHayakawa approved these changes

@smagnani96 smagnani96 smagnani96 approved these changes

Assignees
No one assigned
Labels
area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. feature/wireguard Relates to Cilium's Wireguard feature kind/complexity-issue Relates to BPF complexity or program size issues 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@julianwiedmann @YutaroHayakawa @smagnani96