]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
OVS Agent: limit veth names to 15 chars
authorKevin Benton <blak111@gmail.com>
Fri, 6 Jun 2014 15:15:38 +0000 (08:15 -0700)
committerKevin Benton <blak111@gmail.com>
Sat, 7 Jun 2014 00:51:49 +0000 (17:51 -0700)
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

neutron/agent/linux/ip_lib.py
neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py
neutron/tests/unit/openvswitch/test_ovs_tunnel.py

index bc32139f276026a0550c22711d8865eea5a2d3c1..aae41281fa4ca13c6dfab009cdfec63c5c79dbc4 100644 (file)
@@ -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.
index 06e361cc769ae9f8349b945693e242ba8f6c3818..c3537b818d878951a648d3a0c85e4dca48434ff2 100644 (file)
@@ -14,6 +14,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import hashlib
 import signal
 import sys
 import time
@@ -854,6 +855,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.
 
@@ -885,9 +908,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()
index 7dc65bb20ac90db63325f98e488a0e8947690c28..23cf356e4287a55c806d02a101f4e0dcf2f60215 100644 (file)
@@ -484,6 +484,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
index c192cae650b38287ffe57d8b0d2e7305d9dc3d18..abd268e1f1e8404d762b7f6cec46a09d85f78fd6 100644 (file)
@@ -81,7 +81,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
@@ -121,12 +121,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()]
@@ -198,13 +198,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()
@@ -212,8 +212,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()