From af67d516a5b39b883fa6fb2fca4673fb7602b292 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 | 36 +++++++++++++++++++ .../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 | 15 +++++--- .../openvswitch/agent/ovs_neutron_agent.py | 14 ++++---- .../agent/test_ovs_neutron_agent.py | 11 ++++-- .../openvswitch/agent/test_ovs_tunnel.py | 6 ++-- 8 files changed, 70 insertions(+), 16 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py index f7b83757d3c..3e3ceb25c7f 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/common/constants.py @@ -78,6 +78,23 @@ DROPPED_TRAFFIC_TABLE = 93 ACCEPTED_EGRESS_TRAFFIC_NORMAL_TABLE = 94 +INT_BR_ALL_TABLES = ( + LOCAL_SWITCHING, + DVR_TO_SRC_MAC, + DVR_TO_SRC_MAC_VLAN, + CANARY_TABLE, + ARP_SPOOF_TABLE, + MAC_SPOOF_TABLE, + TRANSIENT_TABLE, + BASE_EGRESS_TABLE, + RULES_EGRESS_TABLE, + ACCEPT_OR_INGRESS_TABLE, + BASE_INGRESS_TABLE, + RULES_INGRESS_TABLE, + ACCEPTED_EGRESS_TRAFFIC_TABLE, + ACCEPTED_INGRESS_TRAFFIC_TABLE, + DROPPED_TRAFFIC_TABLE) + # --- Tunnel bridge (tun_br) # Various tables for tunneling flows @@ -93,6 +110,19 @@ UCAST_TO_TUN = 20 ARP_RESPONDER = 21 FLOOD_TO_TUN = 22 +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) + # --- Physical Bridges (phys_brs) # Various tables for DVR use of physical bridge flows @@ -100,6 +130,12 @@ DVR_PROCESS_VLAN = 1 LOCAL_VLAN_TRANSLATION = 2 DVR_NOT_LEARN_VLAN = 3 +PHY_BR_ALL_TABLES = ( + LOCAL_SWITCHING, + DVR_PROCESS_VLAN, + LOCAL_VLAN_TRANSLATION, + DVR_NOT_LEARN_VLAN) + # --- end of OpenFlow table IDs # type for ARP reply in ARP header 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 34a1fd1bc60..b6a8535481e 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 @@ -38,6 +38,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_goto(dest_table_id=constants.TRANSIENT_TABLE) 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 cdb539f7701..6d9bbfee126 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ofswitch.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/native/ofswitch.py @@ -165,16 +165,21 @@ 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("Deleting flow with cookie 0x%(cookie)x", {'cookie': c}) self.uninstall_flows(cookie=c, cookie_mask=ovs_lib.UINT64_BITMASK) + def cleanup_flows(self): + LOG.info("Reserved cookies for %s: %s", self.br_name, + self.reserved_cookies) + + for table_id in self.of_tables: + self._dump_and_clean(table_id) + def install_goto_next(self, table_id, active_bundle=None): self.install_goto(table_id=table_id, dest_table_id=table_id + 1, active_bundle=active_bundle) 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 14806ac545f..2ea06ecec65 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1917,13 +1917,15 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, return port_stats def cleanup_stale_flows(self): - bridges = [self.int_br] - bridges.extend(self.phys_brs.values()) + LOG.info("Cleaning stale %s flows", self.int_br.br_name) + self.int_br.cleanup_flows() + for pby_br in self.phys_brs.values(): + LOG.info("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("Cleaning stale %s flows", bridge.br_name) - bridge.cleanup_flows() + LOG.info("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 62f567538ff..4ea2dd1023a 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 @@ -2372,21 +2372,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 f0bd31860d7..b7cb46541ad 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 @@ -573,10 +573,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