Edward Haas has uploaded a new change for review.

Change subject: net: Correctly apply MTU values on networks
......................................................................

net: Correctly apply MTU values on networks

Two issues have been resolved by this change:

- New networks with no MTU specification are being
set by default with their connected device (bond,
vlan, nic) mtu (which does not have to be 1500).
Fixed by detecting when no MTU us specified in the
configuration, and adding the default (1500)
explicitly.

- Test fix: The NIC/s mtu should be set to the
maximum mtu of the remaining networks.

Change-Id: Iba24363f5b9ea70392a68885cf5374800ab58549
Signed-off-by: Edward Haas <[email protected]>
---
M lib/vdsm/network/api.py
M tests/functional/networkTests.py
2 files changed, 28 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/97/50397/1

diff --git a/lib/vdsm/network/api.py b/lib/vdsm/network/api.py
index c3d82d4..1d4ea09 100755
--- a/lib/vdsm/network/api.py
+++ b/lib/vdsm/network/api.py
@@ -37,6 +37,7 @@
                           get as netinfo_get, CachingNetInfo,
                           networks as netinfo_networks, nics as netinfo_nics,
                           NET_PATH)
+from vdsm.netinfo.mtus import DEFAULT_MTU
 from vdsm import udevadm
 from vdsm import utils
 from vdsm import ipwrapper
@@ -895,6 +896,8 @@
                  "networks:%r, bondings:%r, options:%r" % (networks,
                                                            bondings, options))
 
+    _compliment_net_defaults(networks)
+
     logging.debug("Validating configuration")
     _validateNetworkSetup(networks, bondings)
 
@@ -961,6 +964,18 @@
     return vlan
 
 
+def _compliment_net_defaults(nets):
+    """
+    Given a network configuration, explicitly add missing defaults.
+    :param nets: The network configuration
+    """
+    _nets_attr = (att for net, att in nets.iteritems()
+                  if 'remove' not in att or att['remove'] is not True)
+    for attr in _nets_attr:
+        if 'mtu' not in attr:
+            attr['mtu'] = DEFAULT_MTU
+
+
 def setSafeNetworkConfig():
     """Declare current network configuration as 'safe'"""
     utils.execCmd([constants.EXT_VDSM_STORE_NET_CONFIG,
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 5e23172..aa25b50 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -40,6 +40,7 @@
 from vdsm.netinfo.dhcp import get_dhclient_ifaces
 from vdsm.netinfo.nics import operstate, OPERSTATE_UNKNOWN, OPERSTATE_UP
 from vdsm.netinfo.routes import getRouteDeviceTo
+from vdsm.netinfo.mtus import DEFAULT_MTU
 from vdsm.netlink import monitor
 from vdsm.network.configurators.ifcfg import (Ifcfg, stop_devices,
                                               NET_CONF_BACK_DIR)
@@ -987,14 +988,13 @@
 
     @cleanupNet
     @permutations([[True], [False]])
-    @brokentest("This test fails because the 2 different networks are getting"
-                " configured with the same MTU. The test should assert that "
-                "the reported MTUs are equal to the requested ones.")
     def testSetupNetworksMtus(self, bridged):
         JUMBO = '9000'
         MIDI = '4000'
 
         with dummyIf(3) as nics:
+            # Add two networks, one with default MTU and the other with MIDI.
+            # Expect to see nics MTU to be MIDI. (MIDI > DEFAULT MTU)
             networks = {NETWORK_NAME + '1':
                         dict(bonding=BONDING_NAME, bridged=bridged,
                              vlan='100'),
@@ -1010,6 +1010,8 @@
             self.assertMtu(MIDI, NETWORK_NAME + '2', BONDING_NAME, nics[0],
                            nics[1])
 
+            # Add a 3rd network, with JUMBO MTU.
+            # Expect to see nics MTU to be JUMBO. (JUMBO > MIDI)
             network = {NETWORK_NAME + '3':
                        dict(bonding=BONDING_NAME, vlan='300', mtu=JUMBO,
                             bridged=bridged)}
@@ -1021,6 +1023,8 @@
             self.assertMtu(JUMBO, NETWORK_NAME + '3', BONDING_NAME, nics[0],
                            nics[1])
 
+            # Remove the 3rd network (with JUMBO MTU)
+            # Expect to see nics MTU to be MIDI.
             status, msg = self.setupNetworks(
                 {NETWORK_NAME + '3': dict(remove=True)}, {}, NOCHK)
 
@@ -1029,21 +1033,24 @@
             self.assertMtu(MIDI, NETWORK_NAME + '2', BONDING_NAME, nics[0],
                            nics[1])
 
-            # Keep last custom MTU on the interfaces
+            # Remove the 2nd network (with MIDI MTU)
+            # Expect to see nics MTU to be DEFAULT.
             status, msg = self.setupNetworks(
                 {NETWORK_NAME + '2': dict(remove=True)}, {}, NOCHK)
 
             self.assertEquals(status, SUCCESS, msg)
 
-            self.assertMtu(MIDI, BONDING_NAME, nics[0], nics[1])
+            self.assertMtu(DEFAULT_MTU, BONDING_NAME, nics[0], nics[1])
 
             # Add additional nic to the bond
+            # Expect to see nics MTU to be DEFAULT.
             status, msg = self.setupNetworks(
                 {}, {BONDING_NAME: dict(nics=nics)}, NOCHK)
 
             self.assertEquals(status, SUCCESS, msg)
 
-            self.assertMtu(MIDI, BONDING_NAME, nics[0], nics[1], nics[2])
+            self.assertMtu(DEFAULT_MTU, BONDING_NAME,
+                           nics[0], nics[1], nics[2])
 
             status, msg = self.setupNetworks(
                 {NETWORK_NAME + '1': dict(remove=True)},


-- 
To view, visit https://gerrit.ovirt.org/50397
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iba24363f5b9ea70392a68885cf5374800ab58549
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Edward Haas <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to