ofagent: Fix VLAN usage for TYPE_FLAT and TYPE_VLAN

while ofagent uses OF1.3, the current coding incorrectly uses
OF1.0 terms in some places.  namely, _local_vlan_for_flat uses
0xffff to mean "no VLAN".  it should use OFPVID_NONE and
pop_vlan/push_vlan appropriately.  the same problem exists for
reclaim_local_vlan.

Closes-Bug: 1301144
Change-Id: I3df821fd72471f8bd84366e3b5a1cc7e3489156c

diff --git a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
index 4f80e217c7d268ec8a69e3f0eeb309b451c483e3..5363ac062f9dc3d80cc44fd1dc09dbd57856d3aa 100644 (file)
--- a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
+++ b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py
@@ -399,54 +399,55 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
         self._provision_local_vlan_inbound_for_tunnel(lvid, network_type,
                                                       segmentation_id)
 
-    def _provision_local_vlan_outbound(self, br, lvid, actions,
-                                       physical_network):
-        match = br.ofparser.OFPMatch(
-            in_port=int(self.phys_ofports[physical_network]),
-            vlan_vid=int(lvid) | ryu_ofp13.OFPVID_PRESENT)
-        instructions = [br.ofparser.OFPInstructionActions(
-            ryu_ofp13.OFPIT_APPLY_ACTIONS, actions)]
-        msg = br.ofparser.OFPFlowMod(br.datapath,
-                                     priority=4,
-                                     match=match,
-                                     instructions=instructions)
+    def _provision_local_vlan_outbound(self, lvid, vlan_vid, physical_network):
+        br = self.phys_brs[physical_network]
+        datapath = br.datapath
+        ofp = datapath.ofproto
+        ofpp = datapath.ofproto_parser
+        match = ofpp.OFPMatch(in_port=int(self.phys_ofports[physical_network]),
+                              vlan_vid=int(lvid) | ofp.OFPVID_PRESENT)
+        if vlan_vid == ofp.OFPVID_NONE:
+            actions = [ofpp.OFPActionPopVlan()]
+        else:
+            actions = [ofpp.OFPActionSetField(vlan_vid=vlan_vid)]
+        actions += [ofpp.OFPActionOutput(ofp.OFPP_NORMAL, 0)]
+        instructions = [
+            ofpp.OFPInstructionActions(ofp.OFPIT_APPLY_ACTIONS, actions),
+        ]
+        msg = ofpp.OFPFlowMod(datapath, priority=4, match=match,
+                              instructions=instructions)
         self.ryu_send_msg(msg)
 
     def _provision_local_vlan_inbound(self, lvid, vlan_vid, physical_network):
-        match = self.int_br.ofparser.OFPMatch(
-            in_port=int(self.int_ofports[physical_network]),
-            vlan_vid=vlan_vid)
-        actions = [self.int_br.ofparser.OFPActionSetField(
-            vlan_vid=int(lvid) | ryu_ofp13.OFPVID_PRESENT),
-            self.int_br.ofparser.OFPActionOutput(
-                ryu_ofp13.OFPP_NORMAL, 0)]
-        instructions = [self.int_br.ofparser.OFPInstructionActions(
-            ryu_ofp13.OFPIT_APPLY_ACTIONS, actions)]
-        msg = self.int_br.ofparser.OFPFlowMod(
-            self.int_br.datapath,
-            priority=3,
-            match=match,
-            instructions=instructions)
+        datapath = self.int_br.datapath
+        ofp = datapath.ofproto
+        ofpp = datapath.ofproto_parser
+        match = ofpp.OFPMatch(in_port=int(self.int_ofports[physical_network]),
+                              vlan_vid=vlan_vid)
+        if vlan_vid == ofp.OFPVID_NONE:
+            actions = [ofpp.OFPActionPushVlan()]
+        else:
+            actions = []
+        actions += [
+            ofpp.OFPActionSetField(vlan_vid=int(lvid) | ofp.OFPVID_PRESENT),
+            ofpp.OFPActionOutput(ofp.OFPP_NORMAL, 0),
+        ]
+        instructions = [
+            ofpp.OFPInstructionActions(ofp.OFPIT_APPLY_ACTIONS, actions),
+        ]
+        msg = ofpp.OFPFlowMod(datapath, priority=3, match=match,
+                              instructions=instructions)
         self.ryu_send_msg(msg)
 
     def _local_vlan_for_flat(self, lvid, physical_network):
-        br = self.phys_brs[physical_network]
-        actions = [br.ofparser.OFPActionPopVlan(),
-                   br.ofparser.OFPActionOutput(ryu_ofp13.OFPP_NORMAL, 0)]
-        self._provision_local_vlan_outbound(
-            br, lvid, actions, physical_network)
-        self._provision_local_vlan_inbound(lvid, 0xffff, physical_network)
+        vlan_vid = ryu_ofp13.OFPVID_NONE
+        self._provision_local_vlan_outbound(lvid, vlan_vid, physical_network)
+        self._provision_local_vlan_inbound(lvid, vlan_vid, physical_network)
 
     def _local_vlan_for_vlan(self, lvid, physical_network, segmentation_id):
-        br = self.phys_brs[physical_network]
-        actions = [br.ofparser.OFPActionSetField(
-            vlan_vid=int(segmentation_id) | ryu_ofp13.OFPVID_PRESENT),
-            br.ofparser.OFPActionOutput(ryu_ofp13.OFPP_NORMAL, 0)]
-        self._provision_local_vlan_outbound(
-            br, lvid, actions, physical_network)
-        self._provision_local_vlan_inbound(
-            lvid, int(segmentation_id) | ryu_ofp13.OFPVID_PRESENT,
-            physical_network)
+        vlan_vid = int(segmentation_id) | ryu_ofp13.OFPVID_PRESENT
+        self._provision_local_vlan_outbound(lvid, vlan_vid, physical_network)
+        self._provision_local_vlan_inbound(lvid, vlan_vid, physical_network)
 
     def provision_local_vlan(self, net_uuid, network_type, physical_network,
                              segmentation_id):
@@ -509,28 +510,31 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
 
     def _reclaim_local_vlan_outbound(self, lvm):
         br = self.phys_brs[lvm.physical_network]
-        match = br.ofparser.OFPMatch(
-            in_port=self.phys_ofports[lvm.physical_network],
-            vlan_vid=int(lvm.vlan) | ryu_ofp13.OFPVID_PRESENT)
-        msg = br.ofparser.OFPFlowMod(br.datapath,
-                                     table_id=ryu_ofp13.OFPTT_ALL,
-                                     command=ryu_ofp13.OFPFC_DELETE,
-                                     out_group=ryu_ofp13.OFPG_ANY,
-                                     out_port=ryu_ofp13.OFPP_ANY,
-                                     match=match)
+        datapath = br.datapath
+        ofp = datapath.ofproto
+        ofpp = datapath.ofproto_parser
+        match = ofpp.OFPMatch(
+            in_port=int(self.phys_ofports[lvm.physical_network]),
+            vlan_vid=int(lvm.vlan) | ofp.OFPVID_PRESENT)
+        msg = ofpp.OFPFlowMod(datapath, table_id=ofp.OFPTT_ALL,
+                              command=ofp.OFPFC_DELETE, out_group=ofp.OFPG_ANY,
+                              out_port=ofp.OFPP_ANY, match=match)
         self.ryu_send_msg(msg)
 
-    def _reclaim_local_vlan_inbound(self, lvm, vlan_vid):
-        br = self.int_br
-        match = br.ofparser.OFPMatch(
-            in_port=self.int_ofports[lvm.physical_network],
-            vlan_vid=vlan_vid)
-        msg = br.ofparser.OFPFlowMod(br.datapath,
-                                     table_id=ryu_ofp13.OFPTT_ALL,
-                                     command=ryu_ofp13.OFPFC_DELETE,
-                                     out_group=ryu_ofp13.OFPG_ANY,
-                                     out_port=ryu_ofp13.OFPP_ANY,
-                                     match=match)
+    def _reclaim_local_vlan_inbound(self, lvm):
+        datapath = self.int_br.datapath
+        ofp = datapath.ofproto
+        ofpp = datapath.ofproto_parser
+        if lvm.network_type == p_const.TYPE_FLAT:
+            vid = ofp.OFPVID_NONE
+        else:  # p_const.TYPE_VLAN
+            vid = lvm.segmentation_id | ofp.OFPVID_PRESENT
+        match = ofpp.OFPMatch(
+            in_port=int(self.int_ofports[lvm.physical_network]),
+            vlan_vid=vid)
+        msg = ofpp.OFPFlowMod(datapath, table_id=ofp.OFPTT_ALL,
+                              command=ofp.OFPFC_DELETE, out_group=ofp.OFPG_ANY,
+                              out_port=ofp.OFPP_ANY, match=match)
         self.ryu_send_msg(msg)
 
     def reclaim_local_vlan(self, net_uuid):
@@ -571,15 +575,10 @@ class OFANeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
                     out_port=ryu_ofp13.OFPP_ANY,
                     match=match)
                 self.ryu_send_msg(msg)
-        elif lvm.network_type == p_const.TYPE_FLAT:
-            if lvm.physical_network in self.phys_brs:
-                self._reclaim_local_vlan_outbound(lvm)
-                self._reclaim_local_vlan_inbound(lvm, 0xffff)
-        elif lvm.network_type == p_const.TYPE_VLAN:
+        elif lvm.network_type in (p_const.TYPE_FLAT, p_const.TYPE_VLAN):
             if lvm.physical_network in self.phys_brs:
                 self._reclaim_local_vlan_outbound(lvm)
-                self._reclaim_local_vlan_inbound(
-                    lvm, lvm.segmentation_id | ryu_ofp13.OFPVID_PRESENT)
+                self._reclaim_local_vlan_inbound(lvm)
         elif lvm.network_type == p_const.TYPE_LOCAL:
             # no flows needed for local networks
             pass

diff --git a/neutron/tests/unit/ofagent/fake_oflib.py b/neutron/tests/unit/ofagent/fake_oflib.py
index 51d2f128ea6fc22035dfdaa059624313eaab7a18..68d21cb6142a2669b7de105a7833159cccb7239c 100644 (file)
--- a/neutron/tests/unit/ofagent/fake_oflib.py
+++ b/neutron/tests/unit/ofagent/fake_oflib.py
@@ -18,7 +18,15 @@
 import mock
 
 
-class _Value(object):
+class _Eq(object):
+    def __eq__(self, other):
+        return repr(self) == repr(other)
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+
+class _Value(_Eq):
     def __or__(self, b):
         return _Op('|', self, b)
 
@@ -45,7 +53,7 @@ class _Op(_Value):
 
 
 def _mkcls(name):
-    class Cls(object):
+    class Cls(_Eq):
         _name = name
 
         def __init__(self, *args, **kwargs):
@@ -62,12 +70,6 @@ def _mkcls(name):
                              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
 
 

diff --git a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
index 97b48d45b35de72163664f485c4733cf876b2b68..f09854d921ae515f8bebbb7a89e1a88a5ec2d8e0 100644 (file)
--- a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
+++ b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py
@@ -218,6 +218,23 @@ class TestOFANeutronAgent(OFAAgentTestCase):
             def start(self, interval=0):
                 self.f()
 
+        def _mk_test_dp(name):
+            ofp = importutils.import_module('ryu.ofproto.ofproto_v1_3')
+            ofpp = importutils.import_module('ryu.ofproto.ofproto_v1_3_parser')
+            dp = mock.Mock()
+            dp.ofproto = ofp
+            dp.ofproto_parser = ofpp
+            dp.__repr__ = lambda _self: name
+            return dp
+
+        def _mk_test_br(name):
+            dp = _mk_test_dp(name)
+            br = mock.Mock()
+            br.datapath = dp
+            br.ofproto = dp.ofproto
+            br.ofparser = dp.ofproto_parser
+            return br
+
         with contextlib.nested(
             mock.patch.object(self.mod_agent.OFANeutronAgent,
                               'setup_integration_br',
@@ -234,18 +251,19 @@ class TestOFANeutronAgent(OFAAgentTestCase):
                        'FixedIntervalLoopingCall',
                        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.agent.tun_br = _mk_test_br('tun_br')
             self.datapath = mock.Mock()
             self.ofparser = mock.Mock()
+            self.agent.phys_brs['phys-net1'] = _mk_test_br('phys_br1')
+            self.agent.phys_ofports['phys-net1'] = 777
+            self.agent.int_ofports['phys-net1'] = 666
             self.datapath.ofparser = self.ofparser
             self.ofparser.OFPMatch = mock.Mock()
             self.ofparser.OFPMatch.return_value = mock.Mock()
             self.ofparser.OFPFlowMod = mock.Mock()
             self.ofparser.OFPFlowMod.return_value = mock.Mock()
             self.agent.int_br.ofparser = self.ofparser
+            self.agent.int_br.datapath = _mk_test_dp('int_br')
 
         self.agent.sg_agent = mock.Mock()
 
@@ -743,7 +761,7 @@ class TestOFANeutronAgent(OFAAgentTestCase):
         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',
+            self.agent.tun_br.datapath,
             instructions=[
                 ofpp.OFPInstructionActions(
                     ofp.OFPIT_APPLY_ACTIONS,
@@ -759,6 +777,195 @@ class TestOFANeutronAgent(OFAAgentTestCase):
             table_id=2)
         sendmsg.assert_has_calls([mock.call(expected_msg)])
 
+    def test__provision_local_vlan_outbound(self):
+        with mock.patch.object(self.agent, 'ryu_send_msg') as sendmsg:
+            self.agent._provision_local_vlan_outbound(888, 999, 'phys-net1')
+
+        ofp = importutils.import_module('ryu.ofproto.ofproto_v1_3')
+        ofpp = importutils.import_module('ryu.ofproto.ofproto_v1_3_parser')
+        expected_msg = ofpp.OFPFlowMod(
+            self.agent.phys_brs['phys-net1'].datapath,
+            instructions=[
+                ofpp.OFPInstructionActions(
+                    ofp.OFPIT_APPLY_ACTIONS,
+                    [
+                        ofpp.OFPActionSetField(vlan_vid=999),
+                        ofpp.OFPActionOutput(ofp.OFPP_NORMAL, 0),
+                    ]
+                )
+            ],
+            match=ofpp.OFPMatch(
+                in_port=777,
+                vlan_vid=888 | ofp.OFPVID_PRESENT
+            ),
+            priority=4)
+        sendmsg.assert_has_calls([mock.call(expected_msg)])
+
+    def test__provision_local_vlan_inbound(self):
+        with mock.patch.object(self.agent, 'ryu_send_msg') as sendmsg:
+            self.agent._provision_local_vlan_inbound(888, 999, 'phys-net1')
+
+        ofp = importutils.import_module('ryu.ofproto.ofproto_v1_3')
+        ofpp = importutils.import_module('ryu.ofproto.ofproto_v1_3_parser')
+        expected_msg = ofpp.OFPFlowMod(
+            self.agent.int_br.datapath,
+            instructions=[
+                ofpp.OFPInstructionActions(
+                    ofp.OFPIT_APPLY_ACTIONS,
+                    [
+                        ofpp.OFPActionSetField(
+                            vlan_vid=888 | ofp.OFPVID_PRESENT
+                        ),
+                        ofpp.OFPActionOutput(ofp.OFPP_NORMAL, 0),
+                    ]
+                )
+            ],
+            match=ofpp.OFPMatch(in_port=666, vlan_vid=999),
+            priority=3)
+        sendmsg.assert_has_calls([mock.call(expected_msg)])
+
+    def test__reclaim_local_vlan_outbound(self):
+        lvm = mock.Mock()
+        lvm.network_type = p_const.TYPE_VLAN
+        lvm.segmentation_id = 555
+        lvm.vlan = 444
+        lvm.physical_network = 'phys-net1'
+        with mock.patch.object(self.agent, 'ryu_send_msg') as sendmsg:
+            self.agent._reclaim_local_vlan_outbound(lvm)
+
+        ofp = importutils.import_module('ryu.ofproto.ofproto_v1_3')
+        ofpp = importutils.import_module('ryu.ofproto.ofproto_v1_3_parser')
+        expected_msg = ofpp.OFPFlowMod(
+            self.agent.phys_brs['phys-net1'].datapath,
+            command=ofp.OFPFC_DELETE,
+            match=ofpp.OFPMatch(
+                in_port=777,
+                vlan_vid=444 | ofp.OFPVID_PRESENT
+            ),
+            out_group=ofp.OFPG_ANY,
+            out_port=ofp.OFPP_ANY,
+            table_id=ofp.OFPTT_ALL)
+        sendmsg.assert_has_calls([mock.call(expected_msg)])
+
+    def test__reclaim_local_vlan_inbound(self):
+        lvm = mock.Mock()
+        lvm.network_type = p_const.TYPE_VLAN
+        lvm.segmentation_id = 555
+        lvm.vlan = 444
+        lvm.physical_network = 'phys-net1'
+        with mock.patch.object(self.agent, 'ryu_send_msg') as sendmsg:
+            self.agent._reclaim_local_vlan_inbound(lvm)
+
+        ofp = importutils.import_module('ryu.ofproto.ofproto_v1_3')
+        ofpp = importutils.import_module('ryu.ofproto.ofproto_v1_3_parser')
+        expected_msg = ofpp.OFPFlowMod(
+            self.agent.int_br.datapath,
+            command=ofp.OFPFC_DELETE,
+            match=ofpp.OFPMatch(
+                in_port=666,
+                vlan_vid=555 | ofp.OFPVID_PRESENT
+            ),
+            out_group=ofp.OFPG_ANY,
+            out_port=ofp.OFPP_ANY,
+            table_id=ofp.OFPTT_ALL)
+        sendmsg.assert_has_calls([mock.call(expected_msg)])
+
+    def test__provision_local_vlan_outbound_flat(self):
+        ofp = importutils.import_module('ryu.ofproto.ofproto_v1_3')
+        ofpp = importutils.import_module('ryu.ofproto.ofproto_v1_3_parser')
+        with mock.patch.object(self.agent, 'ryu_send_msg') as sendmsg:
+            self.agent._provision_local_vlan_outbound(888, ofp.OFPVID_NONE,
+                                                      'phys-net1')
+
+        expected_msg = ofpp.OFPFlowMod(
+            self.agent.phys_brs['phys-net1'].datapath,
+            instructions=[
+                ofpp.OFPInstructionActions(
+                    ofp.OFPIT_APPLY_ACTIONS,
+                    [
+                        ofpp.OFPActionPopVlan(),
+                        ofpp.OFPActionOutput(ofp.OFPP_NORMAL, 0),
+                    ]
+                )
+            ],
+            match=ofpp.OFPMatch(
+                in_port=777,
+                vlan_vid=888 | ofp.OFPVID_PRESENT
+            ),
+            priority=4)
+        sendmsg.assert_has_calls([mock.call(expected_msg)])
+
+    def test__provision_local_vlan_inbound_flat(self):
+        ofp = importutils.import_module('ryu.ofproto.ofproto_v1_3')
+        ofpp = importutils.import_module('ryu.ofproto.ofproto_v1_3_parser')
+        with mock.patch.object(self.agent, 'ryu_send_msg') as sendmsg:
+            self.agent._provision_local_vlan_inbound(888, ofp.OFPVID_NONE,
+                                                     'phys-net1')
+
+        expected_msg = ofpp.OFPFlowMod(
+            self.agent.int_br.datapath,
+            instructions=[
+                ofpp.OFPInstructionActions(
+                    ofp.OFPIT_APPLY_ACTIONS,
+                    [
+                        ofpp.OFPActionPushVlan(),
+                        ofpp.OFPActionSetField(
+                            vlan_vid=888 | ofp.OFPVID_PRESENT
+                        ),
+                        ofpp.OFPActionOutput(ofp.OFPP_NORMAL, 0),
+                    ]
+                )
+            ],
+            match=ofpp.OFPMatch(in_port=666, vlan_vid=ofp.OFPVID_NONE),
+            priority=3)
+        sendmsg.assert_has_calls([mock.call(expected_msg)])
+
+    def test__reclaim_local_vlan_outbound_flat(self):
+        lvm = mock.Mock()
+        lvm.network_type = p_const.TYPE_FLAT
+        lvm.segmentation_id = 555
+        lvm.vlan = 444
+        lvm.physical_network = 'phys-net1'
+        with mock.patch.object(self.agent, 'ryu_send_msg') as sendmsg:
+            self.agent._reclaim_local_vlan_outbound(lvm)
+
+        ofp = importutils.import_module('ryu.ofproto.ofproto_v1_3')
+        ofpp = importutils.import_module('ryu.ofproto.ofproto_v1_3_parser')
+        expected_msg = ofpp.OFPFlowMod(
+            self.agent.phys_brs['phys-net1'].datapath,
+            command=ofp.OFPFC_DELETE,
+            match=ofpp.OFPMatch(
+                in_port=777,
+                vlan_vid=444 | ofp.OFPVID_PRESENT
+            ),
+            out_group=ofp.OFPG_ANY,
+            out_port=ofp.OFPP_ANY,
+            table_id=ofp.OFPTT_ALL)
+        sendmsg.assert_has_calls([mock.call(expected_msg)])
+
+    def test__reclaim_local_vlan_inbound_flat(self):
+        lvm = mock.Mock()
+        lvm.network_type = p_const.TYPE_FLAT
+        lvm.segmentation_id = 555
+        lvm.vlan = 444
+        lvm.physical_network = 'phys-net1'
+        with mock.patch.object(self.agent, 'ryu_send_msg') as sendmsg:
+            self.agent._reclaim_local_vlan_inbound(lvm)
+
+        ofp = importutils.import_module('ryu.ofproto.ofproto_v1_3')
+        ofpp = importutils.import_module('ryu.ofproto.ofproto_v1_3_parser')
+        expected_msg = ofpp.OFPFlowMod(
+            self.agent.int_br.datapath,
+            command=ofp.OFPFC_DELETE,
+            match=ofpp.OFPMatch(
+                in_port=666,
+                vlan_vid=ofp.OFPVID_NONE
+            ),
+            out_group=ofp.OFPG_ANY,
+            out_port=ofp.OFPP_ANY,
+            table_id=ofp.OFPTT_ALL)
+        sendmsg.assert_has_calls([mock.call(expected_msg)])
+
 
 class AncillaryBridgesTest(OFAAgentTestCase):