From b03f9d4c433dbe32d1d18e2b2e6b19cd1f1905b2 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Tue, 21 Apr 2020 10:30:52 +0200 Subject: [PATCH] [DVR] Reconfigure re-created physical bridges for dvr routers In case when physical bridge is removed and created again it is initialized by neutron-ovs-agent. But if agent has enabled distributed routing, dvr related flows wasn't configured again and that lead to connectivity issues in case of DVR routers. This patch fixes it by adding configuration of dvr related flows if distributed routing is enabled in agent's configuration. It also adds reset list of phys_brs in dvr_agent. Without that there were different objects used in ovs agent and dvr_agent classes thus e.g. 2 various cookie ids were set on flows in physical bridge. This was also the same issue in case when openvswitch was restarted and all bridges were reconfigured. Now in such case there is correctly new cookie_id configured for all flows. Change-Id: I710f00f0f542bcf7fa2fc60800797b90f9f77e14 Closes-Bug: #1864822 (cherry picked from commit 91f0bf3c8511bf3b0cc63746f767d8d4dce601bd) --- .../agent/ovs_dvr_neutron_agent.py | 26 +++++++--- .../openvswitch/agent/ovs_neutron_agent.py | 14 +++--- .../agent/test_ovs_neutron_agent.py | 50 +++++++++++++++++++ 3 files changed, 76 insertions(+), 14 deletions(-) diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py index 357dab158f0..2a33ec3db1c 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py @@ -128,10 +128,9 @@ class OVSDVRNeutronAgent(object): self.enable_tunneling = enable_tunneling self.enable_distributed_routing = enable_distributed_routing self.bridge_mappings = bridge_mappings - self.phys_brs = phys_brs self.int_ofports = int_ofports self.phys_ofports = phys_ofports - self.reset_ovs_parameters(integ_br, tun_br, + self.reset_ovs_parameters(integ_br, tun_br, phys_brs, patch_int_ofport, patch_tun_ofport) self.reset_dvr_parameters() self.dvr_mac_address = None @@ -143,17 +142,19 @@ class OVSDVRNeutronAgent(object): def set_firewall(self, firewall=None): self.firewall = firewall - def setup_dvr_flows(self): + def setup_dvr_flows(self, bridge_mappings=None): + bridge_mappings = bridge_mappings or self.bridge_mappings self.setup_dvr_flows_on_integ_br() self.setup_dvr_flows_on_tun_br() - self.setup_dvr_flows_on_phys_br() + self.setup_dvr_flows_on_phys_br(bridge_mappings) self.setup_dvr_mac_flows_on_all_brs() - def reset_ovs_parameters(self, integ_br, tun_br, + def reset_ovs_parameters(self, integ_br, tun_br, phys_brs, patch_int_ofport, patch_tun_ofport): '''Reset the openvswitch parameters''' self.int_br = integ_br self.tun_br = tun_br + self.phys_brs = phys_brs self.patch_int_ofport = patch_int_ofport self.patch_tun_ofport = patch_tun_ofport @@ -164,6 +165,15 @@ class OVSDVRNeutronAgent(object): self.local_ports = {} self.registered_dvr_macs = set() + def reset_dvr_flows(self, integ_br, tun_br, phys_brs, + patch_int_ofport, patch_tun_ofport, + bridge_mappings=None): + '''Reset the openvswitch and DVR parameters and DVR flows''' + self.reset_ovs_parameters( + integ_br, tun_br, phys_brs, patch_int_ofport, patch_tun_ofport) + self.reset_dvr_parameters() + self.setup_dvr_flows(bridge_mappings) + def get_dvr_mac_address(self): try: self.get_dvr_mac_address_with_retry() @@ -239,10 +249,10 @@ class OVSDVRNeutronAgent(object): self.tun_br.install_goto(table_id=constants.DVR_PROCESS, dest_table_id=constants.PATCH_LV_TO_TUN) - def setup_dvr_flows_on_phys_br(self): + def setup_dvr_flows_on_phys_br(self, bridge_mappings=None): '''Setup up initial dvr flows into br-phys''' - - for physical_network in self.bridge_mappings: + bridge_mappings = bridge_mappings or self.bridge_mappings + for physical_network in bridge_mappings: self.phys_brs[physical_network].install_goto( in_port=self.phys_ofports[physical_network], priority=2, 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 b743c5adcd4..1cd5392b6d9 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1449,6 +1449,11 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, if bridge_mappings: sync = True self.setup_physical_bridges(bridge_mappings) + if self.enable_distributed_routing: + self.dvr_agent.reset_dvr_flows( + self.int_br, self.tun_br, self.phys_brs, + self.patch_int_ofport, self.patch_tun_ofport, + bridge_mappings) return sync def _check_bridge_datapath_id(self, bridge, datapath_ids_set): @@ -2493,12 +2498,9 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, # with l2pop fdb entries update self._report_state() if self.enable_distributed_routing: - self.dvr_agent.reset_ovs_parameters(self.int_br, - self.tun_br, - self.patch_int_ofport, - self.patch_tun_ofport) - self.dvr_agent.reset_dvr_parameters() - self.dvr_agent.setup_dvr_flows() + self.dvr_agent.reset_dvr_flows( + self.int_br, self.tun_br, self.phys_brs, + self.patch_int_ofport, self.patch_tun_ofport) # notify that OVS has restarted registry.publish( callback_resources.AGENT, 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 efb64edd77c..f1c67bcd516 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 @@ -3953,6 +3953,56 @@ class TestOvsDvrNeutronAgent(object): self.agent.ancillary_brs = mock.Mock() self._test_scan_ports_failure('scan_ancillary_ports') + def test_ext_br_recreated(self): + self._setup_for_dvr_test() + reset_methods = ( + 'reset_ovs_parameters', 'reset_dvr_parameters', + 'setup_dvr_flows_on_integ_br', 'setup_dvr_flows_on_tun_br', + 'setup_dvr_flows_on_phys_br', 'setup_dvr_mac_flows_on_all_brs') + for method in reset_methods: + mock.patch.object(self.agent.dvr_agent, method).start() + bridge_mappings = {'physnet0': 'br-ex0', + 'physnet1': 'br-ex1'} + ex_br_mocks = [mock.Mock(br_name='br-ex0'), + mock.Mock(br_name='br-ex1')] + phys_bridges = {'physnet0': ex_br_mocks[0], + 'physnet1': ex_br_mocks[1]}, + bridges_added = ['br-ex0'] + with mock.patch.object(self.agent, 'check_ovs_status', + return_value=constants.OVS_NORMAL), \ + mock.patch.object(self.agent, '_agent_has_updates', + side_effect=TypeError('loop exit')), \ + mock.patch.dict(self.agent.bridge_mappings, bridge_mappings, + clear=True), \ + mock.patch.dict(self.agent.phys_brs, phys_bridges, + clear=True), \ + mock.patch.object(self.agent, 'setup_physical_bridges') as \ + setup_physical_bridges, \ + mock.patch.object(self.agent.ovs.ovsdb, 'idl_monitor') as \ + mock_idl_monitor: + mock_idl_monitor.bridges_added = bridges_added + try: + self.agent.rpc_loop(polling_manager=mock.Mock()) + except TypeError: + pass + # Setup bridges should be called once even if it will raise Runtime + # Error because TypeError is raised in _agent_has_updates to stop + # agent after first loop iteration + setup_physical_bridges.assert_called_once_with({'physnet0': 'br-ex0'}) + # Ensure dvr_agent methods were called correctly + self.agent.dvr_agent.reset_ovs_parameters.assert_called_once_with( + self.agent.int_br, self.agent.tun_br, self.agent.phys_brs, + self.agent.patch_int_ofport, self.agent.patch_tun_ofport) + self.agent.dvr_agent.reset_dvr_parameters.assert_called_once_with() + (self.agent.dvr_agent.setup_dvr_flows_on_phys_br. + assert_called_once_with({'physnet0': 'br-ex0'})) + (self.agent.dvr_agent.setup_dvr_flows_on_integ_br. + assert_called_once_with()) + (self.agent.dvr_agent.setup_dvr_flows_on_tun_br. + assert_called_once_with()) + (self.agent.dvr_agent.setup_dvr_mac_flows_on_all_brs. + assert_called_once_with()) + class TestOvsDvrNeutronAgentOSKen(TestOvsDvrNeutronAgent, ovs_test_base.OVSOSKenTestBase):