Merge pull request #5263 from jcaamano/fix-ofmanager-concurrent

Fix panic/race around openflowmanager

Author: trozet

go-controller/pkg/node/gateway_shared_intf.go

@@ -422,7 +422,7 @@ func (npw *nodePortWatcher) updateServiceFlowCache(service *corev1.Service, netI
 				ipPrefix = "ipv6"
 			}
 			// table 2, user-defined network host -> OVN towards default cluster network services
-			defaultNetConfig := npw.ofm.defaultBridge.getActiveNetworkBridgeConfig(types.DefaultNetworkName)
+			defaultNetConfig := npw.ofm.defaultBridge.getActiveNetworkBridgeConfigCopy(types.DefaultNetworkName)
 			// sample flow: cookie=0xdeff105, duration=2319.685s, table=2, n_packets=496, n_bytes=67111, priority=300,
 			//              ip,nw_dst=10.96.0.1 actions=mod_dl_dst:02:42:ac:12:00:03,output:"patch-breth0_ov"
 			// This flow is used for UDNs and advertised UDNs to be able to reach kapi and dns services alone on default network

go-controller/pkg/node/gateway_udn.go

@@ -146,12 +146,18 @@ func (b *bridgeConfiguration) delNetworkBridgeConfig(nInfo util.NetInfo) {
 	delete(b.netConfig, nInfo.GetNetworkName())
 }
 
-// getActiveNetworkBridgeConfig returns a shallow copy of the network configuration corresponding to the
+func (b *bridgeConfiguration) getNetworkBridgeConfig(networkName string) *bridgeUDNConfiguration {
+	b.Lock()
+	defer b.Unlock()
+	return b.netConfig[networkName]
+}
+
+// getActiveNetworkBridgeConfigCopy returns a shallow copy of the network configuration corresponding to the
 // provided netInfo.
 //
 // NOTE: if the network configuration can't be found or if the network is not patched by OVN
 // yet this returns nil.
-func (b *bridgeConfiguration) getActiveNetworkBridgeConfig(networkName string) *bridgeUDNConfiguration {
+func (b *bridgeConfiguration) getActiveNetworkBridgeConfigCopy(networkName string) *bridgeUDNConfiguration {
 	b.Lock()
 	defer b.Unlock()
 
@@ -917,9 +923,18 @@ func (udng *UserDefinedNetworkGateway) Reconcile() {
 func (udng *UserDefinedNetworkGateway) doReconcile() error {
 	klog.Infof("Reconciling gateway with updates for UDN %s", udng.GetNetworkName())
 
+	// shouldn't happen
+	if udng.openflowManager == nil || udng.openflowManager.defaultBridge == nil {
+		return fmt.Errorf("openflow manager with default bridge configuration has not been provided for network %s", udng.GetNetworkName())
+	}
+
 	// update bridge configuration
 	isNetworkAdvertised := util.IsPodNetworkAdvertisedAtNode(udng.NetInfo, udng.node.Name)
-	udng.openflowManager.defaultBridge.netConfig[udng.GetNetworkName()].advertised.Store(isNetworkAdvertised)
+	netConfig := udng.openflowManager.defaultBridge.getNetworkBridgeConfig(udng.GetNetworkName())
+	if netConfig == nil {
+		return fmt.Errorf("missing bridge configuration for network %s", udng.GetNetworkName())
+	}
+	netConfig.advertised.Store(isNetworkAdvertised)
 
 	if err := udng.updateUDNVRFIPRules(isNetworkAdvertised); err != nil {
 		return fmt.Errorf("error while updating ip rule for UDN %s: %s", udng.GetNetworkName(), err)

go-controller/pkg/node/openflow_manager.go

@@ -59,7 +59,7 @@ func (c *openflowManager) delNetwork(nInfo util.NetInfo) {
 }
 
 func (c *openflowManager) getActiveNetwork(nInfo util.NetInfo) *bridgeUDNConfiguration {
-	return c.defaultBridge.getActiveNetworkBridgeConfig(nInfo.GetNetworkName())
+	return c.defaultBridge.getActiveNetworkBridgeConfigCopy(nInfo.GetNetworkName())
 }
 
 // END UDN UTILs
@@ -211,9 +211,15 @@ func (c *openflowManager) Run(stopChan <-chan struct{}, doneWg *sync.WaitGroup)
 }
 
 func (c *openflowManager) updateBridgePMTUDFlowCache(key string, ipAddrs []string) {
+	// protect defaultBridge config from being updated by gw.nodeIPManager
+	c.defaultBridge.Lock()
+	defer c.defaultBridge.Unlock()
+
 	dftFlows := pmtudDropFlows(c.defaultBridge, ipAddrs)
 	c.updateFlowCacheEntry(key, dftFlows)
 	if c.externalGatewayBridge != nil {
+		c.externalGatewayBridge.Lock()
+		defer c.externalGatewayBridge.Unlock()
 		exGWBridgeDftFlows := pmtudDropFlows(c.externalGatewayBridge, ipAddrs)
 		c.updateExBridgeFlowCacheEntry(key, exGWBridgeDftFlows)
 	}