Restore NO_FLOOD to OVS ports after reconnecting the OVS bridge #4654

xliuxu · 2023-02-22T10:21:09Z

The NO_FLOOD configuration is lost when the OVS daemon is restarted. Currently, the only way to recover this configuration is by restarting the agent. This pull request adds logic to recover the configuration when receiving OVS reconnection events.

codecov · 2023-02-22T11:47:36Z

Codecov Report

Merging #4654 (caea932) into main (d5dd02e) will decrease coverage by 0.30%.
The diff coverage is 57.50%.

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

@@            Coverage Diff             @@
##             main    #4654      +/-   ##
==========================================
- Coverage   69.86%   69.57%   -0.30%     
==========================================
  Files         403      402       -1     
  Lines       59778    58421    -1357     
==========================================
- Hits        41765    40644    -1121     
+ Misses      15189    14975     -214     
+ Partials     2824     2802      -22

Flag	Coverage Δ		*Carryforward flag
e2e-tests	`38.31% <57.50%> (-0.02%)`	⬇️
integration-tests	`34.46% <ø> (+0.01%)`	⬆️	Carriedforward from e909b97
kind-e2e-tests	`47.15% <ø> (+0.22%)`	⬆️	Carriedforward from e909b97
unit-tests	`59.79% <ø> (-0.13%)`	⬇️	Carriedforward from e909b97

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

Impacted Files	Coverage Δ
pkg/agent/interfacestore/types.go	`100.00% <ø> (ø)`
...gent/controller/noderoute/node_route_controller.go	`62.36% <57.14%> (-6.23%)`	⬇️
pkg/agent/agent.go	`56.00% <57.57%> (-0.66%)`	⬇️
pkg/agent/util/ethtool/ethtool_linux.go	`0.00% <0.00%> (-70.00%)`	⬇️
...nt/apiserver/handlers/serviceexternalip/handler.go	`29.62% <0.00%> (-22.23%)`	⬇️
pkg/agent/proxy/topology.go	`72.72% <0.00%> (-9.10%)`	⬇️
pkg/agent/controller/egress/egress_controller.go	`75.27% <0.00%> (-8.38%)`	⬇️
pkg/agent/controller/networkpolicy/packetin.go	`70.27% <0.00%> (-6.76%)`	⬇️
pkg/agent/wireguard/client_linux.go	`77.07% <0.00%> (-4.46%)`	⬇️
... and 38 more

xliuxu · 2023-02-22T12:07:24Z

/test-all

hongliangl · 2023-02-23T01:31:53Z

pkg/agent/agent.go

+			} else {
+				klog.InfoS("Set port no-flood success", "PortName", port.Name)
+			}


Suggested change

} else {

klog.InfoS("Set port no-flood success", "PortName", port.Name)

}

}

klog.InfoS("Set port no-flood successfully", "PortName", port.Name)

Fixed. Thanks!

hongliangl · 2023-02-23T01:47:22Z

pkg/agent/agent.go

+			klog.Info("Restoring OF port configs to OVS bridge")
+			err := i.restorePortConfigs()
+			if err != nil {
+				klog.ErrorS(err, "Failed to restore OF port configs")
+			} else {
+				klog.Info("Restore OF port configs completed")
+			}


Suggested change

klog.Info("Restoring OF port configs to OVS bridge")

err := i.restorePortConfigs()

if err != nil {

klog.ErrorS(err, "Failed to restore OF port configs")

} else {

klog.Info("Restore OF port configs completed")

}

klog.InfoS("Restoring OF port configs to OVS bridge")

if err := i.restorePortConfigs(); err != nil {

klog.ErrorS(err, "Failed to restore OF port configs")

} else {

klog.InfoS("Port configs restoration completed")

}

Fixed. Thanks!

hongliangl · 2023-02-23T01:48:27Z

pkg/agent/controller/noderoute/node_route_controller.go

