From b5b919a7a3569ccb93c3d7d523c1edfaeddb7cb9 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Thu, 2 Apr 2015 21:11:06 -0400 Subject: [PATCH] Add ipset element and hashsize tunables Recently, these messages have been noticed in both tempest logs, as well as reported by downstream users syslog: Set IPv4915d358d-2c5b-43b5-9862 is full, maxelem 65536 reached So the default of 64K is not sufficient enough. This change adds two config options to control both the number of elements as well as the hashsize, since they should be tuned together for best performance. Slightly different formats were required for 'ipset create' and 'ipset restore'. The default values for these are now set to 131072 (maxelem) and 2048 (hashsize), which is an increase over their typical default values of 65536/1024 (respectively), in order to fix the errors seen in the tempest tests. DocImpact Change-Id: Ic0b5b38a840e737dc6be938230f4052974c8620f Closes-bug: #1439817 --- etc/neutron.conf | 9 +++++ neutron/agent/common/config.py | 15 ++++++++ neutron/agent/linux/ipset_manager.py | 20 +++++++++- .../unit/agent/linux/test_ipset_manager.py | 37 +++++++++++++++++-- 4 files changed, 75 insertions(+), 6 deletions(-) diff --git a/etc/neutron.conf b/etc/neutron.conf index 7bdfda9a9b7..1ab301ab1cf 100644 --- a/etc/neutron.conf +++ b/etc/neutron.conf @@ -664,6 +664,15 @@ lock_path = $state_path/lock # each rule's purpose. (System must support the iptables comments module.) # comment_iptables_rules = True +# Maximum number of elements which can be stored in an IPset. +# If None is specified, the system default will be used. +# ipset_maxelem = 131072 + +# Initial hash size for an IPset. Must be a power of 2, +# else the kernel will round it up automatically. +# If None is specified, the system default will be used. +# ipset_hashsize = 2048 + # Use the root helper when listing the namespaces on a system. This may not # be required depending on the security configuration. If the root helper is # not required, set this to False for a performance improvement. diff --git a/neutron/agent/common/config.py b/neutron/agent/common/config.py index 7e63ea38789..efc1ca47602 100644 --- a/neutron/agent/common/config.py +++ b/neutron/agent/common/config.py @@ -63,6 +63,17 @@ IPTABLES_OPTS = [ help=_("Add comments to iptables rules.")), ] +IPSET_OPTS = [ + cfg.IntOpt('ipset_maxelem', default=131072, + help=_("Maximum number of elements which can be stored in " + "an IPset. If None is specified, the system default " + "will be used.")), + cfg.IntOpt('ipset_hashsize', default=2048, + help=_("Initial hash size for an IPset. Must be a power of 2, " + "else the kernel will round it up automatically. If " + "None is specified, the system default will be used.")), +] + PROCESS_MONITOR_OPTS = [ cfg.StrOpt('check_child_processes_action', default='respawn', choices=['respawn', 'exit'], @@ -122,6 +133,10 @@ def register_iptables_opts(conf): conf.register_opts(IPTABLES_OPTS, 'AGENT') +def register_ipset_opts(conf): + conf.register_opts(IPSET_OPTS, 'AGENT') + + def register_process_monitor_opts(conf): conf.register_opts(PROCESS_MONITOR_OPTS, 'AGENT') diff --git a/neutron/agent/linux/ipset_manager.py b/neutron/agent/linux/ipset_manager.py index e5ab7a01e9c..33b6379b586 100644 --- a/neutron/agent/linux/ipset_manager.py +++ b/neutron/agent/linux/ipset_manager.py @@ -11,6 +11,9 @@ # See the License for the specific language governing permissions and # limitations under the License. +from oslo_config import cfg + +from neutron.agent.common import config from neutron.agent.linux import utils as linux_utils from neutron.common import utils @@ -29,6 +32,7 @@ class IpsetManager(object): def __init__(self, execute=None, namespace=None): self.execute = execute or linux_utils.execute self.namespace = namespace + config.register_ipset_opts(cfg.CONF) self.ipset_sets = {} @staticmethod @@ -39,6 +43,15 @@ class IpsetManager(object): name = ethertype + id return name[:IPSET_NAME_MAX_LENGTH] + @staticmethod + def get_hashargs(): + args = [] + if cfg.CONF.AGENT.ipset_hashsize: + args.extend(['hashsize', str(cfg.CONF.AGENT.ipset_hashsize)]) + if cfg.CONF.AGENT.ipset_maxelem: + args.extend(['maxelem', str(cfg.CONF.AGENT.ipset_maxelem)]) + return args + def set_exists(self, id, ethertype): """Returns true if the id+ethertype pair is known to the manager.""" set_name = self.get_name(id, ethertype) @@ -85,8 +98,10 @@ class IpsetManager(object): def _refresh_set(self, set_name, member_ips, ethertype): new_set_name = set_name + SWAP_SUFFIX set_type = self._get_ipset_set_type(ethertype) - process_input = ["create %s hash:ip family %s" % (new_set_name, - set_type)] + hash_args = ' '.join(self.get_hashargs()) + process_input = ["create %s hash:ip family %s %s" % (new_set_name, + set_type, + hash_args)] for ip in member_ips: process_input.append("add %s %s" % (new_set_name, ip)) @@ -103,6 +118,7 @@ class IpsetManager(object): def _create_set(self, set_name, ethertype): cmd = ['ipset', 'create', '-exist', set_name, 'hash:ip', 'family', self._get_ipset_set_type(ethertype)] + cmd.extend(self.get_hashargs()) self._apply(cmd) self.ipset_sets[set_name] = [] diff --git a/neutron/tests/unit/agent/linux/test_ipset_manager.py b/neutron/tests/unit/agent/linux/test_ipset_manager.py index cbd156218ff..19fbb7e20e6 100644 --- a/neutron/tests/unit/agent/linux/test_ipset_manager.py +++ b/neutron/tests/unit/agent/linux/test_ipset_manager.py @@ -12,7 +12,9 @@ # limitations under the License. import mock +from oslo_config import cfg +from neutron.agent.common import config as a_cfg from neutron.agent.linux import ipset_manager from neutron.tests import base @@ -25,8 +27,13 @@ FAKE_IPS = ['10.0.0.1', '10.0.0.2', '10.0.0.3', '10.0.0.4', class BaseIpsetManagerTest(base.BaseTestCase): - def setUp(self): + def setUp(self, maxelem=None, hashsize=None): super(BaseIpsetManagerTest, self).setUp() + cfg.CONF.register_opts(a_cfg.IPSET_OPTS, 'AGENT') + cfg.CONF.set_override('ipset_maxelem', maxelem, 'AGENT') + cfg.CONF.set_override('ipset_hashsize', hashsize, 'AGENT') + self.maxelem = maxelem + self.hashsize = hashsize self.ipset = ipset_manager.IpsetManager() self.execute = mock.patch.object(self.ipset, "execute").start() self.expected_calls = [] @@ -36,7 +43,13 @@ class BaseIpsetManagerTest(base.BaseTestCase): self.execute.assert_has_calls(self.expected_calls, any_order=False) def expect_set(self, addresses): - temp_input = ['create IPv4fake_sgid-new hash:ip family inet'] + hash_args = [] + if self.hashsize: + hash_args.extend(['hashsize', str(self.hashsize)]) + if self.maxelem: + hash_args.extend(['maxelem', str(self.maxelem)]) + temp_input = ['create IPv4fake_sgid-new hash:ip family inet %s' % + ' '.join(hash_args)] temp_input.extend('add IPv4fake_sgid-new %s' % ip for ip in addresses) input = '\n'.join(temp_input) self.expected_calls.extend([ @@ -63,9 +76,14 @@ class BaseIpsetManagerTest(base.BaseTestCase): run_as_root=True) for ip in addresses) def expect_create(self): + ipset_call = ['ipset', 'create', '-exist', TEST_SET_NAME, + 'hash:ip', 'family', 'inet'] + if self.hashsize: + ipset_call.extend(['hashsize', str(self.hashsize)]) + if self.maxelem: + ipset_call.extend(['maxelem', str(self.maxelem)]) self.expected_calls.append( - mock.call(['ipset', 'create', '-exist', TEST_SET_NAME, - 'hash:ip', 'family', 'inet'], + mock.call(ipset_call, process_input=None, run_as_root=True)) @@ -85,6 +103,10 @@ class BaseIpsetManagerTest(base.BaseTestCase): class IpsetManagerTestCase(BaseIpsetManagerTest): + """Run all tests, but with maxelem/hashsize values not configured + """ + def setUp(self): + super(IpsetManagerTestCase, self).setUp() def test_set_exists(self): self.add_first_ip() @@ -117,3 +139,10 @@ class IpsetManagerTestCase(BaseIpsetManagerTest): self.expect_destroy() self.ipset.destroy(TEST_SET_ID, ETHERTYPE) self.verify_mock_calls() + + +class IpsetManagerTestCaseHashArgs(IpsetManagerTestCase): + """Run all the above tests, but with maxelem/hashsize values configured + """ + def setUp(self): + super(IpsetManagerTestCase, self).setUp(maxelem=131072, hashsize=2048)