[lxc-devel] [lxd/master] Network: Removes OVN ipv4.routes.external and ipv6.routes.external

tomponline on Github lxc-bot at linuxcontainers.org
Mon Oct 19 16:13:08 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 337 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20201019/7d86b12a/attachment.bin>
-------------- next part --------------
From 71caae866af04c142dbb48cc19a4d7a126e3657a Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 15:56:33 +0100
Subject: [PATCH 01/15] lxd/network/driver/ovn: Only call Validate in
 FillConfig if state is set

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index ea1616d269..cfc5e7fff2 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -1168,7 +1168,7 @@ func (n *ovn) FillConfig(config map[string]string) error {
 		changedConfig = true
 	}
 
-	if changedConfig {
+	if changedConfig && n.state != nil {
 		return n.Validate(config)
 	}
 

From f6add9084341a88c0847398eb4db5610e7e5f1bd Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 15:56:15 +0100
Subject: [PATCH 02/15] lxd/db/projects: Adds GetProject function

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/db/projects.go | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/lxd/db/projects.go b/lxd/db/projects.go
index cf03765ea5..ff613611d1 100644
--- a/lxd/db/projects.go
+++ b/lxd/db/projects.go
@@ -189,3 +189,22 @@ func (c *ClusterTx) InitProjectWithoutImages(project string) error {
 	_, err = c.tx.Exec(stmt, defaultProfileID)
 	return err
 }
+
+// GetProject returns the project with the given key.
+func (c *Cluster) GetProject(projectName string) (*api.Project, error) {
+	var err error
+	var p *api.Project
+	err = c.Transaction(func(tx *ClusterTx) error {
+		p, err = tx.GetProject(projectName)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return p, nil
+}

From fa2da560919d515ecd24a2aebc40b4a213c34fea Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 13:56:53 +0100
Subject: [PATCH 03/15] lxd/network/driver/ovn: Converts instance port
 functions to exported

So they can be accessed by OVN NIC directly.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index cfc5e7fff2..25780813e1 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -1887,8 +1887,8 @@ func (n *ovn) getInstanceDevicePortName(instanceID int, deviceName string) openv
 	return openvswitch.OVNSwitchPort(fmt.Sprintf("%s-%d-%s", n.getIntSwitchInstancePortPrefix(), instanceID, deviceName))
 }
 
-// instanceDevicePortAdd adds an instance device port to the internal logical switch and returns the port name.
-func (n *ovn) instanceDevicePortAdd(instanceID int, instanceName string, deviceName string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error) {
+// InstanceDevicePortAdd adds an instance device port to the internal logical switch and returns the port name.
+func (n *ovn) InstanceDevicePortAdd(instanceID int, instanceName string, deviceName string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error) {
 	var dhcpV4ID, dhcpv6ID string
 
 	revert := revert.New()
@@ -2065,8 +2065,8 @@ func (n *ovn) instanceDevicePortAdd(instanceID int, instanceName string, deviceN
 	return instancePortName, nil
 }
 
-// instanceDevicePortIPs returns the dynamically allocated IPs for a device port.
-func (n *ovn) instanceDevicePortDynamicIPs(instanceID int, deviceName string) ([]net.IP, error) {
+// InstanceDevicePortDynamicIPs returns the dynamically allocated IPs for a device port.
+func (n *ovn) InstanceDevicePortDynamicIPs(instanceID int, deviceName string) ([]net.IP, error) {
 	instancePortName := n.getInstanceDevicePortName(instanceID, deviceName)
 
 	client, err := n.getClient()
@@ -2077,8 +2077,8 @@ func (n *ovn) instanceDevicePortDynamicIPs(instanceID int, deviceName string) ([
 	return client.LogicalSwitchPortDynamicIPs(instancePortName)
 }
 
-// instanceDevicePortDelete deletes an instance device port from the internal logical switch.
-func (n *ovn) instanceDevicePortDelete(instanceID int, deviceName string, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) error {
+// InstanceDevicePortDelete deletes an instance device port from the internal logical switch.
+func (n *ovn) InstanceDevicePortDelete(instanceID int, deviceName string, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) error {
 	instancePortName := n.getInstanceDevicePortName(instanceID, deviceName)
 
 	client, err := n.getClient()

From d5351219834cbb9bb12faba3cf653f0751765898 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 14:02:16 +0100
Subject: [PATCH 04/15] lxd/network/driver/ovn: Removes ipv4.routes.external
 and ipv6.routes.external

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 34 +++++-----------------------------
 1 file changed, 5 insertions(+), 29 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 25780813e1..91463e5477 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -133,13 +133,11 @@ func (n *ovn) Validate(config map[string]string) error {
 
 			return validate.Optional(validate.IsNetworkAddressCIDRV6)(value)
 		},
-		"ipv6.dhcp.stateful":   validate.Optional(validate.IsBool),
-		"ipv4.routes.external": validate.Optional(validate.IsNetworkV4List),
-		"ipv6.routes.external": validate.Optional(validate.IsNetworkV6List),
-		"ipv4.nat":             validate.Optional(validate.IsBool),
-		"ipv6.nat":             validate.Optional(validate.IsBool),
-		"dns.domain":           validate.IsAny,
-		"dns.search":           validate.IsAny,
+		"ipv6.dhcp.stateful": validate.Optional(validate.IsBool),
+		"ipv4.nat":           validate.Optional(validate.IsBool),
+		"ipv6.nat":           validate.Optional(validate.IsBool),
+		"dns.domain":         validate.IsAny,
+		"dns.search":         validate.IsAny,
 
 		// Volatile keys populated automatically as needed.
 		ovnVolatileUplinkIPv4: validate.Optional(validate.IsNetworkAddressV4),
@@ -232,28 +230,6 @@ func (n *ovn) Validate(config map[string]string) error {
 		}
 	}
 
-	// Check IP external routes are within the uplink network's routes and project's subnet restrictions.
-	if config["ipv4.routes.external"] != "" || config["ipv6.routes.external"] != "" {
-		// Parse and validate our external routes.
-		for _, k := range []string{"ipv4.routes.external", "ipv6.routes.external"} {
-			if config[k] == "" {
-				continue
-			}
-
-			routeSubnetList, err := SubnetParseAppend([]*net.IPNet{}, strings.Split(config[k], ",")...)
-			if err != nil {
-				return err
-			}
-
-			for _, routeSubnet := range routeSubnetList {
-				err = n.validateExternalSubnet(uplinkRoutes, projectRestrictedSubnets, routeSubnet)
-				if err != nil {
-					return err
-				}
-			}
-		}
-	}
-
 	return nil
 }
 

From e1668b6f4f92365854ad48eb4ee243186c87fa15 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 17:00:06 +0100
Subject: [PATCH 05/15] lxc/network/driver/ovn: Adds projectRestrictedSubnets
 and uplinkRoutes functions

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 55 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 91463e5477..1e537c7413 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -78,6 +78,61 @@ func (n *ovn) Info() Info {
 	}
 }
 
+// uplinkRoutes parses ipv4.routes and ipv6.routes settings for a named uplink network into a slice of *net.IPNet.
+func (n *ovn) uplinkRoutes(uplinkNetworkName string) ([]*net.IPNet, error) {
+	_, uplink, err := n.state.Cluster.GetNetworkInAnyState(project.Default, uplinkNetworkName)
+	if err != nil {
+		return nil, err
+	}
+
+	var uplinkRoutes []*net.IPNet
+	for _, k := range []string{"ipv4.routes", "ipv6.routes"} {
+		if uplink.Config[k] == "" {
+			continue
+		}
+
+		uplinkRoutes, err = SubnetParseAppend(uplinkRoutes, strings.Split(uplink.Config[k], ",")...)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return uplinkRoutes, nil
+}
+
+// projectRestrictedSubnets parses the restrict.networks.subnets project setting and returns slice of *net.IPNet.
+// Returns nil slice if no project restrictions, or empty slice if no allowed subnets.
+func (n *ovn) projectRestrictedSubnets(p *api.Project, uplinkNetworkName string) ([]*net.IPNet, error) {
+	// Parse project's restricted subnets.
+	var projectRestrictedSubnets []*net.IPNet // Nil value indicates not restricted.
+	if shared.IsTrue(p.Config["restricted"]) && p.Config["restricted.networks.subnets"] != "" {
+		projectRestrictedSubnets = []*net.IPNet{} // Empty slice indicates no allowed subnets.
+
+		for _, subnetRaw := range strings.Split(p.Config["restricted.networks.subnets"], ",") {
+			subnetParts := strings.SplitN(strings.TrimSpace(subnetRaw), ":", 2)
+			if len(subnetParts) != 2 {
+				return nil, fmt.Errorf(`Project subnet %q invalid, must be in the format of "<uplink network>:<subnet>"`, subnetRaw)
+			}
+
+			subnetUplinkName := subnetParts[0]
+			subnetStr := subnetParts[1]
+
+			if subnetUplinkName != uplinkNetworkName {
+				continue // Only include subnets for our uplink.
+			}
+
+			_, restrictedSubnet, err := net.ParseCIDR(subnetStr)
+			if err != nil {
+				return nil, err
+			}
+
+			projectRestrictedSubnets = append(projectRestrictedSubnets, restrictedSubnet)
+		}
+	}
+
+	return projectRestrictedSubnets, nil
+}
+
 // validateExternalSubnet checks the supplied ipNet is allowed within the uplink routes and project
 // restricted subnets. If projectRestrictedSubnets is nil, then it is not checked as this indicates project has
 // no restrictions. Whereas if uplinkRoutes is nil/empty then this will always return an error.

From bbfce066807e2491abe1bd281cccc0ac6c8f7b26 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 17:00:30 +0100
Subject: [PATCH 06/15] lxd/network/driver/ovn: Simplifies Validate by using
 separate data loader functions

Also removes a duplicate project load query from DB.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 60 ++++++---------------------------------
 1 file changed, 9 insertions(+), 51 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 1e537c7413..8d7b55111f 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -205,69 +205,27 @@ func (n *ovn) Validate(config map[string]string) error {
 	}
 
 	// Load the project to get uplink network restrictions.
-	var projectRow *api.Project
-	err = n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
-		projectRow, err = tx.GetProject(n.project)
-		if err != nil {
-			return err
-		}
-
-		return nil
-	})
+	p, err := n.state.Cluster.GetProject(n.project)
 	if err != nil {
-		return errors.Wrapf(err, "Failed to load IP route restrictions for project %q", n.project)
+		return errors.Wrapf(err, "Failed to load network restrictions from project %q", n.project)
 	}
 
 	// Check uplink network is valid and allowed in project.
-	uplinkNetworkName, err := n.validateUplinkNetwork(config["network"])
+	uplinkNetworkName, err := n.validateUplinkNetwork(p, config["network"])
 	if err != nil {
 		return err
 	}
 
-	// Load the uplink network to get uplink routes.
-	_, uplink, err := n.state.Cluster.GetNetworkInAnyState(project.Default, uplinkNetworkName)
+	// Get uplink routes.
+	uplinkRoutes, err := n.uplinkRoutes(uplinkNetworkName)
 	if err != nil {
 		return err
 	}
 
-	// Parse uplink route subnets.
-	var uplinkRoutes []*net.IPNet
-	for _, k := range []string{"ipv4.routes", "ipv6.routes"} {
-		if uplink.Config[k] == "" {
-			continue
-		}
-
-		uplinkRoutes, err = SubnetParseAppend(uplinkRoutes, strings.Split(uplink.Config[k], ",")...)
-		if err != nil {
-			return err
-		}
-	}
-
-	// Parse project's restricted subnets.
-	var projectRestrictedSubnets []*net.IPNet // Nil value indicates not restricted.
-	if shared.IsTrue(projectRow.Config["restricted"]) && projectRow.Config["restricted.networks.subnets"] != "" {
-		projectRestrictedSubnets = []*net.IPNet{} // Empty slice indicates no allowed subnets.
-
-		for _, subnetRaw := range strings.Split(projectRow.Config["restricted.networks.subnets"], ",") {
-			subnetParts := strings.SplitN(strings.TrimSpace(subnetRaw), ":", 2)
-			if len(subnetParts) != 2 {
-				return fmt.Errorf(`Project subnet %q invalid, must be in the format of "<uplink network>:<subnet>"`, subnetRaw)
-			}
-
-			uplinkName := subnetParts[0]
-			subnetStr := subnetParts[1]
-
-			if uplinkName != uplink.Name {
-				continue // Only include subnets for our uplink.
-			}
-
-			_, restrictedSubnet, err := net.ParseCIDR(subnetStr)
-			if err != nil {
-				return err
-			}
-
-			projectRestrictedSubnets = append(projectRestrictedSubnets, restrictedSubnet)
-		}
+	// Get project restricted routes.
+	projectRestrictedSubnets, err := n.projectRestrictedSubnets(p, uplinkNetworkName)
+	if err != nil {
+		return err
 	}
 
 	// If NAT disabled, check subnets are within the uplink network's routes and project's subnet restrictions.

From d84254815bcb4e822fad4ef67f6a2c84a4c5a76e Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 17:01:14 +0100
Subject: [PATCH 07/15] lxd/network/driver/ovn: Passes project into
 allowedUplinkNetworks

Rather than load again from DB.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 22 ++++------------------
 1 file changed, 4 insertions(+), 18 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 8d7b55111f..f401f08fd0 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -1180,7 +1180,7 @@ func (n *ovn) Create(clientType cluster.ClientType) error {
 }
 
 // allowedUplinkNetworks returns a list of allowed networks to use as uplinks based on project restrictions.
-func (n *ovn) allowedUplinkNetworks() ([]string, error) {
+func (n *ovn) allowedUplinkNetworks(p *api.Project) ([]string, error) {
 	// Uplink networks are always from the default project.
 	networks, err := n.state.Cluster.GetNetworks(project.Default)
 	if err != nil {
@@ -1200,34 +1200,20 @@ func (n *ovn) allowedUplinkNetworks() ([]string, error) {
 		}
 	}
 
-	// Load the project to get uplink network restrictions.
-	var project *api.Project
-	err = n.state.Cluster.Transaction(func(tx *db.ClusterTx) error {
-		project, err = tx.GetProject(n.project)
-		if err != nil {
-			return err
-		}
-
-		return nil
-	})
-	if err != nil {
-		return nil, errors.Wrapf(err, "Failed to load restrictions for project %q", n.project)
-	}
-
 	// If project is not restricted, return full network list.
-	if !shared.IsTrue(project.Config["restricted"]) {
+	if !shared.IsTrue(p.Config["restricted"]) {
 		return networks, nil
 	}
 
 	allowedNetworks := []string{}
 
 	// There are no allowed networks if restricted.networks.uplinks is not set.
-	if project.Config["restricted.networks.uplinks"] == "" {
+	if p.Config["restricted.networks.uplinks"] == "" {
 		return allowedNetworks, nil
 	}
 
 	// Parse the allowed uplinks and return any that are present in the actual defined networks.
-	allowedRestrictedUplinks := strings.Split(project.Config["restricted.networks.uplinks"], ",")
+	allowedRestrictedUplinks := strings.Split(p.Config["restricted.networks.uplinks"], ",")
 
 	for _, allowedRestrictedUplink := range allowedRestrictedUplinks {
 		allowedRestrictedUplink = strings.TrimSpace(allowedRestrictedUplink)

From bfd7ac77861bdc804e742948aa0c7c12b38946c2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 17:01:43 +0100
Subject: [PATCH 08/15] lxd/network/driver/ovn: Passes project into
 validateUplinkNetwork

To avoid additional query.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index f401f08fd0..9ea0806cf2 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -1229,8 +1229,8 @@ func (n *ovn) allowedUplinkNetworks(p *api.Project) ([]string, error) {
 // validateUplinkNetwork checks if uplink network is allowed, and if empty string is supplied then tries to select
 // an uplink network from the allowedUplinkNetworks() list if there is only one allowed network.
 // Returns chosen uplink network name to use.
-func (n *ovn) validateUplinkNetwork(uplinkNetworkName string) (string, error) {
-	allowedUplinkNetworks, err := n.allowedUplinkNetworks()
+func (n *ovn) validateUplinkNetwork(p *api.Project, uplinkNetworkName string) (string, error) {
+	allowedUplinkNetworks, err := n.allowedUplinkNetworks(p)
 	if err != nil {
 		return "", err
 	}

From 44e6104b2a3acccb81bbc4a6c2c8f5eca43da058 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 17:02:09 +0100
Subject: [PATCH 09/15] lxd/network/driver/ovn: Load project in setup() to pass
 to n.validateUplinkNetwork()

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 9ea0806cf2..8f568c54c5 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -1276,8 +1276,14 @@ func (n *ovn) setup(update bool) error {
 	// Record updated config so we can store back into DB and n.config variable.
 	updatedConfig := make(map[string]string)
 
+	// Load the project to get uplink network restrictions.
+	p, err := n.state.Cluster.GetProject(n.project)
+	if err != nil {
+		return errors.Wrapf(err, "Failed to load network restrictions from project %q", n.project)
+	}
+
 	// Check project restrictions and get uplink network to use.
-	uplinkNetwork, err := n.validateUplinkNetwork(n.config["network"])
+	uplinkNetwork, err := n.validateUplinkNetwork(p, n.config["network"])
 	if err != nil {
 		return err
 	}

From 3efb1adc6d469353639768c632549ca846b65d38 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 17:02:30 +0100
Subject: [PATCH 10/15] lxd/network/driver/ovn: Adds
 InstanceDevicePortValidateExternalRoutes function

To validate a OVN NIC's external routes against an OVN network's uplink routes and project's restricted subnet.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_ovn.go | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/lxd/network/driver_ovn.go b/lxd/network/driver_ovn.go
index 8f568c54c5..1898ad2f4a 100644
--- a/lxd/network/driver_ovn.go
+++ b/lxd/network/driver_ovn.go
@@ -1868,6 +1868,36 @@ func (n *ovn) getInstanceDevicePortName(instanceID int, deviceName string) openv
 	return openvswitch.OVNSwitchPort(fmt.Sprintf("%s-%d-%s", n.getIntSwitchInstancePortPrefix(), instanceID, deviceName))
 }
 
+// InstanceDevicePortValidateExternalRoutes validates the external routes for an OVN instance port.
+func (n *ovn) InstanceDevicePortValidateExternalRoutes(externalRoutes []*net.IPNet) error {
+	// Load the project to get uplink network restrictions.
+	p, err := n.state.Cluster.GetProject(n.project)
+	if err != nil {
+		return errors.Wrapf(err, "Failed to load network restrictions from project %q", n.project)
+	}
+
+	// Get uplink routes.
+	uplinkRoutes, err := n.uplinkRoutes(n.config["network"])
+	if err != nil {
+		return err
+	}
+
+	// Get project restricted routes.
+	projectRestrictedSubnets, err := n.projectRestrictedSubnets(p, n.config["network"])
+	if err != nil {
+		return err
+	}
+
+	for _, externalRoute := range externalRoutes {
+		err = n.validateExternalSubnet(uplinkRoutes, projectRestrictedSubnets, externalRoute)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // InstanceDevicePortAdd adds an instance device port to the internal logical switch and returns the port name.
 func (n *ovn) InstanceDevicePortAdd(instanceID int, instanceName string, deviceName string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error) {
 	var dhcpV4ID, dhcpv6ID string

From 131a644a8507295a9c25882e88a48ba952e125bf Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 13:55:55 +0100
Subject: [PATCH 11/15] lxd/network/network/utils/ovn: Remvoes unused functions

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/network_utils_ovn.go | 42 --------------------------------
 1 file changed, 42 deletions(-)
 delete mode 100644 lxd/network/network_utils_ovn.go

diff --git a/lxd/network/network_utils_ovn.go b/lxd/network/network_utils_ovn.go
deleted file mode 100644
index 41b8e89d27..0000000000
--- a/lxd/network/network_utils_ovn.go
+++ /dev/null
@@ -1,42 +0,0 @@
-package network
-
-import (
-	"fmt"
-	"net"
-
-	"github.com/lxc/lxd/lxd/network/openvswitch"
-)
-
-// OVNInstanceDevicePortAdd adds a logical port to the OVN network's internal switch and returns the logical
-// port name for use linking an OVS port on the integration bridge to the logical switch port.
-func OVNInstanceDevicePortAdd(network Network, instanceID int, instanceName string, deviceName string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error) {
-	// Check network is of type OVN.
-	n, ok := network.(*ovn)
-	if !ok {
-		return "", fmt.Errorf("Network is not OVN type")
-	}
-
-	return n.instanceDevicePortAdd(instanceID, instanceName, deviceName, mac, ips, internalRoutes, externalRoutes)
-}
-
-// OVNInstanceDevicePortDynamicIPs gets a logical port's dynamic IPs stored in the OVN network's internal switch.
-func OVNInstanceDevicePortDynamicIPs(network Network, instanceID int, deviceName string) ([]net.IP, error) {
-	// Check network is of type OVN.
-	n, ok := network.(*ovn)
-	if !ok {
-		return nil, fmt.Errorf("Network is not OVN type")
-	}
-
-	return n.instanceDevicePortDynamicIPs(instanceID, deviceName)
-}
-
-// OVNInstanceDevicePortDelete deletes a logical port from the OVN network's internal switch.
-func OVNInstanceDevicePortDelete(network Network, instanceID int, deviceName string, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) error {
-	// Check network is of type OVN.
-	n, ok := network.(*ovn)
-	if !ok {
-		return fmt.Errorf("Network is not OVN type")
-	}
-
-	return n.instanceDevicePortDelete(instanceID, deviceName, internalRoutes, externalRoutes)
-}

From 08415e9d128c15abf020089325ba5c090ceb0e7d Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 13:58:09 +0100
Subject: [PATCH 12/15] lxd/device/nic/ovn: Adds ovnNet interface and use OVN
 instance port functions directly from network

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_ovn.go | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index 091888ab60..99c49ea622 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -24,10 +24,20 @@ import (
 	log "github.com/lxc/lxd/shared/log15"
 )
 
+// ovnNet defines an interface for accessing instance specific functions on OVN network.
+type ovnNet interface {
+	network.Network
+
+	InstanceDevicePortValidateExternalRoutes(externalRoutes []*net.IPNet) error
+	InstanceDevicePortAdd(instanceID int, instanceName string, deviceName string, mac net.HardwareAddr, ips []net.IP, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) (openvswitch.OVNSwitchPort, error)
+	InstanceDevicePortDelete(instanceID int, deviceName string, internalRoutes []*net.IPNet, externalRoutes []*net.IPNet) error
+	InstanceDevicePortDynamicIPs(instanceID int, deviceName string) ([]net.IP, error)
+}
+
 type nicOVN struct {
 	deviceCommon
 
-	network network.Network // Populated in validateConfig().
+	network ovnNet // Populated in validateConfig().
 }
 
 // getIntegrationBridgeName returns the OVS integration bridge to use.
@@ -91,7 +101,12 @@ func (d *nicOVN) validateConfig(instConf instance.ConfigReader) error {
 		}
 	}
 
-	d.network = n // Stored loaded instance for use by other functions.
+	ovnNet, ok := n.(ovnNet)
+	if !ok {
+		return fmt.Errorf("Network is not OVN type")
+	}
+
+	d.network = ovnNet // Stored loaded instance for use by other functions.
 	netConfig := d.network.Config()
 
 	if d.config["ipv4.address"] != "" {
@@ -317,14 +332,12 @@ func (d *nicOVN) Start() (*deviceConfig.RunConfig, error) {
 	}
 
 	// Add new OVN logical switch port for instance.
-	logicalPortName, err := network.OVNInstanceDevicePortAdd(d.network, d.inst.ID(), d.inst.Name(), d.name, mac, ips, internalRoutes, externalRoutes)
+	logicalPortName, err := d.network.InstanceDevicePortAdd(d.inst.ID(), d.inst.Name(), d.name, mac, ips, internalRoutes, externalRoutes)
 	if err != nil {
 		return nil, errors.Wrapf(err, "Failed adding OVN port")
 	}
 
-	revert.Add(func() {
-		network.OVNInstanceDevicePortDelete(d.network, d.inst.ID(), d.name, internalRoutes, externalRoutes)
-	})
+	revert.Add(func() { d.network.InstanceDevicePortDelete(d.inst.ID(), d.name, internalRoutes, externalRoutes) })
 
 	// Attach host side veth interface to bridge.
 	integrationBridge, err := d.getIntegrationBridgeName()
@@ -455,7 +468,7 @@ func (d *nicOVN) Stop() (*deviceConfig.RunConfig, error) {
 		}
 	}
 
-	err = network.OVNInstanceDevicePortDelete(d.network, d.inst.ID(), d.name, internalRoutes, externalRoutes)
+	err = d.network.InstanceDevicePortDelete(d.inst.ID(), d.name, internalRoutes, externalRoutes)
 	if err != nil {
 		// Don't fail here as we still want the postStop hook to run to clean up the local veth pair.
 		d.logger.Error("Failed to remove OVN device port", log.Ctx{"err": err})
@@ -531,7 +544,7 @@ func (d *nicOVN) State() (*api.InstanceStateNetwork, error) {
 
 	// OVN only supports dynamic IP allocation if neither IPv4 or IPv6 are statically set.
 	if d.config["ipv4.address"] == "" && d.config["ipv6.address"] == "" {
-		dynamicIPs, err := network.OVNInstanceDevicePortDynamicIPs(d.network, d.inst.ID(), d.name)
+		dynamicIPs, err := d.network.InstanceDevicePortDynamicIPs(d.inst.ID(), d.name)
 		if err == nil {
 			for _, dynamicIP := range dynamicIPs {
 				family := "inet"

From b9b1964381a0851d00c552fac3519dd02daf28ad Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 17:03:40 +0100
Subject: [PATCH 13/15] lxd/device/nic/ovn: Removes validation of external
 routes against network's external routes

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_ovn.go | 50 -------------------------------------------
 1 file changed, 50 deletions(-)

diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index 99c49ea622..16b1dff6cd 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -145,56 +145,6 @@ func (d *nicOVN) validateConfig(instConf instance.ConfigReader) error {
 		}
 	}
 
-	// Check IP external routes are within the network's external routes.
-	if d.config["ipv4.routes.external"] != "" || d.config["ipv6.routes.external"] != "" {
-		// Parse network external route subnets.
-		var networkRoutes []*net.IPNet
-		for _, k := range []string{"ipv4.routes.external", "ipv6.routes.external"} {
-			if netConfig[k] == "" {
-				continue
-			}
-
-			networkRoutes, err = network.SubnetParseAppend(networkRoutes, strings.Split(netConfig[k], ",")...)
-			if err != nil {
-				return err
-			}
-		}
-
-		// Parse and validate our external routes.
-		for _, k := range []string{"ipv4.routes.external", "ipv6.routes.external"} {
-			if d.config[k] == "" {
-				continue
-			}
-
-			externalRoutes, err := network.SubnetParseAppend([]*net.IPNet{}, strings.Split(d.config[k], ",")...)
-			if err != nil {
-				return err
-			}
-
-			for _, externalRoute := range externalRoutes {
-				rOnes, rBits := externalRoute.Mask.Size()
-				if rBits > 32 && rOnes < 122 {
-					return fmt.Errorf("External route %q is too large. Maximum size for IPv6 external route is /122", externalRoute.String())
-				} else if rOnes < 26 {
-					return fmt.Errorf("External route %q is too large. Maximum size for IPv4 external route is /26", externalRoute.String())
-				}
-
-				// Check that the external route is within the network's routes.
-				foundMatch := false
-				for _, networkRoute := range networkRoutes {
-					if network.SubnetContains(networkRoute, externalRoute) {
-						foundMatch = true
-						break
-					}
-				}
-
-				if !foundMatch {
-					return fmt.Errorf("Network %q doesn't contain %q in its external routes", n.Name(), externalRoute.String())
-				}
-			}
-		}
-	}
-
 	// Apply network level config options to device config before validation.
 	d.config["mtu"] = fmt.Sprintf("%s", netConfig["bridge.mtu"])
 

From 78d4ec8e820ad4f41f42887565b2f112442f3fff Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 17:04:11 +0100
Subject: [PATCH 14/15] lxd/device/nic/ovn: Validate NICs external routes using
 d.network.InstanceDevicePortValidateExternalRoutes

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/device/nic_ovn.go | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/lxd/device/nic_ovn.go b/lxd/device/nic_ovn.go
index 16b1dff6cd..f743083165 100644
--- a/lxd/device/nic_ovn.go
+++ b/lxd/device/nic_ovn.go
@@ -156,6 +156,35 @@ func (d *nicOVN) validateConfig(instConf instance.ConfigReader) error {
 		return err
 	}
 
+	// Check IP external routes are within the network's external routes.
+	var externalRoutes []*net.IPNet
+	for _, k := range []string{"ipv4.routes.external", "ipv6.routes.external"} {
+		if d.config[k] == "" {
+			continue
+		}
+
+		externalRoutes, err = network.SubnetParseAppend(externalRoutes, strings.Split(d.config[k], ",")...)
+		if err != nil {
+			return err
+		}
+	}
+
+	if len(externalRoutes) > 0 {
+		for _, externalRoute := range externalRoutes {
+			rOnes, rBits := externalRoute.Mask.Size()
+			if rBits > 32 && rOnes < 122 {
+				return fmt.Errorf("External route %q is too large. Maximum size for IPv6 external route is /122", externalRoute.String())
+			} else if rOnes < 26 {
+				return fmt.Errorf("External route %q is too large. Maximum size for IPv4 external route is /26", externalRoute.String())
+			}
+		}
+
+		err = d.network.InstanceDevicePortValidateExternalRoutes(externalRoutes)
+		if err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 

From 4a2a968c0e4e330d00d2c65d434bf9101f883c43 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Mon, 19 Oct 2020 17:06:19 +0100
Subject: [PATCH 15/15] doc/networks: Removes ipv4.routes.external and
 ipv6.routes.external from ovn network

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 doc/networks.md | 2 --
 1 file changed, 2 deletions(-)

diff --git a/doc/networks.md b/doc/networks.md
index d03da41f9d..dc6e109a2e 100644
--- a/doc/networks.md
+++ b/doc/networks.md
@@ -298,11 +298,9 @@ dns.domain                      | string    | -                     | lxd
 dns.search                      | string    | -                     | -                         | Full comma separated domain search list, defaulting to `dns.domain` value
 ipv4.address                    | string    | standard mode         | random unused subnet      | IPv4 address for the bridge (CIDR notation). Use "none" to turn off IPv4 or "auto" to generate a new one
 ipv4.nat                        | boolean   | ipv4 address          | false                     | Whether to NAT (will default to true if unset and a random ipv4.address is generated)
-ipv4.routes.external            | string    | ipv4 address          | -                         | Comma separated list of additional external IPv4 CIDR subnets that are allowed for OVN NICs ipv4.routes.external setting
 ipv6.address                    | string    | standard mode         | random unused subnet      | IPv6 address for the bridge (CIDR notation). Use "none" to turn off IPv6 or "auto" to generate a new one
 ipv6.dhcp.stateful              | boolean   | ipv6 dhcp             | false                     | Whether to allocate addresses using DHCP
 ipv6.nat                        | boolean   | ipv6 address          | false                     | Whether to NAT (will default to true if unset and a random ipv6.address is generated)
-ipv6.routes.external            | string    | ipv6 address          | -                         | Comma separated list of additional external IPv6 CIDR subnets that are allowed for OVN NICs ipv6.routes.external setting
 network                         | string    | -                     | -                         | Uplink network to use for external network access
 
 ## network: physical


More information about the lxc-devel mailing list