From 1c06a4c230fec741a64a32770f02aa8a8d079fba Mon Sep 17 00:00:00 2001 From: Arata Notsu Date: Fri, 25 Oct 2013 16:34:22 +0900 Subject: [PATCH] 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 --- .../openvswitch/agent/ovs_neutron_agent.py | 8 ++++- .../openvswitch/test_ovs_neutron_agent.py | 36 +++++++++++++++++++ .../tests/unit/openvswitch/test_ovs_tunnel.py | 4 ++- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index 26d142a8aa..b093d0a2d3 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 e7a7c942d4..874ef2011d 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 e3b306cbcb..3715fb07bd 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,