diff --git a/doc/source/admin/config-qos-min-bw.rst b/doc/source/admin/config-qos-min-bw.rst index 7e89b3ec6ab..7ddc7120ef1 100644 --- a/doc/source/admin/config-qos-min-bw.rst +++ b/doc/source/admin/config-qos-min-bw.rst @@ -61,6 +61,26 @@ Limitations effect. That is ports of the QoS policy are not yet used by Nova. Requests to change guarantees of in-use policies are rejected. +* Changing the QoS policy of the port with new ``minimum_bandwidth`` rules + changes placement ``allocations`` from Victoria release. + If the VM was booted with port without QoS policy and ``minimum_bandwidth`` + rules the port update succeeds but placement allocations will not change. + The same is true if the port has no ``binding:profile``, thus no placement + allocation record exists for it. But if the VM was booted with a port with + QoS policy and ``minimum_bandwidth`` rules the update is possible and the + allocations are changed in placement as well. + +.. note:: + + As it is possible to update a port to remove the QoS policy, updating it + back to have QoS policy with ``minimum_bandwidth`` rule will not result in + ``placement allocation`` record, only the dataplane enforcement will happen. + +.. note:: + + updating the ``minimum_bandwidth`` rule of a QoS policy that is attached + to a port which is bound to a VM is still not possible. + * The first data-plane-only Guaranteed Minimum Bandwidth implementation (for SR-IOV egress traffic) was released in the Newton release of Neutron. Because of the known lack of diff --git a/neutron/services/qos/qos_plugin.py b/neutron/services/qos/qos_plugin.py index 728ebc4c566..9ae35dc4cef 100644 --- a/neutron/services/qos/qos_plugin.py +++ b/neutron/services/qos/qos_plugin.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +from keystoneauth1 import exceptions as ks_exc from neutron_lib.api.definitions import port as port_def from neutron_lib.api.definitions import port_resource_request from neutron_lib.api.definitions import portbindings @@ -32,9 +33,12 @@ from neutron_lib.db import api as db_api from neutron_lib.db import resource_extend from neutron_lib import exceptions as lib_exc from neutron_lib.exceptions import qos as qos_exc +from neutron_lib.placement import client as pl_client from neutron_lib.placement import constants as pl_constants from neutron_lib.placement import utils as pl_utils from neutron_lib.services.qos import constants as qos_consts +from oslo_config import cfg +from oslo_log import log as logging from neutron._i18n import _ from neutron.db import db_base_plugin_common @@ -44,10 +48,14 @@ from neutron.objects import network as network_object from neutron.objects import ports as ports_object from neutron.objects.qos import policy as policy_object from neutron.objects.qos import qos_policy_validator as checker +from neutron.objects.qos import rule as rule_object from neutron.objects.qos import rule_type as rule_type_object from neutron.services.qos.drivers import manager +LOG = logging.getLogger(__name__) + + @resource_extend.has_resource_extenders class QoSPlugin(qos.QoSPluginBase): """Implementation of the Neutron QoS Service Plugin. @@ -72,11 +80,19 @@ class QoSPlugin(qos.QoSPluginBase): def __init__(self): super(QoSPlugin, self).__init__() self.driver_manager = manager.QosServiceDriverManager() + self._placement_client = pl_client.PlacementAPIClient(cfg.CONF) callbacks_registry.subscribe( self._validate_create_port_callback, callbacks_resources.PORT, callbacks_events.PRECOMMIT_CREATE) + # TODO(lajoskatona): PORT BEFORE_UPDATE is a notify, so + # "old style" kwargs instead of payload object, let's change it + # to notify and payload. + callbacks_registry.subscribe( + self._check_port_for_placement_allocation_change, + callbacks_resources.PORT, + callbacks_events.BEFORE_UPDATE) callbacks_registry.subscribe( self._validate_update_port_callback, callbacks_resources.PORT, @@ -172,6 +188,81 @@ class QoSPlugin(qos.QoSPluginBase): context.elevated(), id=policy_id) self.validate_policy_for_port(context, policy, port) + def _check_port_for_placement_allocation_change(self, resource, event, + trigger, **kwargs): + context = kwargs['context'] + orig_port = kwargs['original_port'] + original_policy_id = orig_port.get(qos_consts.QOS_POLICY_ID) + policy_id = kwargs['port'].get(qos_consts.QOS_POLICY_ID) + + if policy_id == original_policy_id: + return + + # Do this only for compute bound ports + if (nl_constants.DEVICE_OWNER_COMPUTE_PREFIX in + orig_port['device_owner']): + original_policy = policy_object.QosPolicy.get_object( + context.elevated(), id=original_policy_id) + policy = policy_object.QosPolicy.get_object( + context.elevated(), id=policy_id) + self._change_placement_allocation(original_policy, policy, + orig_port) + + def _prepare_allocation_needs(self, original_rule, desired_rule): + if (isinstance(desired_rule, rule_object.QosMinimumBandwidthRule) or + isinstance(desired_rule, dict)): + o_dir = original_rule.get('direction') + o_minkbps = original_rule.get('min_kbps') + d_minkbps = desired_rule.get('min_kbps') + d_dir = desired_rule.get('direction') + if o_dir == d_dir and o_minkbps != d_minkbps: + diff = d_minkbps - o_minkbps + # TODO(lajoskatona): move this to neutron-lib, see similar + # dict @l125. + if d_dir == 'egress': + drctn = pl_constants.CLASS_NET_BW_EGRESS_KBPS + else: + drctn = pl_constants.CLASS_NET_BW_INGRESS_KBPS + return {drctn: diff} + return {} + + def _change_placement_allocation(self, original_policy, desired_policy, + orig_port): + alloc_diff = {} + original_rules = [] + rp_uuid = orig_port['binding:profile'].get('allocation') + device_id = orig_port['device_id'] + if original_policy: + original_rules = original_policy.get('rules') + if desired_policy: + desired_rules = desired_policy.get('rules') + else: + desired_rules = [{'direction': 'egress', 'min_kbps': 0}, + {'direction': 'ingress', 'min_kbps': 0}] + + any_rules_minbw = any( + [isinstance(r, rule_object.QosMinimumBandwidthRule) + for r in desired_rules]) + if (not original_rules and any_rules_minbw) or not rp_uuid: + LOG.warning("There was no QoS policy with minimum_bandwidth rule " + "attached to the port %s, there is no allocation " + "record in placement for it, only the dataplane " + "enforcement will happen!", orig_port['id']) + return + for o_rule in original_rules: + if isinstance(o_rule, rule_object.QosMinimumBandwidthRule): + for d_rule in desired_rules: + alloc_diff.update( + self._prepare_allocation_needs(o_rule, d_rule)) + if alloc_diff: + try: + self._placement_client.update_qos_minbw_allocation( + consumer_uuid=device_id, minbw_alloc_diff=alloc_diff, + rp_uuid=rp_uuid) + except ks_exc.Conflict: + raise qos_exc.QosPlacementAllocationConflict( + consumer=device_id, rp=rp_uuid) + def _validate_update_port_callback(self, resource, event, trigger, payload=None): context = payload.context diff --git a/neutron/tests/unit/services/qos/test_qos_plugin.py b/neutron/tests/unit/services/qos/test_qos_plugin.py index 3e1b08e0888..5756134b486 100644 --- a/neutron/tests/unit/services/qos/test_qos_plugin.py +++ b/neutron/tests/unit/services/qos/test_qos_plugin.py @@ -13,12 +13,14 @@ import copy from unittest import mock +from keystoneauth1 import exceptions as ks_exc import netaddr from neutron_lib.api.definitions import qos from neutron_lib.callbacks import events from neutron_lib import constants as lib_constants from neutron_lib import context from neutron_lib import exceptions as lib_exc +from neutron_lib.exceptions import placement as pl_exc from neutron_lib.exceptions import qos as qos_exc from neutron_lib.objects import utils as obj_utils from neutron_lib.placement import constants as pl_constants @@ -1221,6 +1223,14 @@ class TestQosPluginDB(base.BaseQosTestCase): qos_policy.create() return qos_policy + def _make_qos_minbw_rule(self, policy_id, direction='ingress', + min_kbps=1000): + qos_rule = rule_object.QosMinimumBandwidthRule( + self.context, project_id=self.project_id, + qos_policy_id=policy_id, direction=direction, min_kbps=min_kbps) + qos_rule.create() + return qos_rule + def _make_port(self, network_id, qos_policy_id=None): base_mac = ['aa', 'bb', 'cc', 'dd', 'ee', 'ff'] mac = netaddr.EUI(next(net_utils.random_mac_generator(base_mac))) @@ -1277,3 +1287,204 @@ class TestQosPluginDB(base.BaseQosTestCase): def test_validate_create_port_callback_no_policy(self): self._test_validate_create_port_callback() + + def _prepare_for_port_placement_allocation_change(self, qos1, qos2): + qos1_id = qos1.id if qos1 else None + qos2_id = qos2.id if qos2 else None + + network = self._make_network() + port = self._make_port(network.id, qos_policy_id=qos1_id) + + return {"context": self.context, + "original_port": { + "id": port.id, + "device_owner": "compute:uu:id", + "qos_policy_id": qos1_id}, + "port": {"id": port.id, "qos_policy_id": qos2_id}} + + def test_check_port_for_placement_allocation_change_no_qos_change(self): + qos1_obj = self._make_qos_policy() + kwargs = self._prepare_for_port_placement_allocation_change( + qos1=qos1_obj, qos2=qos1_obj) + with mock.patch.object( + self.qos_plugin, + '_change_placement_allocation') as mock_alloc_change: + self.qos_plugin._check_port_for_placement_allocation_change( + 'PORT', 'before_update', 'test_plugin', **kwargs) + mock_alloc_change.assert_not_called() + + def test_check_port_for_placement_allocation_change(self): + qos1_obj = self._make_qos_policy() + qos2_obj = self._make_qos_policy() + kwargs = self._prepare_for_port_placement_allocation_change( + qos1=qos1_obj, qos2=qos2_obj) + + with mock.patch.object( + self.qos_plugin, + '_change_placement_allocation') as mock_alloc_change: + self.qos_plugin._check_port_for_placement_allocation_change( + 'PORT', 'before_update', 'test_plugin', **kwargs) + mock_alloc_change.assert_called_once_with( + qos1_obj, qos2_obj, kwargs['original_port']) + + def test_check_port_for_placement_allocation_change_no_new_policy(self): + qos1_obj = self._make_qos_policy() + kwargs = self._prepare_for_port_placement_allocation_change( + qos1=qos1_obj, qos2=None) + + with mock.patch.object( + self.qos_plugin, + '_change_placement_allocation') as mock_alloc_change: + self.qos_plugin._check_port_for_placement_allocation_change( + 'PORT', 'before_update', 'test_plugin', **kwargs) + mock_alloc_change.assert_called_once_with( + qos1_obj, None, kwargs['original_port']) + + def _prepare_port_for_placement_allocation(self, qos1, qos2=None, + min_kbps1=1000, min_kbps2=2000): + rule1_obj = self._make_qos_minbw_rule(qos1.id, min_kbps=min_kbps1) + qos1.rules = [rule1_obj] + if qos2: + rule2_obj = self._make_qos_minbw_rule(qos2.id, min_kbps=min_kbps2) + qos2.rules = [rule2_obj] + orig_port = {'binding:profile': {'allocation': 'rp:uu:id'}, + 'device_id': 'uu:id'} + return orig_port + + def test_change_placement_allocation_increase(self): + qos1 = self._make_qos_policy() + qos2 = self._make_qos_policy() + port = self._prepare_port_for_placement_allocation(qos1, qos2) + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + self.qos_plugin._change_placement_allocation(qos1, qos2, port) + mock_update_qos_alloc.assert_called_once_with( + consumer_uuid='uu:id', + minbw_alloc_diff={'NET_BW_IGR_KILOBIT_PER_SEC': 1000}, + rp_uuid='rp:uu:id') + + def test_test_change_placement_allocation_decrease(self): + qos1 = self._make_qos_policy() + qos2 = self._make_qos_policy() + port = self._prepare_port_for_placement_allocation(qos2, qos1) + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + self.qos_plugin._change_placement_allocation(qos1, qos2, port) + mock_update_qos_alloc.assert_called_once_with( + consumer_uuid='uu:id', + minbw_alloc_diff={'NET_BW_IGR_KILOBIT_PER_SEC': -1000}, + rp_uuid='rp:uu:id') + + def test_change_placement_allocation_no_original_qos(self): + qos1 = None + qos2 = self._make_qos_policy() + rule2_obj = self._make_qos_minbw_rule(qos2.id, min_kbps=1000) + qos2.rules = [rule2_obj] + orig_port = {'id': 'u:u', 'device_id': 'i:d', 'binding:profile': {}} + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + self.qos_plugin._change_placement_allocation( + qos1, qos2, orig_port) + mock_update_qos_alloc.assert_not_called() + + def test_change_placement_allocation_no_original_allocation(self): + qos1 = self._make_qos_policy() + rule1_obj = self._make_qos_minbw_rule(qos1.id, min_kbps=500) + qos1.rules = [rule1_obj] + qos2 = self._make_qos_policy() + rule2_obj = self._make_qos_minbw_rule(qos2.id, min_kbps=1000) + qos2.rules = [rule2_obj] + orig_port = {'id': 'u:u', 'device_id': 'i:d', 'binding:profile': {}} + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + self.qos_plugin._change_placement_allocation( + qos1, qos2, orig_port) + mock_update_qos_alloc.assert_not_called() + + def test_change_placement_allocation_new_policy_empty(self): + qos1 = self._make_qos_policy() + port = self._prepare_port_for_placement_allocation(qos1) + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + self.qos_plugin._change_placement_allocation(qos1, None, port) + mock_update_qos_alloc.assert_called_once_with( + consumer_uuid='uu:id', + minbw_alloc_diff={'NET_BW_IGR_KILOBIT_PER_SEC': -1000}, + rp_uuid='rp:uu:id') + + def test_change_placement_allocation_no_min_bw(self): + qos1 = self._make_qos_policy() + qos2 = self._make_qos_policy() + bw_limit_rule1 = rule_object.QosDscpMarkingRule(dscp_mark=16) + bw_limit_rule2 = rule_object.QosDscpMarkingRule(dscp_mark=18) + qos1.rules = [bw_limit_rule1] + qos2.rules = [bw_limit_rule2] + port = {'binding:profile': {'allocation': 'rp:uu:id'}, + 'device_id': 'uu:id'} + + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + self.qos_plugin._change_placement_allocation(qos1, None, port) + mock_update_qos_alloc.assert_not_called() + + def test_change_placement_allocation_old_rule_not_min_bw(self): + qos1 = self._make_qos_policy() + qos2 = self._make_qos_policy() + bw_limit_rule = rule_object.QosDscpMarkingRule(dscp_mark=16) + port = self._prepare_port_for_placement_allocation(qos1, qos2) + qos1.rules = [bw_limit_rule] + + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + self.qos_plugin._change_placement_allocation(qos1, qos2, port) + mock_update_qos_alloc.assert_not_called() + + def test_change_placement_allocation_new_rule_not_min_bw(self): + qos1 = self._make_qos_policy() + qos2 = self._make_qos_policy() + bw_limit_rule = rule_object.QosDscpMarkingRule(dscp_mark=16) + qos2.rules = [bw_limit_rule] + port = self._prepare_port_for_placement_allocation(qos1) + + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + self.qos_plugin._change_placement_allocation(qos1, qos2, port) + mock_update_qos_alloc.assert_not_called() + + def test_change_placement_allocation_equal_minkbps(self): + qos1 = self._make_qos_policy() + qos2 = self._make_qos_policy() + port = self._prepare_port_for_placement_allocation(qos1, qos2, 1000, + 1000) + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + self.qos_plugin._change_placement_allocation(qos1, qos2, port) + mock_update_qos_alloc.assert_not_called() + + def test_change_placement_allocation_update_conflict(self): + qos1 = self._make_qos_policy() + qos2 = self._make_qos_policy() + port = self._prepare_port_for_placement_allocation(qos1, qos2) + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + mock_update_qos_alloc.side_effect = ks_exc.Conflict( + response={'errors': [{'code': 'placement.concurrent_update'}]} + ) + self.assertRaises( + qos_exc.QosPlacementAllocationConflict, + self.qos_plugin._change_placement_allocation, + qos1, qos2, port) + + def test_change_placement_allocation_update_generation_conflict(self): + qos1 = self._make_qos_policy() + qos2 = self._make_qos_policy() + port = self._prepare_port_for_placement_allocation(qos1, qos2) + with mock.patch.object(self.qos_plugin._placement_client, + 'update_qos_minbw_allocation') as mock_update_qos_alloc: + mock_update_qos_alloc.side_effect = ( + pl_exc.PlacementAllocationGenerationConflict( + consumer='rp:uu:id')) + self.assertRaises( + pl_exc.PlacementAllocationGenerationConflict, + self.qos_plugin._change_placement_allocation, + qos1, qos2, port) diff --git a/releasenotes/notes/update_qos_allocation_for_bound_port-5358620322b66ae9.yaml b/releasenotes/notes/update_qos_allocation_for_bound_port-5358620322b66ae9.yaml new file mode 100644 index 00000000000..8d64c0695dc --- /dev/null +++ b/releasenotes/notes/update_qos_allocation_for_bound_port-5358620322b66ae9.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + Update of an ``already bound port`` with ``QoS minimum_bandwidth`` rule + with new ``QoS policy`` with ``minimum_bandwidth`` rule changes the + allocations in placement as well. ``NOTE``: updating the + ``minimum_bandwidth`` rule of a QoS policy that is attached to a port + which is bound to a VM is still not possible.