Enable IP forwarding on the Windows bridge local interface #3137

hongliangl · 2021-12-14T10:55:12Z

Traffic from the uplink interface will be output to the bridge local
interface directly. When an external client connects to a LoadBalancer
type Service, and the packets of the connection are routed to the
selected backend Pod via the bridge interface, if we do not enable IP
forwarding on the bridge interface, the connection will be discarded
on the bridge interface as the destination of the connection is not the
Node.

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

codecov-commenter · 2021-12-14T11:11:34Z

Codecov Report

Merging #3137 (3f43b54) into main (a5114e7) will increase coverage by 19.63%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             main    #3137       +/-   ##
===========================================
+ Coverage   40.35%   59.98%   +19.63%     
===========================================
  Files         167      302      +135     
  Lines       20879    37135    +16256     
===========================================
+ Hits         8426    22276    +13850     
- Misses      11632    12994     +1362     
- Partials      821     1865     +1044

Flag	Coverage Δ
e2e-tests	`50.69% <ø> (?)`
kind-e2e-tests	`47.72% <ø> (?)`
unit-tests	`40.38% <ø> (+0.02%)`	⬆️

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

Impacted Files	Coverage Δ
pkg/agent/cniserver/ipam/antrea_ipam_controller.go	`12.97% <0.00%> (-52.41%)`	⬇️
pkg/ipam/poolallocator/allocator.go	`21.28% <0.00%> (-41.14%)`	⬇️
pkg/agent/cniserver/ipam/antrea_ipam.go	`42.16% <0.00%> (-34.31%)`	⬇️
pkg/ipam/ipallocator/allocator.go	`54.22% <0.00%> (-29.99%)`	⬇️
pkg/controller/networkpolicy/endpoint_querier.go	`61.46% <0.00%> (-29.97%)`	⬇️
pkg/controller/egress/controller.go	`61.11% <0.00%> (-27.34%)`	⬇️
pkg/controller/ipam/validate.go	`57.14% <0.00%> (-22.86%)`	⬇️
pkg/agent/util/iptables/lock.go	`60.00% <0.00%> (-21.82%)`	⬇️
pkg/controller/externalippool/validate.go	`55.17% <0.00%> (-21.02%)`	⬇️
pkg/controller/egress/validate.go	`47.54% <0.00%> (-16.91%)`	⬇️
... and 286 more

jianjuns · 2021-12-14T18:55:19Z

pkg/agent/agent_windows.go

@@ -188,6 +188,13 @@ func (i *Initializer) prepareOVSBridge() error {
 		err = nil
 		klog.V(4).Infof("Address: %s already exists when configuring IP on interface %s", uplinkNetConfig.IP.String(), brName)
 	}
+	// Enable bridge local interface forwarding. Since now all traffic from uplink interface is output to bridge local interface
+	// directly instead of being resubmitted to UplinkTable, for connection of Service LoadBalancer, when the client is from


This sentence is too long to read.

How about:
Traffic from the uplink interface will be output to the bridge local interface directly. If we do not enable IP forwarding on the bridge interface, when an external client connects to a LoadBalancer type Service and the selected backend Pod is on a remote Node, the connection will be discarded on the bridge local interface.

Thanks, much better. I changed a little according your version

// Enable IP forwarding on the bridge local interface. Traffic from the uplink interface will be output to the bridge // local interface directly. When an external client connects to a LoadBalancer type Service, and the packets of the // connection is routed to the selected backend Pod via the bridge interface, if we do not enable IP forwarding on // the bridge interface, the connection will be discarded on the bridge interface as the source of the connection // is not the Node.

pkg/agent/agent_windows.go

jianjuns

LGTM.

jianjuns · 2021-12-15T01:53:52Z

pkg/agent/agent_windows.go

@@ -188,6 +188,14 @@ func (i *Initializer) prepareOVSBridge() error {
 		err = nil
 		klog.V(4).Infof("Address: %s already exists when configuring IP on interface %s", uplinkNetConfig.IP.String(), brName)
 	}
+	// Enable IP forwarding on the bridge local interface. Traffic from the uplink interface will be output to the bridge
+	// local interface directly. When an external client connects to a LoadBalancer type Service, and the packets of the


I suggest to break it to two sentences:

When an external client connects to a LoadBalancer type Service, the packets of the connection are routed to the selected backend Pond via the bridge interface; if we do not enable...

Thanks, done.

wenyingd · 2021-12-15T03:33:03Z

pkg/agent/agent_windows.go

+	// Enable IP forwarding on the bridge local interface. Traffic from the uplink interface will be output to the bridge
+	// local interface directly. When an external client connects to a LoadBalancer type Service, and the packets of the
+	// connection are routed to the selected backend Pod via the bridge interface; if we do not enable IP forwarding on
+	// the bridge interface, the connection will be discarded on the bridge interface as the source of the connection is


the connection will be discarded on the bridge interface as the destination of the connection is not the Node/

You are right. The reason is that the destination of the connection is not the Node, if IP forwarding is not enabled, the connection will be discarded.

tnqn

I have few questions:

Is this related to AntreaProxy's enablement?
Does NodePort traffic work without the change?
If we don't have tests to detect the issue, could we add one?

pkg/agent/agent_windows.go

hongliangl · 2021-12-17T06:18:34Z

@tnqn

1 .Is this related to AntreaProxy's enablement?

Yes, if IP forwarding is not enabled, the br-int interface will drop the packets whose destination is not the Node, as a result, LB connections (destination is external IP) from external network will be discarded on Nodes.

Does NodePort traffic work without the change?

No, NodePort traffic doesn't need this as the destination IP of NodePort connections is the Node.

If we don't have tests to detect the issue, could we add one?

I think we can add one like egress e2e test. We can add such a case in PR #2950.

wenyingd

LGTM

hongliangl · 2022-01-19T09:02:30Z

/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

Traffic from the uplink interface will be output to the bridge local interface directly. When an external client connects to a LoadBalancer type Service, and the packets of the connection are routed to the selected backend Pod via the bridge interface; if we do not enable IP forwarding on the bridge interface, the connection will be discarded on the bridge interface as the destination of the connection is not the Node. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

tnqn · 2022-02-23T16:57:41Z

/skip-all
/test-windows-all

tnqn · 2022-02-24T02:07:49Z

/skip-all
/test-windows-all

hongliangl · 2022-02-24T07:14:12Z

/test-windows-all

hongliangl · 2022-02-24T08:49:21Z

/test-windows-all

hongliangl · 2022-02-24T14:43:08Z

/test-windows-networkpolicy

tnqn · 2022-02-25T11:34:41Z

/test-windows-networkpolicy

…#3137) Traffic from the uplink interface will be output to the bridge local interface directly. When an external client connects to a LoadBalancer type Service, and the packets of the connection are routed to the selected backend Pod via the bridge interface; if we do not enable IP forwarding on the bridge interface, the connection will be discarded on the bridge interface as the destination of the connection is not the Node. Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

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

hongliangl requested review from antoninbas, jianjuns, tnqn and wenyingd December 14, 2021 14:30

jianjuns reviewed Dec 14, 2021

View reviewed changes

hongliangl force-pushed the windows-br-int-enable-forwarding branch from abe4c5b to 69c4e47 Compare December 15, 2021 01:49

hongliangl changed the title ~~Enable Windows bridge local interface forwarding~~ Enable Windows bridge local interface IP forwarding Dec 15, 2021

hongliangl changed the title ~~Enable Windows bridge local interface IP forwarding~~ Enable IP forwarding on the Windows bridge local interface. Dec 15, 2021

hongliangl force-pushed the windows-br-int-enable-forwarding branch from 69c4e47 to 1f4b9d0 Compare December 15, 2021 01:50

hongliangl requested a review from jianjuns December 15, 2021 01:52

jianjuns reviewed Dec 15, 2021

View reviewed changes

hongliangl force-pushed the windows-br-int-enable-forwarding branch from 1f4b9d0 to ee59bc9 Compare December 15, 2021 01:57

