[lxc-devel] [lxd/master] Device Infiniband MAC improvements

tomponline on Github lxc-bot at linuxcontainers.org
Wed Aug 21 11:03:54 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 610 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190821/1efac475/attachment.bin>
-------------- next part --------------
From 12f55c4cefccb20925243844ab2108e5e8106808 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 21 Aug 2019 11:30:22 +0100
Subject: [PATCH 1/4] device/device/utils/infiniband: Adds IB MAC functions

Supports long (20 byte) and short (8 byte) variants for validating and setting MAC addresses on infiniband devices.

The short form variant is used when you want to specify the last 8 bytes of the MAC address manually without having to worry about what the first 12 bytes are (these indicate the queue and subnet of the device and can not be changed by the ip tool).

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

diff --git a/lxd/device/device_utils_infiniband.go b/lxd/device/device_utils_infiniband.go
index 07d3fb7bae..98b5719e3f 100644
--- a/lxd/device/device_utils_infiniband.go
+++ b/lxd/device/device_utils_infiniband.go
@@ -2,6 +2,7 @@ package device
 
 import (
 	"fmt"
+	"regexp"
 
 	"github.com/lxc/lxd/lxd/device/config"
 	"github.com/lxc/lxd/lxd/state"
@@ -112,3 +113,47 @@ func infinibandAddDevices(s *state.State, devicesPath string, deviceName string,
 
 	return nil
 }
+
+// infinibandValidMAC validates an infiniband MAC address. Supports both short and long variants,
+// e.g. "4a:c8:f9:1b:aa:57:ef:19" and "a0:00:0f:c0:fe:80:00:00:00:00:00:00:4a:c8:f9:1b:aa:57:ef:19".
+func infinibandValidMAC(value string) error {
+	regexHwaddrLong, err := regexp.Compile("^([0-9a-f]{2}:){19}[0-9a-f]{2}$")
+	if err != nil {
+		return err
+	}
+
+	regexHwaddrShort, err := regexp.Compile("^([0-9a-f]{2}:){7}[0-9a-f]{2}$")
+
+	if regexHwaddrShort.MatchString(value) {
+		return nil
+	}
+
+	if regexHwaddrLong.MatchString(value) {
+		return nil
+	}
+
+	return fmt.Errorf("Invalid value, must be either 8 or 20 bytes of lower case hex separated by colons")
+}
+
+// infinibandSetDevMAC detects whether the supplied MAC is a short or long form variant.
+// If the short form variant is supplied then only the last 8 bytes of the ibDev device's hwaddr
+// are changed. If the long form variant is supplied then the full 20 bytes of the ibDev device's
+// hwaddr are changed.
+func infinibandSetDevMAC(ibDev string, hwaddr string) error {
+	// Handle 20 byte variant, e.g. a0:00:14:c0:fe:80:00:00:00:00:00:00:4a:c8:f9:1b:aa:57:ef:19.
+	if len(hwaddr) == 59 {
+		return NetworkSetDevMAC(ibDev, hwaddr)
+	}
+
+	// Handle 8 byte variant, e.g. 4a:c8:f9:1b:aa:57:ef:19.
+	if len(hwaddr) == 23 {
+		curHwaddr, err := NetworkGetDevMAC(ibDev)
+		if err != nil {
+			return err
+		}
+
+		return NetworkSetDevMAC(ibDev, fmt.Sprintf("%s%s", curHwaddr[:36], hwaddr))
+	}
+
+	return fmt.Errorf("Invalid length")
+}

From 607a9647d28196f560357e118ccd22efc8380358 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 21 Aug 2019 11:32:12 +0100
Subject: [PATCH 2/4] device/infiniband/physical: Improves MAC address support

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

diff --git a/lxd/device/infiniband_physical.go b/lxd/device/infiniband_physical.go
index 70e21af1e5..ea61964cce 100644
--- a/lxd/device/infiniband_physical.go
+++ b/lxd/device/infiniband_physical.go
@@ -25,7 +25,17 @@ func (d *infinibandPhysical) validateConfig() error {
 		"mtu",
 		"hwaddr",
 	}
-	err := config.ValidateDevice(nicValidationRules(requiredFields, optionalFields), d.config)
+
+	rules := nicValidationRules(requiredFields, optionalFields)
+	rules["hwaddr"] = func(value string) error {
+		if value == "" {
+			return nil
+		}
+
+		return infinibandValidMAC(value)
+	}
+
+	err := config.ValidateDevice(rules, d.config)
 	if err != nil {
 		return err
 	}
@@ -78,7 +88,7 @@ func (d *infinibandPhysical) Start() (*RunConfig, error) {
 
 	// Set the MAC address.
 	if d.config["hwaddr"] != "" {
-		_, err := shared.RunCommand("ip", "link", "set", "dev", saveData["host_name"], "address", d.config["hwaddr"])
+		err := infinibandSetDevMAC(saveData["host_name"], d.config["hwaddr"])
 		if err != nil {
 			return nil, fmt.Errorf("Failed to set the MAC address: %s", err)
 		}

From dbbfdc5a639e682f06d8ca74596de307f8fec816 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 21 Aug 2019 11:32:37 +0100
Subject: [PATCH 3/4] device/infiniband/sriov: Improves MAC address support

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

diff --git a/lxd/device/infiniband_sriov.go b/lxd/device/infiniband_sriov.go
index 675aacd9a4..76a84b5e8f 100644
--- a/lxd/device/infiniband_sriov.go
+++ b/lxd/device/infiniband_sriov.go
@@ -26,7 +26,17 @@ func (d *infinibandSRIOV) validateConfig() error {
 		"mtu",
 		"hwaddr",
 	}
-	err := config.ValidateDevice(nicValidationRules(requiredFields, optionalFields), d.config)
+
+	rules := nicValidationRules(requiredFields, optionalFields)
+	rules["hwaddr"] = func(value string) error {
+		if value == "" {
+			return nil
+		}
+
+		return infinibandValidMAC(value)
+	}
+
+	err := config.ValidateDevice(rules, d.config)
 	if err != nil {
 		return err
 	}
@@ -100,7 +110,7 @@ func (d *infinibandSRIOV) Start() (*RunConfig, error) {
 
 	// Set the MAC address.
 	if d.config["hwaddr"] != "" {
-		_, err := shared.RunCommand("ip", "link", "set", "dev", saveData["host_name"], "address", d.config["hwaddr"])
+		err := infinibandSetDevMAC(saveData["host_name"], d.config["hwaddr"])
 		if err != nil {
 			return nil, fmt.Errorf("Failed to set the MAC address: %s", err)
 		}

From 86861cdaf0ff19d7181c323fdb12131b25bbceba Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 21 Aug 2019 12:00:40 +0100
Subject: [PATCH 4/4] test: Adds infiniband MAC tests

Also renames infiniband test functions to match their filename for consistency.

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 test/main.sh                                  |  4 +--
 .../container_devices_infiniband_physical.sh  | 25 +++++++++++++--
 .../container_devices_infiniband_sriov.sh     | 32 ++++++++++++++++---
 3 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/test/main.sh b/test/main.sh
index a207dcb28f..33e60d42d2 100755
--- a/test/main.sh
+++ b/test/main.sh
@@ -196,8 +196,8 @@ run_test test_container_devices_nic_physical "container devices - nic - physical
 run_test test_container_devices_nic_macvlan "container devices - nic - macvlan"
 run_test test_container_devices_nic_ipvlan "container devices - nic - ipvlan"
 run_test test_container_devices_nic_sriov "container devices - nic - sriov"
-run_test test_container_devices_ib_physical "container devices - infiniband - physical"
-run_test test_container_devices_ib_sriov "container devices - infiniband - sriov"
+run_test test_container_devices_infiniband_physical "container devices - infiniband - physical"
+run_test test_container_devices_infiniband_sriov "container devices - infiniband - sriov"
 run_test test_container_devices_proxy "container devices - proxy"
 run_test test_container_devices_gpu "container devices - gpu"
 run_test test_security "security features"
diff --git a/test/suites/container_devices_infiniband_physical.sh b/test/suites/container_devices_infiniband_physical.sh
index 04605ed571..e7e7c19a50 100644
--- a/test/suites/container_devices_infiniband_physical.sh
+++ b/test/suites/container_devices_infiniband_physical.sh
@@ -7,7 +7,7 @@
 # sudo mlxconfig --yes -d /dev/mst/mt4099_pciconf0 set LINK_TYPE_P2=2
 # echo "options mlx4_core num_vfs=4 probe_vf=4" | sudo tee /etc/modprobe.d/mellanox.conf
 # reboot
-test_container_devices_ib_physical() {
+test_container_devices_infiniband_physical() {
   ensure_import_testimage
   ensure_has_localhost_remote "${LXD_ADDR}"
 
@@ -19,6 +19,11 @@ test_container_devices_ib_physical() {
   fi
 
   ctName="nt$$"
+  macRand=$(shuf -i 0-9 -n 1)
+  ctMAC="96:29:52:03:73:4b:81:e${macRand}"
+
+  # Get host dev MAC to check MAC restore.
+  parentHostMAC=$(cat /sys/class/net/"${parent}"/address)
 
   # Record how many nics we started with.
   startNicCount=$(find /sys/class/net | wc -l)
@@ -28,7 +33,8 @@ test_container_devices_ib_physical() {
   lxc config device add "${ctName}" eth0 infiniband \
     nictype=physical \
     parent="${parent}" \
-    mtu=1500
+    mtu=1500 \
+    hwaddr=${ctMAC}
   lxc start "${ctName}"
 
   # Check host devices are created.
@@ -45,6 +51,12 @@ test_container_devices_ib_physical() {
     false
   fi
 
+  # Check custom MAC is applied in container on boot.
+  if ! lxc exec "${ctName}" -- grep -i "${ctMAC}" /sys/class/net/ib0/address ; then
+    echo "custom mac not applied"
+    false
+  fi
+
   # Check unprivileged cgroup device rule count.
   cgroupDeviceCount=$(wc -l < /sys/fs/cgroup/devices/lxc.payload/"${ctName}"/devices.list)
   if [ "$cgroupDeviceCount" != "1" ]; then
@@ -59,8 +71,15 @@ test_container_devices_ib_physical() {
     false
   fi
 
-  # Check volatile cleanup on stop.
   lxc stop -f "${ctName}"
+
+  # Check host dev MAC restore.
+  if ! grep -i "${parentHostMAC}" /sys/class/net/"${parent}"/address ; then
+    echo "host mac not restored"
+    false
+  fi
+
+  # Check volatile cleanup on stop.
   if lxc config show "${ctName}" | grep volatile.eth0 | grep -v volatile.eth0.name ; then
     echo "unexpected volatile key remains"
     false
diff --git a/test/suites/container_devices_infiniband_sriov.sh b/test/suites/container_devices_infiniband_sriov.sh
index 3cf17d1f86..2b5e5883fd 100644
--- a/test/suites/container_devices_infiniband_sriov.sh
+++ b/test/suites/container_devices_infiniband_sriov.sh
@@ -7,7 +7,7 @@
 # sudo mlxconfig --yes -d /dev/mst/mt4099_pciconf0 set LINK_TYPE_P2=2
 # echo "options mlx4_core num_vfs=4 probe_vf=4" | sudo tee /etc/modprobe.d/mellanox.conf
 # reboot
-test_container_devices_ib_sriov() {
+test_container_devices_infiniband_sriov() {
   ensure_import_testimage
   ensure_has_localhost_remote "${LXD_ADDR}"
 
@@ -19,6 +19,8 @@ test_container_devices_ib_sriov() {
   fi
 
   ctName="nt$$"
+  macRand=$(shuf -i 0-9 -n 1)
+  ctMAC="96:29:52:03:73:4b:81:e${macRand}"
 
   # Set a known start point config
   ip link set "${parent}" up
@@ -28,14 +30,17 @@ test_container_devices_ib_sriov() {
 
   # Test basic container with SR-IOV IB. Add 2 devices to check reservation system works.
   lxc init testimage "${ctName}"
+
+  # Name the device eth0 rather than ib0 so that check volatile data reset.
   lxc config device add "${ctName}" eth0 infiniband \
     nictype=sriov \
     parent="${parent}" \
     mtu=1500
-  lxc config device add "${ctName}" eth1 infiniband \
+  lxc config device add "${ctName}" ib1 infiniband \
     nictype=sriov \
     parent="${parent}" \
-    mtu=1500
+    mtu=1500 \
+    hwaddr=${ctMAC}
   lxc start "${ctName}"
 
   # Check host devices are created.
@@ -52,6 +57,12 @@ test_container_devices_ib_sriov() {
     false
   fi
 
+  # Check custom MAC is applied in container on boot.
+  if ! lxc exec "${ctName}" -- grep -i "${ctMAC}" /sys/class/net/ib1/address ; then
+    echo "custom mac not applied"
+    false
+  fi
+
   # Check unprivileged cgroup device rule count.
   cgroupDeviceCount=$(wc -l < /sys/fs/cgroup/devices/lxc.payload/"${ctName}"/devices.list)
   if [ "$cgroupDeviceCount" != "1" ]; then
@@ -66,8 +77,19 @@ test_container_devices_ib_sriov() {
     false
   fi
 
-  # Check volatile cleanup on stop.
+  # Get host dev name for ib1 before stop to check MAC restore.
+  ib1HostDev=$(lxc config get "${ctName}" volatile.ib1.host_name)
+  ib1HostMAC=$(lxc config get "${ctName}" volatile.ib1.last_state.hwaddr)
+
   lxc stop -f "${ctName}"
+
+  # Check host dev MAC restore.
+  if ! grep -i "${ib1HostMAC}" /sys/class/net/"${ib1HostDev}"/address ; then
+    echo "host mac not restored"
+    false
+  fi
+
+  # Check volatile cleanup on stop.
   if lxc config show "${ctName}" | grep volatile.eth0 | grep -v volatile.eth0.name ; then
     echo "unexpected volatile key remains"
     false
@@ -101,7 +123,7 @@ test_container_devices_ib_sriov() {
   lxc stop -f "${ctName}"
 
   lxc config device remove "${ctName}" eth0
-  lxc config device remove "${ctName}" eth1
+  lxc config device remove "${ctName}" ib1
 
   # Test hotplugging.
   lxc start "${ctName}"


More information about the lxc-devel mailing list