[lxc-devel] [lxd/master] network: Fix dnsmasq duplicate IP handling and bridge external interfaces logics

stgraber on Github lxc-bot at linuxcontainers.org
Mon Aug 28 22:17:43 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 301 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170828/ab496c52/attachment.bin>
-------------- next part --------------
From 8ff709a719d092dc624b5f090cb30f35bfc6a8dd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 28 Aug 2017 17:47:44 -0400
Subject: [PATCH 1/2] network: Allow for duplicate IPs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #3721

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/container_lxc.go  |   6 +--
 lxd/networks.go       |   2 +-
 lxd/networks_utils.go | 121 +++++++++++++++++++++++++++++---------------------
 3 files changed, 75 insertions(+), 54 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index 5779dc179..1be813345 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -399,7 +399,7 @@ func containerLXCCreate(s *state.State, args db.ContainerArgs) (container, error
 	}
 
 	// Update lease files
-	networkUpdateStatic(s, "", c.name)
+	networkUpdateStatic(s, "")
 
 	logger.Info("Created container", ctxMap)
 
@@ -2879,7 +2879,7 @@ func (c *containerLXC) Delete() error {
 	}
 
 	// Update network files
-	networkUpdateStatic(c.state, "", c.name)
+	networkUpdateStatic(c.state, "")
 	for k, m := range c.expandedDevices {
 		if m["type"] != "nic" || m["nictype"] != "bridged" || (m["ipv4.address"] == "" && m["ipv6.address"] == "") {
 			continue
@@ -4073,7 +4073,7 @@ func (c *containerLXC) Update(args db.ContainerArgs, userRequested bool) error {
 	}
 
 	if needsUpdate {
-		networkUpdateStatic(c.state, "", c.name)
+		networkUpdateStatic(c.state, "")
 	}
 
 	// Success, update the closure to mark that the changes should be kept.
diff --git a/lxd/networks.go b/lxd/networks.go
index de0b7d840..4b36d981d 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -1235,7 +1235,7 @@ func (n *network) Start() error {
 		}
 
 		// Update the static leases
-		err = networkUpdateStatic(n.state, n.name, "")
+		err = networkUpdateStatic(n.state, n.name)
 		if err != nil {
 			return err
 		}
diff --git a/lxd/networks_utils.go b/lxd/networks_utils.go
index 6022efd7d..37ab63d83 100644
--- a/lxd/networks_utils.go
+++ b/lxd/networks_utils.go
@@ -23,8 +23,11 @@ import (
 	"github.com/lxc/lxd/lxd/db"
 	"github.com/lxc/lxd/lxd/state"
 	"github.com/lxc/lxd/shared"
+	"github.com/lxc/lxd/shared/logger"
 )
 
+var networkStaticLock sync.Mutex
+
 func networkAutoAttach(dbObj *sql.DB, devName string) error {
 	_, dbInfo, err := db.NetworkGetInterface(dbObj, devName)
 	if err != nil {
@@ -724,17 +727,15 @@ func networkKillDnsmasq(name string, reload bool) error {
 	return nil
 }
 
-func networkUpdateStatic(s *state.State, networkName string, containerName string) error {
+func networkUpdateStatic(s *state.State, networkName string) error {
+	// We don't want to race with ourselves here
+	networkStaticLock.Lock()
+	defer networkStaticLock.Unlock()
+
 	// Get all the containers
-	containers := []string{}
-	if containerName == "" {
-		var err error
-		containers, err = db.ContainersList(s.DB, db.CTypeRegular)
-		if err != nil {
-			return err
-		}
-	} else {
-		containers = []string{containerName}
+	containers, err := db.ContainersList(s.DB, db.CTypeRegular)
+	if err != nil {
+		return err
 	}
 
 	// Get all the networks
@@ -796,61 +797,81 @@ func networkUpdateStatic(s *state.State, networkName string, containerName strin
 		}
 		config := n.Config()
 
-		// Clean everything up on resets
-		if containerName == "" {
-			// Wipe everything clean
-			entries, err := ioutil.ReadDir(shared.VarPath("networks", network, "dnsmasq.hosts"))
+		// Wipe everything clean
+		files, err := ioutil.ReadDir(shared.VarPath("networks", network, "dnsmasq.hosts"))
+		if err != nil {
+			return err
+		}
+
+		for _, entry := range files {
+			err = os.Remove(shared.VarPath("networks", network, "dnsmasq.hosts", entry.Name()))
 			if err != nil {
 				return err
 			}
-
-			for _, entry := range entries {
-				err = os.Remove(shared.VarPath("networks", network, "dnsmasq.hosts", entry.Name()))
-				if err != nil {
-					return err
-				}
-			}
 		}
 
-		if containerName != "" && len(entries) == 0 {
-			// Wipe the one container clean
-			if shared.PathExists(shared.VarPath("networks", network, "dnsmasq.hosts", containerName)) {
-				err = os.Remove(shared.VarPath("networks", network, "dnsmasq.hosts", containerName))
-				if err != nil {
-					return err
-				}
-			}
-		} else {
-			// Apply the changes
-			for _, entry := range entries {
-				hwaddr := entry[0]
-				cName := entry[1]
-				ipv4Address := entry[2]
-				ipv6Address := entry[3]
-
-				line := hwaddr
+		// Apply the changes
+		for entryIdx, entry := range entries {
+			hwaddr := entry[0]
+			cName := entry[1]
+			ipv4Address := entry[2]
+			ipv6Address := entry[3]
+			line := hwaddr
 
-				if ipv4Address != "" {
-					line += fmt.Sprintf(",%s", ipv4Address)
-				}
-
-				if ipv6Address != "" {
-					line += fmt.Sprintf(",[%s]", ipv6Address)
+			// Look for duplicates
+			duplicate := false
+			for iIdx, i := range entries {
+				if entry[1] == i[1] {
+					// Skip ourselves
+					continue
 				}
 
-				if config["dns.mode"] == "" || config["dns.mode"] == "managed" {
-					line += fmt.Sprintf(",%s", cName)
+				if entry[0] == i[0] {
+					// Find broken configurations
+					logger.Errorf("Duplicate MAC detected: %s and %s", entry[1], i[1])
 				}
 
-				if line == hwaddr {
+				if i[2] == "" && i[3] == "" {
+					// Skip unconfigured
 					continue
 				}
 
-				err := ioutil.WriteFile(shared.VarPath("networks", network, "dnsmasq.hosts", cName), []byte(line+"\n"), 0644)
-				if err != nil {
-					return err
+				if entry[2] == i[2] && entry[3] == i[3] {
+					// Find identical containers (copies with static configuration)
+					if entryIdx > iIdx {
+						duplicate = true
+					} else {
+						line = fmt.Sprintf("%s,%s", line, i[0])
+						logger.Debugf("Found containers with duplicate IPv4/IPv6: %s and %s", entry[1], i[1])
+					}
 				}
 			}
+
+			if duplicate {
+				continue
+			}
+
+			// Generate the dhcp-host line
+			if ipv4Address != "" {
+				line += fmt.Sprintf(",%s", ipv4Address)
+			}
+
+			if ipv6Address != "" {
+				line += fmt.Sprintf(",[%s]", ipv6Address)
+			}
+
+			if config["dns.mode"] == "" || config["dns.mode"] == "managed" {
+				line += fmt.Sprintf(",%s", cName)
+			}
+
+			if line == hwaddr {
+				continue
+			}
+
+			err := ioutil.WriteFile(shared.VarPath("networks", network, "dnsmasq.hosts", cName), []byte(line+"\n"), 0644)
+			if err != nil {
+				return err
+			}
 		}
 
 		// Signal dnsmasq

From 54577b8bf651f4b3e860581c3a9a3ab73bb8ef60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 28 Aug 2017 18:12:30 -0400
Subject: [PATCH 2/2] network: Fix bridging devies with IPv6 link-local
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #3727
Closes #3728

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/networks.go | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lxd/networks.go b/lxd/networks.go
index 4b36d981d..a34e6ba27 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -665,8 +665,19 @@ func (n *network) Start() error {
 				continue
 			}
 
+			unused := true
 			addrs, err := iface.Addrs()
-			if err == nil && len(addrs) != 0 {
+			if err == nil {
+				for _, addr := range addrs {
+					ip, _, err := net.ParseCIDR(addr.String())
+					if ip != nil && err == nil && ip.IsGlobalUnicast() {
+						unused = false
+						break
+					}
+				}
+			}
+
+			if !unused {
 				return fmt.Errorf("Only unconfigured network interfaces can be bridged")
 			}
 


More information about the lxc-devel mailing list