From 138312122ec0d6f95487faf9f7d66c01686a39ed Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Thu, 3 Mar 2016 11:50:34 +0100 Subject: [PATCH] mtu: don't attempt to set link mtu if it's invalid network_device_mtu option does not have an explicit default value set, in which case it is implicitly None. Also, it can be set to e.g. 0. In those both cases, we should not attempt to set 'None' or '0' MTU value on interfaces, since such requests are doomed to fail. Instead of calling to 'ip link' as we do now, validate MTU value first and issue a warning if MTU is not properly configured for the device. Change-Id: I6b4a5e281297ee0c71b6968595480e805ec4279c Co-Authored-By: Sean Mooney Closes-Bug: #1591127 --- vif_plug_linux_bridge/linux_net.py | 10 ++- vif_plug_linux_bridge/tests/test_linux_net.py | 86 +++++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 vif_plug_linux_bridge/tests/test_linux_net.py diff --git a/vif_plug_linux_bridge/linux_net.py b/vif_plug_linux_bridge/linux_net.py index 1915fff..30c58ee 100644 --- a/vif_plug_linux_bridge/linux_net.py +++ b/vif_plug_linux_bridge/linux_net.py @@ -84,9 +84,13 @@ 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]) - # NOTE(vish): set mtu every time to ensure that changes to mtu get - # propogated - _set_device_mtu(interface, mtu) + 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 diff --git a/vif_plug_linux_bridge/tests/test_linux_net.py b/vif_plug_linux_bridge/tests/test_linux_net.py new file mode 100644 index 0000000..c57750e --- /dev/null +++ b/vif_plug_linux_bridge/tests/test_linux_net.py @@ -0,0 +1,86 @@ +# 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 contextlib +import mock +import six +import testtools + +import fixtures +from oslo_concurrency import lockutils +from oslo_concurrency import processutils +from oslo_config import cfg +from oslo_config import fixture as config_fixture +from oslo_log.fixture import logging_error as log_fixture + +from vif_plug_linux_bridge import linux_net +from vif_plug_linux_bridge import privsep + +CONF = cfg.CONF + +if six.PY2: + nested = contextlib.nested +else: + @contextlib.contextmanager + def nested(*contexts): + with contextlib.ExitStack() as stack: + yield [stack.enter_context(c) for c in contexts] + + +class LinuxNetTest(testtools.TestCase): + + def setUp(self): + super(LinuxNetTest, self).setUp() + + privsep.vif_plug.set_client_mode(False) + lock_path = self.useFixture(fixtures.TempDir()).path + self.fixture = self.useFixture( + config_fixture.Config(lockutils.CONF)) + self.fixture.config(lock_path=lock_path, + group='oslo_concurrency') + self.useFixture(log_fixture.get_logging_handle_error_fixture()) + + @mock.patch.object(processutils, "execute") + def test_set_device_mtu(self, execute): + linux_net._set_device_mtu(dev='fakedev', mtu=1500) + expected = ['ip', 'link', 'set', 'fakedev', 'mtu', 1500] + execute.assert_called_with(*expected, check_exit_code=mock.ANY) + + @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(self, mock_set_mtu, + mock_dev_exists, mock_exec): + linux_net._ensure_vlan_privileged(123, 'fake-bridge', + mac_address='fake-mac', + mtu=1500) + self.assertTrue(mock_dev_exists.called) + calls = [mock.call('ip', 'link', 'add', 'link', + 'fake-bridge', 'name', 'vlan123', 'type', + 'vlan', 'id', 123, + check_exit_code=[0, 2, 254]), + mock.call('ip', 'link', 'set', 'vlan123', + 'address', 'fake-mac', + check_exit_code=[0, 2, 254]), + mock.call('ip', 'link', 'set', 'vlan123', 'up', + check_exit_code=[0, 2, 254])] + self.assertEqual(calls, mock_exec.mock_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)