From 77a525dfe96728cbec38e78b8da457aa6955918e Mon Sep 17 00:00:00 2001
Message-Id: <77a525dfe96728cbec38e78b8da457aa6955918e.1386932212.git.jdenemar@redhat.com>
From: Laine Stump <laine@laine.org>
Date: Wed, 11 Dec 2013 04:26:39 -0700
Subject: [PATCH] network: properly update iptables rules during net-update

This patch resolves:

  https://bugzilla.redhat.com/show_bug.cgi?id=1035336

The basic problem is that during a network update, the required
iptables rules sometimes change, and this was being handled by simply
removing and re-adding the rules. However, the removal of the old
rules was done based on the *new* state of the network, which would
mean that some of the rules would not match those currently in the
system, so the old rules wouldn't be removed.

This patch removes the old rules prior to updating the network
definition then adds the new rules as soon as the definition is
updated. Note that this could lead to a stray packet or two during the
interim, but that was already a problem before (the period of limbo is
now just slightly longer).

While moving the location for the rules, I added a few more sections
that should result in the iptables rules being redone:

DHCP_RANGE and DHCP_HOST - these are needed because adding/removing a dhcp
host entry could lead to the dhcp service being started/stopped, which
would require that the mangle rule that fixes up dhcp response
checksums sould need to be added/removed, and this wasn't being done.

(cherry picked from commit 54f9492353170b1ffc78a44c06ed3f9ecaab6ccf)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/network/bridge_driver.c | 48 ++++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 6e31840..54d388b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3307,6 +3307,7 @@ networkUpdate(virNetworkPtr net,
     size_t i;
     virNetworkIpDefPtr ipdef;
     bool oldDhcpActive = false;
+    bool needFirewallRefresh = false;
 
 
     virCheckFlags(VIR_NETWORK_UPDATE_AFFECT_LIVE |
@@ -3348,8 +3349,40 @@ networkUpdate(virNetworkPtr net,
             flags |= VIR_NETWORK_UPDATE_AFFECT_CONFIG;
     }
 
+    if (isActive && (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE)) {
+        /* Take care of anything that must be done before updating the
+         * live NetworkDef.
+         */
+        if (network->def->forward.type == VIR_NETWORK_FORWARD_NONE ||
+            network->def->forward.type == VIR_NETWORK_FORWARD_NAT ||
+            network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
+            switch (section) {
+            case VIR_NETWORK_SECTION_FORWARD:
+            case VIR_NETWORK_SECTION_FORWARD_INTERFACE:
+            case VIR_NETWORK_SECTION_IP:
+            case VIR_NETWORK_SECTION_IP_DHCP_RANGE:
+            case VIR_NETWORK_SECTION_IP_DHCP_HOST:
+                /* these could affect the firewall rules, so remove the
+                 * old rules (and remember to load new ones after the
+                 * update).
+                 */
+                networkRemoveFirewallRules(network);
+                needFirewallRefresh = true;
+                break;
+            default:
+                break;
+            }
+        }
+    }
+
     /* update the network config in memory/on disk */
-    if (virNetworkObjUpdate(network, command, section, parentIndex, xml, flags) < 0)
+    if (virNetworkObjUpdate(network, command, section, parentIndex, xml, flags) < 0) {
+        if (needFirewallRefresh)
+            ignore_value(networkAddFirewallRules(network));
+        goto cleanup;
+    }
+
+    if (needFirewallRefresh && networkAddFirewallRules(network) < 0)
         goto cleanup;
 
     if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) {
@@ -3419,19 +3452,6 @@ networkUpdate(virNetworkPtr net,
                 goto cleanup;
         }
 
-        if ((section == VIR_NETWORK_SECTION_IP ||
-             section == VIR_NETWORK_SECTION_FORWARD ||
-             section == VIR_NETWORK_SECTION_FORWARD_INTERFACE) &&
-           (network->def->forward.type == VIR_NETWORK_FORWARD_NONE ||
-            network->def->forward.type == VIR_NETWORK_FORWARD_NAT ||
-            network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE)) {
-            /* these could affect the iptables rules */
-            networkRemoveFirewallRules(network);
-            if (networkAddFirewallRules(network) < 0)
-                goto cleanup;
-
-        }
-
         /* save current network state to disk */
         if ((ret = virNetworkSaveStatus(driverState->stateDir,
                                         network)) < 0) {
-- 
1.8.5.1