diff --git a/neutron/services/trunk/exceptions.py b/neutron/services/trunk/exceptions.py index 604162085ce..37adbcdddc9 100644 --- a/neutron/services/trunk/exceptions.py +++ b/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.") diff --git a/neutron/services/trunk/plugin.py b/neutron/services/trunk/plugin.py index dc25a8a7c5a..8d6118e10ae 100644 --- a/neutron/services/trunk/plugin.py +++ b/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 diff --git a/neutron/services/trunk/rules.py b/neutron/services/trunk/rules.py index b7ad260a5cd..5f3191d3fc6 100644 --- a/neutron/services/trunk/rules.py +++ b/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 diff --git a/neutron/tests/unit/services/trunk/test_rules.py b/neutron/tests/unit/services/trunk/test_rules.py index 78698cae79a..0787173827b 100644 --- a/neutron/tests/unit/services/trunk/test_rules.py +++ b/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):