Set NO_FLOOD to IPsec tunnel ports #4419

xliuxu · 2022-11-29T13:44:37Z

Set NO_FLOOD to IPsec tunnel ports to avoid ARP flooding.

codecov · 2022-11-29T14:03:16Z

Codecov Report

Merging #4419 (ce5ae9d) into main (a63314f) will increase coverage by 0.08%.
The diff coverage is 50.00%.

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

@@            Coverage Diff             @@
##             main    #4419      +/-   ##
==========================================
+ Coverage   69.58%   69.67%   +0.08%     
==========================================
  Files         400      400              
  Lines       59122    58369     -753     
==========================================
- Hits        41141    40666     -475     
+ Misses      15175    14914     -261     
+ Partials     2806     2789      -17

Flag	Coverage Δ		*Carryforward flag
e2e-tests	`38.28% <25.00%> (-0.06%)`	⬇️
integration-tests	`34.48% <ø> (ø)`		Carriedforward from a63314f
kind-e2e-tests	`47.68% <25.00%> (ø)`		Carriedforward from a63314f
unit-tests	`59.61% <60.00%> (ø)`		Carriedforward from a63314f

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files	Coverage Δ
cmd/antrea-agent/agent.go	`0.00% <0.00%> (ø)`
...gent/controller/noderoute/node_route_controller.go	`67.98% <60.00%> (+4.78%)`	⬆️
pkg/agent/wireguard/client_linux.go	`77.07% <0.00%> (-4.46%)`	⬇️
pkg/agent/util/net_linux.go	`31.27% <0.00%> (-1.70%)`	⬇️
...kg/apiserver/registry/system/supportbundle/rest.go	`83.24% <0.00%> (-1.53%)`	⬇️
pkg/agent/openflow/network_policy.go	`79.83% <0.00%> (-1.46%)`	⬇️
pkg/agent/agent.go	`56.96% <0.00%> (+0.09%)`	⬆️
pkg/agent/config/node_config.go	`94.87% <0.00%> (+1.25%)`	⬆️
pkg/agent/controller/networkpolicy/cache.go	`86.79% <0.00%> (+1.35%)`	⬆️
... and 9 more

tnqn · 2022-11-29T15:39:50Z

pkg/agent/controller/noderoute/node_route_controller.go

+	// Set the port with no-flood to reject ARP flood packets.
+	if err := c.ovsCtlClient.SetPortNoFlood(int(ofPort)); err != nil {
+		return 0, fmt.Errorf("failed to set port %s with no-flood config: %w", portName, err)
+	}


Want to understand the issue: the normal tunnel port doesn't need setting it because "normal" action doesn't flood broadcast packets to flow based tunnel ports but will flood to ipsec tunnels because they are port based?

Yes, the general tunnel port is flow based, and it needs flow to tell it how to encap the tunnel header on a packet. For ARP, we do not have a flow to explicitly set tht tunnel peer, that is why we do not see ARP packets are broadcast to all other Nodes. But for IPSec tunnel, one port is created per peer Node, so when ARP packet outputs ( via normal action) to the port, OVS datapath is able to send it to peer Node. Then the packet hits the "normal" flow on peer Node, and continue flooding.

wenyingd

LGTM

tnqn · 2022-12-01T11:01:06Z

@xliuxu I suppose this should be backported?

tnqn · 2022-12-01T11:01:23Z

/test-all

xliuxu · 2022-12-01T11:02:34Z

@xliuxu I suppose this should be backported?

I think so. I will backport this PR once merged.

xliuxu · 2022-12-01T13:06:48Z

/test-e2e

tnqn · 2022-12-01T15:59:08Z

It failed on testIPSecTunnelConnectivity twice. Is it related to the change?

xliuxu · 2022-12-02T01:38:01Z

/test-e2e

xliuxu · 2022-12-02T01:39:33Z

It failed on testIPSecTunnelConnectivity twice. Is it related to the change?

I cannot reproduce the same error locally. I will trigger a new e2e test and check the results on the testbed if it still fails.

tnqn · 2022-12-02T02:04:51Z

It failed on testIPSecTunnelConnectivity twice. Is it related to the change?

I cannot reproduce the same error locally. I will trigger a new e2e test and check the results on the testbed if it still fails.

Could you check the previous failures too? Even if the 3rd succeeds, we need to understand the failures as they are frequent.

xliuxu · 2022-12-02T16:09:17Z

It failed on testIPSecTunnelConnectivity twice. Is it related to the change?

I cannot reproduce the same error locally. I will trigger a new e2e test and check the results on the testbed if it still fails.

Could you check the previous failures too? Even if the 3rd succeeds, we need to understand the failures as they are frequent.

I cannot reproduce the error in the testbeds. I will trigger more e2e tests to check.
/test-e2e

xliuxu · 2022-12-05T01:46:45Z

/test-latest-e2e

xliuxu · 2022-12-05T02:25:23Z

/test-e2e

xliuxu · 2022-12-05T03:23:21Z

/test-e2e

xliuxu · 2022-12-05T04:41:10Z

/test-e2e

xliuxu · 2022-12-05T06:19:56Z

/test-e2e

xliuxu · 2022-12-05T07:22:34Z

/test-e2e

xliuxu · 2022-12-05T08:19:34Z

/test-e2e

xliuxu · 2022-12-05T09:01:05Z

/test-e2e

xliuxu · 2022-12-05T11:07:58Z

/test-e2e

xliuxu · 2022-12-05T23:51:11Z

/test-e2e

xliuxu · 2022-12-06T01:59:04Z

/test-e2e

xliuxu · 2022-12-14T02:09:39Z

/test-e2e

xliuxu · 2022-12-19T01:01:22Z

/test-e2e

xliuxu · 2022-12-19T07:25:50Z

/test-e2e

xliuxu · 2022-12-19T11:36:42Z

/test-e2e

xliuxu · 2022-12-19T12:20:08Z

/test-e2e

xliuxu · 2022-12-23T01:46:03Z

/test-e2e

xliuxu · 2022-12-23T02:45:54Z

/test-e2e

Set NO_FLOOD to IPsec tunnel ports to avoid ARP flooding. Signed-off-by: Xu Liu <xliu2@vmware.com>

xliuxu · 2023-02-08T02:56:39Z

/test-e2e

xliuxu · 2023-02-08T07:27:19Z

/test-all

xliuxu · 2023-02-08T11:12:30Z

@tnqn The flaky e2e of this PR should be the same as #4477 but I did not find the root cause. I can tell that the configs of strongswan and the xfrm policies are configured correctly when the error occurred. I suspect this could be related to the underlying testbed and it is now not reoccurring. Do you think we can merge this PR?

tnqn · 2023-02-08T14:20:00Z

@tnqn The flaky e2e of this PR should be the same as #4477 but I did not find the root cause. I can tell that the configs of strongswan and the xfrm policies are configured correctly when the error occurred. I suspect this could be related to the underlying testbed and it is now not reoccurring. Do you think we can merge this PR?

Got it. We could revisit it when it happens again.

xliuxu requested review from wenyingd and tnqn November 29, 2022 13:45

tnqn reviewed Nov 29, 2022

View reviewed changes

wenyingd previously approved these changes Nov 30, 2022

View reviewed changes

tnqn added action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. labels Dec 1, 2022

xliuxu dismissed wenyingd’s stale review via f51d79d December 5, 2022 02:23

xliuxu force-pushed the fix_ipsec_arp_flood branch from 11025af to f51d79d Compare December 5, 2022 02:23

xliuxu force-pushed the fix_ipsec_arp_flood branch from f51d79d to b6fdfce Compare December 5, 2022 03:21

xliuxu force-pushed the fix_ipsec_arp_flood branch from b6fdfce to 5953341 Compare December 5, 2022 07:22

xliuxu force-pushed the fix_ipsec_arp_flood branch from 5953341 to 8c23549 Compare December 14, 2022 02:08

xliuxu mentioned this pull request Dec 14, 2022

TestIPSec/testIPSecPSKAuth/testIPSecTunnelConnectivity is flaky #4477

Closed

xliuxu force-pushed the fix_ipsec_arp_flood branch from 76bc543 to 82be664 Compare December 19, 2022 11:36

xliuxu force-pushed the fix_ipsec_arp_flood branch from 82be664 to f80665a Compare December 19, 2022 12:19

xliuxu changed the title ~~Set NO_FLOOD to IPsec tunnel ports~~ [WIP] Set NO_FLOOD to IPsec tunnel ports Dec 19, 2022

Set NO_FLOOD to IPsec tunnel ports

ecf67b4

Set NO_FLOOD to IPsec tunnel ports to avoid ARP flooding. Signed-off-by: Xu Liu <xliu2@vmware.com>

xliuxu force-pushed the fix_ipsec_arp_flood branch from f80665a to ecf67b4 Compare February 8, 2023 02:43

xliuxu changed the title ~~[WIP] Set NO_FLOOD to IPsec tunnel ports~~ Set NO_FLOOD to IPsec tunnel ports Feb 8, 2023

tnqn approved these changes Feb 8, 2023

View reviewed changes

tnqn merged commit 6d76ddf into antrea-io:main Feb 8, 2023

Set NO_FLOOD to IPsec tunnel ports #4419

Set NO_FLOOD to IPsec tunnel ports #4419

Conversation

xliuxu commented Nov 29, 2022

codecov bot commented Nov 29, 2022 • edited Loading

Codecov Report

tnqn Nov 29, 2022

Choose a reason for hiding this comment

wenyingd Nov 30, 2022

Choose a reason for hiding this comment

wenyingd left a comment

Choose a reason for hiding this comment

tnqn commented Dec 1, 2022

tnqn commented Dec 1, 2022

xliuxu commented Dec 1, 2022

xliuxu commented Dec 1, 2022

tnqn commented Dec 1, 2022

xliuxu commented Dec 2, 2022

xliuxu commented Dec 2, 2022

tnqn commented Dec 2, 2022

xliuxu commented Dec 2, 2022

xliuxu commented Dec 5, 2022

xliuxu commented Dec 5, 2022

xliuxu commented Dec 5, 2022

xliuxu commented Dec 5, 2022

xliuxu commented Dec 5, 2022

xliuxu commented Dec 5, 2022

xliuxu commented Dec 5, 2022

xliuxu commented Dec 5, 2022

xliuxu commented Dec 5, 2022

xliuxu commented Dec 5, 2022

xliuxu commented Dec 6, 2022

xliuxu commented Dec 14, 2022

xliuxu commented Dec 19, 2022

xliuxu commented Dec 19, 2022

xliuxu commented Dec 19, 2022

xliuxu commented Dec 19, 2022

xliuxu commented Dec 23, 2022

xliuxu commented Dec 23, 2022

xliuxu commented Feb 8, 2023

xliuxu commented Feb 8, 2023

xliuxu commented Feb 8, 2023

tnqn commented Feb 8, 2023

codecov bot commented Nov 29, 2022 •

edited

Loading