Retain port info from DSCP rule creation.
When a VM is deleted all info except the port number is removed. delete_dscp_marking requires the ofport to be present. This results in an exception being thrown when a port with the DSCP_Marking rule attached is deleted. This patch: - Stores the port info when the dscp_marking rule is updated or created. - Pops the stored info when the dscp_marking rule is removed from the port or the port is deleted. - Expands existing unit tests for the QoS Open vswitch driver to cover this scenario. Change-Id: I77f632fdc7d612267af9a4a3bf0f74288696332b Closes-bug: #1603443
This commit is contained in:
David Shaughnessy
parent
8eaa1808e5
commit
af8ca1bca6
|
|
@ -12,12 +12,15 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import collections
|
||||
|
||||
from oslo_config import cfg
|
||||
from oslo_log import log as logging
|
||||
|
||||
from neutron.agent.l2.extensions import qos
|
||||
from neutron.plugins.ml2.drivers.openvswitch.mech_driver import (
|
||||
mech_openvswitch)
|
||||
from neutron.services.qos import qos_consts
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
|
@ -33,6 +36,7 @@ class QosOVSAgentDriver(qos.QosAgentDriver):
|
|||
self.br_int_name = cfg.CONF.OVS.integration_bridge
|
||||
self.br_int = None
|
||||
self.agent_api = None
|
||||
self.ports = collections.defaultdict(dict)
|
||||
|
||||
def consume_api(self, agent_api):
|
||||
self.agent_api = agent_api
|
||||
|
|
@ -47,7 +51,7 @@ class QosOVSAgentDriver(qos.QosAgentDriver):
|
|||
def update_bandwidth_limit(self, port, rule):
|
||||
vif_port = port.get('vif_port')
|
||||
if not vif_port:
|
||||
port_id = port.get('port_id', None)
|
||||
port_id = port.get('port_id')
|
||||
LOG.debug("update_bandwidth_limit was received for port %s but "
|
||||
"vif_port was not found. It seems that port is already "
|
||||
"deleted", port_id)
|
||||
|
|
@ -65,7 +69,7 @@ class QosOVSAgentDriver(qos.QosAgentDriver):
|
|||
def delete_bandwidth_limit(self, port):
|
||||
vif_port = port.get('vif_port')
|
||||
if not vif_port:
|
||||
port_id = port.get('port_id', None)
|
||||
port_id = port.get('port_id')
|
||||
LOG.debug("delete_bandwidth_limit was received for port %s but "
|
||||
"vif_port was not found. It seems that port is already "
|
||||
"deleted", port_id)
|
||||
|
|
@ -76,7 +80,15 @@ class QosOVSAgentDriver(qos.QosAgentDriver):
|
|||
self.update_dscp_marking(port, rule)
|
||||
|
||||
def update_dscp_marking(self, port, rule):
|
||||
port_name = port['vif_port'].port_name
|
||||
self.ports[port['port_id']][qos_consts.RULE_TYPE_DSCP_MARKING] = port
|
||||
vif_port = port.get('vif_port')
|
||||
if not vif_port:
|
||||
port_id = port.get('port_id')
|
||||
LOG.debug("update_dscp_marking was received for port %s but "
|
||||
"vif_port was not found. It seems that port is already "
|
||||
"deleted", port_id)
|
||||
return
|
||||
port_name = vif_port.port_name
|
||||
port = self.br_int.get_port_ofport(port_name)
|
||||
mark = rule.dscp_mark
|
||||
#mark needs to be bit shifted 2 left to not overwrite the
|
||||
|
|
@ -109,7 +121,12 @@ class QosOVSAgentDriver(qos.QosAgentDriver):
|
|||
actions=actions)
|
||||
|
||||
def delete_dscp_marking(self, port):
|
||||
port_name = port['vif_port'].port_name
|
||||
port = self.br_int.get_port_ofport(port_name)
|
||||
|
||||
self.br_int.delete_flows(in_port=port, table=0, reg2=0)
|
||||
dscp_port = self.ports[port['port_id']].pop(qos_consts.
|
||||
RULE_TYPE_DSCP_MARKING, 0)
|
||||
if dscp_port:
|
||||
port_num = dscp_port['vif_port'].ofport
|
||||
self.br_int.delete_flows(in_port=port_num, table=0, reg2=0)
|
||||
else:
|
||||
LOG.debug("delete_dscp_marking was received for port %s but "
|
||||
"no port information was stored to be deleted",
|
||||
port['port_id'])
|
||||
|
|
|
|||
|
|
@ -46,13 +46,15 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
|
|||
self.qos_driver.br_int = mock.Mock()
|
||||
self.qos_driver.br_int.get_egress_bw_limit_for_port = mock.Mock(
|
||||
return_value=(1000, 10))
|
||||
self.qos_driver.br_int.dump_flows_for = mock.Mock(return_value=None)
|
||||
self.get = self.qos_driver.br_int.get_egress_bw_limit_for_port
|
||||
self.qos_driver.br_int.del_egress_bw_limit_for_port = mock.Mock()
|
||||
self.delete = self.qos_driver.br_int.delete_egress_bw_limit_for_port
|
||||
self.qos_driver.br_int.create_egress_bw_limit_for_port = mock.Mock()
|
||||
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.rules = [self._create_bw_limit_rule_obj(),
|
||||
self._create_dscp_marking_rule_obj()]
|
||||
self.qos_policy = self._create_qos_policy_obj(self.rules)
|
||||
self.port = self._create_fake_port(self.qos_policy.id)
|
||||
|
||||
def _create_bw_limit_rule_obj(self):
|
||||
|
|
@ -63,6 +65,13 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
|
|||
rule_obj.obj_reset_changes()
|
||||
return rule_obj
|
||||
|
||||
def _create_dscp_marking_rule_obj(self):
|
||||
rule_obj = rule.QosDscpMarkingRule()
|
||||
rule_obj.id = uuidutils.generate_uuid()
|
||||
rule_obj.dscp_mark = 32
|
||||
rule_obj.obj_reset_changes()
|
||||
return rule_obj
|
||||
|
||||
def _create_qos_policy_obj(self, rules):
|
||||
policy_dict = {'id': uuidutils.generate_uuid(),
|
||||
'tenant_id': uuidutils.generate_uuid(),
|
||||
|
|
@ -86,6 +95,7 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
|
|||
return {'vif_port': FakeVifPort(),
|
||||
'qos_policy_id': policy_id,
|
||||
'network_qos_policy_id': None,
|
||||
'port_id': uuidutils.generate_uuid(),
|
||||
'device_owner': uuidutils.generate_uuid()}
|
||||
|
||||
def test_create_new_rule(self):
|
||||
|
|
@ -95,19 +105,22 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
|
|||
# Assert create is the last call
|
||||
self.assertEqual(
|
||||
'create_egress_bw_limit_for_port',
|
||||
self.qos_driver.br_int.method_calls[-1][0])
|
||||
self.qos_driver.br_int.method_calls[0][0])
|
||||
self.assertEqual(0, self.delete.call_count)
|
||||
self.create.assert_called_once_with(
|
||||
self.port_name, self.rule.max_kbps,
|
||||
self.rule.max_burst_kbps)
|
||||
self.port_name, self.rules[0].max_kbps,
|
||||
self.rules[0].max_burst_kbps)
|
||||
self._assert_dscp_rule_create_updated()
|
||||
|
||||
def test_create_existing_rules(self):
|
||||
self.qos_driver.create(self.port, self.qos_policy)
|
||||
self._assert_rule_create_updated()
|
||||
self._assert_bw_rule_create_updated()
|
||||
self._assert_dscp_rule_create_updated()
|
||||
|
||||
def test_update_rules(self):
|
||||
self.qos_driver.update(self.port, self.qos_policy)
|
||||
self._assert_rule_create_updated()
|
||||
self._assert_bw_rule_create_updated()
|
||||
self._assert_dscp_rule_create_updated()
|
||||
|
||||
def test_update_rules_no_vif_port(self):
|
||||
port = copy.copy(self.port)
|
||||
|
|
@ -125,12 +138,22 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase):
|
|||
self.qos_driver.delete(port, self.qos_policy)
|
||||
self.delete.assert_not_called()
|
||||
|
||||
def _assert_rule_create_updated(self):
|
||||
def _assert_bw_rule_create_updated(self):
|
||||
# Assert create is the last call
|
||||
self.assertEqual(
|
||||
'create_egress_bw_limit_for_port',
|
||||
self.qos_driver.br_int.method_calls[-1][0])
|
||||
self.qos_driver.br_int.method_calls[0][0])
|
||||
|
||||
self.create.assert_called_once_with(
|
||||
self.port_name, self.rule.max_kbps,
|
||||
self.rule.max_burst_kbps)
|
||||
self.port_name, self.rules[0].max_kbps,
|
||||
self.rules[0].max_burst_kbps)
|
||||
|
||||
def _assert_dscp_rule_create_updated(self):
|
||||
# Assert add_flow is the last call
|
||||
self.assertEqual(
|
||||
'add_flow',
|
||||
self.qos_driver.br_int.method_calls[-1][0])
|
||||
|
||||
self.qos_driver.br_int.add_flow.assert_called_once_with(
|
||||
actions='mod_nw_tos:128,load:55->NXM_NX_REG2[0..5],resubmit(,0)',
|
||||
in_port=mock.ANY, priority=65535, reg2=0, table=0)
|
||||
|
|
|
|||
Loading…
Reference in New Issue