openstack
/
os-vif
Code Issues Proposed changes

set mtu on all code paths 

- This change ensures that the mtu is always set
  by the linux bridge plugin if provided.

related-bug: #bug/1684326
Change-Id: I9f901dcf67823a6dd78b9b18cb3e06a3f6a13e37
This commit is contained in:
Sean Mooney 2017-06-01 16:31:48 +00:00
parent fff78cf0d2
commit e15e8dc46e
4 changed files with 74 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
vif_plug_linux_bridge/linux_bridge.py
View File

@ -95,9 +95,9 @@ class LinuxBridgePlugin(plugin.PluginBase):
network = vif.network
bridge_name = vif.bridge_name
if not network.multi_host and network.should_provide_bridge:
mtu = network.mtu or self.config.network_device_mtu
if network.should_provide_vlan:
iface = self.config.vlan_interface or network.bridge_interface
mtu = network.mtu or self.config.network_device_mtu
linux_net.ensure_vlan_bridge(network.vlan,
bridge_name, iface, mtu=mtu)
else:
@ -105,7 +105,7 @@ class LinuxBridgePlugin(plugin.PluginBase):
# only put in iptables rules if Neutron not filtering
install_filters = not vif.has_traffic_filtering
linux_net.ensure_bridge(bridge_name, iface,
filtering=install_filters)
filtering=install_filters, mtu=mtu)
def unplug(self, vif, instance_info):
# Nothing required to unplug a port for a VIF using standard

29
vif_plug_linux_bridge/linux_net.py
View File

@ -39,8 +39,12 @@ def device_exists(device):
def _set_device_mtu(dev, mtu):
"""Set the device MTU."""
processutils.execute('ip', 'link', 'set', dev, 'mtu', mtu,
check_exit_code=[0, 2, 254])
if mtu:
processutils.execute('ip', 'link', 'set', dev, 'mtu', mtu,
check_exit_code=[0, 2, 254])
else:
LOG.debug("MTU not set on %(interface_name)s interface",
{'interface_name': dev})
def _ip_bridge_cmd(action, params, device):
@ -85,33 +89,32 @@ def _ensure_vlan_privileged(vlan_num, bridge_interface, mac_address, mtu):
check_exit_code=[0, 2, 254])
processutils.execute('ip', 'link', 'set', interface, 'up',
check_exit_code=[0, 2, 254])
if mtu:
# NOTE(vish): set mtu every time to ensure that changes to mtu get
# propogated
_set_device_mtu(interface, mtu)
else:
LOG.debug("MTU not set on %(interface_name)s interface",
{'interface_name': interface})
return interface
@lockutils.synchronized('nova-lock_bridge', external=True)
def ensure_bridge(bridge, interface, net_attrs=None, gateway=True,
filtering=True):
_ensure_bridge_privileged(bridge, interface, net_attrs, gateway, filtering)
filtering=True, mtu=None):
_ensure_bridge_privileged(bridge, interface, net_attrs, gateway,
filtering=filtering, mtu=mtu)
if filtering:
_ensure_bridge_filtering(bridge, gateway)
@privsep.vif_plug.entrypoint
def _ensure_bridge_privileged(bridge, interface, net_attrs, gateway,
filtering=True):
filtering=True, mtu=None):
"""Create a bridge unless it already exists.
:param interface: the interface to create the bridge on.
:param net_attrs: dictionary with attributes used to create bridge.
:param gateway: whether or not the bridge is a gateway.
:param filtering: whether or not to create filters on the bridge.
:param mtu: MTU of bridge.
If net_attrs is set, it will add the net_attrs['gateway'] to the bridge
using net_attrs['broadcast'] and net_attrs['cidr']. It will also add
@ -156,6 +159,8 @@ def _ensure_bridge_privileged(bridge, interface, net_attrs, gateway,
out, err = processutils.execute('ip', 'link', 'set',
interface, 'up', check_exit_code=False)
_set_device_mtu(interface, mtu)
# NOTE(vish): This will break if there is already an ip on the
# interface, so we move any ips to the bridge
# NOTE(danms): We also need to copy routes to the bridge so as
@ -186,6 +191,12 @@ def _ensure_bridge_privileged(bridge, interface, net_attrs, gateway,
for fields in old_routes:
processutils.execute('ip', 'route', 'add', *fields)
# NOTE(sean-k-mooney):
# The bridge mtu cannont be set until after an
# interface is added due to bug:
# https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1399064
_set_device_mtu(bridge, mtu)
def _ensure_bridge_filtering(bridge, gateway):
# This method leaves privsep usage to iptables manager

44
vif_plug_linux_bridge/tests/unit/test_linux_net.py
View File

@ -46,6 +46,11 @@ class LinuxNetTest(testtools.TestCase):
expected = ['ip', 'link', 'set', 'fakedev', 'mtu', 1500]
execute.assert_called_with(*expected, check_exit_code=mock.ANY)
@mock.patch.object(processutils, "execute")
def test_set_device_invalid_mtu(self, mock_exec):
linux_net._set_device_mtu(dev='fakedev', mtu=None)
mock_exec.assert_not_called()
@mock.patch.object(processutils, "execute")
@mock.patch.object(linux_net, "device_exists", return_value=False)
@mock.patch.object(linux_net, "_set_device_mtu")
@ -67,15 +72,6 @@ class LinuxNetTest(testtools.TestCase):
mock_exec.assert_has_calls(calls)
mock_set_mtu.assert_called_once_with('vlan123', 1500)
@mock.patch.object(processutils, "execute")
@mock.patch.object(linux_net, "device_exists", return_value=False)
@mock.patch.object(linux_net, "_set_device_mtu")
def test_ensure_vlan_invalid_mtu(self, mock_set_mtu,
mock_dev_exists, mock_exec):
linux_net._ensure_vlan_privileged(123, 'fake-bridge',
mac_address='fake-mac', mtu=None)
self.assertFalse(mock_set_mtu.called)
@mock.patch.object(processutils, "execute")
@mock.patch.object(linux_net, "device_exists", return_value=True)
def test_ensure_bridge_exists(self, mock_dev_exists, mock_exec):
@ -104,6 +100,36 @@ class LinuxNetTest(testtools.TestCase):
mock_exec.assert_has_calls(calls)
mock_dev_exists.assert_has_calls([mock.call("br0"), mock.call("br0")])
@mock.patch.object(linux_net, "_set_device_mtu")
@mock.patch.object(os.path, "exists", return_value=False)
@mock.patch.object(processutils, "execute")
@mock.patch.object(linux_net, "device_exists", return_value=False)
def test_ensure_bridge_mtu_not_called(self, mock_dev_exists, mock_exec,
mock_path_exists, mock_set_mtu):
"""This test validates that mtus are updated only if an interface
is added to the bridge
"""
linux_net._ensure_bridge_privileged("fake-bridge", None,
None, False, mtu=1500)
mock_set_mtu.assert_not_called()
@mock.patch.object(linux_net, "_set_device_mtu")
@mock.patch.object(os.path, "exists", return_value=False)
@mock.patch.object(processutils, "execute", return_value=("", ""))
@mock.patch.object(linux_net, "device_exists", return_value=False)
def test_ensure_bridge_mtu_order(self, mock_dev_exists, mock_exec,
mock_path_exists, mock_set_mtu):
"""This test validates that when adding an interface
to a bridge, the interface mtu is updated first
followed by the bridge. This is required to work around
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1399064
"""
linux_net._ensure_bridge_privileged("fake-bridge", "fake-interface",
None, False, mtu=1500)
calls = [mock.call('fake-interface', 1500),
mock.call('fake-bridge', 1500)]
mock_set_mtu.assert_has_calls(calls)
@mock.patch.object(os.path, "exists", return_value=False)
@mock.patch.object(processutils, "execute")
@mock.patch.object(linux_net, "device_exists", return_value=False)

22
vif_plug_linux_bridge/tests/unit/test_plugin.py
View File

@ -51,15 +51,23 @@ class PluginTest(testtools.TestCase):
mock_ensure_bridge.assert_not_called()
mock_ensure_vlan_bridge.assert_not_called()
def test_plug_bridge_create_br_mtu_in_model(self):
self._test_plug_bridge_create_br(mtu=1234)
def test_plug_bridge_create_br_mtu_from_config(self):
self._test_plug_bridge_create_br()
@mock.patch.object(linux_net, 'ensure_vlan_bridge')
@mock.patch.object(linux_net, 'ensure_bridge')
def test_plug_bridge_create_br(self, mock_ensure_bridge,
mock_ensure_vlan_bridge):
def _test_plug_bridge_create_br(self, mock_ensure_bridge,
mock_ensure_vlan_bridge,
mtu=None):
network = objects.network.Network(
id='437c6db5-4e6f-4b43-b64b-ed6a11ee5ba7',
bridge='br0',
bridge_interface='eth0',
should_provide_bridge=True)
should_provide_bridge=True,
mtu=mtu)
vif = objects.vif.VIFBridge(
id='b679325f-ca89-4ee0-a8be-6db1409b69ea',
@ -72,13 +80,17 @@ class PluginTest(testtools.TestCase):
plugin = linux_bridge.LinuxBridgePlugin.load("linux_bridge")
plugin.plug(vif, self.instance)
mock_ensure_bridge.assert_called_with("br0", "eth0", filtering=False)
mock_ensure_bridge.assert_called_with("br0", "eth0",
filtering=False,
mtu=mtu or 1500)
mock_ensure_vlan_bridge.assert_not_called()
mock_ensure_bridge.reset_mock()
vif.has_traffic_filtering = False
plugin.plug(vif, self.instance)
mock_ensure_bridge.assert_called_with("br0", "eth0", filtering=True)
mock_ensure_bridge.assert_called_with("br0", "eth0",
filtering=True,
mtu=mtu or 1500)
def test_plug_bridge_create_br_vlan_mtu_in_model(self):
self._test_plug_bridge_create_br_vlan(mtu=1234)