[libvirt] [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 definitionm 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. --- src/network/bridge_driver.c | 47 +++++++++++++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 83dc931..d7f12e1 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2642,6 +2642,7 @@ networkUpdate(virNetworkPtr net, size_t i; virNetworkIpDefPtr ipdef; bool oldDhcpActive = false; + bool needFirewallRefresh = false; virCheckFlags(VIR_NETWORK_UPDATE_AFFECT_LIVE | @@ -2683,8 +2684,39 @@ 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; + 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) { @@ -2754,19 +2786,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.4.2