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

Egress supports except property to allow noSNAT for assigned destination #2707

Closed
leonstack opened this issue Sep 2, 2021 · 9 comments
Closed
Assignees
Labels
kind/design Categorizes issue or PR as related to design. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@leonstack
Copy link
Contributor

leonstack commented Sep 2, 2021

Describe what you are trying to solve
My environment has multi-nic per-node for several network(such as management, monitor, public), and there are two applications in my environment. One application uses antrea(called antrea-app) and another uses hostNetwork(called host-app) . The antrea-app wants to access the external network and also wants to access the host-app(such as monitor network).
If the egressIP function is used for the antrea-app, no matter the destination is the external network or host-app, the packets will always be sent to the node where the egressIP belong to, and then the packets will be sent according to the host route of the node, which will hugely reduce the network performance between antrea-app and host-app.

Describe the solution you have in mind
Add a new property named except to Egress, means if destination matchs except value, these packets won't be sent to the node which egressIP belong to.
After adding the except field, and set the value to the IP or CIDR where host-app listening(such as monitor network). When the egressIP is used for antrea-app, if the destination address is host-app, the packets will no longer be sent to the node where the egressIP belong to, but sent directly from the current node and communicate with host-app directly. If the destination address doesn't match the except, the packets is still sent to the node where egressIP belong to.

@leonstack leonstack added the kind/design Categorizes issue or PR as related to design. label Sep 2, 2021
@leonstack leonstack changed the title Egress supports expect property to allow noSNAT for assigned destination Egress supports except property to allow noSNAT for assigned destination Sep 2, 2021
@tnqn
Copy link
Member

tnqn commented Sep 2, 2021

@leonstack Thanks for the proposal. Supporting excluding specific traffic from being SNATed sounds reasonable to me. Besides your case, I can imagine that if there are BMs or VMs running in the same network as the Kubernetes cluster, user may want their Pod talk to them directly. So having except can help that case too.

@jianjuns @antoninbas @wenqiq Could you share your opinions if any?

@antoninbas
Copy link
Contributor

This sounds good to me. One other issue (#2671) asked for the ability to filter by protocol (TCP / UDP), so maybe we can think of a generic traffic filtering mechanism that covers multiple use cases. I am not totally sure why one would want to filter by protocol though, so maybe @eranshpiner could provide more information about his use case.

@leonstack
Copy link
Contributor Author

hi @tnqn @antoninbas , seems the except should include [cidr, protocol, port] for more situation.
Or we can focus on [cidr, protocol] that I can't think of a use case for port.

leonstack added a commit to leonstack/antrea that referenced this issue Sep 7, 2021
Avoid SNAT for assigned IP block with assigned protocol {and port}?

Fixes: antrea-io#2707

Signed-off-by: Yang Li [email protected]
@tnqn
Copy link
Member

tnqn commented Sep 7, 2021

@leonstack I cannot think of a use case that different protocols and ports expect different egress behavior either. But we can make the API extensible even we only support cidr for now. Something like the following should work:

@@ -226,6 +226,12 @@ type EgressSpec struct {
        // If it is non-empty, the EgressIP will be assigned to a Node specified by the pool automatically and will failover
        // to a different Node when the Node becomes unreachable.
        ExternalIPPool string `json:"externalIPPool"`
+
+       Excepts []Except `json:"excepts,omitempty"`
+}
+
+type Except struct {
+       CIDR string
 }

@jianjuns
Copy link
Contributor

jianjuns commented Sep 7, 2021

Just wonder do we have requirements for per Egress "except", or we just need a global list of excluded CIDRs.

@leonstack
Copy link
Contributor Author

Just wonder do we have requirements for per Egress "except", or we just need a global list of excluded CIDRs.

Global list can solve my problem, but for #2671, I'm not sure it's flexible enough.

leonstack added a commit to leonstack/antrea that referenced this issue Sep 8, 2021
Avoid SNAT for assigned IP block with assigned protocol {and port}?

Fixes: antrea-io#2707

Signed-off-by: Yang Li [email protected]
leonstack added a commit to leonstack/antrea that referenced this issue Sep 8, 2021
Avoid SNAT for assigned IP block if the pod wants to communicate
directly, this can improve network performance

Fixes: antrea-io#2707

Signed-off-by: Yang Li [email protected]
leonstack added a commit to leonstack/antrea that referenced this issue Sep 9, 2021
Avoid SNAT for assigned IP block if the pod wants to communicate
directly, this can improve network performance

Signed-off-by: Yang Li [email protected]
leonstack added a commit to leonstack/antrea that referenced this issue Sep 9, 2021
Avoid SNAT for assigned IP block if the pod wants to communicate
directly, this can improve network performance

Signed-off-by: Yang Li [email protected]
leonstack added a commit to leonstack/antrea that referenced this issue Sep 9, 2021
Avoid SNAT for assigned IP block if the pod wants to communicate
directly, this can improve network performance

Signed-off-by: Yang Li [email protected]
@tnqn
Copy link
Member

tnqn commented Sep 9, 2021

I agree with @jianjuns that these except CIDRs are normally global in a cluster. It seems it doesn't make a lot of sense that an IP is an external address to one Pod but an internal address to another. And repeating the configuration in all Egress objects will be boring. Since we haven't received any feedback for #2671, should we just do the way that makes most sense for now? Maybe an option group for egress feature like this?

egress:
    exceptCIDRs: x.x.x.x/y, ...

It seems it would simplify the implementation too as we only need to classify these except CIDRs only once, instead of once per egress.
What you think?

@leonstack
Copy link
Contributor Author

I agree with @jianjuns that these except CIDRs are normally global in a cluster. It seems it doesn't make a lot of sense that an IP is an external address to one Pod but an internal address to another. And repeating the configuration in all Egress objects will be boring. Since we haven't received any feedback for #2671, should we just do the way that makes most sense for now? Maybe an option group for egress feature like this?

egress:
    exceptCIDRs: x.x.x.x/y, ...

It seems it would simplify the implementation too as we only need to classify these except CIDRs only once, instead of once per egress.
What you think?

Agreed, and the implementation will be more simplified, we can ignore the #2617 that I haven't found a case avoid SNAT by protocol filter.

leonstack pushed a commit to leonstack/antrea that referenced this issue Sep 10, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li [email protected]
leonstack pushed a commit to leonstack/antrea that referenced this issue Sep 10, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li [email protected]
leonstack pushed a commit to leonstack/antrea that referenced this issue Sep 13, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li [email protected]
leonstack added a commit to leonstack/antrea that referenced this issue Sep 13, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
leonstack added a commit to leonstack/antrea that referenced this issue Sep 15, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
leonstack added a commit to leonstack/antrea that referenced this issue Sep 16, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
leonstack added a commit to leonstack/antrea that referenced this issue Sep 17, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
leonstack added a commit to leonstack/antrea that referenced this issue Sep 17, 2021
For some environment, Pod can communicate with some destination
(not podCIDR/svcCIDR) directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
leonstack added a commit to leonstack/antrea that referenced this issue Oct 22, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
leonstack added a commit to leonstack/antrea that referenced this issue Oct 22, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
leonstack added a commit to leonstack/antrea that referenced this issue Oct 22, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
leonstack added a commit to leonstack/antrea that referenced this issue Oct 22, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
leonstack added a commit to leonstack/antrea that referenced this issue Oct 22, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
leonstack added a commit to leonstack/antrea that referenced this issue Oct 22, 2021
For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,
we should avoid SNAT for such destination.

Signed-off-by: Yang Li <[email protected]>
@github-actions
Copy link
Contributor

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/design Categorizes issue or PR as related to design. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants