From fc827a3a6163a9ff2f09ec47793337ab0e5ffa13 Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Tue, 19 Sep 2017 15:54:56 +0000 Subject: [PATCH] of_native: Use int for comparing datapath ID Previously, DP ID was converted to integer and then back to string. As a consequence of the conversion, DP IDs like 000123 were converted to 123 losing leading zeros. In case self._get_dp_by_dpid() method raises a RuntimeError exception current DP ID of the bridge was compared to cached DP ID and if IDs were different, original exception coming from ryu library was swallowed. As conversion for cached DP ID removes leading zeros, original exception was always swallowed if bridge's DP ID started with zero. This patch uses the integer for comparison between current and cached bridge DP ID hence any exception coming from ryu is not swallowed. Closes-bug: #1718235 Conflicts: neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_ovs_bridge.py Change-Id: I445aa61acc758b56c51a9403df4d92d9c1d40ace (cherry picked from commit d739d01b6c98e082127af8fa6d4130e854ff413d) --- .../openvswitch/agent/openflow/native/ovs_bridge.py | 13 ++++++------- .../agent/openflow/native/test_ovs_bridge.py | 6 ++++-- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py index 923f1a09774..14be2687148 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ovs_bridge.py @@ -55,17 +55,16 @@ class OVSAgentBridge(ofswitch.OpenFlowSwitchMixin, # NOTE(yamamoto): Open vSwitch change its dpid on # some events. # REVISIT(yamamoto): Consider to set dpid statically. - old_dpid_str = format(self._cached_dpid, '0x') - new_dpid_str = self.get_datapath_id() - if new_dpid_str != old_dpid_str: + new_dpid = int(self.get_datapath_id(), 16) + if new_dpid != self._cached_dpid: LOG.info("Bridge %(br_name)s changed its " - "datapath-ID from %(old)s to %(new)s", { + "datapath-ID from %(old)x to %(new)x", { "br_name": self.br_name, - "old": old_dpid_str, - "new": new_dpid_str, + "old": self._cached_dpid, + "new": new_dpid, }) ctx.reraise = False - self._cached_dpid = int(new_dpid_str, 16) + self._cached_dpid = new_dpid def setup_controllers(self, conf): controllers = [ diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_ovs_bridge.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_ovs_bridge.py index 68bebad0b1f..d784633be2a 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_ovs_bridge.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_ovs_bridge.py @@ -18,18 +18,20 @@ import mock from neutron.tests.unit.plugins.ml2.drivers.openvswitch.agent \ import ovs_test_base +DPID = "0003e9" + class OVSAgentBridgeTestCase(ovs_test_base.OVSRyuTestBase): def test__get_dp(self): mock.patch( 'neutron.agent.common.ovs_lib.OVSBridge.get_datapath_id', - return_value="3e9").start() + return_value=DPID).start() mock.patch( "neutron.plugins.ml2.drivers.openvswitch.agent.openflow.native." "ofswitch.OpenFlowSwitchMixin._get_dp_by_dpid", side_effect=RuntimeError).start() br = self.br_int_cls('br-int') - br._cached_dpid = int("3e9", 16) + br._cached_dpid = int(DPID, 16) # make sure it correctly raises RuntimeError, not UnboundLocalError as # in LP https://bugs.launchpad.net/neutron/+bug/1588042 self.assertRaises(RuntimeError, br._get_dp)