Browse Source

Add OVS QoS driver cache for minimum bandwidth rules

1. Add OVS QoS driver cache
Added minimum bandwidth rules cache in the OVS QoS driver. If a new
port is detected in the integration bridge, managed by the OVS agent
driver, the QoS extension is applied (if configured).

If this new port does not have a QoS rule, by default the QoS
configuration is reset in this port. Because this port has no Queue
related, the call "QosOVSAgentDriver.delete_minimum_bandwidth" should
not execute the rule cleanup.

This QoS rule cache implementation per porti is currently used in the
OVS QoS driver for the DSCP and the bandwitdh limit rules.

2. If a Queue cannot be deleted, log the QoS registers using it
If a Queue is still used in a QoS register in the OVSDB, it cannot
be deleted. With the current design, only one QoS rule is created in
the database and several Queues are assigned to it.

If something external to the OVS agent is handling the Queues and by
mistake a Queue is assigned to other QoS registers, this error is now
logged before raising the exception.

Closes-Bug: #1845176
Change-Id: Ia9077fc20e4ca360819a2e368c8c1f9250e5a6d8
changes/22/687922/15
Rodolfo Alonso Hernandez 4 months ago
parent
commit
83d7eb961d
5 changed files with 93 additions and 17 deletions
  1. +16
    -0
      neutron/agent/common/ovs_lib.py
  2. +20
    -0
      neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py
  3. +30
    -14
      neutron/tests/fullstack/test_qos.py
  4. +16
    -0
      neutron/tests/functional/agent/common/test_ovs_lib.py
  5. +11
    -3
      neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py

+ 16
- 0
neutron/agent/common/ovs_lib.py View File

@@ -25,6 +25,7 @@ from neutron_lib import exceptions
from neutron_lib.services.qos import constants as qos_constants
from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import excutils
from oslo_utils import uuidutils
from ovsdbapp.backend.ovs_idl import idlutils

@@ -1085,6 +1086,21 @@ class OVSBridge(BaseOVS):
self.ovsdb.db_destroy('Queue', queue_id).execute(check_error=True)
except idlutils.RowNotFound:
LOG.info('OVS Queue %s was already deleted', queue_id)
except RuntimeError as exc:
with excutils.save_and_reraise_exception():
if 'referential integrity violation' not in str(exc):
return
qos_regs = self._list_qos()
qos_uuids = []
for qos_reg in qos_regs:
queue_nums = [num for num, q in qos_reg['queues'].items()
if q.uuid == queue_id]
if queue_nums:
qos_uuids.append(str(qos_reg['_uuid']))
LOG.error('Queue %(queue)s was still in use by the following '
'QoS rules: %(qoses)s',
{'queue': str(queue_id),
'qoses': ', '.join(sorted(qos_uuids))})

def _update_qos(self, qos_id=None, queues=None):
queues = queues or {}

+ 20
- 0
neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py View File

@@ -168,6 +168,10 @@ class QosOVSAgentDriver(qos.QosLinuxAgentDriver):
'vif_port was not found. It seems that port is already '
'deleted', port.get('port_id'))
return

self.ports[port['port_id']][(qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH,
rule.direction)] = port

# queue_num is used to identify the port which traffic come from,
# it needs to be unique across br-int. It is convenient to use ofport
# as queue_num because it is unique in br-int and start from 1.
@@ -187,10 +191,26 @@ class QosOVSAgentDriver(qos.QosLinuxAgentDriver):
'qos_id': qos_id, 'ports': egress_port_names})

def delete_minimum_bandwidth(self, port):
rule_port = self.ports[port['port_id']].pop(
(qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH,
constants.EGRESS_DIRECTION), None)
if not rule_port:
LOG.debug('delete_minimum_bandwidth was received for port %s but '
'no port information was stored to be deleted',
port['port_id'])
return
self.br_int.delete_minimum_bandwidth_queue(port['port_id'])
LOG.debug("Minimum bandwidth rule was deleted for port: %s.",
port['port_id'])

def delete_minimum_bandwidth_ingress(self, port):
rule_port = self.ports[port['port_id']].pop(
(qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH,
constants.INGRESS_DIRECTION), None)
if not rule_port:
LOG.debug('delete_minimum_bandwidth_ingress was received for port '
'%s but no port information was stored to be deleted',
port['port_id'])
return
LOG.debug("Minimum bandwidth rule for ingress direction was deleted "
"for port %s", port['port_id'])

+ 30
- 14
neutron/tests/fullstack/test_qos.py View File

@@ -91,17 +91,19 @@ class BaseQoSRuleTestCase(object):
shared='False', is_default='False')

def _prepare_vm_with_qos_policy(self, rule_add_functions):
qos_policy = self._create_qos_policy()
qos_policy_id = qos_policy['id']
if rule_add_functions:
qos_policy = self._create_qos_policy()
qos_policy_id = qos_policy['id']
for rule_add in rule_add_functions:
rule_add(qos_policy)
else:
qos_policy_id = qos_policy = None

