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

Optimize NodePort performance by reducing request packets CT actions #3862

Merged
merged 1 commit into from
Jun 10, 2022

Conversation

hongliangl
Copy link
Contributor

@hongliangl hongliangl commented Jun 7, 2022

For a NodePort connection sourced from external network or local Node,
destination IP will be DNATed with a virtual IP, then the connection
will be forwarded to OVS via Antrea gateway. However, in UnSNATTable,
a flow is installed to unSNAT replied packets of SNATed connections by
matching the virtual IP as destination IP. The flow is like the following:

table=UnSNAT, priority=200,ip,nw_dst=169.254.0.253 actions=ct(table=ConntrackZone,zone=65521,nat)

Note that, the request packets of a DNATed NodePort connection are also
matched by the flow above, but it is unnecessary. To optimize the
performance of NodePort, another virtual IP is used to DNAT NodePort
connections.

TCP_RR and TCP_CRR improvement is as fowllows:

Test      old TPS     new TPS    delta
TCP_CRR   3510.28     3847.76    +%9.61
TCP_RR    9574.29     10457.6    +%9.23

Signed-off-by: Hongliang Liu lhongliang@vmware.com

@hongliangl hongliangl added this to the Antrea v1.7 release milestone Jun 7, 2022
@hongliangl hongliangl added the action/release-note Indicates a PR that should be included in release notes. label Jun 7, 2022
@hongliangl
Copy link
Contributor Author

/test-e2e
/test-windows-all

@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2022

Codecov Report

Merging #3862 (f143fe7) into main (dcd1019) will increase coverage by 11.13%.
The diff coverage is 45.58%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #3862       +/-   ##
===========================================
+ Coverage   49.22%   60.36%   +11.13%     
===========================================
  Files         258      290       +32     
  Lines       37969    42067     +4098     
===========================================
+ Hits        18691    25392     +6701     
+ Misses      17422    14569     -2853     
- Partials     1856     2106      +250     
Flag Coverage Δ
kind-e2e-tests 44.59% <42.64%> (-4.64%) ⬇️
unit-tests 44.29% <85.18%> (?)

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% <ø> (ø)
pkg/agent/openflow/pipeline.go 68.08% <0.00%> (-4.32%) ⬇️
pkg/agent/route/route_linux.go 27.54% <19.51%> (-20.65%) ⬇️
pkg/agent/proxy/proxier.go 60.00% <50.00%> (+6.17%) ⬆️
pkg/agent/openflow/service.go 67.03% <95.45%> (-16.88%) ⬇️
...g/agent/apiserver/handlers/featuregates/handler.go 4.54% <0.00%> (-77.28%) ⬇️
pkg/apis/controlplane/v1beta2/helper.go 40.00% <0.00%> (-60.00%) ⬇️
pkg/controller/egress/store/egressgroup.go 1.72% <0.00%> (-54.32%) ⬇️
...kg/apiserver/registry/system/supportbundle/rest.go 22.17% <0.00%> (-46.09%) ⬇️
pkg/support/dump_others.go 0.00% <0.00%> (-44.74%) ⬇️
... and 177 more

@hongliangl
Copy link
Contributor Author

/test-e2e
/test-windows-e2e

@hongliangl hongliangl removed this from the Antrea v1.7 release milestone Jun 8, 2022
tnqn
tnqn previously approved these changes Jun 9, 2022
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tnqn tnqn requested review from jianjuns and wenyingd June 9, 2022 15:06
jianjuns
jianjuns previously approved these changes Jun 9, 2022
@@ -1254,6 +1278,26 @@ func (c *Client) AddClusterIPRoute(svcIP net.IP) error {
return nil
}

func (c *Client) addVirtualDNATIPRoute(isIPv6 bool) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: should we name it addVirtualNodePortDNATIPRoute()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

jianjuns
jianjuns previously approved these changes Jun 10, 2022
@jianjuns
Copy link
Contributor

@hongliangl : there is a conflict.

@hongliangl
Copy link
Contributor Author

@jianjuns Thanks, the PR is updated.

jianjuns
jianjuns previously approved these changes Jun 10, 2022
@jianjuns
Copy link
Contributor

/test-all

Please take care of tests.

pkg/agent/config/node_config.go Outdated Show resolved Hide resolved
pkg/agent/config/node_config.go Show resolved Hide resolved
For a NodePort connection sourced from external network or local Node,
destination IP will be DNATed with a virtual IP, then the connection
will be forwarded to OVS via Antrea gateway. However, in UnSNATTable,
a flow is installed to unSNAT replied packets of SNATed connections by
matching the virtual IP as destination IP. The flow is like the following:

```
table=UnSNAT, priority=200,ip,nw_dst=169.254.0.253 actions=ct(table=ConntrackZone,zone=65521,nat)
```

Note that, the request packets of a DNATed NodePort connection are also
matched by the flow above, but it is unnecessary. To optimize the
performance of NodePort, another virtual IP is used to DNAT NodePort
connections.

TCP_RR and TCP_CRR improvement is as fowllows:

```
Test      old TPS     new TPS    delta
TCP_CRR   3510.28     3847.76    +%9.61
TCP_RR    9574.29     10457.6    +%9.23
```

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
@jianjuns
Copy link
Contributor

/test-all

Copy link
Contributor

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jianjuns
Copy link
Contributor

/test-ipv6-e2e

@jianjuns jianjuns merged commit 422feec into antrea-io:main Jun 10, 2022
@hongliangl hongliangl deleted the reduce-svc-ct-action branch June 16, 2022 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants