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

@@ -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
+    _update_device_mtu(dev, mtu, interface_type)
-    # mtu for vhost-user type ports.
+
-    if mtu and interface_type not in [
+
-        constants.OVS_VHOSTUSER_INTERFACE_TYPE,
+@privsep.vif_plug.entrypoint
-        constants.OVS_VHOSTUSER_CLIENT_INTERFACE_TYPE]:
+def update_ovs_vif_port(dev, mtu=None, interface_type=None):
-        _set_device_mtu(dev, mtu)
+    _update_device_mtu(dev, mtu, interface_type)
-    else:
-        LOG.debug("MTU not set on %(interface_name)s interface "
-                  "of type %(interface_type)s.",
-                  {'interface_name': dev,
-                   'interface_type': 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,

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
+            return vif.network.mtu
-        else:
+        return self.config.network_device_mtu
-            mtu = 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."""

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,
+    def test_plug_ovs_bridge(self, mock_sys, ensure_bridge, device_exists,
-                             device_exists, create_veth_pair,
+                             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(
+            '_update_vif_port': [mock.call(self.vif_ovs_hybrid,
-                                 self.vif_ovs_hybrid, 'qvob679325f-ca',
+                                           '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)