From 172855f293b6a48b4b64fd9e47c377c7124e192c Mon Sep 17 00:00:00 2001 From: Jan Gutter Date: Fri, 1 Jun 2018 15:24:28 +0200 Subject: [PATCH] Convert vrouter legacy plugging to os-vif Before this change, the vrouter VIF type used legacy VIF plugging. This changeset ports the plugging methods over to an external os-vif plugin, simplifying the in-tree code. Miscellaneous notes: * There are two "vrouter" Neutron VIF types: * "contrail_vrouter" supporting vhostuser plugging, and * "vrouter", supporting kernel datapath plugging. * The VIFGeneric os-vif type is used for the kernel TAP based plugging when the vnic_type is 'normal'. * For multiqueue support, the minimum version of libvirt 1.3.1 is required. In that case, libvirt creates the TAP device, rather than the os-vif plugin. (This is the minimum version for Rocky and later) ref: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1574957 * The corresponding commit on Tungsten Fabric / OpenContrail for this work is at: https://github.com/Juniper/contrail-nova-vif-driver/commit/ed01d315e5707b4f670468454729dc2031c5f780 Change-Id: I047856982251fddc631679fb2dbcea0f3b0db097 Signed-off-by: Jan Gutter blueprint: vrouter-os-vif-conversion --- nova/network/os_vif_util.py | 20 +++-- nova/privsep/libvirt.py | 32 ------- nova/tests/unit/network/test_os_vif_util.py | 37 +++++++- nova/tests/unit/virt/libvirt/test_vif.py | 87 ++++++++----------- nova/virt/libvirt/vif.py | 73 ---------------- ...parate-os-vif-plugin-5557c9cd6f926fd8.yaml | 12 +++ 6 files changed, 98 insertions(+), 163 deletions(-) create mode 100644 releasenotes/notes/move-vrouter-plug-unplug-to-separate-os-vif-plugin-5557c9cd6f926fd8.yaml diff --git a/nova/network/os_vif_util.py b/nova/network/os_vif_util.py index 1505669c082e..a2aa46243268 100644 --- a/nova/network/os_vif_util.py +++ b/nova/network/os_vif_util.py @@ -426,6 +426,21 @@ def _nova_to_osvif_vif_ivs(vif): return obj +# VIF_TYPE_VROUTER = 'vrouter' +def _nova_to_osvif_vif_vrouter(vif): + vnic_type = vif.get('vnic_type', model.VNIC_TYPE_NORMAL) + if vnic_type == model.VNIC_TYPE_NORMAL: + obj = _get_vif_instance( + vif, + objects.vif.VIFGeneric, + plugin="vrouter", + vif_name=_get_vif_name(vif) + ) + else: + raise NotImplementedError() + return obj + + # VIF_TYPE_DVS = 'dvs' def _nova_to_osvif_vif_dvs(vif): raise NotImplementedError() @@ -461,11 +476,6 @@ def _nova_to_osvif_vif_midonet(vif): raise NotImplementedError() -# VIF_TYPE_VROUTER = 'vrouter' -def _nova_to_osvif_vif_vrouter(vif): - raise NotImplementedError() - - # VIF_TYPE_TAP = 'tap' def _nova_to_osvif_vif_tap(vif): raise NotImplementedError() diff --git a/nova/privsep/libvirt.py b/nova/privsep/libvirt.py index a7035e6880d3..562b4b59c6fa 100644 --- a/nova/privsep/libvirt.py +++ b/nova/privsep/libvirt.py @@ -156,38 +156,6 @@ def unplug_plumgrid_vif(dev): processutils.execute('ifc_ctl', 'gateway', 'del_port', dev) -@nova.privsep.sys_admin_pctxt.entrypoint -def plug_contrail_vif(project_id, vm_id, vm_name, vif_id, net_id, port_type, - dev_name, mac, ip_addr, ip6_addr): - cmd = ( - 'vrouter-port-control', - '--oper=add', - '--vm_project_uuid=%s' % project_id, - '--instance_uuid=%s' % vm_id, - ' --vm_name=%s' % vm_name, - '--uuid=%s' % vif_id, - '--vn_uuid=%s' % net_id, - '--port_type=%s' % port_type, - '--tap_name=%s' % dev_name, - '--mac=%s' % mac, - '--ip_address=%s' % ip_addr, - '--ipv6_address=%s' % ip6_addr, - '--tx_vlan_id=-1', - '--rx_vlan_id=-1', - ) - processutils.execute(*cmd) - - -@nova.privsep.sys_admin_pctxt.entrypoint -def unplug_contrail_vif(port_id): - cmd = ( - 'vrouter-port-control', - '--oper=delete', - '--uuid=%s' % port_id, - ) - processutils.execute(*cmd) - - @nova.privsep.sys_admin_pctxt.entrypoint def readpty(path): # TODO(mikal): I'm not a huge fan that we don't enforce a valid pty path diff --git a/nova/tests/unit/network/test_os_vif_util.py b/nova/tests/unit/network/test_os_vif_util.py index 01d713b811ca..f25415c6dc2a 100644 --- a/nova/tests/unit/network/test_os_vif_util.py +++ b/nova/tests/unit/network/test_os_vif_util.py @@ -1040,7 +1040,8 @@ class OSVIFUtilTestCase(test.NoDBTestCase): subnets=[]),) self.assertIsNone(os_vif_util.nova_to_osvif_vif(vif)) - def test_nova_to_osvif_vhostuser_vrouter(self): + def test_nova_to_osvif_contrail_vrouter(self): + """Test for the Contrail / Tungsten Fabric DPDK datapath.""" vif = model.VIF( id="dc065497-3c8d-4f44-8fb4-e1d33c16a536", type=model.VIF_TYPE_VHOSTUSER, @@ -1077,7 +1078,8 @@ class OSVIFUtilTestCase(test.NoDBTestCase): self.assertObjEqual(expect, actual) - def test_nova_to_osvif_vhostuser_vrouter_no_socket_path(self): + def test_nova_to_osvif_contrail_vrouter_no_socket_path(self): + """Test for the Contrail / Tungsten Fabric DPDK datapath.""" vif = model.VIF( id="dc065497-3c8d-4f44-8fb4-e1d33c16a536", type=model.VIF_TYPE_VHOSTUSER, @@ -1095,3 +1097,34 @@ class OSVIFUtilTestCase(test.NoDBTestCase): self.assertRaises(exception.VifDetailsMissingVhostuserSockPath, os_vif_util.nova_to_osvif_vif, vif) + + def test_nova_to_osvif_vrouter(self): + """Test for the Contrail / Tungsten Fabric kernel datapath.""" + vif = model.VIF( + id="dc065497-3c8d-4f44-8fb4-e1d33c16a536", + type=model.VIF_TYPE_VROUTER, + address="22:52:25:62:e2:aa", + network=model.Network( + id="b82c1929-051e-481d-8110-4669916c7915", + label="Demo Net", + subnets=[]), + ) + + actual = os_vif_util.nova_to_osvif_vif(vif) + + expect = osv_objects.vif.VIFGeneric( + id="dc065497-3c8d-4f44-8fb4-e1d33c16a536", + active=False, + address="22:52:25:62:e2:aa", + plugin="vrouter", + vif_name="nicdc065497-3c", + has_traffic_filtering=False, + preserve_on_delete=False, + network=osv_objects.network.Network( + id="b82c1929-051e-481d-8110-4669916c7915", + bridge_interface=None, + label="Demo Net", + subnets=osv_objects.subnet.SubnetList( + objects=[]))) + + self.assertObjEqual(expect, actual) diff --git a/nova/tests/unit/virt/libvirt/test_vif.py b/nova/tests/unit/virt/libvirt/test_vif.py index 6ed1d85a2abf..2e9c9e5dcb06 100644 --- a/nova/tests/unit/virt/libvirt/test_vif.py +++ b/nova/tests/unit/virt/libvirt/test_vif.py @@ -273,6 +273,15 @@ class LibvirtVifTestCase(test.NoDBTestCase): type=network_model.VIF_TYPE_VROUTER, devname='tap-xxx-yyy-zzz') + vif_contrail_vrouter = network_model.VIF(id=uuids.vif, + address='ca:fe:de:ad:be:ef', + network=network_vrouter, + type=network_model.VIF_TYPE_VHOSTUSER, + details={ + network_model.VIF_DETAILS_VHOSTUSER_MODE: 'server', + network_model.VIF_DETAILS_VHOSTUSER_SOCKET: '/tmp/usv-xxx-yyy-zzz', + network_model.VIF_DETAILS_VHOSTUSER_VROUTER_PLUG: True}) + vif_ib_hostdev = network_model.VIF(id=uuids.vif, address='ca:fe:de:ad:be:ef', network=network_8021, @@ -1012,57 +1021,6 @@ class LibvirtVifTestCase(test.NoDBTestCase): self.vif_iovisor['network']['id'], self.instance.project_id)]) - @mock.patch('nova.privsep.libvirt.unplug_contrail_vif') - def test_unplug_vrouter_with_details(self, mock_unplug_contrail): - d = vif.LibvirtGenericVIFDriver() - d.unplug(self.instance, self.vif_vrouter) - mock_unplug_contrail.assert_called_once_with(self.vif_vrouter['id']) - - @mock.patch('nova.privsep.libvirt.plug_contrail_vif') - @mock.patch('nova.privsep.linux_net.set_device_enabled') - def test_plug_vrouter_with_details(self, mock_enabled, mock_plug_contrail): - d = vif.LibvirtGenericVIFDriver() - instance = mock.Mock() - instance.name = 'instance-name' - instance.uuid = '46a4308b-e75a-4f90-a34a-650c86ca18b2' - instance.project_id = 'b168ea26fa0c49c1a84e1566d9565fa5' - instance.display_name = 'instance1' - instance.image_meta = objects.ImageMeta.from_dict({'properties': {}}) - with mock.patch.object(utils, 'execute') as execute: - d.plug(instance, self.vif_vrouter) - execute.assert_has_calls([ - mock.call('ip', 'tuntap', 'add', 'tap-xxx-yyy-zzz', 'mode', - 'tap', run_as_root=True, check_exit_code=[0, 2, 254])]) - mock_plug_contrail.called_once_with( - instance.project_id, instance.uuid, instance.display_name, - self.vif_vrouter['id'], self.vif_vrouter['network']['id'], - 'NovaVMPort', self.vif_vrouter['devname'], - self.vif_vrouter['address'], '0.0.0.0', None) - mock_enabled.assert_called_once_with('tap-xxx-yyy-zzz') - - @mock.patch('nova.network.linux_utils.create_tap_dev') - @mock.patch('nova.privsep.libvirt.plug_contrail_vif') - def test_plug_vrouter_with_details_multiqueue( - self, mock_plug_contrail, mock_create_tap_dev): - d = vif.LibvirtGenericVIFDriver() - instance = mock.Mock() - instance.name = 'instance-name' - instance.uuid = '46a4308b-e75a-4f90-a34a-650c86ca18b2' - instance.project_id = 'b168ea26fa0c49c1a84e1566d9565fa5' - instance.display_name = 'instance1' - instance.image_meta = objects.ImageMeta.from_dict({ - 'properties': {'hw_vif_multiqueue_enabled': True}}) - instance.flavor.vcpus = 2 - d.plug(instance, self.vif_vrouter) - mock_create_tap_dev.assert_called_once_with('tap-xxx-yyy-zzz', - multiqueue=True) - - mock_plug_contrail.called_once_with( - instance.project_id, instance.uuid, instance.display_name, - self.vif_vrouter['id'], self.vif_vrouter['network']['id'], - 'NovaVMPort', self.vif_vrouter['devname'], - self.vif_vrouter['address'], '0.0.0.0', None) - def _check_ovs_virtualport_driver(self, d, vif, want_iface_id): self.flags(firewall_driver="nova.virt.firewall.NoopFirewallDriver") xml = self._get_instance_xml(d, vif) @@ -1433,6 +1391,33 @@ class LibvirtVifTestCase(test.NoDBTestCase): script = node.find("script") self.assertIsNone(script) + def test_vrouter(self): + """Test for the Contrail / Tungsten Fabric kernel datapath.""" + d = vif.LibvirtGenericVIFDriver() + dev_want = self.vif_vrouter['devname'] + xml = self._get_instance_xml(d, self.vif_vrouter) + node = self._get_node(xml) + self._assertTypeAndMacEquals(node, "ethernet", "target", "dev", + self.vif_vrouter, dev_want) + + def test_contrail_vrouter(self): + """Test for the Contrail / Tungsten Fabric DPDK datapath.""" + d = vif.LibvirtGenericVIFDriver() + xml = self._get_instance_xml(d, + self.vif_contrail_vrouter) + node = self._get_node(xml) + self.assertEqual(node.get("type"), + network_model.VIF_TYPE_VHOSTUSER) + + self._assertTypeEquals(node, network_model.VIF_TYPE_VHOSTUSER, + "source", "mode", "server") + self._assertTypeEquals(node, network_model.VIF_TYPE_VHOSTUSER, + "source", "path", "/tmp/usv-xxx-yyy-zzz") + self._assertTypeEquals(node, network_model.VIF_TYPE_VHOSTUSER, + "source", "type", "unix") + self._assertMacEquals(node, self.vif_contrail_vrouter) + self._assertModel(xml, network_model.VIF_MODEL_VIRTIO) + @mock.patch("nova.network.os_vif_util.nova_to_osvif_instance") @mock.patch("nova.network.os_vif_util.nova_to_osvif_vif") @mock.patch.object(os_vif, "plug") diff --git a/nova/virt/libvirt/vif.py b/nova/virt/libvirt/vif.py index c3989b6147d1..4b846b6c2bb9 100644 --- a/nova/virt/libvirt/vif.py +++ b/nova/virt/libvirt/vif.py @@ -192,10 +192,6 @@ class LibvirtGenericVIFDriver(object): designer.set_vif_host_backend_hostdev_pci_config(conf, pci_slot) return conf - def _is_multiqueue_enabled(self, image_meta, flavor): - _, vhost_queues = self._get_virtio_mq_settings(image_meta, flavor) - return vhost_queues > 1 if vhost_queues is not None else False - def _get_virtio_mq_settings(self, image_meta, flavor): """A methods to set the number of virtio queues, if it has been requested in extra specs. @@ -485,17 +481,6 @@ class LibvirtGenericVIFDriver(object): inst_type, virt_type, host): return self.get_base_hostdev_pci_config(vif) - def get_config_vrouter(self, instance, vif, image_meta, - inst_type, virt_type, host): - conf = self.get_base_config(instance, vif['address'], image_meta, - inst_type, virt_type, vif['vnic_type'], - host) - dev = self.get_vif_devname(vif) - designer.set_vif_host_backend_ethernet_config(conf, dev, host) - - designer.set_vif_bandwidth_config(conf, inst_type) - return conf - def _set_config_VIFGeneric(self, instance, vif, conf, host): dev = vif.vif_name designer.set_vif_host_backend_ethernet_config(conf, dev, host) @@ -721,51 +706,6 @@ class LibvirtGenericVIFDriver(object): def plug_vhostuser(self, instance, vif): pass - def plug_vrouter(self, instance, vif): - """Plug into Contrail's network port - - Bind the vif to a Contrail virtual port. - """ - dev = self.get_vif_devname(vif) - ip_addr = '0.0.0.0' - ip6_addr = None - subnets = vif['network']['subnets'] - for subnet in subnets: - if not subnet['ips']: - continue - ips = subnet['ips'][0] - if not ips['address']: - continue - if (ips['version'] == 4): - if ips['address'] is not None: - ip_addr = ips['address'] - if (ips['version'] == 6): - if ips['address'] is not None: - ip6_addr = ips['address'] - - ptype = 'NovaVMPort' - if (CONF.libvirt.virt_type == 'lxc'): - ptype = 'NameSpacePort' - - try: - multiqueue = self._is_multiqueue_enabled(instance.image_meta, - instance.flavor) - linux_net_utils.create_tap_dev(dev, multiqueue=multiqueue) - nova.privsep.libvirt.plug_contrail_vif( - instance.project_id, - instance.uuid, - instance.display_name, - vif['id'], - vif['network']['id'], - ptype, - dev, - vif['address'], - ip_addr, - ip6_addr, - ) - except processutils.ProcessExecutionError: - LOG.exception(_("Failed while plugging vif"), instance=instance) - def _plug_os_vif(self, instance, vif): instance_info = os_vif_util.nova_to_osvif_instance(instance) @@ -880,19 +820,6 @@ class LibvirtGenericVIFDriver(object): def unplug_vhostuser(self, instance, vif): pass - def unplug_vrouter(self, instance, vif): - """Unplug Contrail's network port - - Unbind the vif from a Contrail virtual port. - """ - dev = self.get_vif_devname(vif) - port_id = vif['id'] - try: - nova.privsep.libvirt.unplug_contrail_vif(port_id) - nova.privsep.linux_net.delete_net_dev(dev) - except processutils.ProcessExecutionError: - LOG.exception(_("Failed while unplugging vif"), instance=instance) - def _unplug_os_vif(self, instance, vif): instance_info = os_vif_util.nova_to_osvif_instance(instance) diff --git a/releasenotes/notes/move-vrouter-plug-unplug-to-separate-os-vif-plugin-5557c9cd6f926fd8.yaml b/releasenotes/notes/move-vrouter-plug-unplug-to-separate-os-vif-plugin-5557c9cd6f926fd8.yaml new file mode 100644 index 000000000000..747f14a8b13b --- /dev/null +++ b/releasenotes/notes/move-vrouter-plug-unplug-to-separate-os-vif-plugin-5557c9cd6f926fd8.yaml @@ -0,0 +1,12 @@ +--- +upgrade: + - | + This release moves the ``vrouter`` VIF plug and unplug code to a + separate package called ``contrail-nova-vif-driver``. This package is a + requirement on compute nodes when using Contrail, OpenContrail or + Tungsten Fabric as a Neutron plugin. + At this time, the reference plugin is hosted on OpenContrail at + https://github.com/Juniper/contrail-nova-vif-driver but is expected to + transition to Tungsten Fabric in the future. + Release ``r5.1.alpha0`` or later of the plugin is required, which will + be included in Tungsten Fabric 5.1.