From b81155ee27d918ba934ee7f08cb70d76715a2163 Mon Sep 17 00:00:00 2001 From: David Shaughnessy Date: Thu, 21 Nov 2019 13:25:58 +0000 Subject: [PATCH] Migrate from ofctl to native QoS DSCP Neutron migrated from using ofctl from the command line in Pike. This patch refactors DSCPs OvS backend to use native rather than ofctl and deprecates the run_ofctl function in ovs_lib.OVSBridge. Change-Id: Id9ab0eaf92b6ec6d5c9197bee60d324ffcb192a8 Related-Bug: #1853171 --- neutron/agent/common/ovs_lib.py | 4 +++ .../agent/extension_drivers/qos_driver.py | 34 ++----------------- .../agent/openflow/native/br_int.py | 19 +++++++++++ .../extension_drivers/test_qos_driver.py | 13 +++---- .../agent/openflow/native/test_br_int.py | 30 ++++++++++++++++ 5 files changed, 63 insertions(+), 37 deletions(-) diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index a55f50fb3f0..d10724b3421 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -27,6 +27,8 @@ from oslo_config import cfg from oslo_log import log as logging from oslo_utils import uuidutils from ovsdbapp.backend.ovs_idl import idlutils + +import debtcollector import six import tenacity @@ -341,6 +343,8 @@ class OVSBridge(BaseOVS): self.ovsdb.del_port(port_name, self.br_name).execute() def run_ofctl(self, cmd, args, process_input=None): + debtcollector.deprecate("Use of run_ofctl is " + "deprecated", removal_version='V') full_args = ["ovs-ofctl", cmd, "-O", self._highest_protocol_needed, self.br_name] + args diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py b/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py index 7c586d95d82..df8daa29ced 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/extension_drivers/qos_driver.py @@ -117,37 +117,9 @@ class QosOVSAgentDriver(qos.QosLinuxAgentDriver): "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 - # lower 2 bits of type of service packet header. - # source: man ovs-ofctl (/mod_nw_tos) - mark = str(mark << 2) - - # reg2 is a metadata field that does not alter packets. - # By loading a value into this field and checking if the value is - # altered it allows the packet to be resubmitted and go through - # the flow table again to be identified by other flows. - flows = self.br_int.dump_flows_for(cookie=self.cookie, table=0, - in_port=port, reg2=0) - if not flows: - actions = ("mod_nw_tos:" + mark + ",load:55->NXM_NX_REG2[0..5]," + - "resubmit(,0)") - self.br_int.add_flow(in_port=port, table=0, priority=65535, - reg2=0, actions=actions) - else: - for flow in flows: - actions = str(flow).partition("actions=")[2] - acts = actions.split(',') - # mod_nw_tos = modify type of service header - # This is the second byte of the IPv4 packet header. - # DSCP makes up the upper 6 bits of this header field. - actions = "mod_nw_tos:" + mark + "," - actions += ','.join([act for act in acts - if "mod_nw_tos:" not in act]) - self.br_int.mod_flow(reg2=0, in_port=port, table=0, - actions=actions) + port = self.br_int.get_port_ofport(vif_port.port_name) + self.br_int.install_dscp_marking_rule(port=port, + dscp_mark=rule.dscp_mark) def delete_dscp_marking(self, port): vif_port = port.get('vif_port') diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py index 44964e4684f..2bac687636e 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_int.py @@ -332,3 +332,22 @@ class OVSIntegrationBridge(ovs_bridge.OVSAgentBridge): def delete_arp_spoofing_allow_rules(self, port): self.uninstall_flows(table_id=constants.ARP_SPOOF_TABLE, in_port=port) + + def install_dscp_marking_rule(self, port, dscp_mark): + # reg2 is a metadata field that does not alter packets. + # By loading a value into this field and checking if the value is + # altered it allows the packet to be resubmitted and go through + # the flow table again to be identified by other flows. + (dp, ofp, ofpp) = self._get_dp() + actions = [ofpp.OFPActionSetField(reg2=1), + ofpp.OFPActionSetField(ip_dscp=dscp_mark), + ofpp.NXActionResubmit(in_port=port)] + instructions = [ + ofpp.OFPInstructionActions(ofp.OFPIT_APPLY_ACTIONS, actions), + ] + self.install_instructions(instructions, table_id=0, + priority=65535, in_port=port, reg2=0, + eth_type=0x0800) + self.install_instructions(instructions, table_id=0, + priority=65535, in_port=port, reg2=0, + eth_type=0x86DD) diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py index ac94133d8f8..513c6b8569b 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/extension_drivers/test_qos_driver.py @@ -52,11 +52,13 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): self.qos_driver.consume_api(self.agent_api) self.qos_driver.initialize() self.qos_driver.br_int = mock.Mock() + self.qos_driver.br_int.get_dp = mock.Mock(return_value=(mock.Mock(), + mock.Mock(), + mock.Mock())) self.qos_driver.br_int.get_egress_bw_limit_for_port = mock.Mock( return_value=(1000, 10)) self.get_egress = self.qos_driver.br_int.get_egress_bw_limit_for_port self.get_ingress = self.qos_driver.br_int.get_ingress_bw_limit_for_port - self.qos_driver.br_int.dump_flows_for = mock.Mock(return_value=None) self.qos_driver.br_int.del_egress_bw_limit_for_port = mock.Mock() self.delete_egress = ( self.qos_driver.br_int.delete_egress_bw_limit_for_port) @@ -188,14 +190,13 @@ class QosOVSAgentDriverTestCase(ovs_test_base.OVSAgentConfigTestBase): self.rules[1].max_burst_kbps) def _assert_dscp_rule_create_updated(self): - # Assert add_flow is the last call + # Assert install_instructions is the last call self.assertEqual( - 'add_flow', + 'install_dscp_marking_rule', 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) + self.qos_driver.br_int.install_dscp_marking_rule.\ + assert_called_once_with(dscp_mark=mock.ANY, port=mock.ANY) def test_create_minimum_bandwidth(self): with mock.patch.object(self.qos_driver, 'update_minimum_bandwidth') \ diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py index bb97748dd3d..837b69812b2 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_int.py @@ -506,3 +506,33 @@ class OVSIntegrationBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase): def test_delete_dvr_dst_mac_for_arp_tunnel(self): self._test_delete_dvr_dst_mac_for_arp(network_type='vxlan') + + def test_install_dscp_marking_rule(self): + test_port = 8888 + test_mark = 38 + self.br.install_dscp_marking_rule(port=test_port, dscp_mark=test_mark) + + (dp, ofp, ofpp) = self._get_dp() + expected = [ + call._send_msg(ofpp.OFPFlowMod(dp, cookie=self.br.default_cookie, + instructions=[ofpp.OFPInstructionActions( + ofp.OFPIT_APPLY_ACTIONS, + [ofpp.OFPActionSetField(reg2=1), + ofpp.OFPActionSetField(ip_dscp=38), + ofpp.NXActionResubmit(in_port=8888)])], + match=ofpp.OFPMatch(eth_type=0x0800, + in_port=8888, reg2=0), + priority=65535, table_id=0), + active_bundle=None), + call._send_msg(ofpp.OFPFlowMod(dp, cookie=self.br.default_cookie, + instructions=[ofpp.OFPInstructionActions( + ofp.OFPIT_APPLY_ACTIONS, + [ofpp.OFPActionSetField(reg2=1), + ofpp.OFPActionSetField(ip_dscp=38), + ofpp.NXActionResubmit(in_port=8888)])], + match=ofpp.OFPMatch(eth_type=0x86DD, + in_port=8888, reg2=0), + priority=65535, table_id=0), + active_bundle=None) + ] + self.assertEqual(expected, self.mock.mock_calls)