From f0539d49271df76222740f76d1b51188ee51a1c3 Mon Sep 17 00:00:00 2001 From: Joe Gordon Date: Mon, 14 Jan 2013 17:30:54 -0800 Subject: [PATCH] Reduce number of iptable-save restore loops Instead of running iptables-save and iptables-restore per table, run iptables save and restore on all tables at once. Change-Id: If610d07c22ddaeb03ab37ee64d3d3a55d5e5b34f --- nova/network/linux_net.py | 33 +++++++++++++------- nova/tests/network/test_linux_net.py | 16 +++------- nova/tests/test_libvirt.py | 45 +++++++++++----------------- nova/tests/test_xenapi.py | 25 ++++++++++++---- nova/tests/xenapi/stubs.py | 14 ++++----- 5 files changed, 70 insertions(+), 63 deletions(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index ea09f69b2f7..d24d52a283c 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -371,19 +371,32 @@ def _apply(self): s += [('ip6tables', self.ipv6)] for cmd, tables in s: + all_tables, _err = self.execute('%s-save' % (cmd,), '-c', + run_as_root=True, + attempts=5) + all_lines = all_tables.split('\n') for table in tables: - current_table, _err = self.execute('%s-save' % (cmd,), '-c', - '-t', '%s' % (table,), - run_as_root=True, - attempts=5) - current_lines = current_table.split('\n') - new_filter = self._modify_rules(current_lines, - tables[table]) - self.execute('%s-restore' % (cmd,), '-c', run_as_root=True, - process_input='\n'.join(new_filter), - attempts=5) + start, end = self._find_table(all_lines, table) + all_lines[start:end] = self._modify_rules( + all_lines[start:end], tables[table]) + self.execute('%s-restore' % (cmd,), '-c', run_as_root=True, + process_input='\n'.join(all_lines), + attempts=5) LOG.debug(_("IPTablesManager.apply completed with success")) + def _find_table(self, lines, table_name): + if len(lines) < 3: + # length only <2 when fake iptables + return (0, 0) + try: + start = lines.index('*%s' % table_name) - 1 + except ValueError: + # Couldn't find table_name + # For Unit Tests + return (0, 0) + end = lines[start:].index('COMMIT') + start + 2 + return (start, end) + def _modify_rules(self, current_lines, table, binary=None): unwrapped_chains = table.unwrapped_chains chains = table.chains diff --git a/nova/tests/network/test_linux_net.py b/nova/tests/network/test_linux_net.py index c0770902d06..8a7865b8313 100644 --- a/nova/tests/network/test_linux_net.py +++ b/nova/tests/network/test_linux_net.py @@ -469,13 +469,9 @@ def fake_ensure(_self, bridge, interface, network, gateway): '--arp-ip-src', dhcp, '-j', 'DROP'), ('ebtables', '-I', 'OUTPUT', '-p', 'ARP', '-o', iface, '--arp-ip-src', dhcp, '-j', 'DROP'), - ('iptables-save', '-c', '-t', 'filter'), + ('iptables-save', '-c'), ('iptables-restore', '-c'), - ('iptables-save', '-c', '-t', 'mangle'), - ('iptables-restore', '-c'), - ('iptables-save', '-c', '-t', 'nat'), - ('iptables-restore', '-c'), - ('ip6tables-save', '-c', '-t', 'filter'), + ('ip6tables-save', '-c'), ('ip6tables-restore', '-c'), ] self.assertEqual(executes, expected) @@ -508,13 +504,9 @@ def fake_remove(_self, bridge, gateway): '--arp-ip-dst', dhcp, '-j', 'DROP'), ('ebtables', '-D', 'OUTPUT', '-p', 'ARP', '-o', iface, '--arp-ip-src', dhcp, '-j', 'DROP'), - ('iptables-save', '-c', '-t', 'filter'), - ('iptables-restore', '-c'), - ('iptables-save', '-c', '-t', 'mangle'), - ('iptables-restore', '-c'), - ('iptables-save', '-c', '-t', 'nat'), + ('iptables-save', '-c'), ('iptables-restore', '-c'), - ('ip6tables-save', '-c', '-t', 'filter'), + ('ip6tables-save', '-c'), ('ip6tables-restore', '-c'), ] self.assertEqual(executes, expected) diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index 53bb1b9844c..286625bd9f9 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -3539,30 +3539,25 @@ def nwfilterDefineXML(*args, **kwargs): fake.FakeVirtAPI(), get_connection=lambda: self.fake_libvirt_connection) - in_nat_rules = [ + in_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_mangle_rules = [ - '# Generated by iptables-save v1.4.12 on Tue Dec 18 15:50:25 201;', - '*mangle', - ':PREROUTING ACCEPT [241:39722]', - ':INPUT ACCEPT [230:39282]', - ':FORWARD ACCEPT [0:0]', - ':OUTPUT ACCEPT [266:26558]', - ':POSTROUTING ACCEPT [267:26590]', - '-A POSTROUTING -o virbr0 -p udp -m udp --dport 68 -j CHECKSUM ' - '--checksum-fill', - 'COMMIT', - '# Completed on Tue Dec 18 15:50:25 2012', - ] - - in_filter_rules = [ + '# Completed on Tue Dec 18 15:50:25 2012', + '# Generated by iptables-save v1.4.12 on Tue Dec 18 15:50:25 201;', + '*mangle', + ':PREROUTING ACCEPT [241:39722]', + ':INPUT ACCEPT [230:39282]', + ':FORWARD ACCEPT [0:0]', + ':OUTPUT ACCEPT [266:26558]', + ':POSTROUTING ACCEPT [267:26590]', + '-A POSTROUTING -o virbr0 -p udp -m udp --dport 68 -j CHECKSUM ' + '--checksum-fill', + 'COMMIT', + '# Completed on Tue Dec 18 15:50:25 2012', '# Generated by iptables-save v1.4.4 on Mon Dec 6 11:54:13 2010', '*filter', ':INPUT ACCEPT [969615:281627771]', @@ -3657,15 +3652,11 @@ def test_static_filters(self): # self.fw.add_instance(instance_ref) def fake_iptables_execute(*cmd, **kwargs): process_input = kwargs.get('process_input', None) - if cmd == ('ip6tables-save', '-c', '-t', 'filter'): + if cmd == ('ip6tables-save', '-c'): return '\n'.join(self.in6_filter_rules), None - if cmd == ('iptables-save', '-c', '-t', 'filter'): - return '\n'.join(self.in_filter_rules), None - if cmd == ('iptables-save', '-c', '-t', 'nat'): - return '\n'.join(self.in_nat_rules), None - if cmd == ('iptables-save', '-c', '-t', 'mangle'): - return '\n'.join(self.in_mangle_rules), None - if cmd == ('iptables-restore', '-c',): + if cmd == ('iptables-save', '-c'): + return '\n'.join(self.in_rules), None + if cmd == ('iptables-restore', '-c'): lines = process_input.split('\n') if '*filter' in lines: self.out_rules = lines @@ -3689,7 +3680,7 @@ def fake_iptables_execute(*cmd, **kwargs): self.fw.apply_instance_filter(instance_ref, network_info) in_rules = filter(lambda l: not l.startswith('#'), - self.in_filter_rules) + self.in_rules) for rule in in_rules: if not 'nova' in rule: self.assertTrue(rule in self.out_rules, diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index 0b1c5d0e754..067e28a1367 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -1822,16 +1822,31 @@ def test_get_all_bw_counters_in_failure_case(self): # Consider abstracting common code in a base class for firewall driver testing. class XenAPIDom0IptablesFirewallTestCase(stubs.XenAPITestBase): - _in_nat_rules = [ + _in_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 = [ + '# Completed on Mon Dec 6 11:54:13 2010', + '# Generated by iptables-save v1.4.4 on Mon Dec 6 11:54:13 2010', + '*mangle', + ':INPUT ACCEPT [969615:281627771]', + ':FORWARD ACCEPT [0:0]', + ':OUTPUT ACCEPT [915599:63811649]', + ':nova-block-ipv4 - [0:0]', + '[0:0] -A INPUT -i virbr0 -p tcp -m tcp --dport 67 -j ACCEPT ', + '[0:0] -A FORWARD -d 192.168.122.0/24 -o virbr0 -m state --state RELATED' + ',ESTABLISHED -j ACCEPT ', + '[0:0] -A FORWARD -s 192.168.122.0/24 -i virbr0 -j ACCEPT ', + '[0:0] -A FORWARD -i virbr0 -o virbr0 -j ACCEPT ', + '[0:0] -A FORWARD -o virbr0 -j REJECT ' + '--reject-with icmp-port-unreachable ', + '[0:0] -A FORWARD -i virbr0 -j REJECT ' + '--reject-with icmp-port-unreachable ', + 'COMMIT', + '# Completed on Mon Dec 6 11:54:13 2010', '# Generated by iptables-save v1.4.4 on Mon Dec 6 11:54:13 2010', '*filter', ':INPUT ACCEPT [969615:281627771]', @@ -1916,7 +1931,7 @@ def _create_test_security_group(self): def _validate_security_group(self): in_rules = filter(lambda l: not l.startswith('#'), - self._in_filter_rules) + self._in_rules) for rule in in_rules: if not 'nova' in rule: self.assertTrue(rule in self._out_rules, diff --git a/nova/tests/xenapi/stubs.py b/nova/tests/xenapi/stubs.py index 85c85b5e214..fa214b23e65 100644 --- a/nova/tests/xenapi/stubs.py +++ b/nova/tests/xenapi/stubs.py @@ -208,12 +208,10 @@ class FakeSessionForFirewallTests(FakeSessionForVMTests): def __init__(self, uri, test_case=None): super(FakeSessionForFirewallTests, self).__init__(uri) - if hasattr(test_case, '_in_filter_rules'): - self._in_filter_rules = test_case._in_filter_rules + if hasattr(test_case, '_in_rules'): + self._in_rules = test_case._in_rules if hasattr(test_case, '_in6_filter_rules'): self._in6_filter_rules = test_case._in6_filter_rules - if hasattr(test_case, '_in_nat_rules'): - self._in_nat_rules = test_case._in_nat_rules self._test_case = test_case def host_call_plugin(self, _1, _2, plugin, method, args): @@ -230,12 +228,10 @@ def host_call_plugin(self, _1, _2, plugin, method, args): else: output = '' process_input = args.get('process_input', None) - if cmd == ['ip6tables-save', '-c', '-t', 'filter']: + if cmd == ['ip6tables-save', '-c']: output = '\n'.join(self._in6_filter_rules) - if cmd == ['iptables-save', '-c', '-t', 'filter']: - output = '\n'.join(self._in_filter_rules) - if cmd == ['iptables-save', '-c', '-t', 'nat']: - output = '\n'.join(self._in_nat_rules) + if cmd == ['iptables-save', '-c']: + output = '\n'.join(self._in_rules) if cmd == ['iptables-restore', '-c', ]: lines = process_input.split('\n') if '*filter' in lines: