Fix missing ICMP SNAT for L2 UDNs GR

On GRs we have an SNAT from the join port IP address to the external IP
address for ICMP needs frag sent by OVN to the external network.

In L2 UDNs, even though there is no join network, the internal port has
two IPs, one for the pod subnet and another for the join subnet. The
ICMP source address will be the first one in lexicographical order which
might be the pod subnet one.

The proper SNAT for the GR pod subnet address gets created in the gateway
initialization. But:
1 when DisableSNATMultipleGWs=false, it is redundant because there is
  already a SNAT for the whole pod subnet
2 when DisableSNATMultipleGWs=true we end up with no SNAT because
  cleanupStalePodSNATs cleans it up becaause it never expected that
  SNAT

This commit fixes #2

Signed-off-by: Jaime Caamaño Ruiz <[email protected]>

go-controller/pkg/ovn/gateway.go

@@ -155,7 +155,7 @@ func WithLoadBalancerGroups(routerLBGroup, clusterLBGroup, switchLBGroup string)
 // NOTE2: egressIP SNATs are synced in EIP controller.
 // TODO (tssurya): Add support cleaning up even if disableSNATMultipleGWs=false, we'd need to remove the perPod
 // SNATs in case someone switches between these modes. See https://github.com/ovn-org/ovn-kubernetes/issues/3232
-func (gw *GatewayManager) cleanupStalePodSNATs(nodeName string, nodeIPs []*net.IPNet) error {
+func (gw *GatewayManager) cleanupStalePodSNATs(nodeName string, nodeIPs []*net.IPNet, gwLRPIPs []net.IP) error {
 	if !config.Gateway.DisableSNATMultipleGWs {
 		return nil
 	}
@@ -209,6 +209,7 @@ func (gw *GatewayManager) cleanupStalePodSNATs(nodeName string, nodeIPs []*net.I
 	}
 
 	nodeIPset := sets.New(util.IPNetsIPToStringSlice(nodeIPs)...)
+	gwLRPIPset := sets.New(util.StringSlice(gwLRPIPs)...)
 	natsToDelete := []*nbdb.NAT{}
 	for _, routerNat := range routerNats {
 		routerNat := routerNat
@@ -221,14 +222,14 @@ func (gw *GatewayManager) cleanupStalePodSNATs(nodeName string, nodeIPs []*net.I
 		if podIPsOnNode.Has(routerNat.LogicalIP) {
 			continue
 		}
+		if gwLRPIPset.Has(routerNat.LogicalIP) {
+			continue
+		}
 		logicalIP := net.ParseIP(routerNat.LogicalIP)
 		if logicalIP == nil {
 			// this is probably a CIDR so not a pod IP
 			continue
 		}
-		if gw.containsJoinIP(logicalIP) {
-			continue
-		}
 		natsToDelete = append(natsToDelete, routerNat)
 	}
 	if len(natsToDelete) > 0 {
@@ -748,7 +749,7 @@ func (gw *GatewayManager) GatewayInit(
 		}
 	}
 
-	if err := gw.cleanupStalePodSNATs(nodeName, l3GatewayConfig.IPAddresses); err != nil {
+	if err := gw.cleanupStalePodSNATs(nodeName, l3GatewayConfig.IPAddresses, gwLRPIPs); err != nil {
 		return fmt.Errorf("failed to sync stale SNATs on node %s: %v", nodeName, err)
 	}
 

go-controller/pkg/ovn/secondary_layer2_network_controller_test.go

@@ -425,7 +425,7 @@ func expectedLayer2EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC
 
 	var nat []string
 	if config.Gateway.DisableSNATMultipleGWs {
-		nat = append(nat, nat1, perPodSNAT, masqSNATUUID1)
+		nat = append(nat, nat1, nat3, perPodSNAT, masqSNATUUID1)
 	} else {
 		nat = append(nat, nat1, nat2, nat3, masqSNATUUID1)
 	}
@@ -464,6 +464,7 @@ func expectedLayer2EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC
 	expectedEntities = append(expectedEntities, expectedExternalSwitchAndLSPs(netInfo, gwConfig, nodeName)...)
 	if config.Gateway.DisableSNATMultipleGWs {
 		expectedEntities = append(expectedEntities, newNATEntry(nat1, dummyMasqueradeIP().IP.String(), gwRouterJoinIPAddress().IP.String(), standardNonDefaultNetworkExtIDs(netInfo)))
+		expectedEntities = append(expectedEntities, newNATEntry(nat3, dummyMasqueradeIP().IP.String(), layer2SubnetGWAddr().IP.String(), standardNonDefaultNetworkExtIDs(netInfo)))
 		expectedEntities = append(expectedEntities, newNATEntry(perPodSNAT, dummyMasqueradeIP().IP.String(), dummyL2TestPodAdditionalNetworkIP(), nil))
 	} else {
 		expectedEntities = append(expectedEntities, newNATEntry(nat1, dummyMasqueradeIP().IP.String(), gwRouterJoinIPAddress().IP.String(), standardNonDefaultNetworkExtIDs(netInfo)))