openstack/neutron
Code Issues Proposed changes

Merge "Allow replacing the QoS policy of bound port"

Browse Source
This commit is contained in:
Zuul 2020-09-30 02:21:39 +00:00 committed by Gerrit Code Review
commit 946057cf27
4 changed files with 330 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
doc/source/admin/config-qos-min-bw.rst
View File

@ -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

91
neutron/services/qos/qos_plugin.py
View File

@ -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

211
neutron/tests/unit/services/qos/test_qos_plugin.py
View File

@ -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)

8
releasenotes/notes/update_qos_allocation_for_bound_port-5358620322b66ae9.yaml Normal file
View File

@ -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.