From 35b0e003e59232d63b8dbd0da511ee8e3877335e Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Wed, 2 Apr 2014 15:41:38 +0900 Subject: [PATCH] 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 --- .../ofagent/agent/ofa_neutron_agent.py | 133 ++++++----- neutron/tests/unit/ofagent/fake_oflib.py | 18 +- .../unit/ofagent/test_ofa_neutron_agent.py | 217 +++++++++++++++++- 3 files changed, 288 insertions(+), 80 deletions(-) diff --git a/neutron/plugins/ofagent/agent/ofa_neutron_agent.py b/neutron/plugins/ofagent/agent/ofa_neutron_agent.py index 4f80e217c7d..5363ac062f9 100644 --- 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: + 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, 0xffff) - elif lvm.network_type == 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 51d2f128ea6..68d21cb6142 100644 --- 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 97b48d45b35..f09854d921a 100644 --- 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):