]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
ovs-agent: use hexadecimal IP address in tunnel port name
authorDarragh O'Reilly <dara2002-openstack@yahoo.com>
Tue, 18 Feb 2014 16:31:08 +0000 (16:31 +0000)
committerDarragh O'Reilly <dara2002-openstack@yahoo.com>
Fri, 28 Feb 2014 08:11:40 +0000 (08:11 +0000)
The remote IP address is used to form the tunnel port name when
the ovs-agent is used with the ML2 plugin. With some OVS/Linux
combinations there is a 15 character limit on port names, so a
name like 'gre-192.168.10.10' does not work. This fix uses the
shorter hexadecimal representation of the IP address instead.

Change-Id: I58caf54219130159525b5a6492eaa451062c2ff5
Closes-Bug: 1281098

neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py

index ec9f5be132558e41fb9a4e916f97464b85c322b5..9ad461c6da31a1f62c1424a6c51a180d48612549 100644 (file)
@@ -19,6 +19,7 @@ import sys
 import time
 
 import eventlet
+import netaddr
 from oslo.config import cfg
 
 from neutron.agent import l2population_rpc
@@ -298,9 +299,9 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
         if not self.enable_tunneling:
             return
         tunnel_ip = kwargs.get('tunnel_ip')
-        tunnel_id = kwargs.get('tunnel_id', tunnel_ip)
+        tunnel_id = kwargs.get('tunnel_id', self.get_ip_in_hex(tunnel_ip))
         if not tunnel_id:
-            tunnel_id = tunnel_ip
+            return
         tunnel_type = kwargs.get('tunnel_type')
         if not tunnel_type:
             LOG.error(_("No tunnel_type specified, cannot create tunnels"))
@@ -330,7 +331,10 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                     ofport = self.tun_br_ofports[
                         lvm.network_type].get(agent_ip)
                     if not ofport:
-                        port_name = '%s-%s' % (lvm.network_type, agent_ip)
+                        remote_ip_hex = self.get_ip_in_hex(agent_ip)
+                        if not remote_ip_hex:
+                            continue
+                        port_name = '%s-%s' % (lvm.network_type, remote_ip_hex)
                         ofport = self.setup_tunnel_port(port_name, agent_ip,
                                                         lvm.network_type)
                         if ofport == 0:
@@ -910,7 +914,8 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
         else:
             for remote_ip, ofport in self.tun_br_ofports[tunnel_type].items():
                 if ofport == tun_ofport:
-                    port_name = '%s-%s' % (tunnel_type, remote_ip)
+                    port_name = '%s-%s' % (tunnel_type,
+                                           self.get_ip_in_hex(remote_ip))
                     self.tun_br.delete_port(port_name)
                     self.tun_br_ofports[tunnel_type].pop(remote_ip, None)
 
@@ -1086,6 +1091,14 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
         # If one of the above opertaions fails => resync with plugin
         return (resync_a | resync_b)
 
+    def get_ip_in_hex(self, ip_address):
+        try:
+            return '%08x' % netaddr.IPAddress(ip_address, version=4)
+        except Exception:
+            LOG.warn(_("Unable to create tunnel port. Invalid remote IP: %s"),
+                     ip_address)
+            return
+
     def tunnel_sync(self):
         resync = False
         try:
@@ -1097,8 +1110,16 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                     tunnels = details['tunnels']
                     for tunnel in tunnels:
                         if self.local_ip != tunnel['ip_address']:
-                            tunnel_id = tunnel.get('id', tunnel['ip_address'])
-                            tun_name = '%s-%s' % (tunnel_type, tunnel_id)
+                            tunnel_id = tunnel.get('id')
+                            # Unlike the OVS plugin, ML2 doesn't return an id
+                            # key. So use ip_address to form port name instead.
+                            # Port name must be <=15 chars, so use shorter hex.
+                            remote_ip = tunnel['ip_address']
+                            remote_ip_hex = self.get_ip_in_hex(remote_ip)
+                            if not tunnel_id and not remote_ip_hex:
+                                continue
+                            tun_name = '%s-%s' % (tunnel_type,
+                                                  tunnel_id or remote_ip_hex)
                             self.setup_tunnel_port(tun_name,
                                                    tunnel['ip_address'],
                                                    tunnel_type)
