[lxc-devel] [lxd/master] network: p2p/bridged static route consistency updates

tomponline on Github lxc-bot at linuxcontainers.org
Wed May 22 16:52:59 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 620 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190522/966c0304/attachment.bin>
-------------- next part --------------
From e3ff176bc99a5c326740af263986820a0bc55295 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 22 May 2019 17:26:39 +0100
Subject: [PATCH 1/4] test: Adds further p2p nic tests for various scenarios

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

diff --git a/test/suites/container_devices_nic_p2p.sh b/test/suites/container_devices_nic_p2p.sh
index 4c1c8deed0..a3f8b12b7e 100644
--- a/test/suites/container_devices_nic_p2p.sh
+++ b/test/suites/container_devices_nic_p2p.sh
@@ -190,6 +190,143 @@ test_container_devices_nic_p2p() {
   lxc launch testimage "${ctName}"
   lxc config device add "${ctName}" eth0 nic \
     nictype=p2p
+
+  # Now add some routes
+  lxc config device set "${ctName}" eth0 ipv4.routes "192.0.2.2${ipRand}/32"
+  lxc config device set "${ctName}" eth0 ipv6.routes "2001:db8::2${ipRand}/128"
+
+  # Check routes are applied on update. The host name is dynamic, so just check routes exist.
+  if ! ip -4 r list | grep "192.0.2.2${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ! ip -6 r list | grep "2001:db8::2${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
+  # Now update routes, check old routes go and new routes added.
+  lxc config device set "${ctName}" eth0 ipv4.routes "192.0.2.3${ipRand}/32"
+  lxc config device set "${ctName}" eth0 ipv6.routes "2001:db8::3${ipRand}/128"
+
+  # Check routes are applied on update. The host name is dynamic, so just check routes exist.
+  if ! ip -4 r list | grep "192.0.2.3${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ! ip -6 r list | grep "2001:db8::3${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
+  # Check old routes removed
+  if ip -4 r list | grep "192.0.2.2${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ip -6 r list | grep "2001:db8::2${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
+  # Now remove device, check routes go
   lxc config device remove "${ctName}" eth0
+
+  if ip -4 r list | grep "192.0.2.3${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ip -6 r list | grep "2001:db8::3${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
+  # Now add a nic to a stopped container with routes.
+  lxc stop "${ctName}"
+  lxc config device add "${ctName}" eth0 nic \
+    nictype=p2p \
+    ipv4.routes="192.0.2.2${ipRand}/32" \
+    ipv6.routes="2001:db8::2${ipRand}/128"
+
+  lxc start "${ctName}"
+
+  # Check routes are applied on start. The host name is dynamic, so just check routes exist.
+  if ! ip -4 r list | grep "192.0.2.2${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ! ip -6 r list | grep "2001:db8::2${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
+  # Now update routes on boot time nic, check old routes go and new routes added.
+  lxc config device set "${ctName}" eth0 ipv4.routes "192.0.2.3${ipRand}/32"
+  lxc config device set "${ctName}" eth0 ipv6.routes "2001:db8::3${ipRand}/128"
+
+  # Check routes are applied on update. The host name is dynamic, so just check routes exist.
+  if ! ip -4 r list | grep "192.0.2.3${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ! ip -6 r list | grep "2001:db8::3${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
+  # Check old routes removed
+  if ip -4 r list | grep "192.0.2.2${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ip -6 r list | grep "2001:db8::2${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
+  # Now remove boot time device
+  lxc config device remove "${ctName}" eth0
+
+  # Check old routes removed
+  if ip -4 r list | grep "192.0.2.3${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ip -6 r list | grep "2001:db8::3${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
+  # Add hot plug device with routes.
+  lxc config device add "${ctName}" eth0 nic \
+    nictype=p2p
+
+  # Now update routes on hotplug nic
+  lxc config device set "${ctName}" eth0 ipv4.routes "192.0.2.2${ipRand}/32"
+  lxc config device set "${ctName}" eth0 ipv6.routes "2001:db8::2${ipRand}/128"
+
+  # Check routes are applied. The host name is dynamic, so just check routes exist.
+  if ! ip -4 r list | grep "192.0.2.2${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ! ip -6 r list | grep "2001:db8::2${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
+  # Now remove hotplug device
+  lxc config device remove "${ctName}" eth0
+
+  # Check old routes removed
+  if ip -4 r list | grep "192.0.2.2${ipRand}" ; then
+    echo "ipv4.routes invalid"
+    false
+  fi
+  if ip -6 r list | grep "2001:db8::2${ipRand}" ; then
+    echo "ipv6.routes invalid"
+    false
+  fi
+
   lxc delete "${ctName}" -f
 }

From 96f32106de0f126f6cc8a77522ed5d51b0c8ba1b Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 22 May 2019 17:28:55 +0100
Subject: [PATCH 2/4] container/lxc: Runs network up hook for all p2p and
 bridged nics

This is so that host_name info can be recorded consistently on boot.

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index f02567325d..8447283230 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -1678,8 +1678,8 @@ func (c *containerLXC) initLXC(config bool) error {
 				}
 			}
 
-			// Check if the container has network specific keys set to avoid unnecessarily running the network up hook.
-			if shared.StringMapHasStringKey(m, containerNetworkKeys...) && shared.StringInSlice(m["nictype"], []string{"bridged", "p2p"}) {
+			// Run network up hook for bridged and p2p nics.
+			if shared.StringInSlice(m["nictype"], []string{"bridged", "p2p"}) {
 				err = lxcSetConfigItem(cc, fmt.Sprintf("%s.%d.script.up", networkKeyPrefix, networkidx), fmt.Sprintf("%s callhook %s %d network-up %s", c.state.OS.ExecPath, shared.VarPath(""), c.id, k))
 				if err != nil {
 					return err

From eb7c7e26370dd25c9a33597a32fd38fdd495a536 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 22 May 2019 17:30:29 +0100
Subject: [PATCH 3/4] container/lxc: Records host_name from LXC on p2p/bridged
 nic start

Records host_name for p2p/bridged nic start in volatile data and updates routes and limits settings to use them.

This allows consistent boot/add/remove/update fof p2p/bridged settings even on older kernels.

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 8447283230..b78669841f 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -3173,7 +3173,7 @@ func (c *containerLXC) cleanupNetworkRoutes() error {
 
 		// Remove any static veth routes
 		if shared.StringInSlice(m["nictype"], []string{"bridged", "p2p"}) {
-			c.removeNetworkRoutes(m)
+			c.removeNetworkRoutes(k, m)
 		}
 
 	}
@@ -3184,7 +3184,23 @@ func (c *containerLXC) cleanupNetworkRoutes() error {
 // OnNetworkUp is called by the LXD callhook when the LXC network up script is run.
 func (c *containerLXC) OnNetworkUp(deviceName string, hostName string) error {
 	device := c.expandedDevices[deviceName]
-	device["host_name"] = hostName
+
+	// This hook is only for bridged and p2p nics currently.
+	if !shared.StringInSlice(device["nictype"], []string{"bridged", "p2p"}) {
+		return nil
+	}
+
+	// Record boot time host name of nic into volatile for use with routes/limits updates later.
+	// Only need to do this if host_name is not specified in nic config.
+	if device["host_name"] == "" {
+		device["host_name"] = hostName
+		hostNameKey := fmt.Sprintf("volatile.%s.host_name", deviceName)
+		err := c.VolatileSet(map[string]string{hostNameKey: hostName})
+		if err != nil {
+			return err
+		}
+	}
+
 	return c.setupHostVethDevice(deviceName, device, types.Device{})
 }
 
@@ -3192,8 +3208,8 @@ func (c *containerLXC) OnNetworkUp(deviceName string, hostName string) error {
 func (c *containerLXC) setupHostVethDevice(deviceName string, device types.Device, oldDevice types.Device) error {
 	// If not populated already, check if volatile data contains the most recently added host_name.
 	if device["host_name"] == "" {
-		configKey := fmt.Sprintf("volatile.%s.host_name", deviceName)
-		device["host_name"] = c.localConfig[configKey]
+		hostNameKey := fmt.Sprintf("volatile.%s.host_name", deviceName)
+		device["host_name"] = c.localConfig[hostNameKey]
 	}
 
 	// Check whether host device resolution succeeded.
@@ -3208,7 +3224,7 @@ func (c *containerLXC) setupHostVethDevice(deviceName string, device types.Devic
 	}
 
 	// Setup static routes to container
-	err = c.setNetworkRoutes(device, oldDevice)
+	err = c.setNetworkRoutes(deviceName, device, oldDevice)
 	if err != nil {
 		return err
 	}
@@ -8299,7 +8315,7 @@ func (c *containerLXC) removeNetworkDevice(name string, m types.Device) error {
 
 	// Remove any static veth routes
 	if shared.StringInSlice(m["nictype"], []string{"bridged", "p2p"}) {
-		c.removeNetworkRoutes(m)
+		c.removeNetworkRoutes(name, m)
 	}
 
 	// If a veth, destroy it
@@ -8852,13 +8868,13 @@ func (c *containerLXC) getHostInterface(name string) string {
 }
 
 // setNetworkRoutes applies any static routes configured from the host to the container nic.
-func (c *containerLXC) setNetworkRoutes(m types.Device, oldDevice types.Device) error {
+func (c *containerLXC) setNetworkRoutes(deviceName string, m types.Device, oldDevice types.Device) error {
 	if !shared.PathExists(fmt.Sprintf("/sys/class/net/%s", m["host_name"])) {
 		return fmt.Errorf("Unknown or missing host side veth: %s", m["host_name"])
 	}
 
 	// Remove any old routes that were setup for this nic device.
-	c.removeNetworkRoutes(oldDevice)
+	c.removeNetworkRoutes(deviceName, oldDevice)
 
 	// Decide whether the route should point to the veth parent or the bridge parent
 	routeDev := m["host_name"]
@@ -8893,7 +8909,13 @@ func (c *containerLXC) setNetworkRoutes(m types.Device, oldDevice types.Device)
 
 // removeNetworkRoutes removes any routes created for this device on the host that were first added
 // with setNetworkRoutes(). Expects to be passed the device config from the oldExpandedDevices.
-func (c *containerLXC) removeNetworkRoutes(m types.Device) {
+func (c *containerLXC) removeNetworkRoutes(deviceName string, m types.Device) {
+	// If not populated already, check if volatile data contains the most recently added host_name.
+	if m["host_name"] == "" {
+		hostNameKey := fmt.Sprintf("volatile.%s.host_name", deviceName)
+		m["host_name"] = c.localConfig[hostNameKey]
+	}
+
 	// Decide whether the route should point to the veth parent or the bridge parent
 	routeDev := m["host_name"]
 	if m["nictype"] == "bridged" {

From 7ab1162fb8cfef3a7d3241696ee0f806f20ba656 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Wed, 22 May 2019 17:32:05 +0100
Subject: [PATCH 4/4] lxc/container: Removes unused getHostInterface()

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index b78669841f..93e255d162 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -8817,56 +8817,6 @@ func (c *containerLXC) setNetworkPriority() error {
 	return nil
 }
 
-func (c *containerLXC) getHostInterface(name string) string {
-	// Pull directly from kernel
-	networks := c.networkState()
-	if networks[name].HostName != "" {
-		return networks[name].HostName
-	}
-
-	// Fallback to poking LXC
-	if c.IsRunning() {
-		networkKeyPrefix := "lxc.net"
-		if !util.RuntimeLiblxcVersionAtLeast(2, 1, 0) {
-			networkKeyPrefix = "lxc.network"
-		}
-
-		for i := 0; i < len(c.c.ConfigItem(networkKeyPrefix)); i++ {
-			nicName := c.c.RunningConfigItem(fmt.Sprintf("%s.%d.name", networkKeyPrefix, i))[0]
-			if nicName != name {
-				continue
-			}
-
-			veth := c.c.RunningConfigItem(fmt.Sprintf("%s.%d.veth.pair", networkKeyPrefix, i))[0]
-			if veth != "" {
-				return veth
-			}
-		}
-	}
-
-	// Fallback to parsing LXD config
-	for _, k := range c.expandedDevices.DeviceNames() {
-		dev := c.expandedDevices[k]
-		if dev["type"] != "nic" && dev["type"] != "infiniband" {
-			continue
-		}
-
-		m, err := c.fillNetworkDevice(k, dev)
-		if err != nil {
-			m = dev
-		}
-
-		if m["name"] != name {
-			continue
-		}
-
-		return m["host_name"]
-	}
-
-	// Fail
-	return ""
-}
-
 // setNetworkRoutes applies any static routes configured from the host to the container nic.
 func (c *containerLXC) setNetworkRoutes(deviceName string, m types.Device, oldDevice types.Device) error {
 	if !shared.PathExists(fmt.Sprintf("/sys/class/net/%s", m["host_name"])) {


More information about the lxc-devel mailing list