[lxc-devel] [lxd/master] lxd/networks: Don't needlessly flush iptables

[stgraber on Github]

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/20190930/a81874d6/attachment.bin>
-------------- next part --------------
From 8049ff1e1ce7165af935442561ccddbbcdb8bd14 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 30 Sep 2019 22:45:34 -0400
Subject: [PATCH 1/2] lxd/networks: Split functions and pass oldConfig
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

diff --git a/lxd/networks.go b/lxd/networks.go
index 5b70543614..3153b56245 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -1050,6 +1050,10 @@ func (n *network) Rename(name string) error {
 }
 
 func (n *network) Start() error {
+	return n.Setup(nil)
+}
+
+func (n *network) Setup(oldConfig map[string]string) error {
 	// If we are in mock mode, just no-op.
 	if n.state.OS.MockMode {
 		return nil
@@ -2051,7 +2055,7 @@ func (n *network) Update(newNetwork api.NetworkPut) error {
 			n.state.Cluster.NetworkUpdate(n.name, n.description, n.config)
 
 			// Reset any change that was made to the bridge
-			n.Start()
+			n.Setup(newConfig)
 		}
 	}()
 
@@ -2131,7 +2135,7 @@ func (n *network) Update(newNetwork api.NetworkPut) error {
 
 	// Restart the network
 	if !userOnly {
-		err = n.Start()
+		err = n.Setup(oldConfig)
 		if err != nil {
 			return err
 		}

From b4acecc914c3b041fe00ea44800bc1876578b6fc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Mon, 30 Sep 2019 23:18:28 -0400
Subject: [PATCH 2/2] lxd/networks: Reduce calls to iptables clear
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #6258

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

diff --git a/lxd/networks.go b/lxd/networks.go
index 3153b56245..bd3245fb87 100644
--- a/lxd/networks.go
+++ b/lxd/networks.go
@@ -1201,19 +1201,23 @@ func (n *network) Setup(oldConfig map[string]string) error {
 	}
 
 	// Remove any existing IPv4 iptables rules
-	err = iptables.NetworkClear("ipv4", n.name, "")
-	if err != nil {
-		return err
-	}
+	if n.config["ipv4.firewall"] == "" || shared.IsTrue(n.config["ipv4.firewall"]) || (oldConfig != nil && (oldConfig["ipv4.firewall"] == "" || shared.IsTrue(oldConfig["ipv4.firewall"]))) {
+		err = iptables.NetworkClear("ipv4", n.name, "")
+		if err != nil {
+			return err
+		}
 
-	err = iptables.NetworkClear("ipv4", n.name, "mangle")
-	if err != nil {
-		return err
+		err = iptables.NetworkClear("ipv4", n.name, "mangle")
+		if err != nil {
+			return err
+		}
 	}
 
-	err = iptables.NetworkClear("ipv4", n.name, "nat")
-	if err != nil {
-		return err
+	if shared.IsTrue(n.config["ipv4.nat"]) || (oldConfig != nil && shared.IsTrue(oldConfig["ipv4.nat"])) {
+		err = iptables.NetworkClear("ipv4", n.name, "nat")
+		if err != nil {
+			return err
+		}
 	}
 
 	// Snapshot container specific IPv4 routes (added with boot proto) before removing IPv4 addresses.
@@ -1399,14 +1403,18 @@ func (n *network) Setup(oldConfig map[string]string) error {
 	}
 
 	// Remove any existing IPv6 iptables rules
-	err = iptables.NetworkClear("ipv6", n.name, "")
-	if err != nil {
-		return err
+	if n.config["ipv6.firewall"] == "" || shared.IsTrue(n.config["ipv6.firewall"]) || (oldConfig != nil && (oldConfig["ipv6.firewall"] == "" || shared.IsTrue(oldConfig["ipv6.firewall"]))) {
+		err = iptables.NetworkClear("ipv6", n.name, "")
+		if err != nil {
+			return err
+		}
 	}
 
-	err = iptables.NetworkClear("ipv6", n.name, "nat")
-	if err != nil {
-		return err
+	if shared.IsTrue(n.config["ipv6.nat"]) || (oldConfig != nil && shared.IsTrue(oldConfig["ipv6.nat"])) {
+		err = iptables.NetworkClear("ipv6", n.name, "nat")
+		if err != nil {
+			return err
+		}
 	}
 
 	// Snapshot container specific IPv6 routes (added with boot proto) before removing IPv6 addresses.
@@ -1970,33 +1978,41 @@ func (n *network) Stop() error {
 	}
 
 	// Cleanup iptables
-	err := iptables.NetworkClear("ipv4", n.name, "")
-	if err != nil {
-		return err
-	}
+	if n.config["ipv4.firewall"] == "" || shared.IsTrue(n.config["ipv4.firewall"]) {
+		err := iptables.NetworkClear("ipv4", n.name, "")
+		if err != nil {
+			return err
+		}
 
-	err = iptables.NetworkClear("ipv4", n.name, "mangle")
-	if err != nil {
-		return err
+		err = iptables.NetworkClear("ipv4", n.name, "mangle")
+		if err != nil {
+			return err
+		}
 	}
 
-	err = iptables.NetworkClear("ipv4", n.name, "nat")
-	if err != nil {
-		return err
+	if shared.IsTrue(n.config["ipv4.nat"]) {
+		err := iptables.NetworkClear("ipv4", n.name, "nat")
+		if err != nil {
+			return err
+		}
 	}
 
-	err = iptables.NetworkClear("ipv6", n.name, "")
-	if err != nil {
-		return err
+	if n.config["ipv6.firewall"] == "" || shared.IsTrue(n.config["ipv6.firewall"]) {
+		err := iptables.NetworkClear("ipv6", n.name, "")
+		if err != nil {
+			return err
+		}
 	}
 
-	err = iptables.NetworkClear("ipv6", n.name, "nat")
-	if err != nil {
-		return err
+	if shared.IsTrue(n.config["ipv6.nat"]) {
+		err := iptables.NetworkClear("ipv6", n.name, "nat")
+		if err != nil {
+			return err
+		}
 	}
 
 	// Kill any existing dnsmasq and forkdns daemon for this network
-	err = dnsmasq.Kill(n.name, false)
+	err := dnsmasq.Kill(n.name, false)
 	if err != nil {
 		return err
 	}