From ca7ed8f84da6962a9c2b8ad21d484931d297f31b Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Fri, 6 Jun 2014 08:15:38 -0700 Subject: [PATCH] OVS Agent: limit veth names to 15 chars The maximum length of a veth interface name is 15 characters. This patch limits the length of the veth names created for integration bridges by switching to a fixed-length hashing mechanism if the normal name would exceed the allowed length. Closes-Bug: #1328288 Change-Id: I432cee62a6dc91f268becbc91f8024c23dd0bfac (cherry-picked from 1222366f6dadf7ce2a810c919344054196134db8) --- neutron/agent/linux/ip_lib.py | 1 + .../openvswitch/agent/ovs_neutron_agent.py | 29 +++++++++++++++++-- .../openvswitch/test_ovs_neutron_agent.py | 10 +++++++ .../tests/unit/openvswitch/test_ovs_tunnel.py | 16 +++++----- 4 files changed, 46 insertions(+), 10 deletions(-) diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index 9b7f2c8615e..3c8e3b07848 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -28,6 +28,7 @@ OPTS = [ ] +VETH_MAX_NAME_LENGTH = 15 LOOPBACK_DEVNAME = 'lo' # NOTE(ethuleau): depend of the version of iproute2, the vlan # interface details vary. diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index 79c03f1de2a..809506b616e 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -14,6 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. +import hashlib import signal import sys import time @@ -767,6 +768,28 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, priority=0, actions="drop") + def get_veth_name(self, prefix, name): + """Construct a veth name based on the prefix and name that does not + exceed the maximum length allowed for a linux device. Longer names + are hashed to help ensure uniqueness. + """ + if len(prefix + name) <= ip_lib.VETH_MAX_NAME_LENGTH: + 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 = ip_lib.VETH_MAX_NAME_LENGTH - len(prefix) - hashlen + new_name = ('%(prefix)s%(truncated)s%(hash)s' % + {'prefix': prefix, 'truncated': name[0:namelen], + 'hash': hashlib.sha1(name).hexdigest()[0:hashlen]}) + LOG.warning(_("Creating an interface named %(name)s exceeds the " + "%(limit)d character limitation. It was shortened to " + "%(new_name)s to fit."), + {'name': name, 'limit': ip_lib.VETH_MAX_NAME_LENGTH, + 'new_name': new_name}) + return new_name + def setup_physical_bridges(self, bridge_mappings): '''Setup the physical network bridges. @@ -798,9 +821,11 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, self.phys_brs[physical_network] = br # create veth to patch physical bridge with integration bridge - int_veth_name = constants.VETH_INTEGRATION_PREFIX + bridge + int_veth_name = self.get_veth_name( + constants.VETH_INTEGRATION_PREFIX, bridge) self.int_br.delete_port(int_veth_name) - phys_veth_name = constants.VETH_PHYSICAL_PREFIX + bridge + phys_veth_name = self.get_veth_name( + constants.VETH_PHYSICAL_PREFIX, bridge) br.delete_port(phys_veth_name) if ip_lib.device_exists(int_veth_name, self.root_helper): ip_lib.IPDevice(int_veth_name, self.root_helper).link.delete() diff --git a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py index 94d3c937108..dfc51625244 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py @@ -476,6 +476,16 @@ class TestOvsNeutronAgent(base.BaseTestCase): self.assertEqual(self.agent.phys_ofports["physnet1"], "int_ofport") + def test_get_veth_name(self): + bridge1 = "A_REALLY_LONG_BRIDGE_NAME1" + bridge2 = "A_REALLY_LONG_BRIDGE_NAME2" + self.assertEqual(len(self.agent.get_veth_name('int-', bridge1)), + ip_lib.VETH_MAX_NAME_LENGTH) + self.assertEqual(len(self.agent.get_veth_name('int-', bridge2)), + ip_lib.VETH_MAX_NAME_LENGTH) + self.assertNotEqual(self.agent.get_veth_name('int-', bridge1), + self.agent.get_veth_name('int-', bridge2)) + def test_port_unbound(self): with mock.patch.object(self.agent, "reclaim_local_vlan") as reclvl_fn: self.agent.enable_tunneling = True diff --git a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py index 126b9ccb1ef..084e8980bbe 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py +++ b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py @@ -75,7 +75,7 @@ class TunnelTest(base.BaseTestCase): self.INT_BRIDGE = 'integration_bridge' self.TUN_BRIDGE = 'tunnel_bridge' - self.MAP_TUN_BRIDGE = 'tunnel_bridge_mapping' + self.MAP_TUN_BRIDGE = 'tun_br_map' self.NET_MAPPING = {'net1': self.MAP_TUN_BRIDGE} self.INT_OFPORT = 11111 self.TUN_OFPORT = 22222 @@ -115,12 +115,12 @@ class TunnelTest(base.BaseTestCase): self.mock_map_tun_bridge_expected = [ mock.call.remove_all_flows(), mock.call.add_flow(priority=1, actions='normal'), - mock.call.delete_port('phy-tunnel_bridge_mapping'), + mock.call.delete_port('phy-%s' % self.MAP_TUN_BRIDGE), mock.call.add_port(self.intb), ] self.mock_int_bridge.add_port.return_value = None self.mock_int_bridge_expected += [ - mock.call.delete_port('int-tunnel_bridge_mapping'), + mock.call.delete_port('int-%s' % self.MAP_TUN_BRIDGE), mock.call.add_port(self.inta) ] self.inta_expected = [mock.call.link.set_up()] @@ -192,13 +192,13 @@ class TunnelTest(base.BaseTestCase): self.device_exists = mock.patch.object(ip_lib, 'device_exists').start() self.device_exists.return_value = True self.device_exists_expected = [ - mock.call('tunnel_bridge_mapping', 'sudo'), - mock.call('int-tunnel_bridge_mapping', 'sudo'), + mock.call(self.MAP_TUN_BRIDGE, 'sudo'), + mock.call('int-%s' % self.MAP_TUN_BRIDGE, 'sudo'), ] self.ipdevice = mock.patch.object(ip_lib, 'IPDevice').start() self.ipdevice_expected = [ - mock.call('int-tunnel_bridge_mapping', 'sudo'), + mock.call('int-%s' % self.MAP_TUN_BRIDGE, 'sudo'), mock.call().link.delete() ] self.ipwrapper = mock.patch.object(ip_lib, 'IPWrapper').start() @@ -206,8 +206,8 @@ class TunnelTest(base.BaseTestCase): add_veth.return_value = [self.inta, self.intb] self.ipwrapper_expected = [ mock.call('sudo'), - mock.call().add_veth('int-tunnel_bridge_mapping', - 'phy-tunnel_bridge_mapping') + mock.call().add_veth('int-%s' % self.MAP_TUN_BRIDGE, + 'phy-%s' % self.MAP_TUN_BRIDGE) ] self.get_bridges = mock.patch.object(ovs_lib, 'get_bridges').start()