index 959101cab143c13a7cb42b526c760c04c27580a8..12bec87992f9f383f9687bc5e50b5946ea4985c4 100644 (file)
@@ -526,7 +526,7 @@ class TestOvsNeutronAgent(base.BaseTestCase):
         lvm2.tun_ofports = set(['1', '2'])
         self.agent.local_vlan_map = {'net1': lvm1, 'net2': lvm2}
         self.agent.tun_br_ofports = {'gre':
-                                     {'ip_agent_1': '1', 'ip_agent_2': '2'}}
+                                     {'1.1.1.1': '1', '2.2.2.2': '2'}}
 
     def test_fdb_ignore_network(self):
         self._prepare_l2_pop_ofports()
@@ -568,7 +568,7 @@ class TestOvsNeutronAgent(base.BaseTestCase):
                      {'network_type': 'gre',
                       'segment_id': 'tun1',
                       'ports':
-                      {'ip_agent_2':
+                      {'2.2.2.2':
                        [['mac', 'ip'],
                         n_const.FLOODING_ENTRY]}}}
         with contextlib.nested(
@@ -596,7 +596,7 @@ class TestOvsNeutronAgent(base.BaseTestCase):
                      {'network_type': 'gre',
                       'segment_id': 'tun2',
                       'ports':
-                      {'ip_agent_2':
+                      {'2.2.2.2':
                        [['mac', 'ip'],
                         n_const.FLOODING_ENTRY]}}}
         with contextlib.nested(
@@ -618,7 +618,7 @@ class TestOvsNeutronAgent(base.BaseTestCase):
         fdb_entry = {'net1':
                      {'network_type': 'gre',
                       'segment_id': 'tun1',
-                      'ports': {'ip_agent_1': [['mac', 'ip']]}}}
+                      'ports': {'1.1.1.1': [['mac', 'ip']]}}}
         with contextlib.nested(
             mock.patch.object(self.agent.tun_br, 'add_flow'),
             mock.patch.object(self.agent.tun_br, 'mod_flow'),
@@ -626,23 +626,22 @@ class TestOvsNeutronAgent(base.BaseTestCase):
         ) as (add_flow_fn, mod_flow_fn, add_tun_fn):
             self.agent.fdb_add(None, fdb_entry)
             self.assertFalse(add_tun_fn.called)
-            fdb_entry['net1']['ports']['ip_agent_3'] = [['mac', 'ip']]
+            fdb_entry['net1']['ports']['10.10.10.10'] = [['mac', 'ip']]
             self.agent.fdb_add(None, fdb_entry)
-            add_tun_fn.assert_called_with('gre-ip_agent_3', 'ip_agent_3',
-                                          'gre')
+            add_tun_fn.assert_called_with('gre-0a0a0a0a', '10.10.10.10', 'gre')
 
     def test_fdb_del_port(self):
         self._prepare_l2_pop_ofports()
         fdb_entry = {'net2':
                      {'network_type': 'gre',
                       'segment_id': 'tun2',
-                      'ports': {'ip_agent_2': [n_const.FLOODING_ENTRY]}}}
+                      'ports': {'2.2.2.2': [n_const.FLOODING_ENTRY]}}}
         with contextlib.nested(
             mock.patch.object(self.agent.tun_br, 'delete_flows'),
             mock.patch.object(self.agent.tun_br, 'delete_port')
         ) as (del_flow_fn, del_port_fn):
             self.agent.fdb_remove(None, fdb_entry)