hongliangl requested a review from jianjuns December 15, 2021 02:09

hongliangl changed the title ~~Enable IP forwarding on the Windows bridge local interface.~~ Enable IP forwarding on the Windows bridge local interface Dec 15, 2021

wenyingd reviewed Dec 15, 2021

View reviewed changes

hongliangl force-pushed the windows-br-int-enable-forwarding branch from ee59bc9 to 53fbae7 Compare December 15, 2021 03:44

tnqn reviewed Dec 16, 2021

View reviewed changes

pkg/agent/agent_windows.go Outdated Show resolved Hide resolved

wenyingd previously approved these changes Jan 11, 2022

View reviewed changes

hongliangl mentioned this pull request Jan 25, 2022

Remove unused OF tables and flows on Windows #3138

Merged

hongliangl dismissed wenyingd’s stale review via d1f6d96 January 25, 2022 13:29

hongliangl force-pushed the windows-br-int-enable-forwarding branch from 3f43b54 to d1f6d96 Compare January 25, 2022 13:29

tnqn approved these changes Feb 23, 2022

View reviewed changes

tnqn merged commit 74a05c5 into antrea-io:main Feb 25, 2022

hongliangl added a commit to hongliangl/antrea that referenced this pull request Mar 21, 2022

Remove modify antrea-io#3137

16f813d

hongliangl added a commit to hongliangl/antrea that referenced this pull request Mar 21, 2022

Remove modification antrea-io#3137

020c35b

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

hongliangl mentioned this pull request Mar 22, 2022

Received duplicate echo reply packets when pinging Windows hosts #3505

Closed

hongliangl deleted the windows-br-int-enable-forwarding branch April 21, 2022 01:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable IP forwarding on the Windows bridge local interface #3137

Enable IP forwarding on the Windows bridge local interface #3137

hongliangl commented Dec 14, 2021 •

edited

codecov-commenter commented Dec 14, 2021 •

edited

jianjuns Dec 14, 2021

hongliangl Dec 15, 2021 •

edited

jianjuns left a comment

jianjuns Dec 15, 2021

hongliangl Dec 15, 2021

wenyingd Dec 15, 2021

hongliangl Dec 15, 2021

tnqn left a comment

hongliangl commented Dec 17, 2021 •

edited

wenyingd left a comment

hongliangl commented Jan 19, 2022

tnqn commented Feb 23, 2022

tnqn commented Feb 24, 2022

hongliangl commented Feb 24, 2022

hongliangl commented Feb 24, 2022

hongliangl commented Feb 24, 2022

tnqn commented Feb 25, 2022

Enable IP forwarding on the Windows bridge local interface #3137

Enable IP forwarding on the Windows bridge local interface #3137

Conversation

hongliangl commented Dec 14, 2021 • edited

codecov-commenter commented Dec 14, 2021 • edited

Codecov Report

jianjuns Dec 14, 2021

Choose a reason for hiding this comment

hongliangl Dec 15, 2021 • edited

Choose a reason for hiding this comment

jianjuns left a comment

Choose a reason for hiding this comment

jianjuns Dec 15, 2021

Choose a reason for hiding this comment

hongliangl Dec 15, 2021

Choose a reason for hiding this comment

wenyingd Dec 15, 2021

Choose a reason for hiding this comment

hongliangl Dec 15, 2021

Choose a reason for hiding this comment

tnqn left a comment

Choose a reason for hiding this comment

hongliangl commented Dec 17, 2021 • edited

wenyingd left a comment

Choose a reason for hiding this comment

hongliangl commented Jan 19, 2022

tnqn commented Feb 23, 2022

tnqn commented Feb 24, 2022

hongliangl commented Feb 24, 2022

hongliangl commented Feb 24, 2022

hongliangl commented Feb 24, 2022

tnqn commented Feb 25, 2022

hongliangl commented Dec 14, 2021 •

edited

codecov-commenter commented Dec 14, 2021 •

edited

hongliangl Dec 15, 2021 •

edited

hongliangl commented Dec 17, 2021 •

edited