merge trunk

5 files changed, 665 insertions, 284 deletions

diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py
index c0bd76adf..9f9d282b6 100644
--- a/nova/network/linux_net.py
+++ b/nova/network/linux_net.py
@@ -17,15 +17,17 @@
 Implements vlans, bridges, and iptables rules using linux utilities.
 """
 
+import inspect
 import os
 
+from eventlet import semaphore
+
 from nova import db
 from nova import exception
 from nova import flags
 from nova import log as logging
 from nova import utils
 
-
 LOG = logging.getLogger("nova.linux_net")
 
 
@@ -52,8 +54,6 @@ flags.DEFINE_string('dhcpbridge', _bin_file('nova-dhcpbridge'),
                         'location of nova-dhcpbridge')
 flags.DEFINE_string('routing_source_ip', '$my_ip',
                     'Public IP of network host')
-flags.DEFINE_bool('use_nova_chains', False,
-                  'use the nova_ routing chains instead of default')
 flags.DEFINE_string('input_chain', 'INPUT',
                     'chain to add nova_input to')
 
@@ -63,79 +63,332 @@ flags.DEFINE_string('dmz_cidr', '10.128.0.0/24',
                     'dmz range that should be accepted')
 
 
+binary_name = os.path.basename(inspect.stack()[-1][1])
+
+
+class IptablesRule(object):
+    """An iptables rule
+
+    You shouldn't need to use this class directly, it's only used by
+    IptablesManager
+    """
+    def __init__(self, chain, rule, wrap=True, top=False):
+        self.chain = chain
+        self.rule = rule
+        self.wrap = wrap
+        self.top = top
+
+    def __eq__(self, other):
+        return ((self.chain == other.chain) and
+                (self.rule == other.rule) and
+                (self.top == other.top) and
+                (self.wrap == other.wrap))
+
+    def __ne__(self, other):
+        return not self == other
+
+    def __str__(self):
+        if self.wrap:
+            chain = '%s-%s' % (binary_name, self.chain)
+        else:
+            chain = self.chain
+        return '-A %s %s' % (chain, self.rule)
+
+
+class IptablesTable(object):
+    """An iptables table"""
+
+    def __init__(self):
+        self.rules = []
+        self.chains = set()
+        self.unwrapped_chains = set()
+
+    def add_chain(self, name, wrap=True):
+        """Adds a named chain to the table
+
+        The chain name is wrapped to be unique for the component creating
+        it, so different components of Nova can safely create identically
+        named chains without interfering with one another.
+
+        At the moment, its wrapped name is <binary name>-<chain name>,
+        so if nova-compute creates a chain named "OUTPUT", it'll actually
+        end up named "nova-compute-OUTPUT".
+        """
+        if wrap:
+            self.chains.add(name)
+        else:
+            self.unwrapped_chains.add(name)
+
+    def remove_chain(self, name, wrap=True):
+        """Remove named chain
+
+        This removal "cascades". All rule in the chain are removed, as are
+        all rules in other chains that jump to it.
+
+        If the chain is not found, this is merely logged.
+        """
+        if wrap:
+            chain_set = self.chains
+        else:
+            chain_set = self.unwrapped_chains
+
+        if name not in chain_set:
+            LOG.debug(_("Attempted to remove chain %s which doesn't exist"),
+                      name)
+            return
+
+        chain_set.remove(name)
+        self.rules = filter(lambda r: r.chain != name, self.rules)
+
+        if wrap:
+            jump_snippet = '-j %s-%s' % (binary_name, name)
+        else:
+            jump_snippet = '-j %s' % (name,)
+
+        self.rules = filter(lambda r: jump_snippet not in r.rule, self.rules)
+
+    def add_rule(self, chain, rule, wrap=True, top=False):
+        """Add a rule to the table
+
+        This is just like what you'd feed to iptables, just without
+        the "-A <chain name>" bit at the start.
+
+        However, if you need to jump to one of your wrapped chains,
+        prepend its name with a '$' which will ensure the wrapping
+        is applied correctly.
+        """
+        if wrap and chain not in self.chains:
+            raise ValueError(_("Unknown chain: %r") % chain)
+
+        if '$' in rule:
+            rule = ' '.join(map(self._wrap_target_chain, rule.split(' ')))
+
+        self.rules.append(IptablesRule(chain, rule, wrap, top))
+
+    def _wrap_target_chain(self, s):
+        if s.startswith('$'):
+            return '%s-%s' % (binary_name, s[1:])
+        return s
+
+    def remove_rule(self, chain, rule, wrap=True, top=False):
+        """Remove a rule from a chain
+
+        Note: The rule must be exactly identical to the one that was added.
+        You cannot switch arguments around like you can with the iptables
+        CLI tool.
+        """
+        try:
+            self.rules.remove(IptablesRule(chain, rule, wrap, top))
+        except ValueError:
+            LOG.debug(_("Tried to remove rule that wasn't there:"
+                        " %(chain)r %(rule)r %(wrap)r %(top)r"),
+                      {'chain': chain, 'rule': rule,
+                       'top': top, 'wrap': wrap})
+
+
+class IptablesManager(object):
+    """Wrapper for iptables
+
+    See IptablesTable for some usage docs
+
+    A number of chains are set up to begin with.
+
+    First, nova-filter-top. It's added at the top of FORWARD and OUTPUT. Its
+    name is not wrapped, so it's shared between the various nova workers. It's
+    intended for rules that need to live at the top of the FORWARD and OUTPUT
+    chains. It's in both the ipv4 and ipv6 set of tables.
+
+    For ipv4 and ipv6, the builtin INPUT, OUTPUT, and FORWARD filter chains are
+    wrapped, meaning that the "real" INPUT chain has a rule that jumps to the
+    wrapped INPUT chain, etc. Additionally, there's a wrapped chain named
+    "local" which is jumped to from nova-filter-top.
+
+    For ipv4, the builtin PREROUTING, OUTPUT, and POSTROUTING nat chains are
+    wrapped in the same was as the builtin filter chains. Additionally, there's
+    a snat chain that is applied after the POSTROUTING chain.
+    """
+    def __init__(self, execute=None):
+        if not execute:
+            if FLAGS.fake_network:
+                self.execute = lambda *args, **kwargs: ('', '')
+            else:
+                self.execute = utils.execute
+        else:
+            self.execute = execute
+
+        self.ipv4 = {'filter': IptablesTable(),
+                     'nat': IptablesTable()}
+        self.ipv6 = {'filter': IptablesTable()}
+
+        # Add a nova-filter-top chain. It's intended to be shared
+        # among the various nova components. It sits at the very top
+        # of FORWARD and OUTPUT.
+        for tables in [self.ipv4, self.ipv6]:
+            tables['filter'].add_chain('nova-filter-top', wrap=False)
+            tables['filter'].add_rule('FORWARD', '-j nova-filter-top',
+                                      wrap=False, top=True)
+            tables['filter'].add_rule('OUTPUT', '-j nova-filter-top',
+                                      wrap=False, top=True)
+
+            tables['filter'].add_chain('local')
+            tables['filter'].add_rule('nova-filter-top', '-j $local',
+                                      wrap=False)
+
+        # Wrap the builtin chains
+        builtin_chains = {4: {'filter': ['INPUT', 'OUTPUT', 'FORWARD'],
+                              'nat': ['PREROUTING', 'OUTPUT', 'POSTROUTING']},
+                          6: {'filter': ['INPUT', 'OUTPUT', 'FORWARD']}}
+
+        for ip_version in builtin_chains:
+            if ip_version == 4:
+                tables = self.ipv4
+            elif ip_version == 6:
+                tables = self.ipv6
+
+            for table, chains in builtin_chains[ip_version].iteritems():
+                for chain in chains:
+                    tables[table].add_chain(chain)
+                    tables[table].add_rule(chain, '-j $%s' % (chain,),
+                                           wrap=False)
+
+        # Add a nova-postrouting-bottom chain. It's intended to be shared
+        # among the various nova components. We set it as the last chain
+        # of POSTROUTING chain.
+        self.ipv4['nat'].add_chain('nova-postrouting-bottom', wrap=False)
+        self.ipv4['nat'].add_rule('POSTROUTING', '-j nova-postrouting-bottom',
+                                  wrap=False)
+
+        # We add a snat chain to the shared nova-postrouting-bottom chain
+        # so that it's applied last.
+        self.ipv4['nat'].add_chain('snat')
+        self.ipv4['nat'].add_rule('nova-postrouting-bottom', '-j $snat',
+                                  wrap=False)
+
+        # And then we add a floating-snat chain and jump to first thing in
+        # the snat chain.
+        self.ipv4['nat'].add_chain('floating-snat')
+        self.ipv4['nat'].add_rule('snat', '-j $floating-snat')
+
+        self.semaphore = semaphore.Semaphore()
+
+    @utils.synchronized('iptables')
+    def apply(self):
+        """Apply the current in-memory set of iptables rules
+
+        This will blow away any rules left over from previous runs of the
+        same component of Nova, and replace them with our current set of
+        rules. This happens atomically, thanks to iptables-restore.
+
+        We wrap the call in a semaphore lock, so that we don't race with
+        ourselves. In the event of a race with another component running
+        an iptables-* command at the same time, we retry up to 5 times.
+        """
+        with self.semaphore:
+            s = [('iptables', self.ipv4)]
+            if FLAGS.use_ipv6:
+                s += [('ip6tables', self.ipv6)]
+
+            for cmd, tables in s:
+                for table in tables:
+                    current_table, _ = self.execute('sudo',
+                                                    '%s-save' % (cmd,),
+                                                    '-t', '%s' % (table,),
+                                                    attempts=5)
+                    current_lines = current_table.split('\n')
+                    new_filter = self._modify_rules(current_lines,
+                                                    tables[table])
+                    self.execute('sudo', '%s-restore' % (cmd,),
+                                 process_input='\n'.join(new_filter),
+                                 attempts=5)
+
+    def _modify_rules(self, current_lines, table, binary=None):
+        unwrapped_chains = table.unwrapped_chains
+        chains = table.chains
+        rules = table.rules
+
+        # Remove any trace of our rules
+        new_filter = filter(lambda line: binary_name not in line,
+                            current_lines)
+
+        seen_chains = False
+        rules_index = 0
+        for rules_index, rule in enumerate(new_filter):
+            if not seen_chains:
+                if rule.startswith(':'):
+                    seen_chains = True
+            else:
+                if not rule.startswith(':'):
+                    break
+
+        our_rules = []
+        for rule in rules:
+            rule_str = str(rule)
+            if rule.top:
+                # rule.top == True means we want this rule to be at the top.
+                # Further down, we weed out duplicates from the bottom of the
+                # list, so here we remove the dupes ahead of time.
+                new_filter = filter(lambda s: s.strip() != rule_str.strip(),
+                                    new_filter)
+            our_rules += [rule_str]
+
+        new_filter[rules_index:rules_index] = our_rules
+
+        new_filter[rules_index:rules_index] = [':%s - [0:0]' % \
+                                               (name,) \
+                                               for name in unwrapped_chains]
+        new_filter[rules_index:rules_index] = [':%s-%s - [0:0]' % \
+                                               (binary_name, name,) \
+                                               for name in chains]
+
+        seen_lines = set()
+
+        def _weed_out_duplicates(line):
+            line = line.strip()
+            if line in seen_lines:
+                return False
+            else:
+                seen_lines.add(line)
+                return True
+
+        # We filter duplicates, letting the *last* occurrence take
+        # precendence.
+        new_filter.reverse()
+        new_filter = filter(_weed_out_duplicates, new_filter)
+        new_filter.reverse()
+        return new_filter
+
+
+iptables_manager = IptablesManager()
+
+
 def metadata_forward():
     """Create forwarding rule for metadata"""
-    _confirm_rule("PREROUTING", '-t', 'nat', '-s', '0.0.0.0/0',
-             '-d', '169.254.169.254/32', '-p', 'tcp', '-m', 'tcp',
-             '--dport', '80', '-j', 'DNAT',
-             '--to-destination',
-             '%s:%s' % (FLAGS.ec2_dmz_host, FLAGS.ec2_port))
+    iptables_manager.ipv4['nat'].add_rule("PREROUTING",
+                                          "-s 0.0.0.0/0 -d 169.254.169.254/32 "
+                                          "-p tcp -m tcp --dport 80 -j DNAT "
+                                          "--to-destination %s:%s" % \
+                                          (FLAGS.ec2_dmz_host, FLAGS.ec2_port))
+    iptables_manager.apply()
 
 
 def init_host():
     """Basic networking setup goes here"""
-
-    if FLAGS.use_nova_chains:
-        _execute('sudo', 'iptables', '-N', 'nova_input', check_exit_code=False)
-        _execute('sudo', 'iptables', '-D', FLAGS.input_chain,
-                 '-j', 'nova_input',
-                 check_exit_code=False)
-        _execute('sudo', 'iptables', '-A', FLAGS.input_chain,
-                 '-j', 'nova_input')
-        _execute('sudo', 'iptables', '-N', 'nova_forward',
-                 check_exit_code=False)
-        _execute('sudo', 'iptables', '-D', 'FORWARD', '-j', 'nova_forward',
-                 check_exit_code=False)
-        _execute('sudo', 'iptables', '-A', 'FORWARD', '-j', 'nova_forward')
-        _execute('sudo', 'iptables', '-N', 'nova_output',
-                 check_exit_code=False)
-        _execute('sudo', 'iptables', '-D', 'OUTPUT', '-j', 'nova_output',
-                 check_exit_code=False)
-        _execute('sudo', 'iptables', '-A', 'OUTPUT', '-j', 'nova_output')
-        _execute('sudo', 'iptables', '-t', 'nat', '-N', 'nova_prerouting',
-                 check_exit_code=False)
-        _execute('sudo', 'iptables', '-t', 'nat', '-D', 'PREROUTING',
-                 '-j', 'nova_prerouting', check_exit_code=False)
-        _execute('sudo', 'iptables', '-t', 'nat', '-A', 'PREROUTING',
-                 '-j', 'nova_prerouting')
-        _execute('sudo', 'iptables', '-t', 'nat', '-N', 'nova_postrouting',
-                 check_exit_code=False)
-        _execute('sudo', 'iptables', '-t', 'nat', '-D', 'POSTROUTING',
-                 '-j', 'nova_postrouting', check_exit_code=False)
-        _execute('sudo', 'iptables', '-t', 'nat', '-A', 'POSTROUTING',
-                 '-j', 'nova_postrouting')
-        _execute('sudo', 'iptables', '-t', 'nat', '-N', 'nova_snatting',
-                 check_exit_code=False)
-        _execute('sudo', 'iptables', '-t', 'nat', '-D', 'POSTROUTING',
-                 '-j nova_snatting', check_exit_code=False)
-        _execute('sudo', 'iptables', '-t', 'nat', '-A', 'POSTROUTING',
-                 '-j', 'nova_snatting')
-        _execute('sudo', 'iptables', '-t', 'nat', '-N', 'nova_output',
-                 check_exit_code=False)
-        _execute('sudo', 'iptables', '-t', 'nat', '-D', 'OUTPUT',
-                 '-j nova_output', check_exit_code=False)
-        _execute('sudo', 'iptables', '-t', 'nat', '-A', 'OUTPUT',
-                 '-j', 'nova_output')
-    else:
-        # NOTE(vish): This makes it easy to ensure snatting rules always
-        #             come after the accept rules in the postrouting chain
-        _execute('sudo', 'iptables', '-t', 'nat', '-N', 'SNATTING',
-                 check_exit_code=False)
-        _execute('sudo', 'iptables', '-t', 'nat', '-D', 'POSTROUTING',
-                 '-j', 'SNATTING', check_exit_code=False)
-        _execute('sudo', 'iptables', '-t', 'nat', '-A', 'POSTROUTING',
-                 '-j', 'SNATTING')
-
     # NOTE(devcamcar): Cloud public SNAT entries and the default
     # SNAT rule for outbound traffic.
-    _confirm_rule("SNATTING", '-t', 'nat', '-s', FLAGS.fixed_range,
-             '-j', 'SNAT', '--to-source', FLAGS.routing_source_ip,
-             append=True)
+    iptables_manager.ipv4['nat'].add_rule("snat",
+                                          "-s %s -j SNAT --to-source %s" % \
+                                           (FLAGS.fixed_range,
+                                            FLAGS.routing_source_ip))
 
-    _confirm_rule("POSTROUTING", '-t', 'nat', '-s', FLAGS.fixed_range,
-                  '-d', FLAGS.dmz_cidr, '-j', 'ACCEPT')
-    _confirm_rule("POSTROUTING", '-t', 'nat', '-s', FLAGS.fixed_range,
-                  '-d', FLAGS.fixed_range, '-j', 'ACCEPT')
+    iptables_manager.ipv4['nat'].add_rule("POSTROUTING",
+                                          "-s %s -d %s -j ACCEPT" % \
+                                          (FLAGS.fixed_range, FLAGS.dmz_cidr))
+
+    iptables_manager.ipv4['nat'].add_rule("POSTROUTING",
+                                          "-s %(range)s -d %(range)s "
+                                          "-j ACCEPT" % \
+                                          {'range': FLAGS.fixed_range})
+    iptables_manager.apply()
 
 
 def bind_floating_ip(floating_ip, check_exit_code=True):
@@ -153,31 +406,36 @@ def unbind_floating_ip(floating_ip):
 
 def ensure_vlan_forward(public_ip, port, private_ip):
     """Sets up forwarding rules for vlan"""
-    _confirm_rule("FORWARD", '-d', private_ip, '-p', 'udp',
-                  '--dport', '1194', '-j', 'ACCEPT')
-    _confirm_rule("PREROUTING", '-t', 'nat', '-d', public_ip, '-p', 'udp',
-                  '--dport', port, '-j', 'DNAT', '--to', '%s:1194'
-                  % private_ip)
+    iptables_manager.ipv4['filter'].add_rule("FORWARD",
+                                             "-d %s -p udp "
+                                             "--dport 1194 "
+                                             "-j ACCEPT" % private_ip)
+    iptables_manager.ipv4['nat'].add_rule("PREROUTING",
+                                          "-d %s -p udp "
+                                          "--dport %s -j DNAT --to %s:1194" %
+                                          (public_ip, port, private_ip))
+    iptables_manager.apply()
 
 
 def ensure_floating_forward(floating_ip, fixed_ip):
     """Ensure floating ip forwarding rule"""
-    _confirm_rule("PREROUTING", '-t', 'nat', '-d', floating_ip, '-j', 'DNAT',
-                  '--to', fixed_ip)
-    _confirm_rule("OUTPUT", '-t', 'nat', '-d', floating_ip, '-j', 'DNAT',
-                  '--to', fixed_ip)
-    _confirm_rule("SNATTING", '-t', 'nat', '-s', fixed_ip, '-j', 'SNAT',
-                  '--to', floating_ip)
+    for chain, rule in floating_forward_rules(floating_ip, fixed_ip):
+        iptables_manager.ipv4['nat'].add_rule(chain, rule)
+    iptables_manager.apply()
 
 
 def remove_floating_forward(floating_ip, fixed_ip):
     """Remove forwarding for floating ip"""
-    _remove_rule("PREROUTING", '-t', 'nat', '-d', floating_ip, '-j', 'DNAT',
-                 '--to', fixed_ip)
-    _remove_rule("OUTPUT", '-t', 'nat', '-d', floating_ip, '-j', 'DNAT',
-                 '--to', fixed_ip)
-    _remove_rule("SNATTING", '-t', 'nat', '-s', fixed_ip, '-j', 'SNAT',
-                 '--to', floating_ip)
+    for chain, rule in floating_forward_rules(floating_ip, fixed_ip):
+        iptables_manager.ipv4['nat'].remove_rule(chain, rule)
+    iptables_manager.apply()
+
+
+def floating_forward_rules(floating_ip, fixed_ip):
+    return [("PREROUTING", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
+            ("OUTPUT", "-d %s -j DNAT --to %s" % (floating_ip, fixed_ip)),
+            ("floating-snat",
+             "-s %s -j SNAT --to %s" % (fixed_ip, floating_ip))]
 
 
 def ensure_vlan_bridge(vlan_num, bridge, net_attrs=None):
@@ -269,19 +527,12 @@ def ensure_bridge(bridge, interface, net_attrs=None):
                            "enslave it to bridge %s.\n" % (interface, bridge)):
             raise exception.Error("Failed to add interface: %s" % err)
 
-    if FLAGS.use_nova_chains:
-        (out, err) = _execute('sudo', 'iptables', '-N', 'nova_forward',
-                              check_exit_code=False)
-        if err != 'iptables: Chain already exists.\n':
-            # NOTE(vish): chain didn't exist link chain
-            _execute('sudo', 'iptables', '-D', 'FORWARD', '-j', 'nova_forward',
-                     check_exit_code=False)
-            _execute('sudo', 'iptables', '-A', 'FORWARD', '-j', 'nova_forward')
-
-    _confirm_rule("FORWARD", '--in-interface', bridge, '-j', 'ACCEPT')
-    _confirm_rule("FORWARD", '--out-interface', bridge, '-j', 'ACCEPT')
-    _execute('sudo', 'iptables', '-N', 'nova-local', check_exit_code=False)
-    _confirm_rule("FORWARD", '-j', 'nova-local')
+    iptables_manager.ipv4['filter'].add_rule("FORWARD",
+                                             "--in-interface %s -j ACCEPT" % \
+                                             bridge)
+    iptables_manager.ipv4['filter'].add_rule("FORWARD",
+                                             "--out-interface %s -j ACCEPT" % \
+                                             bridge)
 
 
 def get_dhcp_hosts(context, network_id):
@@ -401,27 +652,6 @@ def _device_exists(device):
     return not err
 
 
-def _confirm_rule(chain, *cmd, **kwargs):
-    append = kwargs.get('append', False)
-    """Delete and re-add iptables rule"""
-    if FLAGS.use_nova_chains:
-        chain = "nova_%s" % chain.lower()
-    if append:
-        loc = "-A"
-    else:
-        loc = "-I"
-    _execute('sudo', 'iptables', '--delete', chain, *cmd,
-             check_exit_code=False)
-    _execute('sudo', 'iptables', loc, chain, *cmd)
-
-
-def _remove_rule(chain, *cmd):
-    """Remove iptables rule"""
-    if FLAGS.use_nova_chains:
-        chain = "%s" % chain.lower()
-    _execute('sudo', 'iptables', '--delete', chain, *cmd)
-
-
 def _dnsmasq_cmd(net):
     """Builds dnsmasq command"""
     cmd = ['sudo', '-E', 'dnsmasq',
diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py
index 19099ff4c..53e35ce7e 100644
--- a/nova/tests/test_network.py
+++ b/nova/tests/test_network.py
@@ -29,11 +29,153 @@ from nova import log as logging
 from nova import test
 from nova import utils
 from nova.auth import manager
+from nova.network import linux_net
 
 FLAGS = flags.FLAGS
 LOG = logging.getLogger('nova.tests.network')
 
 
+class IptablesManagerTestCase(test.TestCase):
+    sample_filter = ['#Generated by iptables-save on Fri Feb 18 15:17:05 2011',
+                     '*filter',
+                     ':INPUT ACCEPT [2223527:305688874]',
+                     ':FORWARD ACCEPT [0:0]',
+                     ':OUTPUT ACCEPT [2172501:140856656]',
+                     ':nova-compute-FORWARD - [0:0]',
+                     ':nova-compute-INPUT - [0:0]',
+                     ':nova-compute-local - [0:0]',
+                     ':nova-compute-OUTPUT - [0:0]',
+                     ':nova-filter-top - [0:0]',
+                     '-A FORWARD -j nova-filter-top ',
+                     '-A OUTPUT -j nova-filter-top ',
+                     '-A nova-filter-top -j nova-compute-local ',
+                     '-A INPUT -j nova-compute-INPUT ',
+                     '-A OUTPUT -j nova-compute-OUTPUT ',
+                     '-A FORWARD -j nova-compute-FORWARD ',
+                     '-A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT ',
+                     '-A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT ',
+                     '-A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT ',
+                     '-A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT ',
+                     '-A FORWARD -s 192.168.122.0/24 -i virbr0 -j ACCEPT ',
+                     '-A FORWARD -i virbr0 -o virbr0 -j ACCEPT ',
+                     '-A FORWARD -o virbr0 -j REJECT --reject-with '
+                     'icmp-port-unreachable ',
+                     '-A FORWARD -i virbr0 -j REJECT --reject-with '
+                     'icmp-port-unreachable ',
+                     'COMMIT',
+                     '# Completed on Fri Feb 18 15:17:05 2011']
+
+    sample_nat = ['# Generated by iptables-save on Fri Feb 18 15:17:05 2011',
+                  '*nat',
+                  ':PREROUTING ACCEPT [3936:762355]',
+                  ':INPUT ACCEPT [2447:225266]',
+                  ':OUTPUT ACCEPT [63491:4191863]',
+                  ':POSTROUTING ACCEPT [63112:4108641]',
+                  ':nova-compute-OUTPUT - [0:0]',
+                  ':nova-compute-floating-ip-snat - [0:0]',
+                  ':nova-compute-SNATTING - [0:0]',
+                  ':nova-compute-PREROUTING - [0:0]',
+                  ':nova-compute-POSTROUTING - [0:0]',
+                  ':nova-postrouting-bottom - [0:0]',
+                  '-A PREROUTING -j nova-compute-PREROUTING ',
+                  '-A OUTPUT -j nova-compute-OUTPUT ',
+                  '-A POSTROUTING -j nova-compute-POSTROUTING ',
+                  '-A POSTROUTING -j nova-postrouting-bottom ',
+                  '-A nova-postrouting-bottom -j nova-compute-SNATTING ',
+                  '-A nova-compute-SNATTING -j nova-compute-floating-ip-snat ',
+                  'COMMIT',
+                  '# Completed on Fri Feb 18 15:17:05 2011']
+
+    def setUp(self):
+        super(IptablesManagerTestCase, self).setUp()
+        self.manager = linux_net.IptablesManager()
+
+    def test_filter_rules_are_wrapped(self):
+        current_lines = self.sample_filter
+
+        table = self.manager.ipv4['filter']
+        table.add_rule('FORWARD', '-s 1.2.3.4/5 -j DROP')
+        new_lines = self.manager._modify_rules(current_lines, table)
+        self.assertTrue('-A run_tests.py-FORWARD '
+                        '-s 1.2.3.4/5 -j DROP' in new_lines)
+
+        table.remove_rule('FORWARD', '-s 1.2.3.4/5 -j DROP')
+        new_lines = self.manager._modify_rules(current_lines, table)
+        self.assertTrue('-A run_tests.py-FORWARD '
+                        '-s 1.2.3.4/5 -j DROP' not in new_lines)
+
+    def test_nat_rules(self):
+        current_lines = self.sample_nat
+        new_lines = self.manager._modify_rules(current_lines,
+                                               self.manager.ipv4['nat'])
+
+        for line in [':nova-compute-OUTPUT - [0:0]',
+                     ':nova-compute-floating-ip-snat - [0:0]',
+                     ':nova-compute-SNATTING - [0:0]',
+                     ':nova-compute-PREROUTING - [0:0]',
+                     ':nova-compute-POSTROUTING - [0:0]']:
+            self.assertTrue(line in new_lines, "One of nova-compute's chains "
+                                               "went missing.")
+
+        seen_lines = set()
+        for line in new_lines:
+            line = line.strip()
+            self.assertTrue(line not in seen_lines,
+                            "Duplicate line: %s" % line)
+            seen_lines.add(line)
+
+        last_postrouting_line = ''
+
+        for line in new_lines:
+            if line.startswith('-A POSTROUTING'):
+                last_postrouting_line = line
+
+        self.assertTrue('-j nova-postrouting-bottom' in last_postrouting_line,
+                        "Last POSTROUTING rule does not jump to "
+                        "nova-postouting-bottom: %s" % last_postrouting_line)
+
+        for chain in ['POSTROUTING', 'PREROUTING', 'OUTPUT']:
+            self.assertTrue('-A %s -j run_tests.py-%s' \
+                            % (chain, chain) in new_lines,
+                            "Built-in chain %s not wrapped" % (chain,))
+
+    def test_filter_rules(self):
+        current_lines = self.sample_filter
+        new_lines = self.manager._modify_rules(current_lines,
+                                               self.manager.ipv4['filter'])
+
+        for line in [':nova-compute-FORWARD - [0:0]',
+                     ':nova-compute-INPUT - [0:0]',
+                     ':nova-compute-local - [0:0]',
+                     ':nova-compute-OUTPUT - [0:0]']:
+            self.assertTrue(line in new_lines, "One of nova-compute's chains"
+                                               " went missing.")
+
+        seen_lines = set()
+        for line in new_lines:
+            line = line.strip()
+            self.assertTrue(line not in seen_lines,
+                            "Duplicate line: %s" % line)
+            seen_lines.add(line)
+
+        for chain in ['FORWARD', 'OUTPUT']:
+            for line in new_lines:
+                if line.startswith('-A %s' % chain):
+                    self.assertTrue('-j nova-filter-top' in line,
+                                    "First %s rule does not "
+                                    "jump to nova-filter-top" % chain)
+                    break
+
+        self.assertTrue('-A nova-filter-top '
+                        '-j run_tests.py-local' in new_lines,
+                        "nova-filter-top does not jump to wrapped local chain")
+
+        for chain in ['INPUT', 'OUTPUT', 'FORWARD']:
+            self.assertTrue('-A %s -j run_tests.py-%s' \
+                            % (chain, chain) in new_lines,
+                            "Built-in chain %s not wrapped" % (chain,))
+
+
 class NetworkTestCase(test.TestCase):
     """Test cases for network code"""
     def setUp(self):
diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py
index 0625296a4..648de3b77 100644
--- a/nova/tests/test_virt.py
+++ b/nova/tests/test_virt.py
@@ -14,6 +14,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import re
 import os
 
 import eventlet
@@ -301,16 +302,22 @@ class IptablesFirewallTestCase(test.TestCase):
         self.manager.delete_user(self.user)
         super(IptablesFirewallTestCase, self).tearDown()
 
-    in_rules = [
+    in_nat_rules = [
+      '# Generated by iptables-save v1.4.10 on Sat Feb 19 00:03:19 2011',
+      '*nat',
+      ':PREROUTING ACCEPT [1170:189210]',
+      ':INPUT ACCEPT [844:71028]',
+      ':OUTPUT ACCEPT [5149:405186]',
+      ':POSTROUTING ACCEPT [5063:386098]'
+    ]
+
+    in_filter_rules = [
       '# Generated by iptables-save v1.4.4 on Mon Dec  6 11:54:13 2010',
       '*filter',
       ':INPUT ACCEPT [969615:281627771]',
       ':FORWARD ACCEPT [0:0]',
       ':OUTPUT ACCEPT [915599:63811649]',
       ':nova-block-ipv4 - [0:0]',
-      '-A INPUT -i virbr0 -p udp -m udp --dport 53 -j ACCEPT ',
-      '-A INPUT -i virbr0 -p tcp -m tcp --dport 53 -j ACCEPT ',
-      '-A INPUT -i virbr0 -p udp -m udp --dport 67 -j ACCEPT ',
       '-A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT ',
       '-A FORWARD -d 192.168.122.0/24 -o virbr0 -m state --state RELATED'
       ',ESTABLISHED -j ACCEPT ',
@@ -322,7 +329,7 @@ class IptablesFirewallTestCase(test.TestCase):
       '# Completed on Mon Dec  6 11:54:13 2010',
     ]
 
-    in6_rules = [
+    in6_filter_rules = [
       '# Generated by ip6tables-save v1.4.4 on Tue Jan 18 23:47:56 2011',
       '*filter',
       ':INPUT ACCEPT [349155:75810423]',
@@ -385,21 +392,31 @@ class IptablesFirewallTestCase(test.TestCase):
         def fake_iptables_execute(*cmd, **kwargs):
             process_input = kwargs.get('process_input', None)
             if cmd == ('sudo', 'ip6tables-save', '-t', 'filter'):
-                return '\n'.join(self.in6_rules), None
+                return '\n'.join(self.in6_filter_rules), None
             if cmd == ('sudo', 'iptables-save', '-t', 'filter'):
-                return '\n'.join(self.in_rules), None
+                return '\n'.join(self.in_filter_rules), None
+            if cmd == ('sudo', 'iptables-save', '-t', 'nat'):
+                return '\n'.join(self.in_nat_rules), None
             if cmd == ('sudo', 'iptables-restore'):
-                self.out_rules = process_input.split('\n')
+                lines = process_input.split('\n')
+                if '*filter' in lines:
+                    self.out_rules = lines
                 return '', ''
             if cmd == ('sudo', 'ip6tables-restore'):
-                self.out6_rules = process_input.split('\n')
+                lines = process_input.split('\n')
+                if '*filter' in lines:
+                    self.out6_rules = lines
                 return '', ''
-        self.fw.execute = fake_iptables_execute
+            print cmd, kwargs
+
+        from nova.network import linux_net
+        linux_net.iptables_manager.execute = fake_iptables_execute
 
         self.fw.prepare_instance_filter(instance_ref)
         self.fw.apply_instance_filter(instance_ref)
 
-        in_rules = filter(lambda l: not l.startswith('#'), self.in_rules)
+        in_rules = filter(lambda l: not l.startswith('#'),
+                          self.in_filter_rules)
         for rule in in_rules:
             if not 'nova' in rule:
                 self.assertTrue(rule in self.out_rules,
@@ -422,17 +439,18 @@ class IptablesFirewallTestCase(test.TestCase):
         self.assertTrue(security_group_chain,
                         "The security group chain wasn't added")
 
-        self.assertTrue('-A %s -p icmp -s 192.168.11.0/24 -j ACCEPT' % \
-                               security_group_chain in self.out_rules,
+        regex = re.compile('-A .* -p icmp -s 192.168.11.0/24 -j ACCEPT')
+        self.assertTrue(len(filter(regex.match, self.out_rules)) > 0,
                         "ICMP acceptance rule wasn't added")
 
-        self.assertTrue('-A %s -p icmp -s 192.168.11.0/24 -m icmp --icmp-type '
-                        '8 -j ACCEPT' % security_group_chain in self.out_rules,
+        regex = re.compile('-A .* -p icmp -s 192.168.11.0/24 -m icmp '
+                           '--icmp-type 8 -j ACCEPT')
+        self.assertTrue(len(filter(regex.match, self.out_rules)) > 0,
                         "ICMP Echo Request acceptance rule wasn't added")
 
-        self.assertTrue('-A %s -p tcp -s 192.168.10.0/24 -m multiport '
-                        '--dports 80:81 -j ACCEPT' % security_group_chain \
-                            in self.out_rules,
+        regex = re.compile('-A .* -p tcp -s 192.168.10.0/24 -m multiport '
+                           '--dports 80:81 -j ACCEPT')
+        self.assertTrue(len(filter(regex.match, self.out_rules)) > 0,
                         "TCP port 80/81 acceptance rule wasn't added")
         db.instance_destroy(admin_ctxt, instance_ref['id'])
 
diff --git a/nova/utils.py b/nova/utils.py
index 0466fecf4..87e726394 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -139,34 +139,44 @@ def execute(*cmd, **kwargs):
     stdin = kwargs.get('stdin', subprocess.PIPE)
     stdout = kwargs.get('stdout', subprocess.PIPE)
     stderr = kwargs.get('stderr', subprocess.PIPE)
+    attempts = kwargs.get('attempts', 1)
     cmd = map(str, cmd)
 
-    LOG.debug(_("Running cmd (subprocess): %s"), ' '.join(cmd))
-    env = os.environ.copy()
-    if addl_env:
-        env.update(addl_env)
-    obj = subprocess.Popen(cmd, stdin=stdin,
-            stdout=stdout, stderr=stderr, env=env)
-    result = None
-    if process_input != None:
-        result = obj.communicate(process_input)
-    else:
-        result = obj.communicate()
-    obj.stdin.close()
-    if obj.returncode:
-        LOG.debug(_("Result was %s") % obj.returncode)
-        if type(check_exit_code) == types.IntType \
-                and obj.returncode != check_exit_code:
-            (stdout, stderr) = result
-            raise ProcessExecutionError(exit_code=obj.returncode,
-                                        stdout=stdout,
-                                        stderr=stderr,
-                                        cmd=' '.join(cmd))
-    # NOTE(termie): this appears to be necessary to let the subprocess call
-    #               clean something up in between calls, without it two
-    #               execute calls in a row hangs the second one
-    greenthread.sleep(0)
-    return result
+    while attempts > 0:
+        attempts -= 1
+        try:
+            LOG.debug(_("Running cmd (subprocess): %s"), ' '.join(cmd))
+            env = os.environ.copy()
+            if addl_env:
+                env.update(addl_env)
+            obj = subprocess.Popen(cmd, stdin=stdin,
+                    stdout=stdout, stderr=stderr, env=env)
+            result = None
+            if process_input != None:
+                result = obj.communicate(process_input)
+            else:
+                result = obj.communicate()
+            obj.stdin.close()
+            if obj.returncode:
+                LOG.debug(_("Result was %s") % obj.returncode)
+                if type(check_exit_code) == types.IntType \
+                        and obj.returncode != check_exit_code:
+                    (stdout, stderr) = result
+                    raise ProcessExecutionError(exit_code=obj.returncode,
+                                                stdout=stdout,
+                                                stderr=stderr,
+                                                cmd=' '.join(cmd))
+            # NOTE(termie): this appears to be necessary to let the subprocess
+            #               call clean something up in between calls, without
+            #               it two execute calls in a row hangs the second one
+            greenthread.sleep(0)
+            return result
+        except ProcessExecutionError:
+            if not attempts:
+                raise
+            else:
+                LOG.debug(_("%r failed. Retrying."), cmd)
+                greenthread.sleep(random.randint(20, 200) / 100.0)
 
 
 def ssh_execute(ssh, cmd, process_input=None,
diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py
index 44b07213a..61ef256f9 100644
--- a/nova/virt/libvirt_conn.py
+++ b/nova/virt/libvirt_conn.py
@@ -56,7 +56,6 @@ from nova import flags
 from nova import log as logging
 #from nova import test
 from nova import utils
-#from nova.api import context
 from nova.auth import manager
 from nova.compute import instance_types
 from nova.compute import power_state
@@ -1222,10 +1221,14 @@ class NWFilterFirewall(FirewallDriver):
 
 class IptablesFirewallDriver(FirewallDriver):
     def __init__(self, execute=None, **kwargs):
-        self.execute = execute or utils.execute
+        from nova.network import linux_net
+        self.iptables = linux_net.iptables_manager
         self.instances = {}
         self.nwfilter = NWFilterFirewall(kwargs['get_connection'])
 
+        self.iptables.ipv4['filter'].add_chain('sg-fallback')
+        self.iptables.ipv4['filter'].add_rule('sg-fallback', '-j DROP')
+
     def setup_basic_filtering(self, instance):
         """Use NWFilter from libvirt for this."""
         return self.nwfilter.setup_basic_filtering(instance)
@@ -1234,128 +1237,97 @@ class IptablesFirewallDriver(FirewallDriver):
         """No-op. Everything is done in prepare_instance_filter"""
         pass
 
-    def remove_instance(self, instance):
+    def unfilter_instance(self, instance):
         if instance['id'] in self.instances:
             del self.instances[instance['id']]
+            self.remove_filters_for_instance(instance)
+            self.iptables.apply()
         else:
             LOG.info(_('Attempted to unfilter instance %s which is not '
                        'filtered'), instance['id'])
 
-    def add_instance(self, instance):
+    def prepare_instance_filter(self, instance):
         self.instances[instance['id']] = instance
+        self.add_filters_for_instance(instance)
+        self.iptables.apply()
 
-    def unfilter_instance(self, instance):
-        self.remove_instance(instance)
-        self.apply_ruleset()
+    def add_filters_for_instance(self, instance):
+        chain_name = self._instance_chain_name(instance)
 
-    def prepare_instance_filter(self, instance):
-        self.add_instance(instance)
-        self.apply_ruleset()
-
-    def apply_ruleset(self):
-        current_filter, _ = self.execute('sudo', 'iptables-save',
-                                         '-t', 'filter')
-        current_lines = current_filter.split('\n')
-        new_filter = self.modify_rules(current_lines, 4)
-        self.execute('sudo', 'iptables-restore',
-                     process_input='\n'.join(new_filter))
-        if(FLAGS.use_ipv6):
-            current_filter, _ = self.execute('sudo', 'ip6tables-save',
-                                             '-t', 'filter')
-            current_lines = current_filter.split('\n')
-            new_filter = self.modify_rules(current_lines, 6)
-            self.execute('sudo', 'ip6tables-restore',
-                         process_input='\n'.join(new_filter))
-
-    def modify_rules(self, current_lines, ip_version=4):
+        self.iptables.ipv4['filter'].add_chain(chain_name)
+        ipv4_address = self._ip_for_instance(instance)
+        self.iptables.ipv4['filter'].add_rule('local',
+                                              '-d %s -j $%s' %
+                                              (ipv4_address, chain_name))
+
+        if FLAGS.use_ipv6:
+            self.iptables.ipv6['filter'].add_chain(chain_name)
+            ipv6_address = self._ip_for_instance_v6(instance)
+            self.iptables.ipv6['filter'].add_rule('local',
+                                                  '-d %s -j $%s' %
+                                                  (ipv6_address,
+                                                   chain_name))
+
+        ipv4_rules, ipv6_rules = self.instance_rules(instance)
+
+        for rule in ipv4_rules:
+            self.iptables.ipv4['filter'].add_rule(chain_name, rule)
+
+        if FLAGS.use_ipv6:
+            for rule in ipv6_rules:
+                self.iptables.ipv6['filter'].add_rule(chain_name, rule)
+
+    def remove_filters_for_instance(self, instance):
+        chain_name = self._instance_chain_name(instance)
+
+        self.iptables.ipv4['filter'].remove_chain(chain_name)
+        if FLAGS.use_ipv6:
+            self.iptables.ipv6['filter'].remove_chain(chain_name)
+
+    def instance_rules(self, instance):
         ctxt = context.get_admin_context()
-        # Remove any trace of nova rules.
-        new_filter = filter(lambda l: 'nova-' not in l, current_lines)
-
-        seen_chains = False
-        for rules_index in range(len(new_filter)):
-            if not seen_chains:
-                if new_filter[rules_index].startswith(':'):
-                    seen_chains = True
-            elif seen_chains == 1:
-                if not new_filter[rules_index].startswith(':'):
-                    break
 
-        our_chains = [':nova-fallback - [0:0]']
-        our_rules = ['-A nova-fallback -j DROP']
-
-        our_chains += [':nova-local - [0:0]']
-        our_rules += ['-A FORWARD -j nova-local']
-        our_rules += ['-A OUTPUT -j nova-local']
-
-        security_groups = {}
-        # Add our chains
-        # First, we add instance chains and rules
-        for instance_id in self.instances:
-            instance = self.instances[instance_id]
-            chain_name = self._instance_chain_name(instance)
-            if(ip_version == 4):
-                ip_address = self._ip_for_instance(instance)
-            elif(ip_version == 6):
-                ip_address = self._ip_for_instance_v6(instance)
-
-            our_chains += [':%s - [0:0]' % chain_name]
-
-            # Jump to the per-instance chain
-            our_rules += ['-A nova-local -d %s -j %s' % (ip_address,
-                                                         chain_name)]
-
-            # Always drop invalid packets
-            our_rules += ['-A %s -m state --state '
-                          'INVALID -j DROP' % (chain_name,)]
-
-            # Allow established connections
-            our_rules += ['-A %s -m state --state '
-                          'ESTABLISHED,RELATED -j ACCEPT' % (chain_name,)]
-
-            # Jump to each security group chain in turn
-            for security_group in \
-                            db.security_group_get_by_instance(ctxt,
-                                                              instance['id']):
-                security_groups[security_group['id']] = security_group
-
-                sg_chain_name = self._security_group_chain_name(
-                                                          security_group['id'])
+        ipv4_rules = []
+        ipv6_rules = []
 
-                our_rules += ['-A %s -j %s' % (chain_name, sg_chain_name)]
-
-            if(ip_version == 4):
-                # Allow DHCP responses
-                dhcp_server = self._dhcp_server_for_instance(instance)
-                our_rules += ['-A %s -s %s -p udp --sport 67 --dport 68 '
-                                    '-j ACCEPT ' % (chain_name, dhcp_server)]
-                #Allow project network traffic
-                if (FLAGS.allow_project_net_traffic):
-                    cidr = self._project_cidr_for_instance(instance)
-                    our_rules += ['-A %s -s %s -j ACCEPT' % (chain_name, cidr)]
-            elif(ip_version == 6):
-                # Allow RA responses
-                ra_server = self._ra_server_for_instance(instance)
-                if ra_server:
-                    our_rules += ['-A %s -s %s -p icmpv6 -j ACCEPT' %
-                                  (chain_name, ra_server + "/128")]
-                #Allow project network traffic
-                if (FLAGS.allow_project_net_traffic):
-                    cidrv6 = self._project_cidrv6_for_instance(instance)
-                    our_rules += ['-A %s -s %s -j ACCEPT' %
-                                        (chain_name, cidrv6)]
-
-            # If nothing matches, jump to the fallback chain
-            our_rules += ['-A %s -j nova-fallback' % (chain_name,)]
+        # Always drop invalid packets
+        ipv4_rules += ['-m state --state ' 'INVALID -j DROP']
+        ipv6_rules += ['-m state --state ' 'INVALID -j DROP']
 
-        # then, security group chains and rules
-        for security_group_id in security_groups:
-            chain_name = self._security_group_chain_name(security_group_id)
-            our_chains += [':%s - [0:0]' % chain_name]
+        # Allow established connections
+        ipv4_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT']
+        ipv6_rules += ['-m state --state ESTABLISHED,RELATED -j ACCEPT']
+
+        dhcp_server = self._dhcp_server_for_instance(instance)
+        ipv4_rules += ['-s %s -p udp --sport 67 --dport 68 '
+                       '-j ACCEPT' % (dhcp_server,)]
+
+        #Allow project network traffic
+        if FLAGS.allow_project_net_traffic:
+            cidr = self._project_cidr_for_instance(instance)
+            ipv4_rules += ['-s %s -j ACCEPT' % (cidr,)]
+
+        # We wrap these in FLAGS.use_ipv6 because they might cause
+        # a DB lookup. The other ones are just list operations, so
+        # they're not worth the clutter.
+        if FLAGS.use_ipv6:
+            # Allow RA responses
+            ra_server = self._ra_server_for_instance(instance)
+            if ra_server:
+                ipv6_rules += ['-s %s/128 -p icmpv6 -j ACCEPT' % (ra_server,)]
+
+            #Allow project network traffic
+            if FLAGS.allow_project_net_traffic:
+                cidrv6 = self._project_cidrv6_for_instance(instance)
+                ipv6_rules += ['-s %s -j ACCEPT' % (cidrv6,)]
 
-            rules = \
-              db.security_group_rule_get_by_security_group(ctxt,
-                                                          security_group_id)
+        security_groups = db.security_group_get_by_instance(ctxt,
+                                                            instance['id'])
+
+        # then, security group chains and rules
+        for security_group in security_groups:
+            rules = db.security_group_rule_get_by_security_group(ctxt,
+                                                          security_group['id'])
 
             for rule in rules:
                 logging.info('%r', rule)
@@ -1366,14 +1338,16 @@ class IptablesFirewallDriver(FirewallDriver):
                     continue
 
                 version = _get_ip_version(rule.cidr)
-                if version != ip_version:
-                    continue
+                if version == 4:
+                    rules = ipv4_rules
+                else:
+                    rules = ipv6_rules
 
                 protocol = rule.protocol
                 if version == 6 and rule.protocol == 'icmp':
                     protocol = 'icmpv6'
 
-                args = ['-A', chain_name, '-p', protocol, '-s', rule.cidr]
+                args = ['-p', protocol, '-s', rule.cidr]
 
                 if rule.protocol in ['udp', 'tcp']:
                     if rule.from_port == rule.to_port:
@@ -1394,32 +1368,39 @@ class IptablesFirewallDriver(FirewallDriver):
                             icmp_type_arg += '/%s' % icmp_code
 
                     if icmp_type_arg:
-                        if(ip_version == 4):
+                        if version == 4:
                             args += ['-m', 'icmp', '--icmp-type',
                                      icmp_type_arg]
-                        elif(ip_version == 6):
+                        elif version == 6:
                             args += ['-m', 'icmp6', '--icmpv6-type',
                                      icmp_type_arg]
 
                 args += ['-j ACCEPT']
-                our_rules += [' '.join(args)]
+                rules += [' '.join(args)]
+
+        ipv4_rules += ['-j $sg-fallback']
+        ipv6_rules += ['-j $sg-fallback']
 
-        new_filter[rules_index:rules_index] = our_rules
-        new_filter[rules_index:rules_index] = our_chains
-        logging.info('new_filter: %s', '\n'.join(new_filter))
-        return new_filter
+        return ipv4_rules, ipv6_rules
 
     def refresh_security_group_members(self, security_group):
         pass
 
     def refresh_security_group_rules(self, security_group):
-        self.apply_ruleset()
+        for instance in self.instances.values():
+            # We use the semaphore to make sure noone applies the rule set
+            # after we've yanked the existing rules but before we've put in
+            # the new ones.
+            with self.iptables.semaphore:
+                self.remove_filters_for_instance(instance)
+                self.add_filters_for_instance(instance)
+        self.iptables.apply()
 
     def _security_group_chain_name(self, security_group_id):
         return 'nova-sg-%s' % (security_group_id,)
 
     def _instance_chain_name(self, instance):
-        return 'nova-inst-%s' % (instance['id'],)
+        return 'inst-%s' % (instance['id'],)
 
     def _ip_for_instance(self, instance):
         return db.instance_get_fixed_address(context.get_admin_context(),