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

[v1.14] bpf: avoid SNAT tracking for overlay traffic #31797

Merged
merged 7 commits into from
May 7, 2024

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Apr 5, 2024

Manual backport of

Once this PR is merged, a GitHub action will update the labels of these PRs:

 31737 31082 31818 32116

@maintainer-s-little-helper maintainer-s-little-helper bot added backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master. labels Apr 5, 2024
@julianwiedmann julianwiedmann changed the title V1.14 tunnel snat [v1.14] bpf: avoid SNAT tracking for overlay traffic Apr 5, 2024
@julianwiedmann
Copy link
Member Author

/test-backport-1.14

@julianwiedmann julianwiedmann added the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Apr 8, 2024
@julianwiedmann julianwiedmann marked this pull request as ready for review April 9, 2024 09:57
@julianwiedmann julianwiedmann requested a review from a team as a code owner April 9, 2024 09:57
@julianwiedmann
Copy link
Member Author

/test-backport-1.14

[ upstream commit 3e32efc ]

[ backporter's notes: the tests looked a bit different in v1.14 ]

This is an internal macro that's selected by common.h (based on
TUNNEL_MODE and a few other config options). There should be no need to
explicitly set it.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
[ upstream commit 5b37dc9 ]

[ backporter's notes: minor conflicts due to changed context ]

When a packet gets SNATed in to-overlay, there is no point in setting the
SNAT-done mark. Firstly the mark refers to the *inner* packet, but all
subsequent users will only see the outer headers. Also our own netfilter
rules installed by installHostTrafficMarkRule() currently clear the mark
for overlay traffic.

Free up the mark for future usage.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
[ upstream commit 2860ded ]

[ backporter's notes: minor conflict in bpf_overlay, as handle_nat_fwd()
  didn't have the trace_ctx / ext_err parameters ]

To make smarter decisions at the native-device level (in to-netdev), mark
traffic that is created by cilium's overlay interface.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
[ upstream commit 7c789e5 ]

Creating SNAT entries for our own overlay traffic makes little sense. In
particular as the replies will not be addressed to the egressing packet's
source port, but to TUNNEL_PORT.

Avoiding such SNAT tracking reduces the pressure on the CT and NAT maps.

Fixes: cilium#26908
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
[ upstream commit b523a92 ]

Prior to 2860ded ("datapath: mark to-overlay traffic"), overlay
traffic would reach the HostFW egress path in to-netdev with
MARK_MAGIC_HOST set. Restore this behaviour by also assigning HOST_ID for
traffic that has MARK_MAGIC_OVERLAY set.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
[ upstream commit 8984c20 ]

As we now have a mark-derived src_sec_identity available, we might as well
share this bit of information with the user.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
[ upstream commit e984f65 ]

The blamed commit attempted to remove all usage of MARK_MAGIC_SNAT_DONE
inside to-overlay, so that we can use MARK_MAGIC_OVERLAY instead.

But it accidentally also touched the LB's NAT-egress path (which is used
by from-overlay to forward LB'ed requests to the egress path, on their way
to a remote backend). Here we need to maintain the usage of
MARK_MAGIC_SNAT_DONE, so that the egress program (either to-overlay or
to-netdev) doesn't apply a second SNAT operation. Otherwise the LB node is
unable to properly RevNAT the reply traffic.

Fix this by restoring the code that sets the MARK_MAGIC_SNAT_DONE flag
for traffic in from-overlay. When such a request is forwarded to the
overlay interface, then to-overlay will consume the mark, override it with
MARK_MAGIC_OVERLAY and skip the additional SNAT step.

Fixes: 5b37dc9 ("bpf: nodeport: avoid setting SNAT-done mark for to-overlay")
Reported-by: Marco Iorio <marco.iorio@isovalent.com>
Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
Copy link
Member Author

/test-backport-1.14

@julianwiedmann julianwiedmann added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs labels May 3, 2024
@ldelossa ldelossa merged commit eaa8b8c into cilium:v1.14 May 7, 2024
61 checks passed
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 7, 2024
@julianwiedmann julianwiedmann deleted the v1.14-tunnel-snat branch May 7, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.14 This PR represents a backport for Cilium 1.14.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants