Add flows to tunnel bridge with proper cookie.

Without that fix flows applied to br-tun through
DeferredOVSBridge are created without cookie.
That results in l2pop flows being deleted in the process of
cleanup of stale flows.

Solution is to add cookie to all add/mod-flows of OVSBrigde
class in the method do_action_flows.
Also, agent_uuid_stamp moved to a proper place - into the
base OVSBridge class as storing attributes in Mixing was
just a wrong code design.

Change-Id: Ic09a0dbc04fc5da38d30e1392cf2ea27d576040c
Closes-Bug: #1489372

diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py
index fc0927543e19f8a86ea56acfab663e9efd8b6778..930558212d0dcd22c755a37846ecd26d32a5827c 100644 (file)
--- a/neutron/agent/common/ovs_lib.py
+++ b/neutron/agent/common/ovs_lib.py
@@ -152,6 +152,10 @@ class OVSBridge(BaseOVS):
         super(OVSBridge, self).__init__()
         self.br_name = br_name
         self.datapath_type = datapath_type
+        self.agent_uuid_stamp = '0x0'
+
+    def set_agent_uuid_stamp(self, val):
+        self.agent_uuid_stamp = val
 
     def set_controller(self, controllers):
         self.ovsdb.set_controller(self.br_name,
@@ -260,6 +264,10 @@ class OVSBridge(BaseOVS):
                                self.br_name, 'datapath_id')
 
     def do_action_flows(self, action, kwargs_list):
+        if action != 'del':
+            for kw in kwargs_list:
+                if 'cookie' not in kw:
+                    kw['cookie'] = self.agent_uuid_stamp
         flow_strs = [_build_flow_expr_str(kw, action) for kw in kwargs_list]
         self.run_ofctl('%s-flows' % action, ['-'], '\n'.join(flow_strs))
 

diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/ofswitch.py b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/ofswitch.py
index e0d5154c39fd5a98168659a85af0731cab8eb50a..c5460db7f8b2cf8eb132bf0d1e806842e3ca00be 100644 (file)
--- a/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/ofswitch.py
+++ b/neutron/plugins/ml2/drivers/openvswitch/agent/openflow/ovs_ofctl/ofswitch.py
@@ -34,10 +34,6 @@ _keywords = {
 
 class OpenFlowSwitchMixin(object):
     """Mixin to provide common convenient routines for an openflow switch."""
-    agent_uuid_stamp = '0x0'
-
-    def set_agent_uuid_stamp(self, val):
-        self.agent_uuid_stamp = val
 
     @staticmethod
     def _conv_args(kwargs):
@@ -88,14 +84,6 @@ class OpenFlowSwitchMixin(object):
         else:
             super(OpenFlowSwitchMixin, self).remove_all_flows()
 
-    def add_flow(self, **kwargs):
-        kwargs['cookie'] = self.agent_uuid_stamp
-        super(OpenFlowSwitchMixin, self).add_flow(**self._conv_args(kwargs))
-
-    def mod_flow(self, **kwargs):
-        kwargs['cookie'] = self.agent_uuid_stamp
-        super(OpenFlowSwitchMixin, self).mod_flow(**self._conv_args(kwargs))
-
     def _filter_flows(self, flows):
         LOG.debug("Agent uuid stamp used to filter flows: %s",
                   self.agent_uuid_stamp)

diff --git a/neutron/tests/unit/agent/common/test_ovs_lib.py b/neutron/tests/unit/agent/common/test_ovs_lib.py
index b6ab9dd2bbce0f2f1cbf4142e251fbb73a2847e7..a599dc6bb2d7de385781e42e1c92d2dc9af6a549 100644 (file)
--- a/neutron/tests/unit/agent/common/test_ovs_lib.py
+++ b/neutron/tests/unit/agent/common/test_ovs_lib.py
@@ -852,3 +852,19 @@ class TestDeferredOVSBridge(base.BaseTestCase):
     def test_getattr_unallowed_attr_failure(self):
         with ovs_lib.DeferredOVSBridge(self.br) as deferred_br:
             self.assertRaises(AttributeError, getattr, deferred_br, 'failure')
+
+    def test_cookie_passed_to_addmod(self):
+        self.br = ovs_lib.OVSBridge("br-tun")
+        self.br.set_agent_uuid_stamp(1234)
+        expected_calls = [
+            mock.call('add-flows', ['-'],
+                      'hard_timeout=0,idle_timeout=0,priority=1,'
+                      'cookie=1234,actions=drop'),
+            mock.call('mod-flows', ['-'],
+                      'cookie=1234,actions=drop')
+        ]
+        with mock.patch.object(self.br, 'run_ofctl') as f:
+            with ovs_lib.DeferredOVSBridge(self.br) as deferred_br:
+                deferred_br.add_flow(actions='drop')
+                deferred_br.mod_flow(actions='drop')
+            f.assert_has_calls(expected_calls)