openstack/deb-nova
Code Issues Proposed changes

Fix simultaneous timeout with smart iptables usage

Browse Source 
Simultaneous boots were getting timeouts in allocate_for_network.
This was due to our inefficient use of iptables where we attempt
to recreate the same rules over and over using initialize_gateway,
plug, and restart_dhcp.

This patch makes the iptables code a little bit smarter by only
shelling out if the rule list has changed. Note that there is
a small difference in the interpretation of duplicate rules.
Previously, the last copy of the rule would win whereas now
the first copy of the rule will win. Multiple copies of the same
rule were never supported. This means the only thing that one
needs to remember is it is necessary to remove a rule and re-add
it if you want to move it later in the list.

Fixes bug 1199433

Change-Id: I9920f8d6aa269d9b3bfd88a8dbbb00f859deb158
This commit is contained in:
Vishvananda Ishaya
2013-08-07 11:02:59 -07:00
parent aca4ef5b8e
commit 176c483cdf
5 changed files with 62 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
nova/network/linux_net.py
View File

@@ -184,6 +184,7 @@ class IptablesTable(object):
self.chains = set()
self.unwrapped_chains = set()
self.remove_chains = set()
self.dirty = True
def add_chain(self, name, wrap=True):
"""Adds a named chain to the table.
@@ -201,6 +202,7 @@ class IptablesTable(object):
self.chains.add(name)
else:
self.unwrapped_chains.add(name)
self.dirty = True
def remove_chain(self, name, wrap=True):
"""Remove named chain.
@@ -220,6 +222,7 @@ class IptablesTable(object):
LOG.warn(_('Attempted to remove chain %s which does not exist'),
name)
return
self.dirty = True
# non-wrapped chains and rules need to be dealt with specially,
# so we keep a list of them to be iterated over in apply()
@@ -257,7 +260,12 @@ class IptablesTable(object):
if '$' in rule:
rule = ' '.join(map(self._wrap_target_chain, rule.split(' ')))
self.rules.append(IptablesRule(chain, rule, wrap, top))
rule_obj = IptablesRule(chain, rule, wrap, top)
if rule_obj in self.rules:
LOG.debug(_("Skipping duplicate iptables rule addition"))
else:
self.rules.append(IptablesRule(chain, rule, wrap, top))
self.dirty = True
def _wrap_target_chain(self, s):
if s.startswith('$'):
@@ -276,6 +284,7 @@ class IptablesTable(object):
self.rules.remove(IptablesRule(chain, rule, wrap, top))
if not wrap:
self.remove_rules.append(IptablesRule(chain, rule, wrap, top))
self.dirty = True
except ValueError:
LOG.warn(_('Tried to remove rule that was not there:'
' %(chain)r %(rule)r %(wrap)r %(top)r'),
@@ -288,12 +297,17 @@ class IptablesTable(object):
regex = re.compile(regex)
num_rules = len(self.rules)
self.rules = filter(lambda r: not regex.match(str(r)), self.rules)
return num_rules - len(self.rules)
removed = num_rules - len(self.rules)
if removed > 0:
self.dirty = True
return removed
def empty_chain(self, chain, wrap=True):
"""Remove all rules from a chain."""
chained_rules = [rule for rule in self.rules
if rule.chain == chain and rule.wrap == wrap]
if chained_rules:
self.dirty = True
for rule in chained_rules:
self.rules.remove(rule)
@@ -389,13 +403,25 @@ class IptablesManager(object):
def defer_apply_off(self):
self.iptables_apply_deferred = False
self._apply()
self.apply()
def dirty(self):
for table in self.ipv4.itervalues():
if table.dirty:
return True
if CONF.use_ipv6:
for table in self.ipv6.itervalues():
if table.dirty:
return True
return False
def apply(self):
if self.iptables_apply_deferred:
return
self._apply()
if self.dirty():
self._apply()
else:
LOG.debug(_("Skipping apply due to lack of new rules"))
@utils.synchronized('iptables', external=True)
def _apply(self):
@@ -419,6 +445,7 @@ class IptablesManager(object):
start, end = self._find_table(all_lines, table_name)
all_lines[start:end] = self._modify_rules(
all_lines[start:end], table, table_name)
table.dirty = False
self.execute('%s-restore' % (cmd,), '-c', run_as_root=True,
process_input='\n'.join(all_lines),
attempts=5)

2
nova/tests/fake_network.py
View File

@@ -334,7 +334,7 @@ def fake_get_instance_nw_info(stubs, num_networks=1, ips_per_vif=2,
subnet_v6 = dict(
cidr='2001:db8:0:%x::/64' % i,
gateway='fe80::def')
gateway='2001:db8:0:%x::1' % i)
return [subnet_v4, subnet_v6]
def get_network_by_uuid(context, uuid):

4
nova/tests/network/test_manager.py
View File

@@ -177,7 +177,7 @@ class FlatNetworkTestCase(test.TestCase):
'dhcp_server': '192.168.1.1',
'dns': ['192.168.%d.3' % nid, '192.168.%d.4' % nid],
'gateway': '192.168.%d.1' % nid,
'gateway_v6': 'fe80::def',
'gateway_v6': '2001:db8:0:1::1',
'ip6s': 'DONTCARE',
'ips': 'DONTCARE',
'label': 'test%d' % nid,
@@ -197,7 +197,7 @@ class FlatNetworkTestCase(test.TestCase):
check = [{'enabled': 'DONTCARE',
'ip': '2001:db8:0:1::%x' % nid,
'netmask': 64,
'gateway': 'fe80::def'}]
'gateway': '2001:db8:0:1::1'}]
self.assertThat(info['ip6s'], matchers.DictListMatches(check))
num_fixed_ips = len(info['ips'])

25
nova/tests/test_iptables_network.py
View File

@@ -88,6 +88,31 @@ class IptablesManagerTestCase(test.TestCase):
super(IptablesManagerTestCase, self).setUp()
self.manager = linux_net.IptablesManager()
def test_duplicate_rules_no_dirty(self):
table = self.manager.ipv4['filter']
table.dirty = False
num_rules = len(table.rules)
table.add_rule('FORWARD', '-s 1.2.3.4/5 -j DROP')
self.assertEqual(len(table.rules), num_rules + 1)
self.assertTrue(table.dirty)
table.dirty = False
num_rules = len(table.rules)
table.add_rule('FORWARD', '-s 1.2.3.4/5 -j DROP')
self.assertEqual(len(table.rules), num_rules)
self.assertFalse(table.dirty)
def test_clean_tables_no_apply(self):
for table in self.manager.ipv4.itervalues():
table.dirty = False
for table in self.manager.ipv6.itervalues():
table.dirty = False
def error_apply():
raise test.TestingException()
self.stubs.Set(self.manager, '_apply', error_apply)
self.manager.apply()
def test_filter_rules_are_wrapped(self):
current_lines = self.sample_filter

4
nova/tests/virt/xenapi/test_xenapi.py
View File

@@ -619,11 +619,11 @@ class XenAPIVMTestCase(stubs.XenAPITestBase):
{'broadcast': '192.168.1.255',
'dns': ['192.168.1.4', '192.168.1.3'],
'gateway': '192.168.1.1',
'gateway_v6': 'fe80::def',
'gateway_v6': '2001:db8:0:1::1',
'ip6s': [{'enabled': '1',
'ip': '2001:db8:0:1::1',
'netmask': 64,
'gateway': 'fe80::def'}],
'gateway': '2001:db8:0:1::1'}],
'ips': [{'enabled': '1',
'ip': '192.168.1.100',
'netmask': '255.255.255.0',