[lxc-devel] [lxd/master] Network: Adds network property support to sriov NIC driver

tomponline on Github lxc-bot at linuxcontainers.org
Wed Jul 22 07:55:42 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 492 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200722/629610f4/attachment.bin>
-------------- next part --------------
From 9e6fa4e9098cb344e2964350daad251863621262 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jul 2020 16:14:38 +0100
Subject: [PATCH 1/7] lxd: Adds NetworkTypeSriov constant and conversion
 handling

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/db/networks.go | 3 +++
 lxd/networks.go    | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/lxd/db/networks.go b/lxd/db/networks.go
index af26843557..bcd6812145 100644
--- a/lxd/db/networks.go
+++ b/lxd/db/networks.go
@@ -309,6 +309,7 @@ type NetworkType int
 const (
 	NetworkTypeBridge  NetworkType = iota // Network type bridge.
 	NetworkTypeMacvlan                    // Network type macvlan.
+	NetworkTypeSriov                      // Network type sriov.
 )
 
 // GetNetworkInAnyState returns the network with the given name.
@@ -370,6 +371,8 @@ func (c *Cluster) getNetwork(name string, onlyCreated bool) (int64, *api.Network
 		network.Type = "bridge"
 	case NetworkTypeMacvlan:
 		network.Type = "macvlan"
+	case NetworkTypeSriov:
+		network.Type = "sriov"
 	default:
 		network.Type = "" // Unknown
 	}
diff --git a/lxd/networks.go b/lxd/networks.go
index d31c0cf802..76baa06e07 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -130,6 +130,8 @@ func networksPost(d *Daemon, r *http.Request) response.Response {
 		dbNetType = db.NetworkTypeBridge
 	case "macvlan":
 		dbNetType = db.NetworkTypeMacvlan
+	case "sriov":
+		dbNetType = db.NetworkTypeSriov
 	default:
 		return response.BadRequest(fmt.Errorf("Unrecognised network type"))
 	}

From b5b7234688b366a7f1f29c9761af5a1c0babe571 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jul 2020 16:17:38 +0100
Subject: [PATCH 2/7] lxd/network: Adds sriov driver

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 lxd/network/driver_sriov.go | 115 ++++++++++++++++++++++++++++++++++++
 lxd/network/network_load.go |   1 +
 2 files changed, 116 insertions(+)
 create mode 100644 lxd/network/driver_sriov.go

