ofagent: Add a missing push_vlan action

Fix the flow for _provision_local_vlan_inbound_for_tunnel
to push-vlan explicitly.  While the old code happened to
work with older versions of OVS, it was spec-wise incorrect because
it failed to meet the prerequisite of the following set-field.
The latest version of OVS correctly rejects such a flow.

Closes-Bug: #1308927
Change-Id: I66221eec0cb4083d178d7d5651360ee1874e3d1b

diff --git a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
index 8eeb7577b6069f55c65d2b255387ed6417a9c7a7..87da5f5bd1d059851860c6e87fef3766570a0143 100644 (file)
--- a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
+++ b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
@@ -411,8 +411,10 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
         br = self.tun_br
         match = br.ofparser.OFPMatch(
             tunnel_id=int(segmentation_id))
-        actions = [br.ofparser.OFPActionSetField(
-            vlan_vid=int(lvid) | ryu_ofp13.OFPVID_PRESENT)]
+        actions = [
+            br.ofparser.OFPActionPushVlan(),
+            br.ofparser.OFPActionSetField(
+                vlan_vid=int(lvid) | ryu_ofp13.OFPVID_PRESENT)]
         instructions = [
             br.ofparser.OFPInstructionActions(
                 ryu_ofp13.OFPIT_APPLY_ACTIONS, actions),

diff --git a/neutron/tests/unit/ofagent/fake_oflib.py b/neutron/tests/unit/ofagent/fake_oflib.py
index 822c49c5c0c30cfc2bff3fa0509558fa566daa45..51d2f128ea6fc22035dfdaa059624313eaab7a18 100644 (file)
--- a/neutron/tests/unit/ofagent/fake_oflib.py
+++ b/neutron/tests/unit/ofagent/fake_oflib.py
@@ -11,33 +11,101 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
+#
 # @author: Fumihiko Kakuma, VA Linux Systems Japan K.K.
+# @author: YAMAMOTO Takashi, VA Linux Systems Japan K.K.
 
 import mock
 
 
+class _Value(object):
+    def __or__(self, b):
+        return _Op('|', self, b)
+
+    def __ror__(self, a):
+        return _Op('|', a, self)
+
+
+class _SimpleValue(_Value):
+    def __init__(self, name):
+        self.name = name
+
+    def __repr__(self):
+        return self.name
+
+
+class _Op(_Value):
+    def __init__(self, op, a, b):
+        self.op = op
+        self.a = a
+        self.b = b
+
+    def __repr__(self):
+        return '%s%s%s' % (self.a, self.op, self.b)
+
+
+def _mkcls(name):
+    class Cls(object):
+        _name = name
+
+        def __init__(self, *args, **kwargs):
+            self._args = args
+            self._kwargs = kwargs
+            self._hist = []
+
+        def __getattr__(self, name):
+            return name
+
+        def __repr__(self):
+            args = map(repr, self._args)
+            kwargs = sorted(['%s=%s' % (x, y) for x, y in
+                             self._kwargs.items()])
+            return '%s(%s)' % (self._name, ', '.join(args + kwargs))
+
+        def __eq__(self, other):
+            return repr(self) == repr(other)
+
+        def __ne__(self, other):
+            return not self.__eq__(other)
+
+    return Cls
+
+
+class _Mod(object):
+    def __init__(self, name):
+        self._name = name
+
+    def __getattr__(self, name):
+        fullname = '%s.%s' % (self._name, name)
+        if '_' in name:  # constants are named like OFPxxx_yyy_zzz
+            return _SimpleValue(fullname)
+        return _mkcls(fullname)
+
+    def __repr__(self):
+        return 'Mod(%s)' % (self._name,)
+
+
 def patch_fake_oflib_of():
     ryu_mod = mock.Mock()
     ryu_base_mod = ryu_mod.base
     ryu_lib_mod = ryu_mod.lib
     ryu_lib_hub = ryu_lib_mod.hub
     ryu_ofproto_mod = ryu_mod.ofproto
-    ryu_ofproto_of13 = ryu_ofproto_mod.ofproto_v1_3
-    ryu_ofproto_of13.OFPTT_ALL = 0xff
-    ryu_ofproto_of13.OFPG_ANY = 0xffffffff
-    ryu_ofproto_of13.OFPP_ANY = 0xffffffff
-    ryu_ofproto_of13.OFPFC_ADD = 0
-    ryu_ofproto_of13.OFPFC_DELETE = 3
+    ofp = _Mod('ryu.ofproto.ofproto_v1_3')
+    ofpp = _Mod('ryu.ofproto.ofproto_v1_3_parser')
+    ryu_ofproto_mod.ofproto_v1_3 = ofp
+    ryu_ofproto_mod.ofproto_v1_3_parser = ofpp
     ryu_app_mod = ryu_mod.app
     ryu_app_ofctl_mod = ryu_app_mod.ofctl
     ryu_ofctl_api = ryu_app_ofctl_mod.api
-    return mock.patch.dict('sys.modules',
-                           {'ryu': ryu_mod,
-                            'ryu.base': ryu_base_mod,
-                            'ryu.lib': ryu_lib_mod,
-                            'ryu.lib.hub': ryu_lib_hub,
-                            'ryu.ofproto': ryu_ofproto_mod,
-                            'ryu.ofproto.ofproto_v1_3': ryu_ofproto_of13,
-                            'ryu.app': ryu_app_mod,
-                            'ryu.app.ofctl': ryu_app_ofctl_mod,
-                            'ryu.app.ofctl.api': ryu_ofctl_api})
+    modules = {'ryu': ryu_mod,
+               'ryu.base': ryu_base_mod,
+               'ryu.lib': ryu_lib_mod,
+               'ryu.lib.hub': ryu_lib_hub,
+               'ryu.ofproto': ryu_ofproto_mod,
+               'ryu.ofproto.ofproto_v1_3': ofp,
+               'ryu.ofproto.ofproto_v1_3_parser': ofpp,
+               'ryu.app': ryu_app_mod,
+               'ryu.app.ofctl': ryu_app_ofctl_mod,
+               'ryu.app.ofctl.api': ryu_ofctl_api}
+    return mock.patch.dict('sys.modules', modules)

diff --git a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
index 4b2f5556ac16231ceea380530c4f0a7e1a62d4b7..a8ef505df7fdd8bde3203f5144e6bf6b3c6abe39 100644 (file)
--- a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
+++ b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
@@ -14,7 +14,9 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
+#
 # @author: Fumihiko Kakuma, VA Linux Systems Japan K.K.
+# @author: YAMAMOTO Takashi, VA Linux Systems Japan K.K.
 
 import contextlib
 
@@ -232,6 +234,9 @@ class TestOFANeutronAgent(OFAAgentTestCase):
                        new=MockFixedIntervalLoopingCall)):
             self.agent = self.mod_agent.OFANeutronAgent(self.ryuapp, **kwargs)
             self.agent.tun_br = mock.Mock()
+            self.agent.tun_br.ofparser = importutils.import_module(
+                'ryu.ofproto.ofproto_v1_3_parser')
+            self.agent.tun_br.datapath = 'tun_br'
             self.datapath = mock.Mock()
             self.ofparser = mock.Mock()
             self.datapath.ofparser = self.ofparser
@@ -695,6 +700,29 @@ class TestOFANeutronAgent(OFAAgentTestCase):
                                     self.agent.tunnel_types[0])]
         self.agent.setup_tunnel_port.assert_has_calls(expected_calls)
 
+    def test__provision_local_vlan_inbound_for_tunnel(self):
+        with mock.patch.object(self.agent, 'ryu_send_msg') as sendmsg:
+            self.agent._provision_local_vlan_inbound_for_tunnel(1, 'gre', 3)
+
+        ofp = importutils.import_module('ryu.ofproto.ofproto_v1_3')
+        ofpp = importutils.import_module('ryu.ofproto.ofproto_v1_3_parser')
+        expected_msg = ofpp.OFPFlowMod(
+            'tun_br',
+            instructions=[
+                ofpp.OFPInstructionActions(
+                    ofp.OFPIT_APPLY_ACTIONS,
+                    [
+                        ofpp.OFPActionPushVlan(),
+                        ofpp.OFPActionSetField(vlan_vid=1 |
+                                               ofp.OFPVID_PRESENT),
+                    ]),
+                ofpp.OFPInstructionGotoTable(table_id=10),
+            ],
+            match=ofpp.OFPMatch(tunnel_id=3),
+            priority=1,
+            table_id=2)
+        sendmsg.assert_has_calls([mock.call(expected_msg)])
+
 
 class AncillaryBridgesTest(OFAAgentTestCase):