port = self.safe_client.create_port(
self.tenant_id, self.network['id'],
self.environment.hosts[0].hostname,
qos_policy_id)

for rule_add in rule_add_functions:
rule_add(qos_policy)

vm = self.useFixture(
machine.FakeFullstackMachine(
self.environment.hosts[0],
@@ -691,23 +693,37 @@ class TestMinBwQoSOvs(_TestMinBwQoS, base.BaseFullStackTestCase):
self.fail('QoS register not found with queue-num %s' % queue_num)

def test_min_bw_qos_port_removed(self):
"""Test if min BW limit config is properly removed when port removed"""
"""Test if min BW limit config is properly removed when port removed.

In case another port is added without a QoS policy, the L2 agent QoS
extension will call "handle_port" and then it will force the reset of
this port (self._process_reset_port(port)). This test will check that
if the port is not present in the agent QoS cache, the policy is not
removed.
"""
# Create port without qos policy attached
vm_noqos, _ = self._prepare_vm_with_qos_policy(None)

# Create port with qos policy attached
vm, qos_policy = self._prepare_vm_with_qos_policy(
vm_qos, qos_policy = self._prepare_vm_with_qos_policy(
[functools.partial(
self._add_min_bw_rule, MIN_BANDWIDTH, self.direction)])
self._wait_for_min_bw_rule_applied(
vm, MIN_BANDWIDTH, self.direction)
vm_qos, MIN_BANDWIDTH, self.direction)

qos, queue = self._find_agent_qos_and_queue(vm)
# Check QoS policy and Queue rule.
qos, queue = self._find_agent_qos_and_queue(vm_qos)
self.assertEqual({'min-rate': str(MIN_BANDWIDTH * 1000)},
queue.other_config)
queues = vm.bridge._list_queues(port=vm.neutron_port['id'])
queues = vm_qos.bridge._list_queues(port=vm_qos.neutron_port['id'])
self.assertEqual(1, len(queues))
self.assertEqual(queue.uuid, queues[0]['_uuid'])

# Delete port with qos policy attached
vm.destroy()
self._wait_for_min_bw_rule_removed(vm, self.direction)
self.assertEqual([],
vm.bridge._list_queues(port=vm.neutron_port['id']))
vm_qos.destroy()
self._wait_for_min_bw_rule_removed(vm_qos, self.direction)
self.assertEqual(
[],
vm_qos.bridge._list_queues(port=vm_qos.neutron_port['id']))

vm_noqos.destroy()

+ 16
- 0
neutron/tests/functional/agent/common/test_ovs_lib.py View File

@@ -225,6 +225,22 @@ class BaseOVSTestCase(base.BaseSudoTestCase):
self.ovs._delete_queue(queue_id)
self._check_value(None, self._list_queues, queue_id=queue_id)

def test__delete_queue_still_used_in_a_qos(self):
queue_id, port_id = self._create_queue()
queues = {1: queue_id}
qos_id_1 = self._create_qos(queues=queues)
self.ovs._min_bw_qos_id = uuidutils.generate_uuid()
qos_id_2 = self._create_qos(queues=queues)
with mock.patch.object(ovs_lib.LOG, 'error') as mock_error:
self.assertRaises(RuntimeError, self.ovs._delete_queue,
queue_id)

qoses = ', '.join(sorted([str(qos_id_1), str(qos_id_2)]))
msg = ('Queue %(queue)s was still in use by the following QoS rules: '
'%(qoses)s')
mock_error.assert_called_once_with(
msg, {'queue': str(queue_id), 'qoses': qoses})

def test__update_qos_new(self):
queue_id, port_id = self._create_queue()
queues = {1: queue_id}

+ 11
- 3
neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py View File

@@ -15,6 +15,7 @@ import copy
import mock
from neutron_lib import constants
from neutron_lib import context
from neutron_lib.services.qos import constants as qos_consts
from oslo_utils import uuidutils

from neutron.objects.qos import policy
@@ -209,9 +210,16 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
with mock.patch.object(self.qos_driver.br_int,
'delete_minimum_bandwidth_queue') \
as mock_delete_minimum_bandwidth_queue:
self.qos_driver.delete_minimum_bandwidth({'port_id': 'port_id'})
mock_delete_minimum_bandwidth_queue.assert_called_once_with(
'port_id')
self.qos_driver.ports['p_id'] = {}
self.qos_driver.delete_minimum_bandwidth({'port_id': 'p_id'})
mock_delete_minimum_bandwidth_queue.assert_not_called()

mock_delete_minimum_bandwidth_queue.reset_mock()
self.qos_driver.ports['p_id'] = {
(qos_consts.RULE_TYPE_MINIMUM_BANDWIDTH,
constants.EGRESS_DIRECTION): 'rule_port'}
self.qos_driver.delete_minimum_bandwidth({'port_id': 'p_id'})
mock_delete_minimum_bandwidth_queue.assert_called_once_with('p_id')

def test_update_minimum_bandwidth_no_vif_port(self):
with mock.patch.object(self.qos_driver.br_int,

Loading…
Cancel
Save