diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 79f1beab..03773206 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -21,6 +21,18 @@ Removed Security ======== +[4.1] - 2021-10.22 +****************** + +Changed +======= +- Removed 'delete' command persistency +- Faster filtering when removing stored flows + +Fixed +===== +- Fixed ``match13_no_strict`` issue 13 + [4.0] - 2021-05-27 ****************** diff --git a/kytos.json b/kytos.json index fcb4f961..8adca590 100644 --- a/kytos.json +++ b/kytos.json @@ -3,7 +3,7 @@ "username": "kytos", "name": "flow_manager", "description": "Manage switches' flows through a REST API.", - "version": "4.0", + "version": "4.1", "napp_dependencies": ["kytos/of_core", "kytos/storehouse"], "license": "MIT", "url": "https://github.com/kytos/flow_manager.git", diff --git a/main.py b/main.py index f94aa200..bde4d882 100644 --- a/main.py +++ b/main.py @@ -199,12 +199,6 @@ def check_switch_consistency(self, switch): self._install_flows(command, flow, [switch], save=False) log.info(f'Flow forwarded to switch {dpid} to be ' 'installed.') - elif command == 'delete': - log.info('A consistency problem was detected in ' - f'switch {dpid}.') - command = 'delete_strict' - self._install_flows(command, flow, [switch], save=False) - log.info(f'Flow forwarded to switch {dpid} to be deleted.') def check_storehouse_consistency(self, switch): """Check consistency of installed flows for a specific switch.""" @@ -266,22 +260,22 @@ def _store_changed_flows(self, command, flow, switch): f'have not been specified: {switch}') return installed_flow = {} - flow_list = [] installed_flow['command'] = command installed_flow['flow'] = flow - deleted_flows = [] + should_persist_flow = command == "add" + deleted_flows_idxs = set() serializer = FlowFactory.get_class(switch) installed_flow_obj = serializer.from_dict(flow, switch) if switch.id not in stored_flows_box: # Switch not stored, add to box. - flow_list.append(installed_flow) - stored_flows_box[switch.id] = {'flow_list': flow_list} + if should_persist_flow: + stored_flows_box[switch.id] = {'flow_list': [installed_flow]} else: stored_flows = stored_flows_box[switch.id].get('flow_list', []) # Check if flow already stored - for stored_flow in stored_flows: + for i, stored_flow in enumerate(stored_flows): stored_flow_obj = serializer.from_dict(stored_flow['flow'], switch) @@ -290,7 +284,7 @@ def _store_changed_flows(self, command, flow, switch): if installed_flow['command'] == 'delete': # No strict match if match_flow(flow, version, stored_flow['flow']): - deleted_flows.append(stored_flow) + deleted_flows_idxs.add(i) elif installed_flow_obj == stored_flow_obj: if stored_flow['command'] == installed_flow['command']: @@ -302,13 +296,17 @@ def _store_changed_flows(self, command, flow, switch): # is to remove it. In this case, the old instruction is # removed and the new one is stored. stored_flow['command'] = installed_flow.get('command') - deleted_flows.append(stored_flow) + deleted_flows_idxs.add(i) break - # if installed_flow['command'] != 'delete': - stored_flows.append(installed_flow) - for i in deleted_flows: - stored_flows.remove(i) + if deleted_flows_idxs: + stored_flows = [ + flow + for i, flow in enumerate(stored_flows) + if i not in deleted_flows_idxs + ] + if should_persist_flow: + stored_flows.append(installed_flow) stored_flows_box[switch.id]['flow_list'] = stored_flows stored_flows_box['id'] = 'flow_persistence' diff --git a/match.py b/match.py index e1c73015..cacd3b7f 100644 --- a/match.py +++ b/match.py @@ -108,33 +108,30 @@ def match10_no_strict(flow_dict, args): def match13_no_strict(flow_to_install, stored_flow_dict): - """Match a packet againts the stored flow (OF 1.3). + """Match a flow that is either exact or more specific (non-strict) (OF1.3). Return the flow if any fields match, otherwise, return False. """ - if flow_to_install.get('cookie_mask') and 'cookie' in stored_flow_dict: - cookie = flow_to_install['cookie'] & flow_to_install['cookie_mask'] - stored_cookie = (stored_flow_dict['cookie'] & - flow_to_install['cookie_mask']) - if cookie == stored_cookie: - return stored_flow_dict - return False - if 'match' not in flow_to_install: + if 'match' not in flow_to_install or 'match' not in stored_flow_dict: + return stored_flow_dict + if not flow_to_install['match']: + return stored_flow_dict + if len(flow_to_install['match']) > len(stored_flow_dict['match']): return False for key, value in flow_to_install.get('match').items(): - if 'match' not in stored_flow_dict: - return False - if key not in ('ipv4_src', 'ipv4_dst', 'ipv6_src', 'ipv6_dst'): - if value == stored_flow_dict['match'].get(key): - return stored_flow_dict - else: - field = stored_flow_dict['match'].get(key) - if not field: + if key not in stored_flow_dict['match']: + return False + if value != stored_flow_dict['match'].get(key): + return False + + key_masks = {"cookie": "cookie_mask"} + for key, key_mask in key_masks.items(): + if flow_to_install.get(key_mask) and key in stored_flow_dict: + value = flow_to_install[key] & flow_to_install[key_mask] + stored_value = (stored_flow_dict[key] & + flow_to_install[key_mask]) + if value != stored_value: return False - masked_ip_addr = ipaddress.ip_network(value, False) - field_mask = field + "/" + str(masked_ip_addr.netmask) - masked_stored_ip = ipaddress.ip_network(field_mask, False) - if masked_ip_addr == masked_stored_ip: - return stored_flow_dict - return False + + return stored_flow_dict diff --git a/setup.py b/setup.py index 1609a4f6..e5d58069 100644 --- a/setup.py +++ b/setup.py @@ -22,7 +22,7 @@ BASE_ENV = Path(os.environ.get('VIRTUAL_ENV', '/')) NAPP_NAME = 'flow_manager' -NAPP_VERSION = '4.0' +NAPP_VERSION = '4.1' # Kytos var folder VAR_PATH = BASE_ENV / 'var' / 'lib' / 'kytos' diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 39d1872a..1fb88c61 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -408,7 +408,7 @@ def test_check_switch_consistency_delete(self, *args): mock_flow_factory.return_value = serializer self.napp.stored_flows = {dpid: {"flow_list": flow_list}} self.napp.check_switch_consistency(switch) - mock_install_flows.assert_called() + mock_install_flows.assert_not_called() @patch('napps.kytos.flow_manager.main.Main._install_flows') @patch('napps.kytos.flow_manager.main.FlowFactory.get_class') @@ -496,7 +496,7 @@ def test_no_strict_delete_with_ipv4(self, *args): "priority": 10, "cookie": 84114904, "match": { - "ipv4_src": "192.168.1.120", + "ipv4_src": "192.168.1.1", "ipv4_dst": "192.168.0.2", }, "actions": [], @@ -510,14 +510,141 @@ def test_no_strict_delete_with_ipv4(self, *args): "match": {"in_port": 2}, }, } - flow_to_install = {"match": {"ipv4_src": '192.168.1.1/24'}} + flow_to_install = {"match": {"ipv4_src": '192.168.1.1'}} flow_list = {"flow_list": [stored_flow, stored_flow2]} command = "delete" self.napp.stored_flows = {dpid: flow_list} self.napp._store_changed_flows(command, flow_to_install, switch) mock_save_flow.assert_called() - self.assertEqual(len(self.napp.stored_flows[dpid]['flow_list']), 2) + expected_stored = { + "flow_list": [ + { + "command": "add", + "flow": { + "actions": [], + "cookie": 4961162389751548787, + "match": {"in_port": 2}, + }, + } + ] + } + self.assertDictEqual(self.napp.stored_flows[dpid], expected_stored) + + @patch('napps.kytos.flow_manager.main.Main._install_flows') + @patch('napps.kytos.flow_manager.main.FlowFactory.get_class') + @patch("napps.kytos.flow_manager.main.StoreHouse.save_flow") + def test_no_strict_delete_in_port(self, *args): + """Test the non-strict matching method. + + Test non-strict matching to delete a Flow matching in_port. + """ + (mock_save_flow, _, _) = args + dpid = "00:00:00:00:00:00:00:01" + switch = get_switch_mock(dpid, 0x04) + switch.id = dpid + flow_to_install = {"match": {"in_port": 1}} + stored_flow = {"flow_list": [ + { + "command": "add", + "flow": { + "priority": 10, + "cookie": 84114904, + "match": { + "in_port": 1, + "dl_vlan": 100, + }, + "actions": [], + }, + }, + { + "command": "add", + "flow": { + "actions": [], + "match": {"in_port": 2}, + }, + }, + { + "command": "add", + "flow": { + "priority": 20, + "cookie": 84114904, + "match": { + "in_port": 1, + "dl_vlan": 102, + }, + "actions": [], + }, + }, + ]} + command = "delete" + self.napp.stored_flows = {dpid: stored_flow} + + self.napp._store_changed_flows(command, flow_to_install, switch) + mock_save_flow.assert_called() + + expected_stored = { + "flow_list": [ + { + "command": "add", + "flow": { + "actions": [], + "match": { + "in_port": 2 + } + } + } + ] + } + self.assertDictEqual(self.napp.stored_flows[dpid], expected_stored) + + @patch('napps.kytos.flow_manager.main.Main._install_flows') + @patch('napps.kytos.flow_manager.main.FlowFactory.get_class') + @patch("napps.kytos.flow_manager.main.StoreHouse.save_flow") + def test_no_strict_delete_all_if_empty_match(self, *args): + """Test the non-strict matching method. + + Test non-strict matching to delete all if empty match is given. + """ + (mock_save_flow, _, _) = args + dpid = "00:00:00:00:00:00:00:01" + switch = get_switch_mock(dpid, 0x04) + switch.id = dpid + flow_to_install = {"match": {}} + stored_flow = {"flow_list": [ + { + "command": "add", + "flow": { + "priority": 10, + "cookie": 84114904, + "match": { + "in_port": 1, + "dl_vlan": 100, + }, + "actions": [], + }, + }, + { + "command": "add", + "flow": { + "priority": 20, + "cookie": 84114904, + "match": { + "in_port": 1, + "dl_vlan": 102, + }, + "actions": [], + }, + }, + ]} + command = "delete" + self.napp.stored_flows = {dpid: stored_flow} + + self.napp._store_changed_flows(command, flow_to_install, switch) + mock_save_flow.assert_called() + + expected_stored = {"flow_list": []} + self.assertDictEqual(self.napp.stored_flows[dpid], expected_stored) @patch('napps.kytos.flow_manager.main.Main._install_flows') @patch('napps.kytos.flow_manager.main.FlowFactory.get_class') @@ -551,14 +678,38 @@ def test_no_strict_delete_with_ipv4_fail(self, *args): "match": {"in_port": 2}, }, } - flow_to_install = {"match": {"ipv4_src": '192.168.1.1/24'}} + flow_to_install = {"match": {"ipv4_src": '192.168.20.20'}} flow_list = {"flow_list": [stored_flow, stored_flow2]} command = "delete" self.napp.stored_flows = {dpid: flow_list} self.napp._store_changed_flows(command, flow_to_install, switch) mock_save_flow.assert_called() - self.assertEqual(len(self.napp.stored_flows[dpid]['flow_list']), 3) + expected_stored = { + "flow_list": [ + { + "command": "add", + "flow": { + "priority": 10, + "cookie": 84114904, + "match": { + "ipv4_src": "192.168.2.1", + "ipv4_dst": "192.168.0.2", + }, + "actions": [], + }, + }, + { + "command": "add", + "flow": { + "actions": [], + "cookie": 4961162389751548787, + "match": {"in_port": 2}, + }, + } + ] + } + self.assertDictEqual(self.napp.stored_flows[dpid], expected_stored) @patch('napps.kytos.flow_manager.main.Main._install_flows') @patch('napps.kytos.flow_manager.main.FlowFactory.get_class') @@ -625,7 +776,7 @@ def test_no_strict_delete_of10(self, *args): self.napp._store_changed_flows(command, flow_to_install, switch) mock_save_flow.assert_called() - self.assertEqual(len(self.napp.stored_flows[dpid]['flow_list']), 1) + self.assertEqual(len(self.napp.stored_flows[dpid]['flow_list']), 0) @patch('napps.kytos.flow_manager.main.Main._install_flows') @patch('napps.kytos.flow_manager.main.FlowFactory.get_class')