v1.13 backports 2023-05-12 #25424
Merged
v1.13 backports 2023-05-12 #25424
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gentoo-root
added
kind/backports
|
/test-backport-1.13 |
[ upstream commit be111c7 ] Rename ct_lb_lookup4 to ct_lazy_lookup4, because it's going to be used not only in LB flows, but now also in NAT. Add the action parameter to cover ICMP packets in NAT flows. The actual usage comes in the next commit, this one doesn't contain functional changes. Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
[ upstream commit 3f0c2b7 ] The tuple prepared by snat_v4_nat or snat_v4_rev_nat is good enough for ct_lazy_lookup4, so we can spare a call to ct_extract_ports4, reducing complexity, improving performance, but most importantly, allowing to pass ports that differ from the packet header, which will allow to fix a bug in a subsequent commit. Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
[ upstream commit 7cb16c8 ] Rename ct_lb_lookup6 to ct_lazy_lookup6, because it's going to be used not only in LB flows, but now also in NAT. Add the action parameter to cover ICMP packets in NAT flows. The actual usage comes in the next commit, this one doesn't contain functional changes. Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
[ upstream commit ccf7965 ] The tuple prepared by snat_v6_nat or snat_v6_rev_nat is good enough for ct_lazy_lookup6, so we can spare a call to ct_extract_ports6, reducing complexity, improving performance, but most importantly, allowing to pass ports that differ from the packet header, which will allow to fix a bug in a subsequent commit. Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
[ upstream commit 6bbbb2a ] Flag the egress gateway NAT entries as needs_ct, so that reply packets could extend the expiration timeout as well. The next commit is also required for the complete fix, otherwise the needs_ct mark won't be matched. Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
[ upstream commit 4e27e55 ] Fix a bug in connection tracking in the revSNAT flow: reply packets are tracked before revSNAT restores the original tuple, meaning that it won't match the existing CT entry, and a new one will be created. This bug has impact on connections that set needs_ct (host-local and egress gateway connections): packets coming in the reply direction won't get the needs_ct mark and won't extend the expiration timeout. The fix is to convert the tuple before doing the conntrack lookup, so that it matches the tuple after revSNAT. Fixes: cilium#25110 Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
[ upstream commit 86cddf6 ] At the moment, both snat_v{4,6}_track_connection and snat_v{4,6}_new_mapping check the same conditions to determine whether a corresponding CT entry should exist for an SNAT entry. This approach has the following drawbacks: 1. Duplicate code: risk of divergence and a minor performance penalty, because this code is actually executed twice for the first packet. 2. The checks in snat_v{4,6}_track_connection are not optimal, because this function is called from both egress and ingress flows, but the checks are only relevant for the egress flow (because the first packet is egress, and the subsequent packets will have the result of the checks cached in the SNAT state). Solve both drawbacks by moving the checks one layer up, to snat_v{4,6}_nat_handle_mapping (also put the checks to a separate function for clarity). This way, the checks are only made in the egress flow when the SNAT state doesn't exist yet, and the result is reused for both snat_v{4,6}_track_connection and snat_v{4,6}_new_mapping, making the code more robust and slightly more optimized. Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com> Suggested-by: Julian Wiedmann <jwi@isovalent.com> Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
thorn3r
force-pushed
the
pr/v1.13-backport-2023-05-12
branch
from
1a743cf
to
bf63427
Compare
|
/test-backport-1.13 Job 'Cilium-PR-K8s-1.24-kernel-4.19' hit: #25255 (93.81% similarity) Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed: Click to show.Test NameFailure OutputJenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.26-kernel-net-next/2314/ If it is a flake and a GitHub issue doesn't already exist to track it, comment Then please upload the Jenkins artifacts to that issue. Job 'Cilium-PR-K8s-1.25-kernel-4.19' hit: #25255 (89.37% similarity) Job 'Cilium-PR-K8s-1.16-kernel-4.19' failed: Click to show.Test NameFailure OutputJenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.16-kernel-4.19/1499/ If it is a flake and a GitHub issue doesn't already exist to track it, comment Then please upload the Jenkins artifacts to that issue. |
|
/test-1.16-4.19 |
|
/test-1.17-4.19 |
|
/test-1.19-4.19 |
|
/test-1.22-4.19 |
|
/test-1.23-4.19 |
|
/test-1.24-4.19 |
|
/test-1.24-5.4 |
|
/test-1.25-4.19 |
|
/test-1.17-4.19 |
|
/ci-multicluster-1.13 |
|
/ci-aks-1.13 |
|
Thanks @thorn3r for linking the flake issues. Merging the PR based on your assessment. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Once this PR is merged, you can update the PR labels via: