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

wireguard: also account for tunnel overhead #30329

Merged
merged 1 commit into from
Jan 30, 2024
Merged

wireguard: also account for tunnel overhead #30329

merged 1 commit into from
Jan 30, 2024

Conversation

3u13r
Copy link
Contributor

@3u13r 3u13r commented Jan 18, 2024

Since #29000 packets are always encapsulated before they are encrypted with WireGuard. Therefore, we also need to take the tunnel overhead for the route MTU into account.

This fixes a performance regression. Before this commit WireGuard encrypted pod-to-pod traffic the iperf3 bandwidth was ~102 Mbits/sec. With this patch the bandwidth increases to 656 Mbits/sec. Without encryption the bandwidth is ~2 Gbits/sec.

This is related to #28413. But this does not fix all issues, see: #28413 (comment).

Fixes: b67291f

Fix performance regression for pod-to-pod traffic WireGuard and tunneling.

@3u13r 3u13r requested a review from a team as a code owner January 18, 2024 18:24
@3u13r 3u13r requested a review from markpash January 18, 2024 18:24
@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 Jan 18, 2024
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jan 18, 2024
@3u13r
wireguard: also account for tunnel overhead
e877d82 
Since cilium#29000 packets are always encapsulated before they are encrypted with WireGuard.
Therefore, we also need to take the tunnel overhead for the route MTU into account.

This fixes a performance regression. Before this commit WireGuard encrypted
pod-to-pod traffic the iperf3 bandwidth was ~102 Mbits/sec. With this patch
the bandwidth increases to 656 Mbits/sec. Without encryption the bandwidth
is ~2 Gbits/sec.

Fixes: b67291f
Signed-off-by: Leonard Cohnen <lc@edgeless.systems>
@squeed
Copy link
Contributor

squeed commented Jan 23, 2024

cc @brb -- this fixes your commit.

@squeed
Copy link
Contributor

squeed commented Jan 23, 2024

/test

@julianwiedmann julianwiedmann requested review from a team and brb and removed request for a team January 23, 2024 14:29
@gandro gandro self-requested a review January 23, 2024 14:40
@gandro gandro added release-note/bug This PR fixes an issue in a previous release of Cilium. needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Jan 23, 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 Jan 23, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in v1.15.0-rc.1 Jan 23, 2024
gandro
gandro approved these changes Jan 23, 2024
View reviewed changes
Copy link
Member

@gandro gandro left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks! I've added the backport label since the underlying change has also been backported to v1.15. This probably also needs a custom backport to v1.14, where additional encapsulation is possible, but disabled by default.

@gandro gandro added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Jan 23, 2024
@burgerdev burgerdev deleted the fix/wireguard-encap-mtu branch January 24, 2024 16:59
@3u13r 3u13r restored the fix/wireguard-encap-mtu branch January 24, 2024 20:04
@burgerdev burgerdev mentioned this pull request Jan 25, 2024
Merged
7 tasks
brb
brb approved these changes Jan 29, 2024
View reviewed changes
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks! In v1.14 we need to accommodate the overhead only if --wireguard-encapsulate is set.

@julianwiedmann julianwiedmann added feature/wireguard Relates to Cilium's Wireguard feature sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. labels Jan 30, 2024
@julianwiedmann julianwiedmann added this pull request to the merge queue Jan 30, 2024
Merged via the queue into cilium:main with commit 44c3dd0 Jan 30, 2024
62 checks passed
@3u13r 3u13r deleted the fix/wireguard-encap-mtu branch January 30, 2024 13:48
@joamaki joamaki mentioned this pull request Jan 30, 2024
Merged
28 tasks
@joamaki joamaki 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 30, 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 30, 2024
@joamaki joamaki mentioned this pull request Jan 31, 2024
Merged
17 tasks
@joamaki joamaki 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 31, 2024
@aanm aanm added this to Backport pending to v1.15 in 1.15.1 Jan 31, 2024
@aanm aanm removed this from Backport pending to vv1.15.0-rc in v1.15.0-rc.1 Jan 31, 2024
@aanm aanm removed this from Backport pending to v1.15 in 1.15.1 Jan 31, 2024
@aanm aanm 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 31, 2024
@aanm aanm added this to Backport done to v1.15 in v1.15.0-rc.1 Jan 31, 2024
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.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. labels Feb 7, 2024
@michi-covalent michi-covalent mentioned this pull request Feb 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gandro gandro gandro approved these changes

@brb brb brb approved these changes

@markpash markpash Awaiting requested review from markpash markpash is a code owner automatically assigned from cilium/sig-datapath

Assignees
No one assigned
Labels
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. feature/wireguard Relates to Cilium's Wireguard feature kind/community-contribution This was a contribution made by a community member. 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.
Projects
No open projects
cilium v1.14.7
Status: Released
v1.15.0-rc.1
Backport done to v1.15
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@3u13r @squeed @gandro @brb @joamaki @aanm @julianwiedmann