diff --git a/os_vif/tests/unit/test_utils.py b/os_vif/tests/unit/test_utils.py new file mode 100644 index 00000000..c8cc909b --- /dev/null +++ b/os_vif/tests/unit/test_utils.py @@ -0,0 +1,26 @@ +# 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. + +import mock +import random + +from os_vif.tests.unit import base +from os_vif import utils + + +class TestGetRandomMac(base.TestCase): + + @mock.patch.object(random, 'getrandbits', return_value=0xa2) + def test_random_mac_generated(self, mock_rnd): + mac = utils.get_random_mac(['aa', 'bb', '00', 'dd', 'ee', 'ff']) + self.assertEqual('aa:bb:00:a2:a2:a2', mac) + mock_rnd.assert_called_with(8) diff --git a/os_vif/utils.py b/os_vif/utils.py index d74f96bb..4df75c2d 100644 --- a/os_vif/utils.py +++ b/os_vif/utils.py @@ -11,6 +11,24 @@ # under the License. +import random + + +def get_random_mac(base_mac): + """Get a random MAC address string of the specified base format. + The first 3 octets will remain unchanged and the others will be + randomly generated. + + :param base_mac: Base MAC address represented by an array of 6 strings/int + :returns: The MAC address string. + """ + + mac = [int(base_mac[0], 16), int(base_mac[1], 16), + int(base_mac[2], 16), random.getrandbits(8), + random.getrandbits(8), random.getrandbits(8)] + return ':'.join(["%02x" % byte for byte in mac]) + + def set_mask(data, mask): return data | mask diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index 9516f79f..0554195f 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -82,7 +82,15 @@ class OvsPlugin(plugin.PluginBase): cfg.BoolOpt('isolate_vif', default=False, help='Controls if VIF should be isolated when plugged ' 'to the ovs bridge. This should only be set to True ' - 'when using the neutron ovs ml2 agent.') + 'when using the neutron ovs ml2 agent.'), + cfg.StrOpt('cleanup_base_mac', + default='aa:16:3f:00:00:00', + help='The cleanup base MAC address to be used for removed ' + 'DPDK representor ports. ' + 'The first 3 octets will remain unchanged and the others ' + 'will be randomly generated . ' + 'A valid base MAC must have local assignment bit set and ' + 'multicast bit cleared in the first octet.') ) def __init__(self, config): diff --git a/vif_plug_ovs/ovsdb/impl_vsctl.py b/vif_plug_ovs/ovsdb/impl_vsctl.py index 9df1650f..a24fb04c 100644 --- a/vif_plug_ovs/ovsdb/impl_vsctl.py +++ b/vif_plug_ovs/ovsdb/impl_vsctl.py @@ -372,7 +372,12 @@ def _set_colval_args(*col_values): # op. Will try to find a better way to default this op to '=' for entry in col_values: if len(entry) == 2: - col, op, val = entry[0], '=', entry[1] + # Escape colons for MAC address since ovs-vsctl command tries + # to parse it as JSON and colons throw it off + if entry[0] == 'mac': + col, op, val = entry[0], '=', entry[1].replace(':', '\:') + else: + col, op, val = entry[0], '=', entry[1] else: col, op, val = entry if isinstance(val, collections.Mapping): diff --git a/vif_plug_ovs/ovsdb/ovsdb_lib.py b/vif_plug_ovs/ovsdb/ovsdb_lib.py index 2d5cc158..cb90552f 100644 --- a/vif_plug_ovs/ovsdb/ovsdb_lib.py +++ b/vif_plug_ovs/ovsdb/ovsdb_lib.py @@ -14,6 +14,7 @@ import sys from oslo_log import log as logging +from os_vif import utils as os_vif_utils from vif_plug_ovs import constants from vif_plug_ovs import linux_net from vif_plug_ovs.ovsdb import api as ovsdb_api @@ -29,6 +30,14 @@ class BaseOVS(object): self.connection = config.ovsdb_connection self.interface = config.ovsdb_interface self.ovsdb = ovsdb_api.get_instance(self) + self.cleanup_base_mac = config.cleanup_base_mac + + def _is_dpdk_representor_port(self, dev): + options = self.ovsdb.db_get('Interface', dev, 'options').execute() + if options.get("dpdk-devargs") and\ + "representor" in options.get("dpdk-devargs"): + return True + return False def _ovs_supports_mtu_requests(self): return self.ovsdb.has_table_column('Interface', 'mtu_request') @@ -101,6 +110,7 @@ class BaseOVS(object): PF_PCI=pf_pci, VF_NUM=vf_num) col_values.append(('options', {'dpdk-devargs': devargs_string})) + col_values.append(('mac', mac)) with self.ovsdb.transaction() as txn: txn.add(self.ovsdb.add_port(bridge, dev)) txn.add(self.ovsdb.db_set('Interface', dev, *col_values)) @@ -109,7 +119,16 @@ class BaseOVS(object): def update_ovs_vif_port(self, dev, mtu=None, interface_type=None): self.update_device_mtu(dev, mtu, interface_type=interface_type) + def _get_ovs_port_options(self, dev): + return self.ovsdb.db_get('Interface', dev, 'options').execute() + def delete_ovs_vif_port(self, bridge, dev, delete_netdev=True): + if self._is_dpdk_representor_port(dev): + # Cleanup dpdk representor MAC address by setting a random MAC + col_values = [('mac', os_vif_utils.get_random_mac( + self.cleanup_base_mac.split(':')))] + self.ovsdb.db_set('Interface', dev, *col_values).execute() + self.ovsdb.del_port(dev, bridge=bridge, if_exists=True).execute() if delete_netdev: linux_net.delete_net_dev(dev) diff --git a/vif_plug_ovs/tests/unit/ovsdb/test_impl_vsctl.py b/vif_plug_ovs/tests/unit/ovsdb/test_impl_vsctl.py new file mode 100644 index 00000000..0f518216 --- /dev/null +++ b/vif_plug_ovs/tests/unit/ovsdb/test_impl_vsctl.py @@ -0,0 +1,22 @@ +# 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 os_vif.tests.unit import base +from vif_plug_ovs.ovsdb import impl_vsctl + + +class TestModuleLevelMethods(base.TestCase): + + def test__set_colval_args(self): + col_values = [('mac', "aa:aa:aa:aa:aa:aa")] + args = impl_vsctl._set_colval_args(*col_values) + self.assertEqual(['mac=aa\:aa\:aa\:aa\:aa\:aa'], args) diff --git a/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py b/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py index 19e12dc6..78ae6945 100644 --- a/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py +++ b/vif_plug_ovs/tests/unit/ovsdb/test_ovsdb_lib.py @@ -17,6 +17,7 @@ from oslo_concurrency import processutils from oslo_config import cfg from oslo_utils import uuidutils +from os_vif import utils from vif_plug_ovs import constants from vif_plug_ovs import linux_net from vif_plug_ovs.ovsdb import ovsdb_lib @@ -37,6 +38,9 @@ class BaseOVSTest(testtools.TestCase): test_vif_plug_ovs_group) CONF.register_opt(cfg.StrOpt('ovsdb_connection', default=None), test_vif_plug_ovs_group) + CONF.register_opt(cfg.StrOpt('cleanup_base_mac', + default='aa:16:3f:00:00:00'), + test_vif_plug_ovs_group) self.br = ovsdb_lib.BaseOVS(cfg.CONF.test_vif_plug_ovs) self.mock_db_set = mock.patch.object(self.br.ovsdb, 'db_set').start() self.mock_del_port = mock.patch.object(self.br.ovsdb, @@ -132,7 +136,8 @@ class BaseOVSTest(testtools.TestCase): values = [('external_ids', external_ids), ('type', interface_type), ('options', {'dpdk-devargs': - '0000:02:00.1,representor=[0]'})] + '0000:02:00.1,representor=[0]'}), + ('mac', 'ca:fe:ca:fe:ca:fe')] with mock.patch.object(self.br, 'update_device_mtu', return_value=True) as mock_update_device_mtu, \ mock.patch.object(self.br, '_ovs_supports_mtu_requests', @@ -158,19 +163,44 @@ class BaseOVSTest(testtools.TestCase): interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE)]) @mock.patch.object(linux_net, 'delete_net_dev') - def test_delete_ovs_vif_port(self, mock_delete_net_dev): + @mock.patch.object(ovsdb_lib.BaseOVS, '_is_dpdk_representor_port', + return_value=False) + def test_delete_ovs_vif_port(self, mock_is_dpdk_representor_port, + mock_delete_net_dev): self.br.delete_ovs_vif_port('bridge', 'device') self.mock_del_port.assert_has_calls( [mock.call('device', bridge='bridge', if_exists=True)]) mock_delete_net_dev.assert_has_calls([mock.call('device')]) @mock.patch.object(linux_net, 'delete_net_dev') - def test_delete_ovs_vif_port_no_delete_netdev(self, mock_delete_net_dev): - self.br.delete_ovs_vif_port('bridge', 'device', delete_netdev=False) + @mock.patch.object(ovsdb_lib.BaseOVS, '_is_dpdk_representor_port', + return_value=False) + def test_delete_ovs_vif_port_no_delete_netdev(self, + mock_is_dpdk_representor_port, mock_delete_net_dev): + self.br.delete_ovs_vif_port('bridge', 'device', + delete_netdev=False) self.mock_del_port.assert_has_calls( [mock.call('device', bridge='bridge', if_exists=True)]) mock_delete_net_dev.assert_not_called() + @mock.patch.object(linux_net, 'delete_net_dev') + @mock.patch.object(utils, 'get_random_mac', + return_value='aa:16:3f:00:00:00') + @mock.patch.object(ovsdb_lib.BaseOVS, '_is_dpdk_representor_port', + return_value=True) + def test_delete_ovs_dpdk_representor_port(self, + mock_is_dpdk_representor_port, mock_get_random_mac, + mock_delete_net_dev): + self.br.delete_ovs_vif_port('bridge', 'device') + self.mock_del_port.assert_has_calls( + [mock.call('device', bridge='bridge', if_exists=True)]) + self.mock_db_set.assert_has_calls( + [mock.call('Interface', 'device', + ('mac', 'aa:16:3f:00:00:00'))]) + mock_delete_net_dev.assert_has_calls([mock.call('device')]) + mock_get_random_mac.assert_has_calls([mock.call( + ['aa', '16', '3f', '00', '00', '00'])]) + def test_ensure_ovs_bridge(self): self.br.ensure_ovs_bridge('bridge', constants.OVS_DATAPATH_SYSTEM) self.mock_add_br('bridge', may_exist=True,