From 486962d0e23b027c11420a0b220d555994249b05 Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Wed, 12 Feb 2014 16:38:10 +0900 Subject: [PATCH] nec plugin: Compare OFS datapath_id as hex int Previously NEC plugin compares old and new datapath_ids as a string and zero padding in hex notation is not taken into account when compared. This causes unintended deletion and recreation of a port on OpenFlow controller. This patch fixes this issue by comparing datapath_ids as hex int. Change-Id: I6aa0a041e98c9bc489af89bb642ec5f86eaecce5 Closes-Bug: 1278349 --- neutron/plugins/nec/common/utils.py | 24 ++++++++++++++++ neutron/plugins/nec/nec_plugin.py | 8 ++++-- neutron/tests/unit/nec/test_portbindings.py | 19 +++++++++++++ neutron/tests/unit/nec/test_utils.py | 31 +++++++++++++++++++++ 4 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 neutron/plugins/nec/common/utils.py create mode 100644 neutron/tests/unit/nec/test_utils.py diff --git a/neutron/plugins/nec/common/utils.py b/neutron/plugins/nec/common/utils.py new file mode 100644 index 0000000000..a628d8ef07 --- /dev/null +++ b/neutron/plugins/nec/common/utils.py @@ -0,0 +1,24 @@ +# Copyright 2014 NEC Corporation. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +def cmp_dpid(dpid_a, dpid_b): + """Compare two datapath IDs as hexadecimal int. + + It returns True if equal, otherwise False. + """ + try: + return (int(dpid_a, 16) == int(dpid_b, 16)) + except Exception: + return False diff --git a/neutron/plugins/nec/nec_plugin.py b/neutron/plugins/nec/nec_plugin.py index 552ebda853..41aaf61f7c 100644 --- a/neutron/plugins/nec/nec_plugin.py +++ b/neutron/plugins/nec/nec_plugin.py @@ -45,6 +45,7 @@ from neutron.openstack.common import uuidutils from neutron.plugins.common import constants as svc_constants from neutron.plugins.nec.common import config from neutron.plugins.nec.common import exceptions as nexc +from neutron.plugins.nec.common import utils as necutils from neutron.plugins.nec.db import api as ndb from neutron.plugins.nec.db import router as rdb from neutron.plugins.nec import extensions @@ -461,7 +462,8 @@ class NECPluginV2(db_base_plugin_v2.NeutronDbPluginV2, portinfo = self._validate_portinfo(profile) portinfo_changed = 'ADD' if cur_portinfo: - if (portinfo['datapath_id'] == cur_portinfo.datapath_id and + if (necutils.cmp_dpid(portinfo['datapath_id'], + cur_portinfo.datapath_id) and portinfo['port_no'] == cur_portinfo.port_no): return ndb.del_portinfo(context.session, port['id']) @@ -707,7 +709,7 @@ class NECPluginV2RPCCallbacks(object): id = p['id'] portinfo = ndb.get_portinfo(session, id) if portinfo: - if (portinfo.datapath_id == datapath_id and + if (necutils.cmp_dpid(portinfo.datapath_id, datapath_id) and portinfo.port_no == p['port_no']): LOG.debug(_("update_ports(): ignore unchanged portinfo in " "port_added message (port_id=%s)."), id) @@ -732,7 +734,7 @@ class NECPluginV2RPCCallbacks(object): "due to portinfo for port_id=%s was not " "registered"), id) continue - if portinfo.datapath_id != datapath_id: + if not necutils.cmp_dpid(portinfo.datapath_id, datapath_id): LOG.debug(_("update_ports(): ignore port_removed message " "received from different host " "(registered_datapath_id=%(registered)s, " diff --git a/neutron/tests/unit/nec/test_portbindings.py b/neutron/tests/unit/nec/test_portbindings.py index e534e2a5fc..41ec4339a4 100644 --- a/neutron/tests/unit/nec/test_portbindings.py +++ b/neutron/tests/unit/nec/test_portbindings.py @@ -213,6 +213,25 @@ class TestNecPortBindingPortInfo(test_nec_plugin.NecPluginV2TestCase): self.assertEqual(self.ofc.create_ofc_port.call_count, 1) self.assertEqual(self.ofc.delete_ofc_port.call_count, 0) + def test_port_update_for_existing_port_with_different_padding_dpid(self): + ctx = context.get_admin_context() + with self.port() as port: + port_id = port['port']['id'] + portinfo = {'id': port_id, 'port_no': 123} + self.rpcapi_update_ports(datapath_id='0x000000000000abcd', + added=[portinfo]) + self.assertEqual(1, self.ofc.create_ofc_port.call_count) + self.assertEqual(0, self.ofc.delete_ofc_port.call_count) + + profile_arg = {portbindings.PROFILE: + self._get_portinfo(datapath_id='0xabcd', + port_no=123)} + self._update('ports', port_id, {'port': profile_arg}, + neutron_context=ctx) + # Check create_ofc_port/delete_ofc_port are not called. + self.assertEqual(1, self.ofc.create_ofc_port.call_count) + self.assertEqual(0, self.ofc.delete_ofc_port.call_count) + def test_port_create_portinfo_non_admin(self): with self.network(set_context=True, tenant_id='test') as net1: with self.subnet(network=net1) as subnet1: diff --git a/neutron/tests/unit/nec/test_utils.py b/neutron/tests/unit/nec/test_utils.py new file mode 100644 index 0000000000..c8a8ac7bc2 --- /dev/null +++ b/neutron/tests/unit/nec/test_utils.py @@ -0,0 +1,31 @@ +# Copyright 2014 NEC Corporation. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from neutron.plugins.nec.common import utils +from neutron.tests import base + + +class NecUtilsTest(base.BaseTestCase): + + def test_cmp_dpid(self): + self.assertTrue(utils.cmp_dpid('0xabcd', '0xabcd')) + self.assertTrue(utils.cmp_dpid('abcd', '0xabcd')) + self.assertTrue(utils.cmp_dpid('0x000000000000abcd', '0xabcd')) + self.assertTrue(utils.cmp_dpid('0x000000000000abcd', '0x00abcd')) + self.assertFalse(utils.cmp_dpid('0x000000000000abcd', '0xabc0')) + self.assertFalse(utils.cmp_dpid('0x000000000000abcd', '0x00abc0')) + + def test_cmp_dpid_with_exception(self): + self.assertFalse(utils.cmp_dpid('0xabcx', '0xabcx')) + self.assertFalse(utils.cmp_dpid(None, None))