From 01da454fc8f50f2c86200df377dab4d21cecb753 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 8 Dec 2016 19:08:34 +0000 Subject: [PATCH] vif_plug_ovs: Always set MTU when plugging devices Recent versions of neutron have made significant changes to how MTUs are calculated. This work appears to have been completed and it's had knock-on effects for some installers [1] which have changed their default MTUs accordingly. In general, it is not recommended that one change the MTU of a network segment in-flight as it can result in dropped packets (depending on the change). However, the combination of a live upgrade and these recent neutron changes means this exact thing is now happening in some deployments. Make life a little easier for the users who see these issues by configuring as much of the network "plumbing" as possible to use the the latest MTU whenever we plug interfaces. This won't necessarily resolve all packet losses immediately - the guest will have to wait for the new MTU to be propogated with a new DHCP lease or have it set manually by a user - but it will make the issue eventually solvable. [1] https://bugs.launchpad.net/tripleo/+bug/1590101 Change-Id: If09eda334cddc74910dda7a4fb498b7987714be3 Closes-bug: #1649845 --- vif_plug_ovs/linux_net.py | 40 +++++++++++++++++++--------- vif_plug_ovs/ovs.py | 22 ++++++++++------ vif_plug_ovs/tests/test_plugin.py | 43 +++++++++++++++++++++++++------ 3 files changed, 77 insertions(+), 28 deletions(-) diff --git a/vif_plug_ovs/linux_net.py b/vif_plug_ovs/linux_net.py index 59e96995..87ff9aee 100644 --- a/vif_plug_ovs/linux_net.py +++ b/vif_plug_ovs/linux_net.py @@ -75,17 +75,12 @@ def create_ovs_vif_port(bridge, dev, iface_id, mac, instance_id, _ovs_vsctl(_create_ovs_vif_cmd(bridge, dev, iface_id, mac, instance_id, interface_type, vhost_server_path), timeout=timeout) - # Note at present there is no support for setting the - # mtu for vhost-user type ports. - if mtu and interface_type not in [ - constants.OVS_VHOSTUSER_INTERFACE_TYPE, - constants.OVS_VHOSTUSER_CLIENT_INTERFACE_TYPE]: - _set_device_mtu(dev, mtu) - else: - LOG.debug("MTU not set on %(interface_name)s interface " - "of type %(interface_type)s.", - {'interface_name': dev, - 'interface_type': interface_type}) + _update_device_mtu(dev, mtu, interface_type) + + +@privsep.vif_plug.entrypoint +def update_ovs_vif_port(dev, mtu=None, interface_type=None): + _update_device_mtu(dev, mtu, interface_type) @privsep.vif_plug.entrypoint @@ -125,7 +120,14 @@ def create_veth_pair(dev1_name, dev2_name, mtu): for dev in [dev1_name, dev2_name]: processutils.execute('ip', 'link', 'set', dev, 'up') processutils.execute('ip', 'link', 'set', dev, 'promisc', 'on') - _set_device_mtu(dev, mtu) + _update_device_mtu(dev, mtu) + + +@privsep.vif_plug.entrypoint +def update_veth_pair(dev1_name, dev2_name, mtu): + """Update a pair of veth devices with new configuration.""" + for dev in [dev1_name, dev2_name]: + _update_device_mtu(dev, mtu) @privsep.vif_plug.entrypoint @@ -166,6 +168,20 @@ def add_bridge_port(bridge, dev): processutils.execute('brctl', 'addif', bridge, dev) +def _update_device_mtu(dev, mtu, interface_type=None): + # Note at present there is no support for setting the + # mtu for vhost-user type ports. + if mtu and interface_type not in [ + constants.OVS_VHOSTUSER_INTERFACE_TYPE, + constants.OVS_VHOSTUSER_CLIENT_INTERFACE_TYPE]: + _set_device_mtu(dev, mtu) + else: + LOG.debug("MTU not set on %(interface_name)s interface " + "of type %(interface_type)s.", + {'interface_name': dev, + 'interface_type': interface_type}) + + def _set_device_mtu(dev, mtu): """Set the device MTU.""" processutils.execute('ip', 'link', 'set', dev, 'mtu', mtu, diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index 44b4cabe..aede60f9 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -82,11 +82,13 @@ class OvsPlugin(plugin.PluginBase): max_version="1.0") ]) - def _create_vif_port(self, vif, vif_name, instance_info, **kwargs): + def _get_mtu(self, vif): if vif.network and vif.network.mtu: - mtu = vif.network.mtu - else: - mtu = self.config.network_device_mtu + return vif.network.mtu + return self.config.network_device_mtu + + def _create_vif_port(self, vif, vif_name, instance_info, **kwargs): + mtu = self._get_mtu(vif) linux_net.create_ovs_vif_port( vif.network.bridge, vif_name, @@ -96,6 +98,10 @@ class OvsPlugin(plugin.PluginBase): timeout=self.config.ovs_vsctl_timeout, **kwargs) + def _update_vif_port(self, vif, vif_name): + mtu = self._get_mtu(vif) + linux_net.update_ovs_vif_port(vif_name, mtu) + def _plug_vhostuser(self, vif, instance_info): linux_net.ensure_ovs_bridge(vif.network.bridge, constants.OVS_DATAPATH_NETDEV) @@ -126,16 +132,16 @@ class OvsPlugin(plugin.PluginBase): linux_net.ensure_bridge(vif.bridge_name) + mtu = self._get_mtu(vif) if not linux_net.device_exists(v2_name): - if vif.network and vif.network.mtu: - mtu = vif.network.mtu - else: - mtu = self.config.network_device_mtu linux_net.create_veth_pair(v1_name, v2_name, mtu) linux_net.add_bridge_port(vif.bridge_name, v1_name) linux_net.ensure_ovs_bridge(vif.network.bridge, constants.OVS_DATAPATH_SYSTEM) self._create_vif_port(vif, v2_name, instance_info) + else: + linux_net.update_veth_pair(v1_name, v2_name, mtu) + self._update_vif_port(vif, v2_name) def _plug_vif_windows(self, vif, instance_info): """Create a per-VIF OVS port.""" diff --git a/vif_plug_ovs/tests/test_plugin.py b/vif_plug_ovs/tests/test_plugin.py index d442591d..590b8319 100644 --- a/vif_plug_ovs/tests/test_plugin.py +++ b/vif_plug_ovs/tests/test_plugin.py @@ -134,41 +134,68 @@ class PluginTest(testtools.TestCase): self.vif_ovs.network.bridge, constants.OVS_DATAPATH_SYSTEM) @mock.patch.object(linux_net, 'ensure_ovs_bridge') + @mock.patch.object(ovs.OvsPlugin, '_update_vif_port') @mock.patch.object(ovs.OvsPlugin, '_create_vif_port') @mock.patch.object(linux_net, 'add_bridge_port') + @mock.patch.object(linux_net, 'update_veth_pair') @mock.patch.object(linux_net, 'create_veth_pair') - @mock.patch.object(linux_net, 'device_exists', return_value=False) + @mock.patch.object(linux_net, 'device_exists') @mock.patch.object(linux_net, 'ensure_bridge') @mock.patch.object(ovs, 'sys') - def test_plug_ovs_bridge(self, mock_sys, ensure_bridge, - device_exists, create_veth_pair, + def test_plug_ovs_bridge(self, mock_sys, ensure_bridge, device_exists, + create_veth_pair, update_veth_pair, add_bridge_port, _create_vif_port, - ensure_ovs_bridge): + _update_vif_port, ensure_ovs_bridge): calls = { 'device_exists': [mock.call('qvob679325f-ca')], 'create_veth_pair': [mock.call('qvbb679325f-ca', 'qvob679325f-ca', 1500)], + 'update_veth_pair': [mock.call('qvbb679325f-ca', + 'qvob679325f-ca', + 1500)], 'ensure_bridge': [mock.call('qbrvif-xxx-yyy')], 'add_bridge_port': [mock.call('qbrvif-xxx-yyy', 'qvbb679325f-ca')], - '_create_vif_port': [mock.call( - self.vif_ovs_hybrid, 'qvob679325f-ca', - self.instance)], + '_update_vif_port': [mock.call(self.vif_ovs_hybrid, + 'qvob679325f-ca')], + '_create_vif_port': [mock.call(self.vif_ovs_hybrid, + 'qvob679325f-ca', + self.instance)], 'ensure_ovs_bridge': [mock.call('br0', constants.OVS_DATAPATH_SYSTEM)] } + # plugging new devices should result in devices being created + + device_exists.return_value = False mock_sys.platform = 'linux' - plugin = ovs.OvsPlugin.load("ovs") + plugin = ovs.OvsPlugin.load('ovs') plugin.plug(self.vif_ovs_hybrid, self.instance) ensure_bridge.assert_has_calls(calls['ensure_bridge']) device_exists.assert_has_calls(calls['device_exists']) create_veth_pair.assert_has_calls(calls['create_veth_pair']) + self.assertFalse(update_veth_pair.called) + self.assertFalse(_update_vif_port.called) add_bridge_port.assert_has_calls(calls['add_bridge_port']) _create_vif_port.assert_has_calls(calls['_create_vif_port']) ensure_ovs_bridge.assert_has_calls(calls['ensure_ovs_bridge']) + # reset call stacks + + create_veth_pair.reset_mock() + _create_vif_port.reset_mock() + + # plugging existing devices should result in devices being updated + + device_exists.return_value = True + self.assertTrue(linux_net.device_exists('test')) + plugin.plug(self.vif_ovs_hybrid, self.instance) + self.assertFalse(create_veth_pair.called) + self.assertFalse(_create_vif_port.called) + update_veth_pair.assert_has_calls(calls['update_veth_pair']) + _update_vif_port.assert_has_calls(calls['_update_vif_port']) + @mock.patch.object(linux_net, 'ensure_ovs_bridge') @mock.patch.object(ovs.OvsPlugin, '_create_vif_port') @mock.patch.object(linux_net, 'device_exists', return_value=False)