From ea3d844c75541cc2be17865bad6336cd1b8385c4 Mon Sep 17 00:00:00 2001 From: LIU Yulong Date: Wed, 20 Feb 2019 19:46:53 +0800 Subject: [PATCH] Divide-and-conquer local bridge flows beasts The dump-flows action will get a very large sets of flow information if there are enormous ports or openflow security group rules. For now we can meet some known exception during such action, for instance, memory issue, timeout issue. So after this patch, the cleanup action of the bridge stale flows will be done one table by one table. But note, this only supports for 'native' OpenFlow interface driver. Related-Bug: #1813703 Related-Bug: #1813712 Related-Bug: #1813709 Related-Bug: #1813708 Change-Id: Ie06d1bebe83ffeaf7130dcbb8ca21e5e59a220fb (cherry picked from commit f898ffd71fba4f9b8fd9f4cb851fc3976d72396a) --- .../openvswitch/agent/common/constants.py | 33 +++++++++++++++++++ .../agent/openflow/native/br_int.py | 2 ++ .../agent/openflow/native/br_phys.py | 1 + .../agent/openflow/native/br_tun.py | 1 + .../agent/openflow/native/ofswitch.py | 18 ++++++---- .../openvswitch/agent/ovs_neutron_agent.py | 14 ++++---- .../agent/test_ovs_neutron_agent.py | 11 +++++-- .../openvswitch/agent/test_ovs_tunnel.py | 6 ++-- 8 files changed, 69 insertions(+), 17 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py index 8322c0fddb0..520cded3e67 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py @@ -69,6 +69,18 @@ OVS_FIREWALL_TABLES = ( ) ## Tunnel bridge (tun_br) +INT_BR_ALL_TABLES = ( + LOCAL_SWITCHING, + DVR_TO_SRC_MAC, + DVR_TO_SRC_MAC_VLAN, + CANARY_TABLE, + ARP_SPOOF_TABLE, + MAC_SPOOF_TABLE, + BASE_EGRESS_TABLE, + RULES_EGRESS_TABLE, + ACCEPT_OR_INGRESS_TABLE, + BASE_INGRESS_TABLE, + RULES_INGRESS_TABLE) # Various tables for tunneling flows DVR_PROCESS = 1 @@ -83,14 +95,35 @@ UCAST_TO_TUN = 20 ARP_RESPONDER = 21 FLOOD_TO_TUN = 22 + ## Physical Bridges (phys_brs) +TUN_BR_ALL_TABLES = ( + LOCAL_SWITCHING, + DVR_PROCESS, + PATCH_LV_TO_TUN, + GRE_TUN_TO_LV, + VXLAN_TUN_TO_LV, + GENEVE_TUN_TO_LV, + DVR_NOT_LEARN, + LEARN_FROM_TUN, + UCAST_TO_TUN, + ARP_RESPONDER, + FLOOD_TO_TUN) + # Various tables for DVR use of physical bridge flows DVR_PROCESS_VLAN = 1 LOCAL_VLAN_TRANSLATION = 2 DVR_NOT_LEARN_VLAN = 3 + ### end of OpenFlow table IDs +PHY_BR_ALL_TABLES = ( + LOCAL_SWITCHING, + DVR_PROCESS_VLAN, + LOCAL_VLAN_TRANSLATION, + DVR_NOT_LEARN_VLAN) + # type for ARP reply in ARP header ARP_REPLY = '0x2' 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 ccaeb23c59c..25a1fd1871f 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 @@ -39,6 +39,8 @@ LOG = logging.getLogger(__name__) class OVSIntegrationBridge(ovs_bridge.OVSAgentBridge): """openvswitch agent br-int specific logic.""" + of_tables = constants.INT_BR_ALL_TABLES + def setup_default_table(self): self.setup_canary_table() self.install_normal() diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_phys.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_phys.py index 4bcbd2c9756..bad8b1d771a 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_phys.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_phys.py @@ -28,6 +28,7 @@ class OVSPhysicalBridge(ovs_bridge.OVSAgentBridge, # Used by OVSDVRProcessMixin dvr_process_table_id = constants.DVR_PROCESS_VLAN dvr_process_next_table_id = constants.LOCAL_VLAN_TRANSLATION + of_tables = constants.PHY_BR_ALL_TABLES def setup_default_table(self): self.install_normal() diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_tun.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_tun.py index c4172c149cf..7b655c06bb7 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_tun.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/br_tun.py @@ -46,6 +46,7 @@ class OVSTunnelBridge(ovs_bridge.OVSAgentBridge, # Used by OVSDVRProcessMixin dvr_process_table_id = constants.DVR_PROCESS dvr_process_next_table_id = constants.PATCH_LV_TO_TUN + of_tables = constants.TUN_BR_ALL_TABLES def setup_default_table(self, patch_int_ofport, arp_responder_enabled): (dp, ofp, ofpp) = self._get_dp() diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ofswitch.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ofswitch.py index cc36b54b41b..85833e3654b 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ofswitch.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ofswitch.py @@ -23,7 +23,7 @@ from oslo_utils import timeutils import ryu.app.ofctl.api as ofctl_api import ryu.exception as ryu_exc -from neutron._i18n import _, _LW +from neutron._i18n import _, _LW, _LI LOG = logging.getLogger(__name__) @@ -134,11 +134,9 @@ class OpenFlowSwitchMixin(object): flows += rep.body return flows - def cleanup_flows(self): - cookies = set([f.cookie for f in self.dump_flows()]) - \ - self.reserved_cookies - LOG.debug("Reserved cookies for %s: %s", self.br_name, - self.reserved_cookies) + def _dump_and_clean(self, table_id=None): + cookies = set([f.cookie for f in self.dump_flows(table_id)]) - \ + self.reserved_cookies for c in cookies: LOG.warning(_LW("Deleting flow with cookie 0x%(cookie)x"), {'cookie': c}) @@ -147,6 +145,14 @@ class OpenFlowSwitchMixin(object): def install_goto_next(self, table_id): self.install_goto(table_id=table_id, dest_table_id=table_id + 1) + def cleanup_flows(self): + LOG.info(_LI("Reserved cookies for %(br_name)s: %(cookie)s"), + {'br_name': self.br_name, + 'cookie': self.reserved_cookies}) + + for table_id in self.of_tables: + self._dump_and_clean(table_id) + def install_output(self, port, table_id=0, priority=0, match=None, **match_kwargs): (_dp, ofp, ofpp) = self._get_dp() diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index a72f46802a5..49475eb3383 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1813,13 +1813,15 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, return port_stats def cleanup_stale_flows(self): - bridges = [self.int_br] - bridges.extend(self.phys_brs.values()) + LOG.info(_LI("Cleaning stale %s flows"), self.int_br.br_name) + self.int_br.cleanup_flows() + for pby_br in self.phys_brs.values(): + LOG.info(_LI("Cleaning stale %s flows"), pby_br.br_name) + pby_br.cleanup_flows() + if self.enable_tunneling: - bridges.append(self.tun_br) - for bridge in bridges: - LOG.info(_LI("Cleaning stale %s flows"), bridge.br_name) - bridge.cleanup_flows() + LOG.info(_LI("Cleaning stale %s flows"), self.tun_br.br_name) + self.tun_br.cleanup_flows() def process_port_info(self, start, polling_manager, sync, ovs_restarted, ports, ancillary_ports, updated_ports_copy, diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py index 12f7a5c752a..38301eec125 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_neutron_agent.py @@ -2144,21 +2144,28 @@ class TestOvsNeutronAgentRyu(TestOvsNeutronAgent, mock.patch.object(self.agent.int_br, 'uninstall_flows') as uninstall_flows: self.agent.int_br.set_agent_uuid_stamp(1234) - dump_flows.return_value = [ + fake_flows = [ # mock ryu.ofproto.ofproto_v1_3_parser.OFPFlowStats mock.Mock(cookie=1234, table_id=0), mock.Mock(cookie=17185, table_id=2), mock.Mock(cookie=9029, table_id=2), mock.Mock(cookie=1234, table_id=3), ] + dump_flows.return_value = fake_flows self.agent.iter_num = 3 self.agent.cleanup_stale_flows() + + dump_flows_expected = [ + mock.call(tid) for tid in constants.INT_BR_ALL_TABLES] + dump_flows.assert_has_calls(dump_flows_expected) + expected = [mock.call(cookie=17185, cookie_mask=uint64_max), mock.call(cookie=9029, cookie_mask=uint64_max)] uninstall_flows.assert_has_calls(expected, any_order=True) - self.assertEqual(len(expected), len(uninstall_flows.mock_calls)) + self.assertEqual(len(constants.INT_BR_ALL_TABLES) * len(expected), + len(uninstall_flows.mock_calls)) class AncillaryBridgesTest(object): diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py index 8a94785ec72..ae0e4421af5 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py @@ -565,10 +565,10 @@ class TunnelTest(object): mock.call.cleanup_flows(), mock.call.check_canary_table() ] - self.mock_tun_bridge_expected += [ - mock.call.cleanup_flows() - ] self.mock_map_tun_bridge_expected += [ + mock.call.cleanup_flows(), + ] + self.mock_tun_bridge_expected += [ mock.call.cleanup_flows() ] # No cleanup is expected on ancillary bridge