bpf: Introduce per-cluster conntrack maps #22857
Merged
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
YutaroHayakawa
added
sig/datapath
github-actions
bot
added
the
kind/community-contribution
YutaroHayakawa
force-pushed
the
yutaro/clustermesh-overlapping-podcidr-oss-per-cluster-ct
branch
from
3e71591
to
b3c2b39
Compare
ti-mo
requested changes
Dec 26, 2022
pkg/maps/ctmap/per_cluster_ctmap.go
Outdated
| // key with ClusterID, we chose to create CT map per-cluster. When | ||
| // we have a good way of extending global CT maps in the future, we | ||
| // should retire this entire file. | ||
| type PerClusterCtMap struct { |
There was a problem hiding this comment.
PerClusterCTMap? https://go.dev/talks/2014
pkg/maps/ctmap/per_cluster_ctmap.go
Outdated
| ) | ||
|
|
||
| // An interface to interact with all per-cluster CT maps | ||
| type PerClusterCtMapsIface interface { |
There was a problem hiding this comment.
Following the Go convention: PerClusterCTMapper
| @@ -0,0 +1,199 @@ | |||
| // SPDX-License-Identifier: Apache-2.0 | |||
There was a problem hiding this comment.
Feel free to rename this to per_cluster_ctmap_test.go. With the removal of the privileged_tests build tag, privileged tests no longer need to be split into separate files, and the filename is getting rather long.
| c.Assert(err, IsNil) | ||
|
|
||
| // Per-cluster CT map depends on the map information | ||
| InitMapInfo(option.CTMapEntriesGlobalTCPDefault, option.CTMapEntriesGlobalAnyDefault, true, true, true) |
There was a problem hiding this comment.
This is already called in ctmap_privileged_test.go:init().
sayboras
reviewed
Jan 3, 2023
| @@ -15,6 +15,18 @@ const ( | |||
| ClusterIDMax = 255 | |||
| ) | |||
|
|
|||
| func ValidateClusterID(clusterID uint32) error { | |||
There was a problem hiding this comment.
is this change already part of #22875 ?
There was a problem hiding this comment.
Yes, but both PRs depend on this change, so let me rebase the branch after the one is merged into the master.
YutaroHayakawa
force-pushed
the
yutaro/clustermesh-overlapping-podcidr-oss-per-cluster-ct
branch
from
b3c2b39
to
eef31a9
Compare
|
Applied the fix suggested by Timo. |
ti-mo
approved these changes
Jan 9, 2023
NikAleksandrov
approved these changes
Jan 9, 2023
|
/test |
|
To distinguish two same IP addresses belong to the different clusters, we need to extend conntrack maps' keys to contain ClusterID. However, currently it is impossible to change conntrack maps without breaking user's connection. Thus, we'll take a different approach that creates conntrack maps per cluster. Signed-off-by: Yutaro Hayakawa <yutaro.hayakawa@isovalent.com>
YutaroHayakawa
force-pushed
the
yutaro/clustermesh-overlapping-podcidr-oss-per-cluster-ct
branch
from
eef31a9
to
0ebbc7c
Compare
|
I surrounded the per-cluster CT maps with guard macro And added new maps to ignore list. |
|
/test |
YutaroHayakawa
removed
the
kind/community-contribution
maintainer-s-little-helper
bot
added
the
ready-to-merge
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.
This is a part of the Cluster Mesh with overlapping PodCIDR support. When the remote cluster has an overlapping PodCIDR, we must identify the remote endpoint using IP + ClusterID. CT is not an exception, so it's ideal for extending the CT map's key with a new ClusterID field. However, the problem is we cannot extend the CT map without breaking users' connections now. Thus, we created a new map-in-map containing the CT maps for each cluster.
This PR only contains basic map definitions and tests. No one uses it. We also don't have GC for per-cluster maps because GC depends on the NAT map. Once I implement per-cluster NAT (it has the same problem), I'll publish another PR for GC.