From c3d2526727241ac1f6d812dc5fc1fd7e3636b3fc Mon Sep 17 00:00:00 2001 From: Andreas Scheuring Date: Wed, 18 Nov 2015 15:23:36 +0100 Subject: [PATCH] ovs: Make interface name hashing algorithm common and extend it The OVS device name hashing algorithm shortens interface names that are too long. To ensure uniqueness it makes use of a hashing algorithm. Move this function to a common place where it can be shared between ml2 drivers and agents. Extend the function to support defining the max device length to be used. Change LOG level to info to help deployers figuring out the unhashed name of an hashed inteface. Adapt OVS agent to use this common function instead of its own implementation. Change-Id: I5c04f39928d070aa7e372934fcb2675609d2761c Partial-Bug: #1495960 --- neutron/plugins/common/utils.py | 43 +++++++++++ .../openvswitch/agent/ovs_neutron_agent.py | 37 ++-------- neutron/tests/unit/plugins/common/__init__.py | 0 .../tests/unit/plugins/common/test_utils.py | 71 +++++++++++++++++++ .../agent/test_ovs_neutron_agent.py | 11 --- 5 files changed, 119 insertions(+), 43 deletions(-) create mode 100644 neutron/tests/unit/plugins/common/__init__.py create mode 100644 neutron/tests/unit/plugins/common/test_utils.py diff --git a/neutron/plugins/common/utils.py b/neutron/plugins/common/utils.py index 61e7c164f..d938d8893 100644 --- a/neutron/plugins/common/utils.py +++ b/neutron/plugins/common/utils.py @@ -16,12 +16,21 @@ Common utilities and helper functions for Openstack Networking Plugins. """ +import hashlib + +from oslo_log import log as logging +import six import webob.exc from neutron.api.v2 import attributes +from neutron.common import constants as n_const from neutron.common import exceptions as n_exc +from neutron.i18n import _LI from neutron.plugins.common import constants as p_const +INTERFACE_HASH_LEN = 6 +LOG = logging.getLogger(__name__) + def is_valid_vlan_tag(vlan): return p_const.MIN_VLAN_TAG <= vlan <= p_const.MAX_VLAN_TAG @@ -138,3 +147,37 @@ def create_port(core_plugin, context, port, check_allow_post=True): port.get('port', {}), check_allow_post=check_allow_post) return core_plugin.create_port(context, {'port': port_data}) + + +def get_interface_name(name, prefix='', max_len=n_const.DEVICE_NAME_MAX_LEN): + """Construct an interface name based on the prefix and name. + + The interface name can not exceed the maximum length passed in. Longer + names are hashed to help ensure uniqueness. + """ + requested_name = prefix + name + + if len(requested_name) <= max_len: + return requested_name + + # We can't just truncate because interfaces may be distinguished + # by an ident at the end. A hash over the name should be unique. + # Leave part of the interface name on for easier identification + if (len(prefix) + INTERFACE_HASH_LEN) > max_len: + raise ValueError(_("Too long prefix provided. New name would exceed " + "given length for an interface name.")) + + namelen = max_len - len(prefix) - INTERFACE_HASH_LEN + if isinstance(name, six.text_type): + hashed_name = hashlib.sha1(name.encode('utf-8')) + else: + hashed_name = hashlib.sha1(name) + new_name = ('%(prefix)s%(truncated)s%(hash)s' % + {'prefix': prefix, 'truncated': name[0:namelen], + 'hash': hashed_name.hexdigest()[0:INTERFACE_HASH_LEN]}) + LOG.info(_LI("The requested interface name %(requested_name)s exceeds the " + "%(limit)d character limitation. It was shortened to " + "%(new_name)s to fit."), + {'requested_name': requested_name, + 'limit': max_len, 'new_name': new_name}) + return new_name diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index ea51b95af..fa3cb7339 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -14,7 +14,6 @@ # under the License. import collections -import hashlib import signal import sys import time @@ -46,6 +45,7 @@ from neutron.common import utils as n_utils from neutron import context from neutron.i18n import _LE, _LI, _LW from neutron.plugins.common import constants as p_const +from neutron.plugins.common import utils as p_utils from neutron.plugins.ml2.drivers.l2pop.rpc_manager import l2population_rpc from neutron.plugins.ml2.drivers.openvswitch.agent.common \ import constants @@ -1044,33 +1044,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, self.tun_br.setup_default_table(self.patch_int_ofport, self.arp_responder_enabled) - def get_peer_name(self, prefix, name): - """Construct a peer name based on the prefix and name. - - The peer name can not exceed the maximum length allowed for a linux - device. Longer names are hashed to help ensure uniqueness. - """ - if len(prefix + name) <= n_const.DEVICE_NAME_MAX_LEN: - return prefix + name - # We can't just truncate because bridges may be distinguished - # by an ident at the end. A hash over the name should be unique. - # Leave part of the bridge name on for easier identification - hashlen = 6 - namelen = n_const.DEVICE_NAME_MAX_LEN - len(prefix) - hashlen - if isinstance(name, six.text_type): - hashed_name = hashlib.sha1(name.encode('utf-8')) - else: - hashed_name = hashlib.sha1(name) - new_name = ('%(prefix)s%(truncated)s%(hash)s' % - {'prefix': prefix, 'truncated': name[0:namelen], - 'hash': hashed_name.hexdigest()[0:hashlen]}) - LOG.warning(_LW("Creating an interface named %(name)s exceeds the " - "%(limit)d character limitation. It was shortened to " - "%(new_name)s to fit."), - {'name': name, 'limit': n_const.DEVICE_NAME_MAX_LEN, - 'new_name': new_name}) - return new_name - def setup_physical_bridges(self, bridge_mappings): '''Setup the physical network bridges. @@ -1104,10 +1077,10 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, self.phys_brs[physical_network] = br # interconnect physical and integration bridges using veth/patchs - int_if_name = self.get_peer_name(constants.PEER_INTEGRATION_PREFIX, - bridge) - phys_if_name = self.get_peer_name(constants.PEER_PHYSICAL_PREFIX, - bridge) + int_if_name = p_utils.get_interface_name( + bridge, prefix=constants.PEER_INTEGRATION_PREFIX) + phys_if_name = p_utils.get_interface_name( + bridge, prefix=constants.PEER_PHYSICAL_PREFIX) # Interface type of port for physical and integration bridges must # be same, so check only one of them. int_type = self.int_br.db_get_val("Interface", int_if_name, "type") diff --git a/neutron/tests/unit/plugins/common/__init__.py b/neutron/tests/unit/plugins/common/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/neutron/tests/unit/plugins/common/test_utils.py b/neutron/tests/unit/plugins/common/test_utils.py new file mode 100644 index 000000000..c4dfdfe27 --- /dev/null +++ b/neutron/tests/unit/plugins/common/test_utils.py @@ -0,0 +1,71 @@ +# Copyright (c) 2015 IBM Corp. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import hashlib +import mock + +from neutron.common import constants +from neutron.plugins.common import utils +from neutron.tests import base + +LONG_NAME1 = "A_REALLY_LONG_INTERFACE_NAME1" +LONG_NAME2 = "A_REALLY_LONG_INTERFACE_NAME2" +SHORT_NAME = "SHORT" +MOCKED_HASH = "mockedhash" + + +class MockSHA(object): + def hexdigest(self): + return MOCKED_HASH + + +class TestUtils(base.BaseTestCase): + + @mock.patch.object(hashlib, 'sha1', return_value=MockSHA()) + def test_get_interface_name(self, mock_sha1): + prefix = "pre-" + prefix_long = "long_prefix" + prefix_exceeds_max_dev_len = "much_too_long_prefix" + hash_used = MOCKED_HASH[0:6] + + self.assertEqual("A_REALLY_" + hash_used, + utils.get_interface_name(LONG_NAME1)) + self.assertEqual("SHORT", + utils.get_interface_name(SHORT_NAME)) + self.assertEqual("pre-A_REA" + hash_used, + utils.get_interface_name(LONG_NAME1, prefix=prefix)) + self.assertEqual("pre-SHORT", + utils.get_interface_name(SHORT_NAME, prefix=prefix)) + # len(prefix) > max_device_len - len(hash_used) + self.assertRaises(ValueError, utils.get_interface_name, SHORT_NAME, + prefix_long) + # len(prefix) > max_device_len + self.assertRaises(ValueError, utils.get_interface_name, SHORT_NAME, + prefix=prefix_exceeds_max_dev_len) + + def test_get_interface_uniqueness(self): + prefix = "prefix-" + if_prefix1 = utils.get_interface_name(LONG_NAME1, prefix=prefix) + if_prefix2 = utils.get_interface_name(LONG_NAME2, prefix=prefix) + self.assertNotEqual(if_prefix1, if_prefix2) + + @mock.patch.object(hashlib, 'sha1', return_value=MockSHA()) + def test_get_interface_max_len(self, mock_sha1): + self.assertEqual(constants.DEVICE_NAME_MAX_LEN, + len(utils.get_interface_name(LONG_NAME1))) + self.assertEqual(10, len(utils.get_interface_name(LONG_NAME1, + max_len=10))) + self.assertEqual(12, len(utils.get_interface_name(LONG_NAME1, + prefix="pre-", + max_len=12))) diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 71b5637f8..c87c86e30 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -972,17 +972,6 @@ class TestOvsNeutronAgent(object): self.assertEqual(self.agent.phys_ofports["physnet1"], "phy_ofport") - def test_get_peer_name(self): - bridge1 = "A_REALLY_LONG_BRIDGE_NAME1" - bridge2 = "A_REALLY_LONG_BRIDGE_NAME2" - self.agent.use_veth_interconnection = True - self.assertEqual(len(self.agent.get_peer_name('int-', bridge1)), - n_const.DEVICE_NAME_MAX_LEN) - self.assertEqual(len(self.agent.get_peer_name('int-', bridge2)), - n_const.DEVICE_NAME_MAX_LEN) - self.assertNotEqual(self.agent.get_peer_name('int-', bridge1), - self.agent.get_peer_name('int-', bridge2)) - def test_setup_tunnel_br(self): self.tun_br = mock.Mock() with mock.patch.object(self.agent.int_br, -- 2.45.2