Optimize the flows for forwarding the packets with unknown destination in L3ForwardingTable #3809

hongliangl · 2022-05-18T13:49:41Z

Currently, when Egress and AntreaIPAM are enabled, there are several flows in
L3ForwardingTable like the follows:

1. table=L3Forwarding, priority=190,ip,reg0=0/0x200,reg8=0/0xfff,nw_dst=10.10.0.0/24 actions=resubmit(,L2ForwardingCalc)
2. table=L3Forwarding, priority=190,ct_mark=0x10/0x10,reg0=0x200/0x200,reg4=0/0x100000 actions=mod_dl_dst:d2:35:24:7f:3a:f8,load:0x2->NXM_NX_REG0[4..7],resubmit(,L3DecTTL)
3. table=L3Forwarding, priority=190,ct_mark=0x10/0x10,reg4=0x100000/0x100000 actions=resubmit(,L3DecTTL)
4. table=L3Forwarding, priority=190,ct_state=-rpl+trk,ip,reg0=0x3/0xf actions=resubmit(,EgressMark)
5. table=L3Forwarding, priority=190,ct_state=-rpl+trk,ip,reg0=0x1/0xf actions=mod_dl_dst:d2:35:24:7f:3a:f8,resubmit(,EgressMark)
6. table=L3Forwarding, priority=0 actions=load:0x2->NXM_NX_REG0[4..7],resubmit(,L2ForwardingCalc)

Flow 1 is used to forward the packets of non-Service connections between
local Pods.
Flow 2 is used to forward the packets of Service connections sourced from
local non-AntreaIPAM Pods and destined for external network Endpoint.
Flow 3 is used to forward the packets of Service connections sourced from
local AntreaIPAM Pods and destined for external network Endpoint.
Flow 4 is used to forward the packets sourced from local Pods and destined
for external network, and the flow is for Egress.
Flow 5 is used to forward the packets sourced from tunnel and destined for
external network, and the flow is also for Egress.
Flow 6 is the default flow. load:0x2->NXM_NX_REG0[4..7] means to Gateway.

For request packets sourced from a local Pod and destined for another local
Pod, they are expected to be matched by flow 1, however, they can be matched
by flow 4, and this leads the unexpected result in #3806.

In addition, when Egress is enabled, if a local Pod accesses to a Service
whose Endpoint is on external Network, the request packets are expected to
be matched by flow 4, but they can be also matched by flow 2.

To resolve above issues, the new flows are like the follows:

1. table=L3Forwarding, priority=200,ip,reg0=0/0x200,reg8=0/0xfff,nw_dst=10.10.0.0/24 actions=resubmit(,L2ForwardingCalc)
2. table=L3Forwarding, priority=190,ct_mark=0x10/0x10,reg0=0x202/0x20f actions=mod_dl_dst:d2:35:24:7f:3a:f8,load:0x2->NXM_NX_REG0[4..7],resubmit(,L3DecTTL)
3. table=L3Forwarding, priority=190,ct_state=-rpl+trk,ip,reg0=0x3/0xf,reg4=0/0x100000 actions=resubmit(,EgressMark)
4. table=L3Forwarding, priority=190,ct_state=-rpl+trk,ip,reg0=0x1/0xf actions=mod_dl_dst:d2:35:24:7f:3a:f8,resubmit(,EgressMark)
5. table=L3Forwarding, priority=0 actions=load:0x2->NXM_NX_REG0[4..7],resubmit(,L2ForwardingCalc)

Issue #3806 is fixed by raising the priority of the legacy flow 1 to
normal (200). It is the new flow 1 now.

In legacy flow 2, packets of Service connections with unknown destination
can be either sourced from local Pods or Antrea gateway. In the new flow 2,
only the packets of Service connections sourced Antrea gateway are matched.

Packets of connections sourced from local Pods and destined for external
network are matched by the new flow 3 (Egress is enabled) or the new flow
5 (Egress is disabled).

Packets of connections sourced from local AntreaIPAM Pods (AntreaIPAM is
enabled) and destined for external network are matched by the new flow 5.

Other modifications: fix some stale comments.

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

codecov-commenter · 2022-05-18T14:48:50Z

Codecov Report

Merging #3809 (c89f339) into main (3a51abe) will decrease coverage by 17.30%.
The diff coverage is 80.00%.

❗ Current head c89f339 differs from pull request most recent head a6e15bf. Consider uploading reports for the commit a6e15bf to get more accurate results

@@             Coverage Diff             @@
##             main    #3809       +/-   ##
===========================================
- Coverage   63.38%   46.08%   -17.31%     
===========================================
  Files         280      247       -33     
  Lines       39750    35881     -3869     
===========================================
- Hits        25197    16537     -8660     
- Misses      12593    17708     +5115     
+ Partials     1960     1636      -324

Flag	Coverage Δ
e2e-tests	`46.08% <80.00%> (?)`
kind-e2e-tests	`?`
unit-tests	`?`

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

Impacted Files	Coverage Δ
pkg/agent/controller/networkpolicy/packetin.go	`16.90% <0.00%> (-38.03%)`	⬇️
pkg/agent/openflow/client.go	`64.70% <ø> (-3.17%)`	⬇️
pkg/apis/controlplane/types.go	`0.00% <ø> (ø)`
pkg/log/log_file.go	`58.92% <ø> (-11.25%)`	⬇️
pkg/agent/openflow/packetin.go	`63.63% <66.66%> (ø)`
pkg/log/log.go	`83.33% <82.14%> (-16.67%)`	⬇️
pkg/agent/openflow/pipeline.go	`67.87% <100.00%> (-3.85%)`	⬇️
pkg/agent/openflow/service.go	`66.66% <100.00%> (-11.50%)`	⬇️
pkg/apis/stats/register.go	`75.00% <100.00%> (+3.57%)`	⬆️
pkg/apis/stats/v1alpha1/register.go	`86.66% <100.00%> (+2.05%)`	⬆️
... and 171 more

gran-vmv

LGTM

gran-vmv · 2022-05-19T01:51:22Z

/test-all
/test-ipv6-all
/test-ipv6-only-all
/test-flexible-ipam-e2e
/test-windows-all

gran-vmv · 2022-05-19T02:20:19Z

/test-all
/test-ipv6-all
/test-ipv6-only-all
/test-flexible-ipam-e2e
/test-windows-all

pkg/agent/openflow/pipeline.go

hongliangl · 2022-05-25T11:55:24Z

/test-all-features-conformance
/test-conformance
/test-e2e
/test-flexible-ipam-e2e
/test-ipv6-conformance
/test-ipv6-e2e
/test-ipv6-networkpolicy
/test-ipv6-only-conformance
/test-ipv6-only-e2e
/test-ipv6-only-networkpolicy
/test-multicluster-e2e
/test-networkpolicy
/test-windows-conformance
/test-windows-e2e
/test-windows-networkpolicy
/test-windows-proxyall-e2e
/test-integration

hongliangl · 2022-05-25T15:48:34Z

/test-all-features-conformance
/test-conformance
/test-e2e
/test-flexible-ipam-e2e
/test-ipv6-conformance
/test-ipv6-e2e
/test-ipv6-networkpolicy
/test-ipv6-only-conformance
/test-ipv6-only-e2e
/test-ipv6-only-networkpolicy
/test-multicluster-e2e
/test-networkpolicy
/test-windows-conformance
/test-windows-e2e
/test-windows-networkpolicy
/test-windows-proxyall-e2e
/test-integration

hongliangl · 2022-05-26T02:23:42Z

/test-flexible-ipam-e2e
/test-windows-e2e
/test-multicluster-e2e
/test-windows-proxyall-e2e
/test-ipv6-conformance

hongliangl · 2022-05-26T13:35:16Z

/test-flexible-ipam-e2e

hongliangl · 2022-05-26T13:36:39Z

/test-windows-e2e
/test-windows-proxyall-e2e
/test-multicluster-e2e

hongliangl · 2022-05-26T15:50:25Z

/test-flexible-ipam-e2e

hongliangl · 2022-05-26T15:50:53Z

/test-windows-e2e
/test-windows-proxyall-e2e
/test-multicluster-e2e

hongliangl · 2022-05-26T20:22:53Z

/test-flexible-ipam-e2e
/test-windows-e2e
/test-windows-proxyall-e2e
/test-multicluster-e2e

hongliangl · 2022-05-27T03:59:46Z

/test-flexible-ipam-e2e

gran-vmv · 2022-06-01T11:21:22Z

/test-flexible-ipam-e2e

gran-vmv · 2022-06-01T23:52:47Z

/test-flexible-ipam-e2e

gran-vmv · 2022-06-06T01:54:20Z

/test-flexible-ipam-e2e

gran-vmv · 2022-06-06T03:21:15Z

/test-flexible-ipam-e2e

hongliangl · 2022-06-07T03:34:29Z

@tnqn @wenyingd Do you have any further comments? Thanks.

wenyingd

LGTM

tnqn

