Merge "Check MTU sanity of trunk port subports"

neutron/services/trunk/exceptions.py

@@ -41,6 +41,11 @@ class ParentPortInUse(n_exc.InUse):
                 "eligible for use as a parent port.")
 
 
+class SubPortMtuGreaterThanTrunkPortMtu(n_exc.Conflict):
+    message = _("MTU %(port_mtu)s of subport %(port_id)s cannot be greater "
+                "than MTU %(trunk_mtu)s of trunk %(trunk_id)s.")
+
+
 class PortInUseAsTrunkParent(n_exc.InUse):
     message = _("Port %(port_id)s is currently a parent port "
                 "for trunk %(trunk_id)s.")

neutron/services/trunk/plugin.py

@@ -273,16 +273,18 @@ class TrunkPlugin(service_base.ServicePluginBase,
     @db_base_plugin_common.convert_result_to_dict
     def add_subports(self, context, trunk_id, subports):
         """Add one or more subports to trunk."""
-        # Check for basic validation since the request body here is not
-        # automatically validated by the API layer.
-        subports = subports['sub_ports']
-        subports_validator = rules.SubPortsValidator(
-            self._segmentation_types, subports)
-        subports = subports_validator.validate(context, basic_validation=True)
-        added_subports = []
-
         with db_api.autonested_transaction(context.session):
             trunk = self._get_trunk(context, trunk_id)
+
+            # Check for basic validation since the request body here is not
+            # automatically validated by the API layer.
+            subports = subports['sub_ports']
+            subports_validator = rules.SubPortsValidator(
+                self._segmentation_types, subports, trunk['port_id'])
+            subports = subports_validator.validate(
+                context, basic_validation=True)
+            added_subports = []
+
             rules.trunk_can_be_managed(context, trunk)
             original_trunk = copy.deepcopy(trunk)
             # NOTE(status_police): the trunk status should transition to

neutron/services/trunk/rules.py

@@ -17,9 +17,11 @@ from neutron_lib.api import validators
 from neutron_lib import exceptions as n_exc
 
 from neutron._i18n import _
+from neutron.common import utils as n_utils
 from neutron.extensions import portbindings
 from neutron import manager
 from neutron.objects import trunk as trunk_objects
+from neutron.plugins.ml2 import driver_api as api
 from neutron.services.trunk import exceptions as trunk_exc
 from neutron.services.trunk import utils
 
@@ -131,18 +133,56 @@ class SubPortsValidator(object):
             msg = validators.validate_subports(self.subports)
             if msg:
                 raise n_exc.InvalidInput(error_message=msg)
+        trunk_port_mtu = self._get_port_mtu(context, self.trunk_port_id)
         if trunk_validation:
-            return [self._validate(context, s) for s in self.subports]
+            return [self._validate(context, s, trunk_port_mtu)
+                    for s in self.subports]
         else:
             return self.subports
 
-    def _validate(self, context, subport):
+    def _get_port_mtu(self, context, port_id):
+        """
+        Return MTU for the network where the given port belongs to.
+        If the network or port cannot be obtained, or if MTU is not defined,
+        returns None.
+        """
+        core_plugin = manager.NeutronManager.get_plugin()
+
+        if not n_utils.is_extension_supported(core_plugin, 'net-mtu'):
+            return
+
+        try:
+            port = core_plugin.get_port(context, port_id)
+            net = core_plugin.get_network(context, port['network_id'])
+        except (n_exc.PortNotFound, n_exc.NetworkNotFound):
+            # A concurrent request might have made the port or network
+            # disappear; though during DB insertion, the subport request
+            # will fail on integrity constraint, it is safer to return
+            # a None MTU here.
+            return
+
+        return net[api.MTU]
+
+    def _validate(self, context, subport, trunk_port_mtu):
         # Check that the subport doesn't reference the same port_id as a
         # trunk we may be in the middle of trying to create, in other words
         # make the validation idiot proof.
         if subport['port_id'] == self.trunk_port_id:
             raise trunk_exc.ParentPortInUse(port_id=subport['port_id'])
 
+        # Check MTU sanity - subport MTU must not exceed trunk MTU.
+        # If for whatever reason trunk_port_mtu is not available,
+        # the MTU sanity check cannot be enforced.
+        if trunk_port_mtu:
+            port_mtu = self._get_port_mtu(context, subport['port_id'])
+            if port_mtu and port_mtu > trunk_port_mtu:
+                raise trunk_exc.SubPortMtuGreaterThanTrunkPortMtu(
+                    port_id=subport['port_id'],
+                    port_mtu=port_mtu,
+                    trunk_id=self.trunk_port_id,
+                    trunk_mtu=trunk_port_mtu
+                )
+
         # If the segmentation details are missing, we will need to
         # figure out defaults when the time comes to support Ironic.
         # We can reasonably expect segmentation details to be provided

neutron/tests/unit/services/trunk/test_rules.py

@@ -22,6 +22,7 @@ from oslo_utils import uuidutils
 
 from neutron import manager
 from neutron.plugins.common import utils
+from neutron.plugins.ml2 import driver_api as api
 from neutron.services.trunk import constants
 from neutron.services.trunk import drivers
 from neutron.services.trunk import exceptions as trunk_exc
@@ -40,6 +41,9 @@ class SubPortsValidatorTestCase(base.BaseTestCase):
         self.segmentation_types = {constants.VLAN: utils.is_valid_vlan_tag}
         self.context = mock.ANY
 
+        mock.patch.object(rules.SubPortsValidator, '_get_port_mtu',
+                          return_value=None).start()
+
     def test_validate_subport_subport_and_trunk_shared_port_id(self):
         shared_id = uuidutils.generate_uuid()
         validator = rules.SubPortsValidator(
@@ -119,6 +123,78 @@ class SubPortsValidatorTestCase(base.BaseTestCase):
                           self.context, basic_validation=True)
 
 
+class SubPortsValidatorMtuSanityTestCase(test_plugin.Ml2PluginV2TestCase):
+
+    def setUp(self):
+        super(SubPortsValidatorMtuSanityTestCase, self).setUp()
+        self.segmentation_types = {constants.VLAN: utils.is_valid_vlan_tag}
+
+    def test_validate_subport_mtu_same_as_trunk(self):
+        self._test_validate_subport_trunk_mtu(1500, 1500)
+
+    def test_validate_subport_mtu_smaller_than_trunks(self):
+        self._test_validate_subport_trunk_mtu(500, 1500)
+
+    def test_validate_subport_mtu_greater_than_trunks(self):
+        self._test_validate_subport_trunk_mtu(1500, 500)
+
+    def test_validate_subport_mtu_unset_trunks_set(self):
+        self._test_validate_subport_trunk_mtu(None, 500)
+
+    def test_validate_subport_mtu_set_trunks_unset(self):
+        self._test_validate_subport_trunk_mtu(500, None)
+
+    def test_validate_subport_mtu_set_trunks_net_exception(self):
+        self._test_validate_subport_trunk_mtu(1500, 'exc')
+
+    def test_validate_subport_mtu_net_exception_trunks_set(self):
+        self._test_validate_subport_trunk_mtu('exc', 1500)
+
+    def _test_validate_subport_trunk_mtu(
+            self, subport_net_mtu, trunk_net_mtu):
+        plugin = manager.NeutronManager.get_plugin()
+        orig_get_network = plugin.get_network
+
+        def get_network_adjust_mtu(*args, **kwargs):
+            res = orig_get_network(*args, **kwargs)
+            if res['name'] == 'net_trunk':
+                if trunk_net_mtu == 'exc':
+                    raise n_exc.NetworkNotFound(net_id='net-id')
+                res[api.MTU] = trunk_net_mtu
+            elif res['name'] == 'net_subport':
+                if subport_net_mtu == 'exc':
+                    raise n_exc.NetworkNotFound(net_id='net-id')
+                res[api.MTU] = subport_net_mtu
+            return res
+
+        with self.network('net_trunk') as trunk_net,\
+            self.subnet(network=trunk_net) as trunk_subnet,\
+            self.port(subnet=trunk_subnet) as trunk_port,\
+            self.network('net_subport') as subport_net,\
+            self.subnet(network=subport_net) as subport_subnet,\
+            self.port(subnet=subport_subnet) as subport,\
+            mock.patch.object(plugin, "get_network",
+                              side_effect=get_network_adjust_mtu):
+            trunk = {'port_id': trunk_port['port']['id'],
+                     'tenant_id': 'test_tenant',
+                     'sub_ports': [{'port_id': subport['port']['id'],
+                                    'segmentation_type': 'vlan',
+                                    'segmentation_id': 2}]}
+
+            validator = rules.SubPortsValidator(
+                self.segmentation_types, trunk['sub_ports'], trunk['port_id'])
+
+            if subport_net_mtu is None or trunk_net_mtu is None:
+                validator.validate(self.context)
+            elif subport_net_mtu == 'exc' or trunk_net_mtu == 'exc':
+                validator.validate(self.context)
+            elif subport_net_mtu <= trunk_net_mtu:
+                validator.validate(self.context)
+            else:
+                self.assertRaises(trunk_exc.SubPortMtuGreaterThanTrunkPortMtu,
+                                  validator.validate, self.context)
+
+
 class TrunkPortValidatorTestCase(test_plugin.Ml2PluginV2TestCase):
 
     def setUp(self):