From: Arata Notsu <notsu@virtualtech.jp>
Date: Fri, 25 Oct 2013 07:34:22 +0000 (+0900)
Subject: Do not run "ovs-ofctl add-flow" with an invalid in_port
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=edddad586332335578477df464be13f5a2b4a45f;p=openstack-build%2Fneutron-build.git

Do not run "ovs-ofctl add-flow" with an invalid in_port

Since ofport is a string, we have to convert it to integer to
check it expresses a negative number or not.

Also unit tests are added/corrected.

Change-Id: If85595f1b8ecb40ea41edda52706fb1bd41cb1ab
Related-Bug: 1238445
---

diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
index 26d142a8a..b093d0a2d 100644
--- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
+++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
@@ -862,7 +862,13 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
                                              self.local_ip,
                                              tunnel_type,
                                              self.vxlan_udp_port)
-        if ofport < 0:
+        ofport_int = -1
+        try:
+            ofport_int = int(ofport)
+        except (TypeError, ValueError):
+            LOG.exception(_("ofport should have a value that can be "
+                            "interpreted as an integer"))
+        if ofport_int < 0:
             LOG.error(_("Failed to set-up %(type)s tunnel port to %(ip)s"),
                       {'type': tunnel_type, 'ip': remote_ip})
             return 0
diff --git a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py
index e7a7c942d..874ef2011 100644
--- a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py
+++ b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py
@@ -591,6 +591,42 @@ class TestOvsNeutronAgent(base.BaseTestCase):
         mock_get_pm.assert_called_with(False, 'sudo')
         mock_loop.called_once()
 
+    def test_setup_tunnel_port_error_negative(self):
+        with contextlib.nested(
+            mock.patch.object(self.agent.tun_br, 'add_tunnel_port',
+                              return_value='-1'),
+            mock.patch.object(ovs_neutron_agent.LOG, 'error')
+        ) as (add_tunnel_port_fn, log_error_fn):
+            ofport = self.agent.setup_tunnel_port(
+                'gre-1', 'remote_ip', constants.TYPE_GRE)
+            add_tunnel_port_fn.assert_called_once_with(
+                'gre-1', 'remote_ip', self.agent.local_ip, constants.TYPE_GRE,
+                self.agent.vxlan_udp_port)
+            log_error_fn.assert_called_once_with(
+                _("Failed to set-up %(type)s tunnel port to %(ip)s"),
+                {'type': constants.TYPE_GRE, 'ip': 'remote_ip'})
+            self.assertEqual(ofport, 0)
+
+    def test_setup_tunnel_port_error_not_int(self):
+        with contextlib.nested(
+            mock.patch.object(self.agent.tun_br, 'add_tunnel_port',
+                              return_value=None),
+            mock.patch.object(ovs_neutron_agent.LOG, 'exception'),
+            mock.patch.object(ovs_neutron_agent.LOG, 'error')
+        ) as (add_tunnel_port_fn, log_exc_fn, log_error_fn):
+            ofport = self.agent.setup_tunnel_port(
+                'gre-1', 'remote_ip', constants.TYPE_GRE)
+            add_tunnel_port_fn.assert_called_once_with(
+                'gre-1', 'remote_ip', self.agent.local_ip, constants.TYPE_GRE,
+                self.agent.vxlan_udp_port)
+            log_exc_fn.assert_called_once_with(
+                _("ofport should have a value that can be "
+                  "interpreted as an integer"))
+            log_error_fn.assert_called_once_with(
+                _("Failed to set-up %(type)s tunnel port to %(ip)s"),
+                {'type': constants.TYPE_GRE, 'ip': 'remote_ip'})
+            self.assertEqual(ofport, 0)
+
 
 class AncillaryBridgesTest(base.BaseTestCase):
 
diff --git a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py
index e3b306cbc..3715fb07b 100644
--- a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py
+++ b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py
@@ -401,7 +401,9 @@ class TunnelTest(base.BaseTestCase):
 
     def test_tunnel_update(self):
         self.mock_tun_bridge.add_tunnel_port('gre-1', '10.0.10.1', '10.0.0.1',
-                                             'gre', 4789)
+                                             'gre', 4789).AndReturn('9999')
+        self.mock_tun_bridge.add_flow(actions='resubmit(,2)', in_port='9999',
+                                      priority=1)
         self.mox.ReplayAll()
         a = ovs_neutron_agent.OVSNeutronAgent(self.INT_BRIDGE,
                                               self.TUN_BRIDGE,