From ba6f7bf83e6f17048a97f781aa16bf4a643a53d2 Mon Sep 17 00:00:00 2001 From: Jakub Libosvar <libosvar@redhat.com> Date: Wed, 26 Jul 2023 18:28:29 +0000 Subject: [PATCH] dvr: Avoid installing non-dvr openflow rule on startup The tunneling bridge uses different openflow rules depending if the agent is running in DVR mode or not. With DVR enabled initial rule was installed that caused traffic coming from the integration bridge to be flooded to all tunnels. After a few miliseconds this flow was replaced by a DVR specific flow, correctly dropping the traffic. This small time window caused a network loop on the compute node with restarted agent. This patch skips installing the non-dvr specific flow in case OVS agent is working in DVR mode. Hence the traffic is never flooded to the tunnels. Closes-bug: #2028795 Signed-off-by: Jakub Libosvar <libosvar@redhat.com> Change-Id: I3ce026054286c8e28ec1500f1a4aa607fe73f337 --- .../agent/openflow/native/br_tun.py | 16 ++++++--- .../agent/ovs_dvr_neutron_agent.py | 18 ++++++---- .../openvswitch/agent/ovs_neutron_agent.py | 3 +- .../tests/functional/agent/test_ovs_flows.py | 9 +++-- .../agent/openflow/native/test_br_tun.py | 33 +++++++++++++++++-- .../openvswitch/agent/test_ovs_tunnel.py | 9 +++-- 6 files changed, 69 insertions(+), 19 deletions(-) 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 921254e6fe9..3af64bd1012 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 @@ -33,13 +33,19 @@ class OVSTunnelBridge(ovs_bridge.OVSAgentBridge, 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): + def setup_default_table( + self, patch_int_ofport, arp_responder_enabled, dvr_enabled): (dp, ofp, ofpp) = self._get_dp() - # Table 0 (default) will sort incoming traffic depending on in_port - self.install_goto(dest_table_id=constants.PATCH_LV_TO_TUN, - priority=1, - in_port=patch_int_ofport) + if not dvr_enabled: + # Table 0 (default) will sort incoming traffic depending on in_port + # This table is needed only for non-dvr environment because + # OVSDVRProcessMixin overwrites this flow in its + # install_dvr_process() method. + self.install_goto(dest_table_id=constants.PATCH_LV_TO_TUN, + priority=1, + in_port=patch_int_ofport) + self.install_drop() # default drop if arp_responder_enabled: 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 7b8c7b19a3e..971ca122506 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 @@ -264,16 +264,20 @@ class OVSDVRNeutronAgent(object): if not self.enable_tunneling: return - self.tun_br.install_goto(dest_table_id=ovs_constants.DVR_PROCESS, - priority=1, - in_port=self.patch_int_ofport) + self._setup_dvr_flows_on_tun_br(self.tun_br, self.patch_int_ofport) + + @staticmethod + def _setup_dvr_flows_on_tun_br(tun_br, patch_int_ofport): + tun_br.install_goto(dest_table_id=ovs_constants.DVR_PROCESS, + priority=1, + in_port=patch_int_ofport) # table-miss should be sent to learning table - self.tun_br.install_goto(table_id=ovs_constants.DVR_NOT_LEARN, - dest_table_id=ovs_constants.LEARN_FROM_TUN) + tun_br.install_goto(table_id=ovs_constants.DVR_NOT_LEARN, + dest_table_id=ovs_constants.LEARN_FROM_TUN) - self.tun_br.install_goto(table_id=ovs_constants.DVR_PROCESS, - dest_table_id=ovs_constants.PATCH_LV_TO_TUN) + tun_br.install_goto(table_id=ovs_constants.DVR_PROCESS, + dest_table_id=ovs_constants.PATCH_LV_TO_TUN) def setup_dvr_flows_on_phys_br(self, bridge_mappings=None): '''Setup up initial dvr flows into br-phys''' 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 724a050451d..ad8603bceba 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1563,7 +1563,8 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, Add all flows to the tunnel bridge. ''' self.tun_br.setup_default_table(self.patch_int_ofport, - self.arp_responder_enabled) + self.arp_responder_enabled, + self.enable_distributed_routing) def _reconfigure_physical_bridges(self, bridges): try: diff --git a/neutron/tests/functional/agent/test_ovs_flows.py b/neutron/tests/functional/agent/test_ovs_flows.py index d11c23cd960..130ef426e93 100644 --- a/neutron/tests/functional/agent/test_ovs_flows.py +++ b/neutron/tests/functional/agent/test_ovs_flows.py @@ -24,6 +24,8 @@ from neutron.agent.common import utils from neutron.agent.linux import ip_lib from neutron.cmd.sanity import checks from neutron.common import utils as common_utils +from neutron.plugins.ml2.drivers.openvswitch.agent \ + import ovs_dvr_neutron_agent as ovsdvragt from neutron.plugins.ml2.drivers.openvswitch.agent \ import ovs_neutron_agent as ovsagt from neutron.tests.common import base as common_base @@ -311,8 +313,9 @@ class OVSFlowTestCase(OVSAgentTestBase): """ def setUp(self): + dvr_enabled = True cfg.CONF.set_override('enable_distributed_routing', - True, + dvr_enabled, group='AGENT') super(OVSFlowTestCase, self).setUp() self.phys_br = self.useFixture(net_helpers.OVSBridgeFixture()).bridge @@ -334,7 +337,9 @@ class OVSFlowTestCase(OVSAgentTestBase): prefix=cfg.CONF.OVS.tun_peer_patch_port), common_utils.get_rand_device_name( prefix=cfg.CONF.OVS.int_peer_patch_port)) - self.br_tun.setup_default_table(self.tun_p, True) + self.br_tun.setup_default_table(self.tun_p, True, dvr_enabled) + ovsdvragt.OVSDVRNeutronAgent._setup_dvr_flows_on_tun_br(self.br_tun, + self.tun_p) def test_provision_local_vlan(self): kwargs = {'port': 123, 'lvid': 888, 'segmentation_id': 777} diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_tun.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_tun.py index 0b3ab1c894c..45da220cd39 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_tun.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_tun.py @@ -52,7 +52,8 @@ class OVSTunnelBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase, patch_int_ofport = 5555 arp_responder_enabled = False self.br.setup_default_table(patch_int_ofport=patch_int_ofport, - arp_responder_enabled=arp_responder_enabled) + arp_responder_enabled=arp_responder_enabled, + dvr_enabled=False) (dp, ofp, ofpp) = self._get_dp() expected = [ call._send_msg(ofpp.OFPFlowMod(dp, @@ -160,7 +161,8 @@ class OVSTunnelBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase, patch_int_ofport = 5555 arp_responder_enabled = True self.br.setup_default_table(patch_int_ofport=patch_int_ofport, - arp_responder_enabled=arp_responder_enabled) + arp_responder_enabled=arp_responder_enabled, + dvr_enabled=False) (dp, ofp, ofpp) = self._get_dp() expected = [ call._send_msg(ofpp.OFPFlowMod(dp, @@ -280,6 +282,33 @@ class OVSTunnelBridgeTest(ovs_bridge_test_base.OVSBridgeTestBase, ] self.assertEqual(expected, self.mock.mock_calls) + def _test_setup_default_table_dvr_helper(self, dvr_enabled): + patch_int_ofport = 5555 + arp_responder_enabled = True + self.br.setup_default_table(patch_int_ofport=patch_int_ofport, + arp_responder_enabled=arp_responder_enabled, + dvr_enabled=dvr_enabled) + (dp, ofp, ofpp) = self._get_dp() + non_dvr_specific_call = call._send_msg( + ofpp.OFPFlowMod( + dp, + cookie=self.stamp, + instructions=[ofpp.OFPInstructionGotoTable(table_id=2)], + match=ofpp.OFPMatch(in_port=patch_int_ofport), + priority=1, table_id=0), + active_bundle=None) + + if dvr_enabled: + self.assertNotIn(non_dvr_specific_call, self.mock.mock_calls) + else: + self.assertIn(non_dvr_specific_call, self.mock.mock_calls) + + def test_setup_default_table_dvr_enabled(self): + self._test_setup_default_table_dvr_helper(dvr_enabled=True) + + def test_setup_default_table_dvr_disabled(self): + self._test_setup_default_table_dvr_helper(dvr_enabled=False) + def test_provision_local_vlan(self): network_type = 'vxlan' lvid = 888 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 5ee8357834f..aedd89d852c 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 @@ -188,7 +188,8 @@ class TunnelTest(object): '_check_bridge_datapath_id').start() self._define_expected_calls() - def _define_expected_calls(self, arp_responder=False, igmp_snooping=False): + def _define_expected_calls( + self, arp_responder=False, igmp_snooping=False): self.mock_int_bridge_cls_expected = [ mock.call(self.INT_BRIDGE, datapath_type=mock.ANY), @@ -268,7 +269,11 @@ class TunnelTest(object): ] self.mock_tun_bridge_expected += [ - mock.call.setup_default_table(self.INT_OFPORT, arp_responder), + # NOTE: Parameters passed to setup_default_table() method are named + # in the production code. That's why we can't use keyword parameter + # here. The last parameter passed below is dvr_enabled set to False + mock.call.setup_default_table( + self.INT_OFPORT, arp_responder, False), ] self.ipdevice_expected = []