Network: Consider volatile IP in ovnNetworkExternalSubnets (#14344)

This PR updates `ovn.ovnNetworkExternalSubnets` to consider the volatile IP of each OVN network using a given uplink. This is added to prevent the use of one OVN network's volatile IP on another OVN network, namely in the cases of creating network forwards and IP passthrough.
canonical · Oct 30, 2024 · 0cbfbf7 · 0cbfbf7
2 parents 1572ecf + 4d2e505
commit 0cbfbf7
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 58 deletions.
diff --git a/lxd/network/driver_common.go b/lxd/network/driver_common.go
@@ -63,6 +63,7 @@ const (
 	subnetUsageNetworkLoadBalancer
 	subnetUsageInstance
 	subnetUsageProxy
+	subnetUsageVolatileIP
 )
 
 // externalSubnetUsage represents usage of a subnet by a network or NIC.

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
@@ -9,6 +9,7 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"slices"
 	"sort"
 	"strconv"
 	"strings"
@@ -4460,15 +4461,32 @@ func (n *ovn) ovnNetworkExternalSubnets(ovnProjectNetworksWithOurUplink map[stri
 					})
 				}
 
+				subnetSize := 128
+				if keyPrefix == "ipv4" {
+					subnetSize = 32
+				}
+
 				// Find any external subnets used for network SNAT.
 				if netInfo.Config[fmt.Sprintf("%s.nat.address", keyPrefix)] != "" {
 					key := fmt.Sprintf("%s.nat.address", keyPrefix)
 
-					subnetSize := 128
-					if keyPrefix == "ipv4" {
-						subnetSize = 32
+					_, ipNet, err := net.ParseCIDR(fmt.Sprintf("%s/%d", netInfo.Config[key], subnetSize))
+					if err != nil {
+						return nil, fmt.Errorf("Failed parsing %q of %q in project %q: %w", key, netInfo.Name, netProject, err)
 					}
 
+					externalSubnets = append(externalSubnets, externalSubnetUsage{
+						subnet:         *ipNet,
+						networkProject: netProject,
+						networkName:    netInfo.Name,
+						usageType:      subnetUsageNetworkSNAT,
+					})
+				}
+
+				// Find the volatile IP for the network.
+				if netInfo.Config[fmt.Sprintf("volatile.network.%s.address", keyPrefix)] != "" {
+					key := fmt.Sprintf("volatile.network.%s.address", keyPrefix)
+
 					_, ipNet, err := net.ParseCIDR(fmt.Sprintf("%s/%d", netInfo.Config[key], subnetSize))
 					if err != nil {
 						return nil, fmt.Errorf("Failed parsing %q of %q in project %q: %w", key, netInfo.Name, netProject, err)
@@ -4478,7 +4496,7 @@ func (n *ovn) ovnNetworkExternalSubnets(ovnProjectNetworksWithOurUplink map[stri
 						subnet:         *ipNet,
 						networkProject: netProject,
 						networkName:    netInfo.Name,
-						usageType:      subnetUsageNetworkSNAT,
+						usageType:      subnetUsageVolatileIP,
 					})
 				}
 			}
@@ -4775,31 +4793,6 @@ func (n *ovn) ForwardCreate(forward api.NetworkForwardsPost, clientType request.
 			return nil, err
 		}
 
-		externalSubnetsInUse, err := n.getExternalSubnetInUse(n.config["network"])
-		if err != nil {
-			return nil, err
-		}
-
-		checkAddressNotInUse := func(netip *net.IPNet) (bool, error) {
-			// Check the listen address subnet doesn't fall within any existing OVN network external subnets.
-			for _, externalSubnetUser := range externalSubnetsInUse {
-				// Check if usage is from our own network.
-				if externalSubnetUser.networkProject == n.project && externalSubnetUser.networkName == n.name {
-					// Skip checking conflict with our own network's subnet or SNAT address.
-					// But do not allow other conflict with other usage types within our own network.
-					if externalSubnetUser.usageType == subnetUsageNetwork || externalSubnetUser.usageType == subnetUsageNetworkSNAT {
-						continue
-					}
-				}
-
-				if SubnetContains(&externalSubnetUser.subnet, netip) || SubnetContains(netip, &externalSubnetUser.subnet) {
-					return false, nil
-				}
-			}
-
-			return true, nil
-		}
-
 		// We're auto-allocating the external IP address if the given listen address is unspecified.
 		if listenAddressNet.IP.IsUnspecified() {
 			ipVersion := 4
@@ -4810,7 +4803,7 @@ func (n *ovn) ForwardCreate(forward api.NetworkForwardsPost, clientType request.
 			ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 			defer cancel()
 
-			listenAddressNet, err = n.randomExternalAddress(ctx, ipVersion, uplinkRoutes, projectRestrictedSubnets, checkAddressNotInUse)
+			listenAddressNet, err = n.randomExternalAddress(ctx, ipVersion, uplinkRoutes, projectRestrictedSubnets, n.checkAddressNotInUse)
 			if err != nil {
 				return nil, fmt.Errorf("Failed to allocate an IPv%d address: %w", ipVersion, err)
 			}
@@ -4824,7 +4817,7 @@ func (n *ovn) ForwardCreate(forward api.NetworkForwardsPost, clientType request.
 				return nil, err
 			}
 
-			isValid, err := checkAddressNotInUse(listenAddressNet)
+			isValid, err := n.checkAddressNotInUse(listenAddressNet)
 			if err != nil {
 				return nil, err
 			} else if !isValid {
@@ -5150,31 +5143,6 @@ func (n *ovn) LoadBalancerCreate(loadBalancer api.NetworkLoadBalancersPost, clie
 			return nil, err
 		}
 
-		externalSubnetsInUse, err := n.getExternalSubnetInUse(n.config["network"])
-		if err != nil {
-			return nil, err
-		}
-
-		checkAddressNotInUse := func(netip *net.IPNet) (bool, error) {
-			// Check the listen address subnet doesn't fall within any existing OVN network external subnets.
-			for _, externalSubnetUser := range externalSubnetsInUse {
-				// Check if usage is from our own network.
-				if externalSubnetUser.networkProject == n.project && externalSubnetUser.networkName == n.name {
-					// Skip checking conflict with our own network's subnet or SNAT address.
-					// But do not allow other conflict with other usage types within our own network.
-					if externalSubnetUser.usageType == subnetUsageNetwork || externalSubnetUser.usageType == subnetUsageNetworkSNAT {
-						continue
-					}
-				}
-
-				if SubnetContains(&externalSubnetUser.subnet, netip) || SubnetContains(netip, &externalSubnetUser.subnet) {
-					return false, nil
-				}
-			}
-
-			return true, nil
-		}
-
 		// We're auto-allocating the external IP address if the given listen address is unspecified.
 		if listenAddressNet.IP.IsUnspecified() {
 			ipVersion := 4
@@ -5185,7 +5153,7 @@ func (n *ovn) LoadBalancerCreate(loadBalancer api.NetworkLoadBalancersPost, clie
 			ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 			defer cancel()
 
-			listenAddressNet, err = n.randomExternalAddress(ctx, ipVersion, uplinkRoutes, projectRestrictedSubnets, checkAddressNotInUse)
+			listenAddressNet, err = n.randomExternalAddress(ctx, ipVersion, uplinkRoutes, projectRestrictedSubnets, n.checkAddressNotInUse)
 			if err != nil {
 				return nil, fmt.Errorf("Failed to allocate an IPv%d address: %w", ipVersion, err)
 			}
@@ -5199,7 +5167,7 @@ func (n *ovn) LoadBalancerCreate(loadBalancer api.NetworkLoadBalancersPost, clie
 				return nil, err
 			}
 
-			isValid, err := checkAddressNotInUse(listenAddressNet)
+			isValid, err := n.checkAddressNotInUse(listenAddressNet)
 			if err != nil {
 				return nil, err
 			} else if !isValid {
@@ -5954,3 +5922,29 @@ func (n *ovn) forPeers(f func(targetOVNNet *ovn) error) error {
 
 	return nil
 }
+
+// checkAddressNotInUse checks that a given network subnet does not fall within
+// any existing OVN network external subnets on the same uplink.
+func (n *ovn) checkAddressNotInUse(netip *net.IPNet) (bool, error) {
+	externalSubnetsInUse, err := n.getExternalSubnetInUse(n.config["network"])
+	if err != nil {
+		return false, err
+	}
+
+	for _, externalSubnetUser := range externalSubnetsInUse {
+		// Check if usage is from our own network.
+		if externalSubnetUser.networkProject == n.project && externalSubnetUser.networkName == n.name {
+			// Skip checking conflict with our own network's subnet, SNAT address, or volatile IP.
+			// But do not allow other conflict with other usage types within our own network.
+			if slices.Contains([]subnetUsageType{subnetUsageNetwork, subnetUsageNetworkSNAT, subnetUsageVolatileIP}, externalSubnetUser.usageType) {
+				continue
+			}
+		}
+
+		if SubnetContains(&externalSubnetUser.subnet, netip) || SubnetContains(netip, &externalSubnetUser.subnet) {
+			return false, nil
+		}
+	}
+
+	return true, nil
+}