[lxc-devel] [lxd/master] network: Static routes for bridged nic types

tomponline on Github lxc-bot at linuxcontainers.org
Tue May 14 16:57:07 UTC 2019


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 498 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20190514/c24ecc1b/attachment.bin>
-------------- next part --------------
From 649dc35dbfefbdebad096376d86bfa4b0116cff0 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 14 May 2019 17:52:51 +0100
Subject: [PATCH 1/3] shared/container: Allows storing volatile keys ending
 .routes and .dev

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

diff --git a/shared/container.go b/shared/container.go
index 6e609959d0..f59d8b51c0 100644
--- a/shared/container.go
+++ b/shared/container.go
@@ -331,6 +331,14 @@ func ConfigKeyChecker(key string) (func(value string) error, error) {
 		if strings.HasSuffix(key, ".host_name") {
 			return IsAny, nil
 		}
+
+		if strings.HasSuffix(key, ".routes") {
+			return IsAny, nil
+		}
+
+		if strings.HasSuffix(key, ".dev") {
+			return IsAny, nil
+		}
 	}
 
 	if strings.HasPrefix(key, "environment.") {

From 343e53955039599926ae0ecf3cea551cd9017999 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 14 May 2019 17:53:24 +0100
Subject: [PATCH 2/3] lxd/container: Adds static routes for bridged veth
 devices

Re-works previous static routes implementation to detect if a bridged nic is in use and setup static routes to the bridge interface instead.

Manages route clean up using volatile config keys.

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

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 59e6ff453f..e6e898a36c 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -3073,6 +3073,12 @@ func (c *containerLXC) OnStop(target string) error {
 		logger.Error("Failed to set container state", log.Ctx{"container": c.Name(), "err": err})
 	}
 
+	// Clean up networking devices
+	err = c.cleanupNetworkDevices()
+	if err != nil {
+		logger.Error("Failed to cleanup network devices: ", log.Ctx{"container": c.Name(), "err": err})
+	}
+
 	go func(c *containerLXC, target string, op *lxcContainerOperation) {
 		c.fromHook = false
 		err = nil
@@ -3128,15 +3134,33 @@ func (c *containerLXC) OnStop(target string) error {
 	return nil
 }
 
+// cleanupNetworkDevices performs any needed network device cleanup steps when container is stopped.
+func (c *containerLXC) cleanupNetworkDevices() error {
+	for _, k := range c.expandedDevices.DeviceNames() {
+		m := c.expandedDevices[k]
+		if m["type"] != "nic" {
+			continue
+		}
+
+		// Remove any static veth routes
+		if shared.StringInSlice(m["nictype"], []string{"bridged", "p2p"}) {
+			c.removeNetworkRoutes(k)
+		}
+
+	}
+
+	return nil
+}
+
 // 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
-	return c.setupHostVethDevice(device)
+	return c.setupHostVethDevice(deviceName, device)
 }
 
 // setupHostVethDevice configures a nic device's host side veth settings.
-func (c *containerLXC) setupHostVethDevice(device types.Device) error {
+func (c *containerLXC) setupHostVethDevice(deviceName string, device types.Device) error {
 	// If not already, populate network device with host name.
 	if device["host_name"] == "" {
 		device["host_name"] = c.getHostInterface(device["name"])
@@ -3154,7 +3178,7 @@ func (c *containerLXC) setupHostVethDevice(device types.Device) error {
 	}
 
 	// Setup static routes to container
-	err = c.setNetworkRoutes(device)
+	err = c.setNetworkRoutes(deviceName, device)
 	if err != nil {
 		return err
 	}
@@ -4907,7 +4931,7 @@ func (c *containerLXC) Update(args db.ContainerArgs, userRequested bool) error {
 						return err
 					}
 
-					err = c.setupHostVethDevice(m)
+					err = c.setupHostVethDevice(k, m)
 					if err != nil {
 						return err
 					}
@@ -5039,7 +5063,7 @@ func (c *containerLXC) Update(args db.ContainerArgs, userRequested bool) error {
 						return fmt.Errorf("LXC doesn't know about this device and the host_name property isn't set, can't find host side veth name")
 					}
 
-					err = c.setupHostVethDevice(m)
+					err = c.setupHostVethDevice(k, m)
 					if err != nil {
 						return err
 					}
@@ -8245,6 +8269,11 @@ 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(name)
+	}
+
 	// If a veth, destroy it
 	if m["nictype"] != "physical" && m["nictype"] != "sriov" {
 		deviceRemoveInterface(hostName)
@@ -8795,31 +8824,55 @@ 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) error {
+func (c *containerLXC) setNetworkRoutes(deviceName string, m types.Device) (rerr 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"])
 	}
 
-	// Flush all IPv4 routes
-	_, err := shared.RunCommand("ip", "-4", "route", "flush", "dev", m["host_name"], "proto", "static")
-	if err != nil {
-		return err
-	}
+	// Remove any old routes that were setup for this nic device.
+	c.removeNetworkRoutes(deviceName)
 
-	// Flush all IPv6 routes
-	_, err = shared.RunCommand("ip", "-6", "route", "flush", "dev", m["host_name"], "proto", "static")
-	if err != nil {
-		return err
+	// Decide whether the route should point to the veth parent or the bridge parent
+	routeDev := m["host_name"]
+	if m["nictype"] == "bridged" {
+		routeDev = m["parent"]
 	}
 
+	ipv4Routes := make([]string, 0)
+	ipv6Routes := make([]string, 0)
+
+	// Setup deferred function to store any routes added into volatile data for removal later
+	defer func() {
+		volatile := make(map[string]string)
+		if len(ipv4Routes) > 0 {
+			routesKey := "volatile." + deviceName + ".last.ipv4.routes"
+			volatile[routesKey] = strings.Join(ipv4Routes, ",")
+		}
+		if len(ipv6Routes) > 0 {
+			routesKey := "volatile." + deviceName + ".last.ipv6.routes"
+			volatile[routesKey] = strings.Join(ipv6Routes, ",")
+		}
+
+		if len(ipv4Routes) > 0 || len(ipv6Routes) > 0 {
+			routesDevKey := "volatile." + deviceName + ".last.ip.routes.dev"
+			volatile[routesDevKey] = routeDev
+		}
+
+		err := c.VolatileSet(volatile)
+		if err != nil && rerr == nil {
+			rerr = err
+		}
+	}()
+
 	// Add additional IPv4 routes
 	if m["ipv4.routes"] != "" {
 		for _, route := range strings.Split(m["ipv4.routes"], ",") {
 			route = strings.TrimSpace(route)
-			_, err := shared.RunCommand("ip", "-4", "route", "add", "dev", m["host_name"], route, "proto", "static")
+			_, err := shared.RunCommand("ip", "-4", "route", "add", route, "dev", routeDev, "proto", "static")
 			if err != nil {
 				return err
 			}
+			ipv4Routes = append(ipv4Routes, route)
 		}
 	}
 
@@ -8827,16 +8880,72 @@ func (c *containerLXC) setNetworkRoutes(m types.Device) error {
 	if m["ipv6.routes"] != "" {
 		for _, route := range strings.Split(m["ipv6.routes"], ",") {
 			route = strings.TrimSpace(route)
-			_, err := shared.RunCommand("ip", "-6", "route", "add", "dev", m["host_name"], route, "proto", "static")
+			_, err := shared.RunCommand("ip", "-6", "route", "add", route, "dev", routeDev, "proto", "static")
 			if err != nil {
 				return err
 			}
+			ipv6Routes = append(ipv6Routes, route)
 		}
 	}
 
 	return nil
 }
 
+// removeNetworkRoutes removes any routes created for this device on the host using the volatile
+// data that was stored when the routes were first added with setNetworkRoutes().
+func (c *containerLXC) removeNetworkRoutes(deviceName string) {
+	ipRoutesDevKey := "volatile." + deviceName + ".last.ip.routes.dev"
+	ipv4RoutesKey := "volatile." + deviceName + ".last.ipv4.routes"
+	ipv6RoutesKey := "volatile." + deviceName + ".last.ipv6.routes"
+	ipRoutesDev := c.localConfig[ipRoutesDevKey]
+	ipv4Routes := c.localConfig[ipv4RoutesKey]
+	ipv6Routes := c.localConfig[ipv6RoutesKey]
+
+	volatile := map[string]string{
+		ipRoutesDevKey: "",
+		ipv4RoutesKey:  "",
+		ipv6RoutesKey:  "",
+	}
+
+	err := c.VolatileSet(volatile)
+	if err != nil {
+		logger.Errorf("Failed to remove volatile route config for %s: %s", deviceName, err)
+	}
+
+	if ipv4Routes != "" || ipv6Routes != "" {
+		if ipRoutesDev == "" {
+			logger.Errorf("Failed to remove static routes for %s as route dev isn't set", deviceName)
+			return
+		}
+
+		if !shared.PathExists(fmt.Sprintf("/sys/class/net/%s", ipRoutesDev)) {
+			return //Routes will already be gone if device doesn't exist.
+		}
+	}
+
+	// Remove IPv4 routes
+	if ipv4Routes != "" {
+		for _, route := range strings.Split(ipv4Routes, ",") {
+			route = strings.TrimSpace(route)
+			_, err := shared.RunCommand("ip", "-4", "route", "flush", route, "dev", ipRoutesDev, "proto", "static")
+			if err != nil {
+				logger.Errorf("Failed to remove static route: %s to %s: %s", route, ipRoutesDev, err)
+			}
+		}
+	}
+
+	// Remove IPv6 routes
+	if ipv6Routes != "" {
+		for _, route := range strings.Split(ipv6Routes, ",") {
+			route = strings.TrimSpace(route)
+			_, err := shared.RunCommand("ip", "-6", "route", "flush", route, "dev", ipRoutesDev, "proto", "static")
+			if err != nil {
+				logger.Errorf("Failed to remove static route: %s to %s: %s", route, ipRoutesDev, err)
+			}
+		}
+	}
+}
+
 func (c *containerLXC) setNetworkLimits(m types.Device) error {
 	var err error
 	// We can only do limits on some network type

From c0ad487d2fbc2a19fc93c1cc51994411dbbfbe97 Mon Sep 17 00:00:00 2001
From: Thomas Parrott <thomas.parrott at canonical.com>
Date: Tue, 14 May 2019 17:54:49 +0100
Subject: [PATCH 3/3] test: Re-works nic p2p and bridged tests to check for
 static routes working

Signed-off-by: Thomas Parrott <thomas.parrott at canonical.com>
---
 test/suites/container_devices_nic_bridged.sh | 64 +++++++++++++++++---
 test/suites/container_devices_nic_p2p.sh     | 10 +++
 2 files changed, 64 insertions(+), 10 deletions(-)

diff --git a/test/suites/container_devices_nic_bridged.sh b/test/suites/container_devices_nic_bridged.sh
index ada516a500..1be60bd9de 100644
--- a/test/suites/container_devices_nic_bridged.sh
+++ b/test/suites/container_devices_nic_bridged.sh
@@ -15,9 +15,11 @@ test_container_devices_nic_bridged() {
   lxc network set "${brName}" ipv6.routing false
   lxc network set "${brName}" ipv6.dhcp.stateful true
   lxc network set "${brName}" bridge.hwaddr 00:11:22:33:44:55
+  lxc network set "${brName}" ipv4.address 192.0.2.1/24
+  lxc network set "${brName}" ipv6.address 2001:db8::1/64
   [ "$(cat /sys/class/net/${brName}/address)" = "00:11:22:33:44:55" ]
 
-  # Test pre-launch profile config is applied at launch.
+  # Test pre-launch profile config is applied at launch
   lxc profile copy default "${ct_name}"
   lxc profile device set "${ct_name}" eth0 ipv4.routes "192.0.2.1${ipRand}/32"
   lxc profile device set "${ct_name}" eth0 ipv6.routes "2001:db8::1${ipRand}/128"
@@ -25,14 +27,16 @@ test_container_devices_nic_bridged() {
   lxc profile device set "${ct_name}" eth0 limits.egress 2Mbit
   lxc profile device set "${ct_name}" eth0 host_name "${veth_host_name}"
   lxc profile device set "${ct_name}" eth0 mtu "1400"
+  lxc profile device set "${ct_name}" eth0 parent "${brName}"
+  lxc profile device set "${ct_name}" eth0 nictype "bridged"
   lxc launch testimage "${ct_name}" -p "${ct_name}"
 
   # Check profile routes are applied on boot.
-  if ! ip -4 r list dev "${veth_host_name}" | grep "192.0.2.1${ipRand}" ; then
+  if ! ip -4 r list dev "${brName}" | grep "192.0.2.1${ipRand}" ; then
     echo "ipv4.routes invalid"
     false
   fi
-  if ! ip -6 r list dev "${veth_host_name}" | grep "2001:db8::1${ipRand}" ; then
+  if ! ip -6 r list dev "${brName}" | grep "2001:db8::1${ipRand}" ; then
     echo "ipv4.routes invalid"
     false
   fi
@@ -53,6 +57,14 @@ test_container_devices_nic_bridged() {
     false
   fi
 
+  # Add IP alias to container and check routes actually work.
+  lxc exec "${ct_name}" -- ip -4 addr add "192.0.2.1${ipRand}/32" dev eth0
+  lxc exec "${ct_name}" -- ip -4 route add default dev eth0
+  ping -c2 -W1 "192.0.2.1${ipRand}"
+  lxc exec "${ct_name}" -- ip -6 addr add "2001:db8::1${ipRand}/128" dev eth0
+  sleep 1 #Wait for link local gateway advert.
+  ping6 -c2 -W1 "2001:db8::1${ipRand}"
+
   # Test hot plugging a container nic with different settings to profile with the same name.
   lxc config device add "${ct_name}" eth0 nic \
     nictype=bridged \
@@ -65,12 +77,22 @@ test_container_devices_nic_bridged() {
     host_name="${veth_host_name}" \
     mtu=1401
 
+  # Check profile routes are removed on hot-plug.
+  if ip -4 r list dev "${brName}" | grep "192.0.2.1${ipRand}" ; then
+    echo "ipv4.routes remain"
+    false
+  fi
+  if ip -6 r list dev "${brName}" | grep "2001:db8::1${ipRand}" ; then
+    echo "ipv4.routes remain"
+    false
+  fi
+
   # Check routes are applied on hot-plug.
-  if ! ip -4 r list dev "${veth_host_name}" | grep "192.0.2.2${ipRand}" ; then
+  if ! ip -4 r list dev "${brName}" | grep "192.0.2.2${ipRand}" ; then
     echo "ipv4.routes invalid"
     false
   fi
-  if ! ip -6 r list dev "${veth_host_name}" | grep "2001:db8::2${ipRand}" ; then
+  if ! ip -6 r list dev "${brName}" | grep "2001:db8::2${ipRand}" ; then
     echo "ipv4.routes invalid"
     false
   fi
@@ -94,12 +116,22 @@ test_container_devices_nic_bridged() {
   # Test removing hot plugged device and check profile nic is restored.
   lxc config device remove "${ct_name}" eth0
 
+  # Check routes are removed on hot-plug.
+  if ip -4 r list dev "${brName}" | grep "192.0.2.2${ipRand}" ; then
+    echo "ipv4.routes remain"
+    false
+  fi
+  if ip -6 r list dev "${brName}" | grep "2001:db8::2${ipRand}" ; then
+    echo "ipv4.routes remain"
+    false
+  fi
+
   # Check profile routes are applied on hot-removal.
-  if ! ip -4 r list dev "${veth_host_name}" | grep "192.0.2.1${ipRand}" ; then
+  if ! ip -4 r list dev "${brName}" | grep "192.0.2.1${ipRand}" ; then
     echo "ipv4.routes invalid"
     false
   fi
-  if ! ip -6 r list dev "${veth_host_name}" | grep "2001:db8::1${ipRand}" ; then
+  if ! ip -6 r list dev "${brName}" | grep "2001:db8::1${ipRand}" ; then
     echo "ipv4.routes invalid"
     false
   fi
@@ -125,7 +157,9 @@ test_container_devices_nic_bridged() {
     nictype=bridged \
     name=eth0 \
     parent=${brName} \
-    host_name="${veth_host_name}"
+    host_name="${veth_host_name}" \
+    ipv4.routes="192.0.2.1${ipRand}/32" \
+    ipv6.routes="2001:db8::1${ipRand}/128"
 
   lxc config device set "${ct_name}" eth0 ipv4.routes "192.0.2.2${ipRand}/32"
   lxc config device set "${ct_name}" eth0 ipv6.routes "2001:db8::2${ipRand}/128"
@@ -133,12 +167,22 @@ test_container_devices_nic_bridged() {
   lxc config device set "${ct_name}" eth0 limits.egress 4Mbit
   lxc config device set "${ct_name}" eth0 mtu 1402
 
+  # Check profile routes are removed on hot-plug.
+  if ip -4 r list dev "${brName}" | grep "192.0.2.1${ipRand}" ; then
+    echo "ipv4.routes remain"
+    false
+  fi
+  if ip -6 r list dev "${brName}" | grep "2001:db8::1${ipRand}" ; then
+    echo "ipv4.routes remain"
+    false
+  fi
+
   # Check routes are applied on update.
-  if ! ip -4 r list dev "${veth_host_name}" | grep "192.0.2.2${ipRand}" ; then
+  if ! ip -4 r list dev "${brName}" | grep "192.0.2.2${ipRand}" ; then
     echo "ipv4.routes invalid"
     false
   fi
-  if ! ip -6 r list dev "${veth_host_name}" | grep "2001:db8::2${ipRand}" ; then
+  if ! ip -6 r list dev "${brName}" | grep "2001:db8::2${ipRand}" ; then
     echo "ipv4.routes invalid"
     false
   fi
diff --git a/test/suites/container_devices_nic_p2p.sh b/test/suites/container_devices_nic_p2p.sh
index 73cf41be6b..d8f9813653 100644
--- a/test/suites/container_devices_nic_p2p.sh
+++ b/test/suites/container_devices_nic_p2p.sh
@@ -43,6 +43,16 @@ test_container_devices_nic_p2p() {
     false
   fi
 
+  # Add IP alias to container and check routes actually work.
+  lxc exec "${ct_name}" -- ip -4 addr add "192.0.2.1${ipRand}/32" dev eth0
+  lxc exec "${ct_name}" -- ip -4 route add default dev eth0
+  ping -c2 -W1 "192.0.2.1${ipRand}"
+  ip -6 addr add 2001:db8::1/128 dev "${veth_host_name}"
+  lxc exec "${ct_name}" -- ip -6 addr add "2001:db8::1${ipRand}/128" dev eth0
+  lxc exec "${ct_name}" -- ip -6 route add default dev eth0
+  sleep 1
+  ping6 -c2 -W1 "2001:db8::1${ipRand}"
+
   # Test hot plugging a container nic with different settings to profile with the same name.
   lxc config device add "${ct_name}" eth0 nic \
     nictype=p2p \


More information about the lxc-devel mailing list