diff --git a/lxd/network/driver_sriov.go b/lxd/network/driver_sriov.go
new file mode 100644
index 0000000000..3b8260609d
--- /dev/null
+++ b/lxd/network/driver_sriov.go
@@ -0,0 +1,115 @@
+package network
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+
+	"github.com/lxc/lxd/lxd/revert"
+	"github.com/lxc/lxd/shared"
+	"github.com/lxc/lxd/shared/api"
+	log "github.com/lxc/lxd/shared/log15"
+)
+
+// sriov represents a LXD sriov network.
+type sriov struct {
+	common
+}
+
+// Validate network config.
+func (n *sriov) Validate(config map[string]string) error {
+	rules := map[string]func(value string) error{
+		"parent": func(value string) error {
+			if err := ValidNetworkName(value); err != nil {
+				return errors.Wrapf(err, "Invalid interface name %q", value)
+			}
+
+			return nil
+		},
+		"maas.subnet.ipv4": shared.IsAny,
+		"maas.subnet.ipv6": shared.IsAny,
+	}
+
+	err := n.validate(config, rules)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// Delete deletes a network.
+func (n *sriov) Delete(clusterNotification bool) error {
+	n.logger.Debug("Delete", log.Ctx{"clusterNotification": clusterNotification})
+	return n.common.delete(clusterNotification)
+}
+
+// Rename renames a network.
+func (n *sriov) Rename(newName string) error {
+	n.logger.Debug("Rename", log.Ctx{"newName": newName})
+
+	// Sanity checks.
+	inUse, err := n.IsUsed()
+	if err != nil {
+		return err
+	}
+
+	if inUse {
+		return fmt.Errorf("The network is currently in use")
+	}
+
+	// Rename common steps.
+	err = n.common.rename(newName)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// Start starts is a no-op.
+func (n *sriov) Start() error {
+	if n.status == api.NetworkStatusPending {
+		return fmt.Errorf("Cannot start pending network")
+	}
+
+	return nil
+}
+
+// Stop stops is a no-op.
+func (n *sriov) Stop() error {
+	return nil
+}
+
+// Update updates the network. Accepts notification boolean indicating if this update request is coming from a
+// cluster notification, in which case do not update the database, just apply local changes needed.
+func (n *sriov) Update(newNetwork api.NetworkPut, targetNode string, clusterNotification bool) error {
+	n.logger.Debug("Update", log.Ctx{"clusterNotification": clusterNotification, "newNetwork": newNetwork})
+
+	dbUpdateNeeeded, _, oldNetwork, err := n.common.configChanged(newNetwork)
+	if err != nil {
+		return err
+	}
+
+	if !dbUpdateNeeeded {
+		return nil // Nothing changed.
+	}
+
+	revert := revert.New()
+	defer revert.Fail()
+
+	// Define a function which reverts everything.
+	revert.Add(func() {
+		// Reset changes to all nodes and database.
+		n.common.update(oldNetwork, targetNode, clusterNotification)
+	})
+
+	// Apply changes to database.
+	err = n.common.update(newNetwork, targetNode, clusterNotification)
+	if err != nil {
+		return err
+	}
+
+	revert.Success()
+	return nil
+}
diff --git a/lxd/network/network_load.go b/lxd/network/network_load.go
index a515779fb3..8638987cfd 100644
--- a/lxd/network/network_load.go
+++ b/lxd/network/network_load.go
@@ -8,6 +8,7 @@ import (
 var drivers = map[string]func() Network{
 	"bridge":  func() Network { return &bridge{} },
 	"macvlan": func() Network { return &macvlan{} },
+	"sriov":   func() Network { return &sriov{} },
 }
 
 // LoadByName loads the network info from the database by name.

From 4085f133ee537f2691e5068d0d06509f83198b8f Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jul 2020 16:29:32 +0100
Subject: [PATCH 3/7] lxd/networks: Remove database record on error in
 networksPost

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

diff --git a/lxd/networks.go b/lxd/networks.go
index 76baa06e07..94771ec9a7 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -201,18 +201,26 @@ func networksPost(d *Daemon, r *http.Request) response.Response {
 		return response.BadRequest(fmt.Errorf("The network already exists"))
 	}
 
+	revert := revert.New()
+	defer revert.Fail()
+
 	// Create the database entry.
 	_, err = d.cluster.CreateNetwork(req.Name, req.Description, dbNetType, req.Config)
 	if err != nil {
 		return response.SmartError(errors.Wrapf(err, "Error inserting %q into database", req.Name))
 	}
 
+	revert.Add(func() {
+		d.cluster.DeleteNetwork(req.Name)
+	})
+
 	// Create network and pass false to clusterNotification so the database record is removed on error.
 	err = doNetworksCreate(d, req, false)
 	if err != nil {
 		return response.SmartError(err)
 	}
 
+	revert.Success()
 	return resp
 }
 

From 3f9271e1da1ec944577c256eace56091b45bc0b2 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jul 2020 16:31:43 +0100
Subject: [PATCH 4/7] lxd/device/nic/sriov: Adds network key support

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

diff --git a/lxd/device/nic_sriov.go b/lxd/device/nic_sriov.go
index 724d432e71..83f7294254 100644
--- a/lxd/device/nic_sriov.go
+++ b/lxd/device/nic_sriov.go
@@ -17,9 +17,11 @@ import (
 	deviceConfig "github.com/lxc/lxd/lxd/device/config"
 	"github.com/lxc/lxd/lxd/instance"
 	"github.com/lxc/lxd/lxd/instance/instancetype"
+	"github.com/lxc/lxd/lxd/network"
 	"github.com/lxc/lxd/lxd/revert"
 	"github.com/lxc/lxd/lxd/util"
 	"github.com/lxc/lxd/shared"
+	"github.com/lxc/lxd/shared/api"
 )
 
 type nicSRIOV struct {
@@ -32,9 +34,11 @@ func (d *nicSRIOV) validateConfig(instConf instance.ConfigReader) error {
 		return ErrUnsupportedDevType
 	}
 
-	requiredFields := []string{"parent"}
+	var requiredFields []string
 	optionalFields := []string{
 		"name",
+		"network",
+		"parent",
 		"hwaddr",
 		"vlan",
 		"security.mac_filtering",
@@ -43,6 +47,48 @@ func (d *nicSRIOV) validateConfig(instConf instance.ConfigReader) error {
 		"boot.priority",
 	}
 
+	// Check that if network proeperty is set that conflicting keys are not present.
+	if d.config["network"] != "" {
+		requiredFields = append(requiredFields, "network")
+
+		bannedKeys := []string{"nictype", "parent", "mtu", "maas.subnet.ipv4", "maas.subnet.ipv6"}
+		for _, bannedKey := range bannedKeys {
+			if d.config[bannedKey] != "" {
+				return fmt.Errorf("Cannot use %q property in conjunction with %q property", bannedKey, "network")
+			}
+		}
+
+		// If network property is specified, lookup network settings and apply them to the device's config.
+		n, err := network.LoadByName(d.state, d.config["network"])
+		if err != nil {
+			return errors.Wrapf(err, "Error loading network config for %q", d.config["network"])
+		}
+
+		if n.Status() == api.NetworkStatusPending {
+			return fmt.Errorf("Specified network is not fully created")
+		}
+
+		if n.Type() != "sriov" {
+			return fmt.Errorf("Specified network must be of type macvlan")
+		}
+
+		netConfig := n.Config()
+
+		// Get actual parent device from network's parent setting.
+		d.config["parent"] = netConfig["parent"]
+
+		// Copy certain keys verbatim from the network's settings.
+		inheritKeys := []string{"maas.subnet.ipv4", "maas.subnet.ipv6"}
+		for _, inheritKey := range inheritKeys {
+			if _, found := netConfig[inheritKey]; found {
+				d.config[inheritKey] = netConfig[inheritKey]
+			}
+		}
+	} else {
+		// If no network property supplied, then parent property is required.
+		requiredFields = append(requiredFields, "parent")
+	}
+
 	// For VMs only NIC properties that can be specified on the parent's VF settings are controllable.
 	if instConf.Type() == instancetype.Container {
 		optionalFields = append(optionalFields, "mtu")

From 7abacec2487d4c06b8af8635be36d5126c39b5db Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 21 Jul 2020 16:31:58 +0100
Subject: [PATCH 5/7] lxd/device/nictype: Adds sriov support

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

diff --git a/lxd/device/nictype/nictype.go b/lxd/device/nictype/nictype.go
index fb8e02530d..c3ad6811ab 100644
--- a/lxd/device/nictype/nictype.go
+++ b/lxd/device/nictype/nictype.go
@@ -30,6 +30,8 @@ func NICType(s *state.State, d deviceConfig.Device) (string, error) {
 				nicType = "bridged"
 			case "macvlan":
 				nicType = "macvlan"
+			case "sriov":
+				nicType = "sriov"
 			default:
 				return "", fmt.Errorf("Unrecognised NIC network type for network %q", d["network"])
 			}

From 77d2fafebb0bcae4f5748d02ec0b38bb080f08d6 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 22 Jul 2020 08:52:19 +0100
Subject: [PATCH 6/7] test: sriov NIC comment ending consistency

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 test/suites/container_devices_nic_sriov.sh | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/test/suites/container_devices_nic_sriov.sh b/test/suites/container_devices_nic_sriov.sh
index 062818d084..7a03f0d3af 100644
--- a/test/suites/container_devices_nic_sriov.sh
+++ b/test/suites/container_devices_nic_sriov.sh
@@ -38,7 +38,7 @@ test_container_devices_nic_sriov() {
     parent="${parent}"
   lxc start "${ctName}"
 
-  # Check spoof checking has been disabled (the default)
+  # Check spoof checking has been disabled (the default).
   vfID=$(lxc config get "${ctName}" volatile.eth0.last_state.vf.id)
   if ip link show "${parent}" | grep "vf ${vfID}" | grep "spoof checking on"; then
     echo "spoof checking is still enabled"
@@ -47,7 +47,7 @@ test_container_devices_nic_sriov() {
 
   lxc config device set "${ctName}" eth0 vlan 1234
 
-  # Check custom vlan has been enabled
+  # Check custom vlan has been enabled.
   vfID=$(lxc config get "${ctName}" volatile.eth0.last_state.vf.id)
   if ! ip link show "${parent}" | grep "vf ${vfID}" | grep "vlan 1234"; then
     echo "vlan not set"
@@ -65,7 +65,7 @@ test_container_devices_nic_sriov() {
 
   lxc config device set "${ctName}" eth0 vlan 0
 
-  # Check custom vlan has been disabled
+  # Check custom vlan has been disabled.
   vfID=$(lxc config get "${ctName}" volatile.eth0.last_state.vf.id)
   if ip link show "${parent}" | grep "vf ${vfID}" | grep "vlan"; then
     # Mellanox cards display vlan 0 as vlan 4095!
@@ -89,7 +89,7 @@ test_container_devices_nic_sriov() {
   lxc config device set "${ctName}" eth0 hwaddr "${ctMAC1}"
   lxc start "${ctName}"
 
-  # Check custom MAC is applied
+  # Check custom MAC is applied.
   vfID=$(lxc config get "${ctName}" volatile.eth0.last_state.vf.id)
   if ! ip link show "${parent}" | grep "vf ${vfID}" | grep "${ctMAC1}"; then
     echo "eth0 MAC not set"
@@ -98,24 +98,24 @@ test_container_devices_nic_sriov() {
 
   lxc stop -f "${ctName}"
 
-  # Disable mac filtering and try fresh boot
+  # Disable mac filtering and try fresh boot.
   lxc config device set "${ctName}" eth0 security.mac_filtering false
   lxc start "${ctName}"
 
-  # Check spoof checking has been disabled (the default)
+  # Check spoof checking has been disabled (the default).
   vfID=$(lxc config get "${ctName}" volatile.eth0.last_state.vf.id)
   if ! ip link show "${parent}" | grep "vf ${vfID}" | grep "spoof checking off"; then
     echo "spoof checking is still enabled"
     false
   fi
 
-  # Hot plug fresh device
+  # Hot plug fresh device.
   lxc config device add "${ctName}" eth1 nic \
     nictype=sriov \
     parent="${parent}" \
     security.mac_filtering=true
 
-  # Check spoof checking has been enabled
+  # Check spoof checking has been enabled.
   vfID=$(lxc config get "${ctName}" volatile.eth1.last_state.vf.id)
   if ! ip link show "${parent}" | grep "vf ${vfID}" | grep "spoof checking on"; then
     echo "spoof checking is still disabled"
@@ -124,11 +124,11 @@ test_container_devices_nic_sriov() {
 
   lxc stop -f "${ctName}"
 
-  # Test setting MAC offline
+  # Test setting MAC offline.
   lxc config device set "${ctName}" eth1 hwaddr "${ctMAC2}"
   lxc start "${ctName}"
 
-  # Check custom MAC is applied
+  # Check custom MAC is applied.
   vfID=$(lxc config get "${ctName}" volatile.eth1.last_state.vf.id)
   if ! ip link show "${parent}" | grep "vf ${vfID}" | grep "${ctMAC2}"; then
     echo "eth1 MAC not set"

From 5846bd8ea334c5d1ddd87f7844b035b2b34993a4 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 22 Jul 2020 08:52:56 +0100
Subject: [PATCH 7/7] test: sriov network test

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 test/suites/container_devices_nic_sriov.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/test/suites/container_devices_nic_sriov.sh b/test/suites/container_devices_nic_sriov.sh
index 7a03f0d3af..a128404f0a 100644
--- a/test/suites/container_devices_nic_sriov.sh
+++ b/test/suites/container_devices_nic_sriov.sh
@@ -135,6 +135,28 @@ test_container_devices_nic_sriov() {
     false
   fi
 
+  lxc stop -f "${ctName}"
+  lxc config device remove "${ctName}" eth0
+  lxc config device remove "${ctName}" eth1
+
+  # Create sriov network and add NIC device using that network.
+  lxc network create "${ctName}net" --type=sriov parent="${parent}"
+  lxc config device add "${ctName}" eth0 nic \
+    network="${ctName}net" \
+    name=eth0 \
+    hwaddr="${ctMAC1}"
+  lxc start "${ctName}"
+
+  # Check custom MAC is applied.
+  vfID=$(lxc config get "${ctName}" volatile.eth0.last_state.vf.id)
+  if ! ip link show "${parent}" | grep "vf ${vfID}" | grep "${ctMAC1}"; then
+    echo "eth0 MAC not set"
+    false
+  fi
+
+  lxc config device remove "${ctName}" eth0
+  lxc network delete "${ctName}net"
+
   lxc delete -f "${ctName}"
 
   # Check we haven't left any NICS lying around.


More information about the lxc-devel mailing list