From 47ec363f5faefd85dfa33223c0087f4444afb5b9 Mon Sep 17 00:00:00 2001 From: Darragh O'Reilly Date: Mon, 13 Jul 2020 14:48:10 +0000 Subject: [PATCH] Ensure drop flows on br-int at agent startup for DVR too Commit 90212b12 changed the OVS agent so adding vital drop flows on br-int (table 0 priority 2) for packets from physical bridges was deferred until DVR initialization later on. But if br-int has no flows from a previous run (eg after host reboot), then these packets will hit the NORMAL flow in table 60. And if there is more than one physical bridge, then the physical interfaces from the different bridges are now essentially connected at layer 2 and a network loop is possible in the time before the flows are added by DVR. Also the DVR code won't add them until after RPC calls to the server, so a loop is more likely if the server is not available. This patch restores adding these flows to when the physical bridges are first configured. Also updated a comment that was no longer correct and updated the unit test. Change-Id: I42c33fefaae6a7bee134779c840f35632823472e Closes-Bug: #1887148 Related-Bug: #1869808 (cherry picked from commit c1a77ef8b74bb9b5abbc5cb03fb3201383122eb8) (cherry picked from commit 143fe8ff89ba776618ed6291af9d5e28e4662bdb) (cherry picked from commit 6a861b8c8c28e5675ec2208057298b811ba2b649) (cherry picked from commit 8181c5dbfe799ac6c832ab67b7eab3bcef4098b9) Conflicts: neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py --- .../openvswitch/agent/ovs_neutron_agent.py | 14 ++++++-------- .../openvswitch/agent/test_ovs_neutron_agent.py | 16 +++++++++++++--- 2 files changed, 19 insertions(+), 11 deletions(-) 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 04d4dc76931..e1d43f52449 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1326,15 +1326,13 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, self.int_ofports[physical_network] = int_ofport self.phys_ofports[physical_network] = phys_ofport - # following drop operations are not necessary for - # dvr agent setup_dvr_flows. So skip it if dvr enabled - # the reason is for br_int it is duplicate - # for br_physical drop_port is dangerous because when dvr - # enabled the highest flow on table=0 is 2 which means - # basically everything will be dropped until setup_dvr_flows - # got executed. + # Drop packets from physical bridges that have not matched a higher + # priority flow to set a local vlan. This prevents these stray + # packets from being forwarded to other physical bridges which + # could cause a network loop in the physical network. + self.int_br.drop_port(in_port=int_ofport) + if not self.enable_distributed_routing: - self.int_br.drop_port(in_port=int_ofport) br.drop_port(in_port=phys_ofport) if self.use_veth_interconnection: 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 b00b5d46261..fe53e43a87b 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 @@ -1361,7 +1361,9 @@ class TestOvsNeutronAgent(object): self.assertIn('added_port_id', port_info['added']) self.assertNotIn('activated_port_id', port_info['added']) - def _test_setup_physical_bridges(self, port_exists=False): + def _test_setup_physical_bridges(self, port_exists=False, + dvr_enabled=False): + self.agent.enable_distributed_routing = dvr_enabled with mock.patch.object(ip_lib.IPDevice, "exists") as devex_fn,\ mock.patch.object(sys, "exit"),\ mock.patch.object(self.agent, 'br_phys_cls') as phys_br_cls,\ @@ -1418,8 +1420,13 @@ class TestOvsNeutronAgent(object): 'phy-br-eth', constants.NONEXISTENT_PEER), ] expected_calls += [ - mock.call.int_br.drop_port(in_port='int_ofport'), - mock.call.phys_br.drop_port(in_port='phy_ofport'), + mock.call.int_br.drop_port(in_port='int_ofport') + ] + if not dvr_enabled: + expected_calls += [ + mock.call.phys_br.drop_port(in_port='phy_ofport') + ] + expected_calls += [ mock.call.int_br.set_db_attribute('Interface', 'int-br-eth', 'options', {'peer': 'phy-br-eth'}), @@ -1439,6 +1446,9 @@ class TestOvsNeutronAgent(object): def test_setup_physical_bridges_port_exists(self): self._test_setup_physical_bridges(port_exists=True) + def test_setup_physical_bridges_dvr_enabled(self): + self._test_setup_physical_bridges(dvr_enabled=True) + def test_setup_physical_bridges_using_veth_interconnection(self): self.agent.use_veth_interconnection = True with mock.patch.object(ip_lib.IPDevice, "exists") as devex_fn,\