Add global except list for egress to avoid SNAT #2749

leonstack · 2021-09-10T08:11:11Z

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 yang.li@transwarp.io

leonstack · 2021-09-13T03:14:15Z

/test-all

leonstack · 2021-09-13T03:20:30Z

/test-ipv6

leonstack · 2021-09-13T07:20:00Z

/test-e2e

leonstack · 2021-09-13T07:22:22Z

/test-e2e

leonstack · 2021-09-14T02:38:37Z

/test-all

leonstack · 2021-09-14T02:39:20Z

@tnqn hi, can you help review this MR...

tnqn

@leonstack Thanks for the PR, I have some comments.

build/yamls/base/conf/antrea-agent.conf

cmd/antrea-agent/agent.go

pkg/agent/config/node_config.go

pkg/agent/openflow/client.go

pkg/agent/openflow/pipeline.go

leonstack · 2021-09-15T13:57:38Z

/test-all

build/yamls/base/conf/antrea-agent.conf

cmd/antrea-agent/config.go

pkg/agent/config/node_config.go

pkg/agent/openflow/pipeline.go

test/integration/agent/openflow_test.go

build/yamls/base/conf/antrea-agent.conf

tnqn · 2021-09-16T16:41:57Z

For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,

How about:

For some environment, Pod can communicate with some destination 
(not podCIDR/svcCIDR) directly for better network performance,

leonstack · 2021-09-17T12:09:53Z

/test-all

leonstack · 2021-09-18T01:16:51Z

For some environment, some destination(not podCIDR/svcCIDR) can be
communicate with each other directly for better network performance,

How about:
For some environment, Pod can communicate with some destination 
(not podCIDR/svcCIDR) directly for better network performance,

Good suggestion, I have fixed it.

jianjuns

This implementation does not work for Windows? Could we add Windows support too? It might be required for running Antrea on GKE Windows.

leonstack · 2021-09-22T03:27:19Z

This implementation does not work for Windows? Could we add Windows support too? It might be required for running Antrea on GKE Windows.

HI, like @tnqn said, Egress feature is not supported on Windows yet: https://github.com/antrea-io/antrea/blob/main/docs/feature-gates.md#requirements-for-this-feature-6
We can add Windows support in another issue

jianjuns · 2021-09-22T16:58:49Z

This implementation does not work for Windows? Could we add Windows support too? It might be required for running Antrea on GKE Windows.

HI, like @tnqn said, Egress feature is not supported on Windows yet: https://github.com/antrea-io/antrea/blob/main/docs/feature-gates.md#requirements-for-this-feature-6
We can add Windows support in another issue

We need a way to bypass the default SNAT / masquerade on Windows. I am fine to have a separate PR for that. But we need it. @wenyingd @lzhecheng

tnqn

@leonstack Antrea 1.4 code freeze is this week, could you resolve the conflict and address the remaining comment if this is supposed to be included?

build/yamls/base/conf/antrea-agent.conf

leonstack · 2021-10-22T06:38:21Z

@tnqn Hi, I have fixed the conflict and the remaining comment, please help review again~

leonstack · 2021-10-22T06:40:06Z

/test-all

pkg/agent/openflow/pipeline.go

codecov-commenter · 2021-10-22T06:47:49Z

Codecov Report

Merging #2749 (5a7d220) into main (6ea909a) will increase coverage by 12.05%.
The diff coverage is 22.22%.

@@             Coverage Diff             @@
##             main    #2749       +/-   ##
===========================================
+ Coverage   40.63%   52.69%   +12.05%     
===========================================
  Files         158      283      +125     
  Lines       19907    23824     +3917     
===========================================
+ Hits         8089    12553     +4464     
+ Misses      11044     9875     -1169     
- Partials      774     1396      +622