@@ -655,7 +655,6 @@ func getPodCIDRsOnNode(node *corev1.Node) []string {
 func (c *Controller) createIPSecTunnelPort(nodeName string, nodeIP net.IP) (int32, error) {
 	portName := util.GenerateNodeTunnelInterfaceName(nodeName)
 	interfaceConfig, exists := c.interfaceStore.GetNodeTunnelInterface(nodeName)
-


Revert this.

Done. Thanks!

tnqn · 2023-02-27T07:21:57Z

pkg/agent/agent.go

+	ovsCtlClient := ovsctl.NewClient(i.ovsBridge)
+	ovsPorts, err := i.ovsBridgeClient.GetPortList()
+	if err != nil {
+		return fmt.Errorf("failed to list OVS ports: %w", err)
+	}
+	for _, port := range ovsPorts {


don't we already cache all TrafficControl and IPSec interfaces? I think they can be fetched via GetInterfacesByType more efficiently?

Done. Thanks for the suggestion!

tnqn · 2023-02-27T07:29:17Z

pkg/agent/controller/noderoute/node_route_controller.go

@@ -714,6 +717,10 @@ func (c *Controller) createIPSecTunnelPort(nodeName string, nodeIP net.IP) (int3
 		// Let NodeRouteController retry at errors.
 		return 0, fmt.Errorf("failed to get of_port of IPsec tunnel port for Node %s", nodeName)
 	}
+	// Set external_ids to the port for upgrade case.
+	if err := c.ovsBridgeClient.SetPortExternalIDs(portName, ovsExternalIDs); err != nil {


initInterfaceStore has a logic to fill missed AntreaInterfaceTypeKey, I think this could be moved there to unify the logic.

Done. Thanks!

tnqn · 2023-02-27T07:33:36Z

pkg/agent/interfacestore/types.go

@@ -43,6 +43,7 @@ const (
 	AntreaUplink           = "uplink"
 	AntreaHost             = "host"
 	AntreaTrafficControl   = "traffic-control"
+	AntreaIPsec            = "ipsec"


perhaps ipsec-tunnel to be more specific.

xliuxu · 2023-02-28T02:28:25Z

/test-all

The NO_FLOOD configuration is lost when the OVS daemon is restarted. Currently, the only way to recover this configuration is by restarting the agent. This pull request adds logic to recover the configuration when receiving OVS reconnection events. Signed-off-by: Xu Liu <[email protected]>

xliuxu · 2023-02-28T05:54:06Z

/test-e2e

tnqn

LGTM

tnqn · 2023-03-01T04:50:40Z

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

tnqn · 2023-03-01T07:07:08Z

/skip-conformance its failure is because the tested svc IP which was supposed to be unreachable happened to be a real server IP.

…ea-io#4654) The NO_FLOOD configuration is lost when the OVS daemon is restarted. Currently, the only way to recover this configuration is by restarting the agent. This pull request adds logic to recover the configuration when receiving OVS reconnection events. Signed-off-by: Xu Liu <[email protected]>

xliuxu force-pushed the fix_no_flood_bridge_reconnect branch from d35922c to 23f835a Compare February 22, 2023 12:06

xliuxu force-pushed the fix_no_flood_bridge_reconnect branch from 23f835a to 383a006 Compare February 22, 2023 12:08

xliuxu requested review from wenyingd, hongliangl and tnqn February 22, 2023 12:17

xliuxu added the action/backport Indicates a PR that requires backports. label Feb 22, 2023

hongliangl reviewed Feb 23, 2023

View reviewed changes

xliuxu force-pushed the fix_no_flood_bridge_reconnect branch 2 times, most recently from 6a528dc to 8f29ecb Compare February 24, 2023 12:01

tnqn reviewed Feb 27, 2023

View reviewed changes

xliuxu force-pushed the fix_no_flood_bridge_reconnect branch 2 times, most recently from d2472a2 to 35afabf Compare February 27, 2023 13:59

xliuxu force-pushed the fix_no_flood_bridge_reconnect branch from 35afabf to 161f575 Compare February 28, 2023 03:05

xliuxu force-pushed the fix_no_flood_bridge_reconnect branch from 161f575 to fe02848 Compare February 28, 2023 03:54

tnqn approved these changes Mar 1, 2023

View reviewed changes

tnqn added the action/release-note Indicates a PR that should be included in release notes. label Mar 1, 2023

tnqn merged commit 26cecc6 into antrea-io:main Mar 1, 2023

This was referenced Mar 2, 2023

Support ProxyTerminatingEndpoints in AntreaProxy #4607

Merged

antrea-agent DaemonSet not ready consistently when running e2e with all features enabled #4673

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restore NO_FLOOD to OVS ports after reconnecting the OVS bridge #4654

Restore NO_FLOOD to OVS ports after reconnecting the OVS bridge #4654

xliuxu commented Feb 22, 2023

codecov bot commented Feb 22, 2023 •

edited

Loading

xliuxu commented Feb 22, 2023

hongliangl Feb 23, 2023

xliuxu Feb 24, 2023

hongliangl Feb 23, 2023

xliuxu Feb 24, 2023

hongliangl Feb 23, 2023

xliuxu Feb 24, 2023

tnqn Feb 27, 2023

xliuxu Feb 27, 2023

tnqn Feb 27, 2023

xliuxu Feb 27, 2023

tnqn Feb 27, 2023

xliuxu Feb 27, 2023

xliuxu commented Feb 28, 2023

xliuxu commented Feb 28, 2023

tnqn left a comment

tnqn commented Mar 1, 2023

tnqn commented Mar 1, 2023

Restore NO_FLOOD to OVS ports after reconnecting the OVS bridge #4654

Restore NO_FLOOD to OVS ports after reconnecting the OVS bridge #4654

Conversation

xliuxu commented Feb 22, 2023

codecov bot commented Feb 22, 2023 • edited Loading

Codecov Report

xliuxu commented Feb 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xliuxu commented Feb 28, 2023

xliuxu commented Feb 28, 2023

tnqn left a comment

Choose a reason for hiding this comment

tnqn commented Mar 1, 2023

tnqn commented Mar 1, 2023

codecov bot commented Feb 22, 2023 •

edited

Loading