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

Conflicts:
	neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py

Signed-off-by: Jakub Libosvar <[email protected]>
Change-Id: I3ce026054286c8e28ec1500f1a4aa607fe73f337
(cherry picked from commit ba6f7bf)

Author: cubeek

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:

neutron/plugins/ml2/drivers/openvswitch/agent/ovs_dvr_neutron_agent.py

@@ -263,16 +263,20 @@ def setup_dvr_flows_on_tun_br(self):
         if not self.enable_tunneling:
             return
 
-        self.tun_br.install_goto(dest_table_id=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=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=constants.DVR_NOT_LEARN,
-                                 dest_table_id=constants.LEARN_FROM_TUN)
+        tun_br.install_goto(table_id=constants.DVR_NOT_LEARN,
+                            dest_table_id=constants.LEARN_FROM_TUN)
 
-        self.tun_br.install_goto(table_id=constants.DVR_PROCESS,
-                                 dest_table_id=constants.PATCH_LV_TO_TUN)
+        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, bridge_mappings=None):
         '''Setup up initial dvr flows into br-phys'''

neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py

@@ -1475,7 +1475,8 @@ def setup_tunnel_br_flows(self):
         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:

neutron/tests/functional/agent/test_ovs_flows.py

@@ -24,6 +24,8 @@
 from neutron.cmd.sanity import checks
 from neutron.common import utils as common_utils
 from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants
+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
@@ -299,8 +301,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
@@ -322,7 +325,9 @@ def setUp(self):
                 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}

neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_br_tun.py

@@ -52,7 +52,8 @@ def test_setup_default_table(self):
         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 @@ def test_setup_default_table_arp_responder_enabled(self):
         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 @@ def test_setup_default_table_arp_responder_enabled(self):
         ]
         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

neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_ovs_tunnel.py

@@ -188,7 +188,8 @@ def lookup_br(br_name, *args, **kwargs):
             '_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 @@ def _define_expected_calls(self, arp_responder=False, igmp_snooping=False):
         ]
 
         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 = []