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

bpf: avoid SNAT tracking for overlay traffic #31082

Merged
merged 5 commits into from Apr 5, 2024

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Mar 1, 2024

In certain configs (basically when the node-to-node IPv4 address is also chosen as IPV4_MASQUERADE) the masquerade code in to-netdev decides that tunnel traffic is potentially conflicting with masqueraded traffic. We thus need to reserve the packet's source port ("track it"), and potentially even SNAT it.

Creating SNAT entries for tunnel traffic makes little sense - in particular as the replies will not be addressed to the packet's source port, but to TUNNEL_PORT. Doing it anyway results in pressure on the CT and NAT maps.

Improve this by marking such traffic in to-overlay, and then bypassing the masquerading logic in to-netdev accordingly. Also use the mark to detect overlay traffic for the recently introduced encrypted-overlay feature. For wireguard we're a bit more careful, to avoid missing unmarked traffic during an upgrade.

Fixes: #26908

Skip overlay traffic in the BPF SNAT processing, and thus reduce pressure on the BPF Connection tracking and NAT maps.

@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 1, 2024
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann force-pushed the 1.16-tunnel-mark branch 2 times, most recently from b4c2415 to 31024f6 Compare March 18, 2024 16:57
@julianwiedmann julianwiedmann changed the title 1.16 tunnel mark bpf: avoid SNAT tracking for overlay traffic Mar 18, 2024
@julianwiedmann julianwiedmann added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. feature/snat Relates to SNAT or Masquerading of traffic labels Mar 18, 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 18, 2024
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann added the dont-merge/preview-only Only for preview or testing, don't merge it. label Mar 18, 2024
@julianwiedmann julianwiedmann marked this pull request as ready for review March 18, 2024 18:09
@julianwiedmann julianwiedmann requested a review from a team as a code owner March 18, 2024 18:09
@julianwiedmann julianwiedmann requested a review from a team as a code owner March 19, 2024 08:50
@julianwiedmann julianwiedmann requested a review from brb March 19, 2024 08:50
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann added the feature/wireguard Relates to Cilium's Wireguard feature label Mar 19, 2024
bpf/lib/wireguard.h Outdated Show resolved Hide resolved
Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

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

Changes to the encrypted overlay bits check out to me.

I'm having a bit of trouble tracing out where TC runs in the egress vxlan driver path.
I'm going to assume we are setting the mark on the packet post-encap.

If we are not, the mark will get cleaned, and I assume this isn't occuring if the PR is tested.

@ldelossa
Copy link
Contributor

ldelossa commented Apr 4, 2024

Above comment is addressed here:
#31082 (comment)

@julianwiedmann
Copy link
Member Author

If we are not, the mark will get cleaned, and I assume this isn't occuring if the PR is tested.

To summarize some offline discussion - Louis' concern was that https://elixir.bootlin.com/linux/latest/source/net/ipv4/ip_tunnel_core.c#L60 would clear the mark. But that doesn't apply, as we don't switch net namespace (xnet is false).

@julianwiedmann julianwiedmann added this pull request to the merge queue Apr 5, 2024
Merged via the queue into cilium:main with commit 42d745e Apr 5, 2024
62 checks passed
@julianwiedmann julianwiedmann deleted the 1.16-tunnel-mark branch April 5, 2024 06:57
@julianwiedmann julianwiedmann added the backport/author The backport will be carried out by the author of the PR. label Apr 5, 2024
@julianwiedmann julianwiedmann 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 Apr 5, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.15 in 1.15.4 Apr 5, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.15 in 1.15.4 Apr 5, 2024
@julianwiedmann julianwiedmann added the backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. label Apr 5, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Backport pending to v1.14 in 1.14.10 Apr 5, 2024
@github-actions github-actions bot 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 10, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed this from Backport pending to v1.15 in 1.15.4 Apr 10, 2024
@asauber asauber added this to Backport pending to v1.14 in 1.14.11 Apr 11, 2024
@asauber asauber removed this from Backport pending to v1.14 in 1.14.10 Apr 11, 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 May 7, 2024
@nebril nebril moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.11 May 10, 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.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/snat Relates to SNAT or Masquerading of traffic feature/wireguard Relates to Cilium's Wireguard feature ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
1.14.11
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

SNAT: Masquerading logic can end up tracking tunnel traffic
7 participants