No network devices on network attached qos policies
Network devices, like internal router legs, or dhcp ports should not be affected by bandwidth limiting rules. This patch disables application of network attached policies to network/neutron owned ports. Closes-bug: #1486039 DocImpact Change-Id: I75d80227f1e6c4b3f5fa7762b8dc3b0c0f1abd46
This commit is contained in:
parent
cc698b2ba5
commit
fe236bdaad
|
@ -84,8 +84,14 @@ for a port or a network:
|
|||
|
||||
Each QoS policy contains zero or more QoS rules. A policy is then applied to a
|
||||
network or a port, making all rules of the policy applied to the corresponding
|
||||
Neutron resource (for a network, applying a policy means that the policy will
|
||||
be applied to all ports that belong to it).
|
||||
Neutron resource.
|
||||
|
||||
When applied through a network association, policy rules could apply or not
|
||||
to neutron internal ports (like router, dhcp, load balancer, etc..). The QosRule
|
||||
base object provides a default should_apply_to_port method which could be
|
||||
overridden. In the future we may want to have a flag in QoSNetworkPolicyBinding
|
||||
or QosRule to enforce such type of application (for example when limiting all
|
||||
the ingress of routers devices on an external network automatically).
|
||||
|
||||
From database point of view, following objects are defined in schema:
|
||||
|
||||
|
|
|
@ -104,9 +104,13 @@ class QosAgentDriver(object):
|
|||
|
||||
def _handle_update_create_rules(self, action, port, qos_policy):
|
||||
for rule in self._iterate_rules(qos_policy.rules):
|
||||
handler_name = "".join((action, "_", rule.rule_type))
|
||||
handler = getattr(self, handler_name)
|
||||
handler(port, rule)
|
||||
if rule.should_apply_to_port(port):
|
||||
handler_name = "".join((action, "_", rule.rule_type))
|
||||
handler = getattr(self, handler_name)
|
||||
handler(port, rule)
|
||||
else:
|
||||
LOG.debug("Port %(port)s excluded from QoS rule %(rule)s",
|
||||
{'port': port, 'rule': rule.id})
|
||||
|
||||
|
||||
class PortPolicyMap(object):
|
||||
|
@ -204,7 +208,9 @@ class QosAgentExtension(agent_extension.AgentCoreResourceExtension):
|
|||
Update events are handled in _handle_notification.
|
||||
"""
|
||||
port_id = port['port_id']
|
||||
qos_policy_id = port.get('qos_policy_id')
|
||||
port_qos_policy_id = port.get('qos_policy_id')
|
||||
network_qos_policy_id = port.get('network_qos_policy_id')
|
||||
qos_policy_id = port_qos_policy_id or network_qos_policy_id
|
||||
if qos_policy_id is None:
|
||||
self._process_reset_port(port)
|
||||
return
|
||||
|
|
|
@ -41,6 +41,8 @@ DEVICE_OWNER_ROUTER_SNAT = "network:router_centralized_snat"
|
|||
DEVICE_OWNER_LOADBALANCER = "neutron:LOADBALANCER"
|
||||
DEVICE_OWNER_LOADBALANCERV2 = "neutron:LOADBALANCERV2"
|
||||
|
||||
DEVICE_OWNER_PREFIXES = ["network:", "neutron:"]
|
||||
|
||||
# Collection used to identify devices owned by router interfaces.
|
||||
# DEVICE_OWNER_ROUTER_HA_INTF is a special case and so is not included.
|
||||
ROUTER_INTERFACE_OWNERS = (DEVICE_OWNER_ROUTER_INTF,
|
||||
|
|
|
@ -20,6 +20,7 @@ from oslo_versionedobjects import base as obj_base
|
|||
from oslo_versionedobjects import fields as obj_fields
|
||||
import six
|
||||
|
||||
from neutron.common import constants
|
||||
from neutron.common import utils
|
||||
from neutron.db import api as db_api
|
||||
from neutron.db.qos import models as qos_db_model
|
||||
|
@ -57,6 +58,22 @@ class QosRule(base.NeutronDbObject):
|
|||
dict_['type'] = self.rule_type
|
||||
return dict_
|
||||
|
||||
def should_apply_to_port(self, port):
|
||||
"""Check whether a rule can be applied to a specific port.
|
||||
|
||||
This function has the logic to decide whether a rule should
|
||||
be applied to a port or not, depending on the source of the
|
||||
policy (network, or port). Eventually rules could override
|
||||
this method, or we could make it abstract to allow different
|
||||
rule behaviour.
|
||||
"""
|
||||
is_network_rule = self.qos_policy_id != port[qos_consts.QOS_POLICY_ID]
|
||||
is_network_device_port = any(port['device_owner'].startswith(prefix)
|
||||
for prefix
|
||||
in constants.DEVICE_OWNER_PREFIXES)
|
||||
|
||||
return not (is_network_rule and is_network_device_port)
|
||||
|
||||
|
||||
@obj_base.VersionedObjectRegistry.register
|
||||
class QosBandwidthLimitRule(QosRule):
|
||||
|
|
|
@ -109,9 +109,8 @@ class RpcCallbacks(type_tunnel.TunnelRpcCallbackMixin):
|
|||
host,
|
||||
port_context.network.current)
|
||||
|
||||
qos_policy_id = (port.get(qos_consts.QOS_POLICY_ID) or
|
||||
port_context.network._network.get(
|
||||
qos_consts.QOS_POLICY_ID))
|
||||
network_qos_policy_id = port_context.network._network.get(
|
||||
qos_consts.QOS_POLICY_ID)
|
||||
entry = {'device': device,
|
||||
'network_id': port['network_id'],
|
||||
'port_id': port['id'],
|
||||
|
@ -124,7 +123,8 @@ class RpcCallbacks(type_tunnel.TunnelRpcCallbackMixin):
|
|||
'device_owner': port['device_owner'],
|
||||
'allowed_address_pairs': port['allowed_address_pairs'],
|
||||
'port_security_enabled': port.get(psec.PORTSECURITY, True),
|
||||
'qos_policy_id': qos_policy_id,
|
||||
'qos_policy_id': port.get(qos_consts.QOS_POLICY_ID),
|
||||
'network_qos_policy_id': network_qos_policy_id,
|
||||
'profile': port[portbindings.PROFILE]}
|
||||
if 'security_groups' in port:
|
||||
entry['security_groups'] = port['security_groups']
|
||||
|
|
|
@ -31,11 +31,13 @@ TEST_POLICY_ID1 = "a2d72369-4246-4f19-bd3c-af51ec8d70cd"
|
|||
TEST_POLICY_ID2 = "46ebaec0-0570-43ac-82f6-60d2b03168c5"
|
||||
TEST_BW_LIMIT_RULE_1 = rule.QosBandwidthLimitRule(
|
||||
context=None,
|
||||
qos_policy_id=TEST_POLICY_ID1,
|
||||
id="5f126d84-551a-4dcf-bb01-0e9c0df0c793",
|
||||
max_kbps=1000,
|
||||
max_burst_kbps=10)
|
||||
TEST_BW_LIMIT_RULE_2 = rule.QosBandwidthLimitRule(
|
||||
context=None,
|
||||
qos_policy_id=TEST_POLICY_ID2,
|
||||
id="fa9128d9-44af-49b2-99bb-96548378ad42",
|
||||
max_kbps=900,
|
||||
max_burst_kbps=9)
|
||||
|
@ -80,6 +82,7 @@ class OVSAgentQoSExtensionTestFramework(base.OVSAgentTestFramework):
|
|||
port_dict = super(OVSAgentQoSExtensionTestFramework,
|
||||
self)._create_test_port_dict()
|
||||
port_dict['qos_policy_id'] = policy_id
|
||||
port_dict['network_qos_policy_id'] = None
|
||||
return port_dict
|
||||
|
||||
def _get_device_details(self, port, network):
|
||||
|
|
|
@ -69,10 +69,14 @@ class QosAgentDriverTestCase(base.BaseTestCase):
|
|||
self.policy = TEST_POLICY
|
||||
self.rule = (
|
||||
rule.QosBandwidthLimitRule(context=None, id='fake_rule_id',
|
||||
qos_policy_id=self.policy.id,
|
||||
max_kbps=100, max_burst_kbps=200))
|
||||
self.policy.rules = [self.rule]
|
||||
self.port = object()
|
||||
self.fake_rule = QosFakeRule(context=None, id='really_fake_rule_id')
|
||||
self.port = {'qos_policy_id': None, 'network_qos_policy_id': None,
|
||||
'device_owner': 'random-device-owner'}
|
||||
|
||||
self.fake_rule = QosFakeRule(context=None, id='really_fake_rule_id',
|
||||
qos_policy_id=self.policy.id)
|
||||
|
||||
def test_create(self):
|
||||
self.driver.create(self.port, self.policy)
|
||||
|
@ -98,6 +102,15 @@ class QosAgentDriverTestCase(base.BaseTestCase):
|
|||
self.assertEqual(1, len(rules))
|
||||
self.assertIsInstance(rules[0], rule.QosBandwidthLimitRule)
|
||||
|
||||
def test__handle_update_create_rules_checks_should_apply_to_port(self):
|
||||
self.rule.should_apply_to_port = mock.Mock(return_value=False)
|
||||
self.driver.create(self.port, self.policy)
|
||||
self.assertFalse(self.driver.create_bandwidth_limit.called)
|
||||
|
||||
self.rule.should_apply_to_port = mock.Mock(return_value=True)
|
||||
self.driver.create(self.port, self.policy)
|
||||
self.assertTrue(self.driver.create_bandwidth_limit.called)
|
||||
|
||||
|
||||
class QosExtensionBaseTestCase(base.BaseTestCase):
|
||||
|
||||
|
|
|
@ -10,12 +10,56 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
from neutron.common import constants
|
||||
from neutron.objects.qos import policy
|
||||
from neutron.objects.qos import rule
|
||||
from neutron.services.qos import qos_consts
|
||||
from neutron.tests import base as neutron_test_base
|
||||
from neutron.tests.unit.objects import test_base
|
||||
from neutron.tests.unit import testlib_api
|
||||
|
||||
POLICY_ID_A = 'policy-id-a'
|
||||
POLICY_ID_B = 'policy-id-b'
|
||||
DEVICE_OWNER_COMPUTE = 'compute:None'
|
||||
|
||||
|
||||
class QosRuleObjectTestCase(neutron_test_base.BaseTestCase):
|
||||
|
||||
def _test_should_apply_to_port(self, rule_policy_id, port_policy_id,
|
||||
device_owner, expected_result):
|
||||
test_rule = rule.QosRule(qos_policy_id=rule_policy_id)
|
||||
port = {qos_consts.QOS_POLICY_ID: port_policy_id,
|
||||
'device_owner': device_owner}
|
||||
self.assertEqual(expected_result, test_rule.should_apply_to_port(port))
|
||||
|
||||
def test_should_apply_to_port_with_network_port_and_net_policy(self):
|
||||
self._test_should_apply_to_port(
|
||||
rule_policy_id=POLICY_ID_B,
|
||||
port_policy_id=POLICY_ID_A,
|
||||
device_owner=constants.DEVICE_OWNER_ROUTER_INTF,
|
||||
expected_result=False)
|
||||
|
||||
def test_should_apply_to_port_with_network_port_and_port_policy(self):
|
||||
self._test_should_apply_to_port(
|
||||
rule_policy_id=POLICY_ID_A,
|
||||
port_policy_id=POLICY_ID_A,
|
||||
device_owner=constants.DEVICE_OWNER_ROUTER_INTF,
|
||||
expected_result=True)
|
||||
|
||||
def test_should_apply_to_port_with_compute_port_and_net_policy(self):
|
||||
self._test_should_apply_to_port(
|
||||
rule_policy_id=POLICY_ID_B,
|
||||
port_policy_id=POLICY_ID_A,
|
||||
device_owner=DEVICE_OWNER_COMPUTE,
|
||||
expected_result=True)
|
||||
|
||||
def test_should_apply_to_port_with_compute_port_and_port_policy(self):
|
||||
self._test_should_apply_to_port(
|
||||
rule_policy_id=POLICY_ID_A,
|
||||
port_policy_id=POLICY_ID_A,
|
||||
device_owner=DEVICE_OWNER_COMPUTE,
|
||||
expected_result=True)
|
||||
|
||||
|
||||
class QosBandwidthLimitRuleObjectTestCase(test_base.BaseObjectIfaceTestCase):
|
||||
|
||||
|
|
|
@ -22,6 +22,7 @@ from neutron.objects.qos import rule
|
|||
from neutron.plugins.ml2.drivers.mech_sriov.agent.common import exceptions
|
||||
from neutron.plugins.ml2.drivers.mech_sriov.agent.extension_drivers import (
|
||||
qos_driver)
|
||||
from neutron.services.qos import qos_consts
|
||||
from neutron.tests import base
|
||||
|
||||
|
||||
|
@ -42,7 +43,7 @@ class QosSRIOVAgentDriverTestCase(base.BaseTestCase):
|
|||
self.clear_max_rate_mock = self.qos_driver.eswitch_mgr.clear_max_rate
|
||||
self.rule = self._create_bw_limit_rule_obj()
|
||||
self.qos_policy = self._create_qos_policy_obj([self.rule])
|
||||
self.port = self._create_fake_port()
|
||||
self.port = self._create_fake_port(self.qos_policy.id)
|
||||
|
||||
def _create_bw_limit_rule_obj(self):
|
||||
rule_obj = rule.QosBandwidthLimitRule()
|
||||
|
@ -61,12 +62,18 @@ class QosSRIOVAgentDriverTestCase(base.BaseTestCase):
|
|||
'rules': rules}
|
||||
policy_obj = policy.QosPolicy(self.context, **policy_dict)
|
||||
policy_obj.obj_reset_changes()
|
||||
for policy_rule in policy_obj.rules:
|
||||
policy_rule.qos_policy_id = policy_obj.id
|
||||
policy_rule.obj_reset_changes()
|
||||
|
||||
return policy_obj
|
||||
|
||||
def _create_fake_port(self):
|
||||
def _create_fake_port(self, qos_policy_id):
|
||||
return {'port_id': uuidutils.generate_uuid(),
|
||||
'profile': {'pci_slot': self.PCI_SLOT},
|
||||
'device': self.ASSIGNED_MAC}
|
||||
'device': self.ASSIGNED_MAC,
|
||||
qos_consts.QOS_POLICY_ID: qos_policy_id,
|
||||
'device_owner': uuidutils.generate_uuid()}
|
||||
|
||||
def test_create_rule(self):
|
||||
self.qos_driver.create(self.port, self.qos_policy)
|
||||
|
|
|
@ -39,7 +39,7 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
|
|||
self.create = self.qos_driver.br_int.create_egress_bw_limit_for_port
|
||||
self.rule = self._create_bw_limit_rule_obj()
|
||||
self.qos_policy = self._create_qos_policy_obj([self.rule])
|
||||
self.port = self._create_fake_port()
|
||||
self.port = self._create_fake_port(self.qos_policy.id)
|
||||
|
||||
def _create_bw_limit_rule_obj(self):
|
||||
rule_obj = rule.QosBandwidthLimitRule()
|
||||
|
@ -58,15 +58,21 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
|
|||
'rules': rules}
|
||||
policy_obj = policy.QosPolicy(self.context, **policy_dict)
|
||||
policy_obj.obj_reset_changes()
|
||||
for policy_rule in policy_obj.rules:
|
||||
policy_rule.qos_policy_id = policy_obj.id
|
||||
policy_rule.obj_reset_changes()
|
||||
return policy_obj
|
||||
|
||||
def _create_fake_port(self):
|
||||
def _create_fake_port(self, policy_id):
|
||||
self.port_name = 'fakeport'
|
||||
|
||||
class FakeVifPort(object):
|
||||
port_name = self.port_name
|
||||
|
||||
return {'vif_port': FakeVifPort()}
|
||||
return {'vif_port': FakeVifPort(),
|
||||
'qos_policy_id': policy_id,
|
||||
'network_qos_policy_id': None,
|
||||
'device_owner': uuidutils.generate_uuid()}
|
||||
|
||||
def test_create_new_rule(self):
|
||||
self.qos_driver.br_int.get_egress_bw_limit_for_port = mock.Mock(
|
||||
|
|
|
@ -144,16 +144,16 @@ class RpcCallbacksTestCase(base.BaseTestCase):
|
|||
res = self.callbacks.get_device_details(mock.Mock(), host='fake')
|
||||
self.assertIsNone(res['qos_policy_id'])
|
||||
|
||||
def test_get_device_details_qos_policy_id_inherited_from_network(self):
|
||||
def test_get_device_details_network_qos_policy_id(self):
|
||||
port = collections.defaultdict(lambda: 'fake_port')
|
||||
self.plugin.get_bound_port_context().current = port
|
||||
self.plugin.get_bound_port_context().network._network = (
|
||||
{"id": "fake_network",
|
||||
qos_consts.QOS_POLICY_ID: 'test-policy-id'})
|
||||
res = self.callbacks.get_device_details(mock.Mock(), host='fake')
|
||||
self.assertEqual('test-policy-id', res['qos_policy_id'])
|
||||
self.assertEqual('test-policy-id', res['network_qos_policy_id'])
|
||||
|
||||
def test_get_device_details_qos_policy_id_taken_from_port(self):
|
||||
def test_get_device_details_qos_policy_id_from_port(self):
|
||||
port = collections.defaultdict(
|
||||
lambda: 'fake_port',
|
||||
{qos_consts.QOS_POLICY_ID: 'test-port-policy-id'})
|
||||
|
|
Loading…
Reference in New Issue