Code LGTM but comments need adjustment.

tnqn · 2022-06-07T16:57:45Z

pkg/agent/openflow/pipeline.go

+// gateway and destined for external network. Note that, the destination MAC address of the packets should be rewritten to
+// local Antrea gateway's so that the packets can be forwarded to external network via local Antrea gateway.
+func (f *featureService) l3FwdFlowToExternalEndpoint() binding.Flow {
+	// This generates the flow to match the packets sourced from per-Node IPAM Pod and destined for external network.


The comment means the opposite of the code

tnqn · 2022-06-07T17:05:43Z

pkg/agent/openflow/pipeline.go

+// l3FwdFlowToExternal generates the flow to forward the packets destined for external network. Corresponding cases are
+// listed in the follows:
+// Assuming that AntreaProxy is enabled,
+//  - when Egress is disabled, request packets of Service or non-Service connections sourced from local non-AntreaIPAM


If a flow is designed for both Service and non-Service connections, which already means it's Service irrelevant. better to remove "of Service or non-Service connections" from comments together to avoid unnecessary attention and being verbose.
So do the commit message.

…n in L3ForwardingTable Fix antrea-io#3806 Currently, when Egress and AntreaIPAM are enabled, there are several flows in L3ForwardingTable like the follows: ``` 1. table=L3Forwarding, priority=190,ip,reg0=0/0x200,reg8=0/0xfff,nw_dst=10.10.0.0/24 actions=resubmit(,L2ForwardingCalc) 2. table=L3Forwarding, priority=190,ct_mark=0x10/0x10,reg0=0x200/0x200,reg4=0/0x100000 actions=mod_dl_dst:d2:35:24:7f:3a:f8,load:0x2->NXM_NX_REG0[4..7],resubmit(,L3DecTTL) 3. table=L3Forwarding, priority=190,ct_mark=0x10/0x10,reg4=0x100000/0x100000 actions=resubmit(,L3DecTTL) 4. table=L3Forwarding, priority=190,ct_state=-rpl+trk,ip,reg0=0x3/0xf actions=resubmit(,EgressMark) 5. table=L3Forwarding, priority=190,ct_state=-rpl+trk,ip,reg0=0x1/0xf actions=mod_dl_dst:d2:35:24:7f:3a:f8,resubmit(,EgressMark) 6. table=L3Forwarding, priority=0 actions=load:0x2->NXM_NX_REG0[4..7],resubmit(,L2ForwardingCalc) ``` - Flow 1 is used to forward the packets of non-Service connections between local Pods. - Flow 2 is used to forward the packets of Service connections sourced from local non-AntreaIPAM Pods and destined for external network Endpoint. - Flow 3 is used to forward the packets of Service connections sourced from local AntreaIPAM Pods and destined for external network Endpoint. - Flow 4 is used to forward the packets sourced from local Pods and destined for external network, and the flow is for Egress. - Flow 5 is used to forward the packets sourced from tunnel and destined for external network, and the flow is also for Egress. - Flow 6 is the default flow. `load:0x2->NXM_NX_REG0[4..7]` means `to Gateway`. For request packets sourced from a local Pod and destined for another local Pod, they are expected to be matched by flow 1, however, they can be matched by flow 4, and this leads the unexpected result in antrea-io#3806. In addition, when Egress is enabled, if a local Pod accesses to a Service whose Endpoint is on external Network, the request packets are expected to be matched by flow 4, but they can be also matched by flow 2. To resolve above issues, the new flows are like the follows: ``` 1. table=L3Forwarding, priority=200,ip,reg0=0/0x200,reg8=0/0xfff,nw_dst=10.10.0.0/24 actions=resubmit(,L2ForwardingCalc) 2. table=L3Forwarding, priority=190,ct_mark=0x10/0x10,reg0=0x202/0x20f actions=mod_dl_dst:d2:35:24:7f:3a:f8,load:0x2->NXM_NX_REG0[4..7],resubmit(,L3DecTTL) 3. table=L3Forwarding, priority=190,ct_state=-rpl+trk,ip,reg0=0x3/0xf,reg4=0/0x100000 actions=resubmit(,EgressMark) 4. table=L3Forwarding, priority=190,ct_state=-rpl+trk,ip,reg0=0x1/0xf actions=mod_dl_dst:d2:35:24:7f:3a:f8,resubmit(,EgressMark) 5. table=L3Forwarding, priority=0 actions=load:0x2->NXM_NX_REG0[4..7],resubmit(,L2ForwardingCalc) ``` Issue antrea-io#3806 is fixed by raising the priority of the legacy flow 1 to normal (200). It is the new flow 1 now. In legacy flow 2, packets of Service connections with unknown destination can be either sourced from local Pods or Antrea gateway. In the new flow 2, only the packets of Service connections sourced Antrea gateway are matched. Packets of Service connections sourced from local Pods and destined for unknown destination (external network) are matched by the new flow 5. Packets of connections sourced from local Pods and destined for external network are matched by the new flow 3 (Egress is enabled) or the new flow 5 (Egress is disabled). Packets of connections sourced from local AntreaIPAM Pods (AntreaIPAM is enabled) and destined for external network are matched by the new flow 5. Other modifications: fix some stale comments. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

hongliangl · 2022-06-08T00:19:29Z

/test-all-features-conformance
/test-conformance
/test-e2e
/test-flexible-ipam-e2e
/test-ipv6-conformance
/test-ipv6-e2e
/test-ipv6-networkpolicy
/test-ipv6-only-conformance
/test-ipv6-only-e2e
/test-ipv6-only-networkpolicy
/test-multicluster-e2e
/test-networkpolicy
/test-windows-conformance
/test-windows-e2e
/test-windows-networkpolicy
/test-windows-proxyall-e2e
/test-integration

gran-vmv · 2022-06-08T01:10:42Z

/test-flexible-ipam-e2e

gran-vmv · 2022-06-08T01:13:12Z

Fixed space issue on flexible-ipam testbed. Please ignore TestPrometheus failure on flexible-ipam which will be fixed by #3868

hongliangl · 2022-06-08T02:04:03Z

/test-e2e
/test-ipv6-only-e2e
/test-conformance

hongliangl · 2022-06-08T04:55:29Z

/test-ipv6-only-e2e

hongliangl · 2022-06-08T05:39:15Z

/test-ipv6-only-e2e

hongliangl · 2022-06-08T05:39:26Z

/test-windows-conformance

tnqn · 2022-06-09T04:36:43Z

In legacy flow 2, packets of Service connections with unknown destination
can be either sourced from local Pods or Antrea gateway. In the new flow 2,
only the packets of Service connections sourced Antrea gateway are matched.
Packets of Service connections sourced from local Pods and destined for
unknown destination (external network) are matched by the new flow 5.

Packets of connections sourced from local Pods and destined for external
network are matched by the new flow 3 (Egress is enabled) or the new flow
5 (Egress is disabled).

The last sentence is inaccurate and redundant with the following paragraph?

tnqn · 2022-06-09T05:02:54Z

@hongliangl please backport it to appliable releases

hongliangl · 2022-06-09T09:22:40Z

Thanks for reviewing and merging this PR. I'll backport it to 1.6

hongliangl requested review from gran-vmv, wenyingd and tnqn May 18, 2022 13:50

hongliangl marked this pull request as ready for review May 18, 2022 13:50

gran-vmv previously approved these changes May 19, 2022

View reviewed changes

wenyingd reviewed May 19, 2022

View reviewed changes

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

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

gran-vmv added this to the Antrea v1.7 release milestone May 20, 2022

gran-vmv added the action/backport Indicates a PR that requires backports. label May 20, 2022

hongliangl dismissed gran-vmv’s stale review via 3e60fd5 May 20, 2022 03:40

hongliangl force-pushed the fix-3806 branch 2 times, most recently from 3e60fd5 to 6118729 Compare May 25, 2022 10:55

hongliangl requested a review from wenyingd May 25, 2022 10:56

hongliangl force-pushed the fix-3806 branch from 6118729 to e402225 Compare May 25, 2022 11:51

hongliangl changed the title ~~Add L3ForwardingDefaultTable to forward packets to external network~~ Optimize the flows for forwarding the packets with unknown destination in L3Forwarding May 25, 2022

hongliangl changed the title ~~Optimize the flows for forwarding the packets with unknown destination in L3Forwarding~~ Optimize the flows for forwarding the packets with unknown destination in L3ForwardingTable May 25, 2022

hongliangl force-pushed the fix-3806 branch from e402225 to 3ba7c8f Compare May 25, 2022 11:53

hongliangl requested a review from gran-vmv May 25, 2022 11:53

hongliangl force-pushed the fix-3806 branch from 3ba7c8f to a6e15bf Compare May 25, 2022 15:48

hongliangl requested a review from wenyingd June 7, 2022 03:32

wenyingd previously approved these changes Jun 7, 2022

View reviewed changes

hongliangl added the action/release-note Indicates a PR that should be included in release notes. label Jun 7, 2022

tnqn reviewed Jun 7, 2022

View reviewed changes

hongliangl dismissed wenyingd’s stale review via 8420ebb June 7, 2022 23:55

hongliangl force-pushed the fix-3806 branch 2 times, most recently from 8420ebb to 1f8f8a9 Compare June 8, 2022 00:16

hongliangl force-pushed the fix-3806 branch from 1f8f8a9 to 1b58806 Compare June 8, 2022 00:18

hongliangl requested a review from tnqn June 8, 2022 00:19

tnqn approved these changes Jun 8, 2022

View reviewed changes

wenyingd mentioned this pull request Jun 9, 2022

ipv6 e2e tests keeps failing on TestAntreaPolicy #3842

Closed

tnqn merged commit ea99ee5 into antrea-io:main Jun 9, 2022

hongliangl mentioned this pull request Jun 9, 2022

Automated cherry pick of #3809: Optimize the flows for forwarding the packets with unknown #3878

Merged

hongliangl deleted the fix-3806 branch December 5, 2023 05:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize the flows for forwarding the packets with unknown destination in L3ForwardingTable #3809

Optimize the flows for forwarding the packets with unknown destination in L3ForwardingTable #3809

hongliangl commented May 18, 2022 •

edited

Loading

codecov-commenter commented May 18, 2022 •

edited

Loading

gran-vmv left a comment

gran-vmv commented May 19, 2022

gran-vmv commented May 19, 2022

hongliangl commented May 25, 2022

hongliangl commented May 25, 2022

hongliangl commented May 26, 2022

hongliangl commented May 26, 2022

hongliangl commented May 26, 2022

hongliangl commented May 26, 2022

hongliangl commented May 26, 2022

hongliangl commented May 26, 2022

hongliangl commented May 27, 2022

gran-vmv commented Jun 1, 2022

gran-vmv commented Jun 1, 2022

gran-vmv commented Jun 6, 2022

gran-vmv commented Jun 6, 2022

hongliangl commented Jun 7, 2022

wenyingd left a comment

tnqn left a comment

tnqn Jun 7, 2022

hongliangl Jun 8, 2022

tnqn Jun 7, 2022

hongliangl Jun 8, 2022

hongliangl commented Jun 8, 2022

gran-vmv commented Jun 8, 2022

gran-vmv commented Jun 8, 2022

hongliangl commented Jun 8, 2022

hongliangl commented Jun 8, 2022

hongliangl commented Jun 8, 2022

hongliangl commented Jun 8, 2022

tnqn commented Jun 9, 2022

tnqn commented Jun 9, 2022

hongliangl commented Jun 9, 2022

Optimize the flows for forwarding the packets with unknown destination in L3ForwardingTable #3809

Optimize the flows for forwarding the packets with unknown destination in L3ForwardingTable #3809

Conversation

hongliangl commented May 18, 2022 • edited Loading

codecov-commenter commented May 18, 2022 • edited Loading

Codecov Report

gran-vmv left a comment

Choose a reason for hiding this comment

gran-vmv commented May 19, 2022

gran-vmv commented May 19, 2022

hongliangl commented May 25, 2022

hongliangl commented May 25, 2022

hongliangl commented May 26, 2022

hongliangl commented May 26, 2022

hongliangl commented May 26, 2022

hongliangl commented May 26, 2022

hongliangl commented May 26, 2022

hongliangl commented May 26, 2022

hongliangl commented May 27, 2022

gran-vmv commented Jun 1, 2022

gran-vmv commented Jun 1, 2022

gran-vmv commented Jun 6, 2022

gran-vmv commented Jun 6, 2022

hongliangl commented Jun 7, 2022

wenyingd left a comment

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

tnqn Jun 7, 2022

Choose a reason for hiding this comment

hongliangl Jun 8, 2022

Choose a reason for hiding this comment

tnqn Jun 7, 2022

Choose a reason for hiding this comment

hongliangl Jun 8, 2022

Choose a reason for hiding this comment

hongliangl commented Jun 8, 2022

gran-vmv commented Jun 8, 2022

gran-vmv commented Jun 8, 2022

hongliangl commented Jun 8, 2022

hongliangl commented Jun 8, 2022

hongliangl commented Jun 8, 2022

hongliangl commented Jun 8, 2022

tnqn commented Jun 9, 2022

tnqn commented Jun 9, 2022

hongliangl commented Jun 9, 2022

hongliangl commented May 18, 2022 •

edited

Loading

codecov-commenter commented May 18, 2022 •

edited

Loading