-            del_port_fn.assert_called_once_with('gre-ip_agent_2')
+            del_port_fn.assert_called_once_with('gre-02020202')
 
     def test_recl_lv_port_to_preserve(self):
         self._prepare_l2_pop_ofports()
@@ -663,7 +662,7 @@ class TestOvsNeutronAgent(base.BaseTestCase):
             mock.patch.object(self.agent.tun_br, 'delete_flows')
         ) as (del_port_fn, del_flow_fn):
             self.agent.reclaim_local_vlan('net2')
-            del_port_fn.assert_called_once_with('gre-ip_agent_2')
+            del_port_fn.assert_called_once_with('gre-02020202')
 
     def test_daemon_loop_uses_polling_manager(self):
         with mock.patch(
@@ -710,6 +709,57 @@ class TestOvsNeutronAgent(base.BaseTestCase):
                 {'type': p_const.TYPE_GRE, 'ip': 'remote_ip'})
             self.assertEqual(ofport, 0)
 
+    def test_tunnel_sync_with_ovs_plugin(self):
+        fake_tunnel_details = {'tunnels': [{'id': '42',
+                                            'ip_address': '100.101.102.103'}]}
+        with contextlib.nested(
+            mock.patch.object(self.agent.plugin_rpc, 'tunnel_sync',
+                              return_value=fake_tunnel_details),
+            mock.patch.object(self.agent, 'setup_tunnel_port')
+        ) as (tunnel_sync_rpc_fn, setup_tunnel_port_fn):
+            self.agent.tunnel_types = ['gre']
+            self.agent.tunnel_sync()
+            expected_calls = [mock.call('gre-42', '100.101.102.103', 'gre')]
+            setup_tunnel_port_fn.assert_has_calls(expected_calls)
+
+    def test_tunnel_sync_with_ml2_plugin(self):
+        fake_tunnel_details = {'tunnels': [{'ip_address': '100.101.31.15'}]}
+        with contextlib.nested(
+            mock.patch.object(self.agent.plugin_rpc, 'tunnel_sync',
+                              return_value=fake_tunnel_details),
+            mock.patch.object(self.agent, 'setup_tunnel_port')
+        ) as (tunnel_sync_rpc_fn, setup_tunnel_port_fn):
+            self.agent.tunnel_types = ['vxlan']
+            self.agent.tunnel_sync()
+            expected_calls = [mock.call('vxlan-64651f0f',
+                                        '100.101.31.15', 'vxlan')]
+            setup_tunnel_port_fn.assert_has_calls(expected_calls)
+
+    def test_tunnel_sync_invalid_ip_address(self):
+        fake_tunnel_details = {'tunnels': [{'ip_address': '300.300.300.300'},
+                                           {'ip_address': '100.100.100.100'}]}
+        with contextlib.nested(
+            mock.patch.object(self.agent.plugin_rpc, 'tunnel_sync',
+                              return_value=fake_tunnel_details),
+            mock.patch.object(self.agent, 'setup_tunnel_port')
+        ) as (tunnel_sync_rpc_fn, setup_tunnel_port_fn):
+            self.agent.tunnel_types = ['vxlan']
+            self.agent.tunnel_sync()
+            setup_tunnel_port_fn.assert_called_once_with('vxlan-64646464',
+                                                         '100.100.100.100',
+                                                         'vxlan')
+
+    def test_tunnel_update(self):
+        kwargs = {'tunnel_ip': '10.10.10.10',
+                  'tunnel_type': 'gre'}
+        self.agent.setup_tunnel_port = mock.Mock()
+        self.agent.enable_tunneling = True
+        self.agent.tunnel_types = ['gre']
+        self.agent.l2_pop = False
+        self.agent.tunnel_update(context=None, **kwargs)
+        expected_calls = [mock.call('gre-0a0a0a0a', '10.10.10.10', 'gre')]
+        self.agent.setup_tunnel_port.assert_has_calls(expected_calls)
+
 
 class AncillaryBridgesTest(base.BaseTestCase):