Flag	Coverage Δ
kind-e2e-tests	`32.33% <22.22%> (?)`
unit-tests	`40.57% <0.00%> (-0.07%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/agent/config/node_config.go	`100.00% <ø> (+63.63%)`	⬆️
pkg/agent/openflow/pipeline.go	`57.44% <0.00%> (+34.92%)`	⬆️
pkg/agent/openflow/client.go	`48.44% <37.50%> (+24.34%)`	⬆️
pkg/agent/agent.go	`51.86% <50.00%> (+30.40%)`	⬆️
pkg/agent/controller/egress/egress_controller.go	`38.32% <0.00%> (-1.47%)`	⬇️
pkg/apiserver/handlers/webhook/mutation_labels.go	`24.71% <0.00%> (ø)`
...t/informers/externalversions/security/interface.go	`100.00% <0.00%> (ø)`
pkg/agent/route/route_linux.go	`28.16% <0.00%> (ø)`
pkg/legacyapis/core/v1alpha2/register.go	`80.00% <0.00%> (ø)`
pkg/legacyapis/stats/install/install.go	`100.00% <0.00%> (ø)`
... and 210 more

build/yamls/antrea-aks.yml

leonstack · 2021-10-22T09:32:24Z

/test-conformance

leonstack · 2021-10-22T09:32:30Z

/test-e2e

leonstack · 2021-10-22T09:32:55Z

/test-networkpolicy

leonstack · 2021-10-22T09:58:07Z

--- FAIL: TestFlowAggregator (230.49s)
    --- FAIL: TestFlowAggregator/IPv4 (115.23s)
        --- FAIL: TestFlowAggregator/IPv4/IntraNodeFlows (18.21s)
        --- FAIL: TestFlowAggregator/IPv4/IntraNodeDenyConnIngressANP (6.45s)
        --- FAIL: TestFlowAggregator/IPv4/IntraNodeDenyConnEgressANP (2.77s)
        --- FAIL: TestFlowAggregator/IPv4/IntraNodeDenyConnNP (8.03s)

Hi @tnqn, some cases failed in Kind/E2e test, I'm not sure these cases have any relationship for this PR.

leonstack · 2021-10-22T10:04:39Z

/test-all

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 <yang.li@transwarp.io>

leonstack · 2021-10-22T10:20:47Z

/test-networkpolicy
/test-e2e
/test-conformance

leonstack · 2021-10-22T15:24:32Z

@tnqn Hi, the required check all passed, could you help to review this PR again:-)

jianjuns

LGTM.

build/yamls/base/conf/antrea-agent.conf

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 <yang.li@transwarp.io>

leonstack · 2021-10-25T02:29:49Z

/test-conformance
/test-e2e
/test-networkpolicy

leonstack · 2021-10-25T08:50:35Z

/test-all

tnqn · 2021-10-25T13:18:56Z

/test-integration

tnqn

LGTM

leonstack force-pushed the except_list branch 3 times, most recently from c5a5781 to 00a59f8 Compare September 13, 2021 03:09

tnqn reviewed Sep 14, 2021

View reviewed changes

leonstack force-pushed the except_list branch from 768e4b2 to fa8a33f Compare September 15, 2021 11:27

Jexf reviewed Sep 16, 2021

View reviewed changes

build/yamls/base/conf/antrea-agent.conf Show resolved Hide resolved

leonstack force-pushed the except_list branch from be1563e to 92f2307 Compare September 16, 2021 11:57

tnqn reviewed Sep 16, 2021

View reviewed changes

leonstack force-pushed the except_list branch 2 times, most recently from 247cde8 to 9a5337f Compare September 17, 2021 08:01

jianjuns reviewed Sep 22, 2021

View reviewed changes

tnqn reviewed Oct 18, 2021

View reviewed changes

build/yamls/base/conf/antrea-agent.conf Outdated Show resolved Hide resolved

tnqn mentioned this pull request Oct 19, 2021

Can antrea co-exist with AWS VPC CNI? #2901

Closed

leonstack force-pushed the except_list branch 2 times, most recently from 22ab277 to 6521b29 Compare October 22, 2021 06:36

tnqn reviewed Oct 22, 2021

View reviewed changes

pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved

leonstack force-pushed the except_list branch from 6521b29 to 22e8642 Compare October 22, 2021 06:44

leonstack changed the title ~~Add global except list for egress to avoid SNAT (#2707)~~ Add global except list for egress to avoid SNAT Oct 22, 2021

leonstack force-pushed the except_list branch from 22e8642 to 7d7a266 Compare October 22, 2021 07:17

tnqn reviewed Oct 22, 2021

View reviewed changes

build/yamls/antrea-aks.yml Outdated Show resolved Hide resolved

leonstack force-pushed the except_list branch 2 times, most recently from de47f98 to affc90e Compare October 22, 2021 07:46

leonstack force-pushed the except_list branch from affc90e to fae1187 Compare October 22, 2021 10:14

jianjuns reviewed Oct 22, 2021

View reviewed changes

build/yamls/base/conf/antrea-agent.conf Outdated Show resolved Hide resolved

leonstack force-pushed the except_list branch from fae1187 to 5a7d220 Compare October 23, 2021 07:10

tnqn approved these changes Oct 25, 2021

View reviewed changes

tnqn merged commit 2718266 into antrea-io:main Oct 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add global except list for egress to avoid SNAT #2749

Add global except list for egress to avoid SNAT #2749

leonstack commented Sep 10, 2021

leonstack commented Sep 13, 2021

leonstack commented Sep 13, 2021

leonstack commented Sep 13, 2021

leonstack commented Sep 13, 2021

leonstack commented Sep 14, 2021

leonstack commented Sep 14, 2021

tnqn left a comment

leonstack commented Sep 15, 2021

tnqn commented Sep 16, 2021

leonstack commented Sep 17, 2021

leonstack commented Sep 18, 2021

jianjuns left a comment

leonstack commented Sep 22, 2021

jianjuns commented Sep 22, 2021

tnqn left a comment

leonstack commented Oct 22, 2021 •

edited

Loading

leonstack commented Oct 22, 2021

codecov-commenter commented Oct 22, 2021 •

edited

Loading

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

jianjuns left a comment

leonstack commented Oct 25, 2021

leonstack commented Oct 25, 2021

tnqn commented Oct 25, 2021

tnqn left a comment

Add global except list for egress to avoid SNAT #2749

Add global except list for egress to avoid SNAT #2749

Conversation

leonstack commented Sep 10, 2021

leonstack commented Sep 13, 2021

leonstack commented Sep 13, 2021

leonstack commented Sep 13, 2021

leonstack commented Sep 13, 2021

leonstack commented Sep 14, 2021

leonstack commented Sep 14, 2021

tnqn left a comment

Choose a reason for hiding this comment

leonstack commented Sep 15, 2021

tnqn commented Sep 16, 2021

leonstack commented Sep 17, 2021

leonstack commented Sep 18, 2021

jianjuns left a comment

Choose a reason for hiding this comment

leonstack commented Sep 22, 2021

jianjuns commented Sep 22, 2021

tnqn left a comment

Choose a reason for hiding this comment

leonstack commented Oct 22, 2021 • edited Loading

leonstack commented Oct 22, 2021

codecov-commenter commented Oct 22, 2021 • edited Loading

Codecov Report

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

leonstack commented Oct 22, 2021

jianjuns left a comment

Choose a reason for hiding this comment

leonstack commented Oct 25, 2021

leonstack commented Oct 25, 2021

tnqn commented Oct 25, 2021

tnqn left a comment

Choose a reason for hiding this comment

leonstack commented Oct 22, 2021 •

edited

Loading

codecov-commenter commented Oct 22, 2021 •

edited

Loading