From c1e3f5eb1736585181319478f676c7a23e0aa49c Mon Sep 17 00:00:00 2001 From: Carlos Magno Date: Thu, 1 Apr 2021 17:21:20 -0300 Subject: [PATCH 01/10] Add support for non-strict exclusion in the consistency check Currently consistency check is not supported for no-strict deletion. This modification begins to solve this problem by adding support for non-strict deletion using a cookie. --- main.py | 23 +++++++++++++++++++---- settings.py | 2 ++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/main.py b/main.py index ea4e2b39..53a86bfe 100644 --- a/main.py +++ b/main.py @@ -1,5 +1,6 @@ """kytos/flow_manager NApp installs, lists and deletes switch flows.""" from collections import OrderedDict +from copy import deepcopy from flask import jsonify, request from pyof.foundation.base import UBIntBase @@ -8,6 +9,7 @@ from kytos.core import KytosEvent, KytosNApp, log, rest from kytos.core.helpers import listen_to +from napps.kytos.flow_manager.match import match_flows from napps.kytos.flow_manager.storehouse import StoreHouse from napps.kytos.of_core.flow import FlowFactory @@ -172,7 +174,7 @@ def _store_changed_flows(self, command, flow, switch): flow: Flows to be stored switch: Switch target """ - stored_flows_box = self.stored_flows.copy() + stored_flows_box = deepcopy(self.stored_flows) # if the flow has a destination dpid it can be stored. if not switch: log.info('The Flow cannot be stored, the destination switch ' @@ -182,6 +184,7 @@ def _store_changed_flows(self, command, flow, switch): flow_list = [] installed_flow['command'] = command installed_flow['flow'] = flow + deleted_flows = [] serializer = FlowFactory.get_class(switch) installed_flow_obj = serializer.from_dict(flow, switch) @@ -196,7 +199,16 @@ def _store_changed_flows(self, command, flow, switch): for stored_flow in stored_flows: stored_flow_obj = serializer.from_dict(stored_flow['flow'], switch) - if installed_flow_obj == stored_flow_obj: + + version = switch.connection.protocol.version + + if installed_flow['command'] == 'delete': + # It is necessary to check whether this exclusion is + # strict or not strict + if match_flows(flow, version, stored_flow['flow']): + deleted_flows.append(stored_flow) + + elif installed_flow_obj == stored_flow_obj: if stored_flow['command'] == installed_flow['command']: log.debug('Data already stored.') return @@ -206,16 +218,19 @@ 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') - stored_flows.remove(stored_flow) + deleted_flows.append(stored_flow) break + # if installed_flow['command'] != 'delete': stored_flows.append(installed_flow) + for i in deleted_flows: + stored_flows.remove(i) stored_flows_box[switch.id]['flow_list'] = stored_flows stored_flows_box['id'] = 'flow_persistence' self.storehouse.save_flow(stored_flows_box) del stored_flows_box['id'] - self.stored_flows = stored_flows_box.copy() + self.stored_flows = deepcopy(stored_flows_box) @rest('v2/flows') @rest('v2/flows/') diff --git a/settings.py b/settings.py index 11bc877b..1a3bb2ae 100644 --- a/settings.py +++ b/settings.py @@ -5,3 +5,5 @@ # Time (in seconds) to wait retrieve box from storehouse BOX_RESTORE_TIMER = 0.1 CONSISTENCY_INTERVAL = 60 + +IPV4 = 2048 From 9c957344fc30cef3e8d2d902e476f45e0e686da0 Mon Sep 17 00:00:00 2001 From: Carlos Magno Date: Thu, 1 Apr 2021 17:33:00 -0300 Subject: [PATCH 02/10] Add new methods to compare whether two Flows are equal --- match.py | 177 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 match.py diff --git a/match.py b/match.py new file mode 100644 index 00000000..f62c604c --- /dev/null +++ b/match.py @@ -0,0 +1,177 @@ +"""Switch match.""" + +import ipaddress + +from pyof.v0x01.common.flow_match import FlowWildCards + +from kytos.core import log + +from .settings import IPV4 + + +def format_request(request): + """Format user request to match function format.""" + args = {} + for key, value in request.items(): + args[key] = value + return args + + +def do_match(flow_to_install, version, stored_flow_dict): + """Match a packet against this flow.""" + if version == 0x01: + return match10(stored_flow_dict, flow_to_install) + elif version == 0x04: + return match13_no_strict(flow_to_install, stored_flow_dict) + return None + + +def _get_match_fields(flow_dict): + """Generate match fields.""" + match_fields = {} + if 'match' in flow_dict: + for key, value in flow_dict['match'].items(): + match_fields[key] = value + return match_fields + + +# pylint: disable=too-many-return-statements, too-many-statements, R0912 +def match10(flow_dict, args): + """Match a packet against this flow (OF1.0).""" + log.debug('Matching packet') + match_fields = _get_match_fields(flow_dict) + if not match_fields.get('wildcards') & FlowWildCards.OFPFW_IN_PORT: + if 'in_port' not in args: + return False + if match_fields.get('in_port') != int(args.get('in_port')): + return False + if not match_fields.get('wildcards') & FlowWildCards.OFPFW_DL_VLAN_PCP: + if 'vlan_pcp' not in args: + return False + if match_fields.get('vlan_pcp') != int(args.get('vlan_pcp')): + return False + if not match_fields.get('wildcards') & FlowWildCards.OFPFW_DL_VLAN: + if 'vlan_vid' not in args: + return False + if match_fields.get('vlan_vid') != args.get('vlan_vid')[-1]: + return False + if not match_fields.get('wildcards') & FlowWildCards.OFPFW_DL_SRC: + if 'eth_src' not in args: + return False + if match_fields.get('eth_src') != args.get('eth_src'): + return False + if not match_fields.get('wildcards') & FlowWildCards.OFPFW_DL_DST: + if 'eth_dst' not in args: + return False + if match_fields.get('eth_dst') != args.get('eth_dst'): + return False + if not match_fields.get('wildcards') & FlowWildCards.OFPFW_DL_TYPE: + if 'eth_type' not in args: + return False + if match_fields.get('eth_type') != int(args.get('eth_type')): + return False + if match_fields['eth_type'] == IPV4: + flow_ip_int = int(ipaddress.IPv4Address(match_fields.get('ipv4_src'))) + if flow_ip_int != 0: + mask = (match_fields.get('wildcards') + & FlowWildCards.OFPFW_NW_SRC_MASK) >> \ + FlowWildCards.OFPFW_NW_SRC_SHIFT + if mask > 32: + mask = 32 + if mask != 32 and 'ipv4_src' not in args: + return False + mask = (0xffffffff << mask) & 0xffffffff + ip_int = int(ipaddress.IPv4Address(args.get('ipv4_src'))) + if ip_int & mask != flow_ip_int & mask: + return False + + flow_ip_int = int(ipaddress.IPv4Address(match_fields['ipv4_dst'])) + if flow_ip_int != 0: + mask = (match_fields.get('wildcards') + & FlowWildCards.OFPFW_NW_DST_MASK) >> \ + FlowWildCards.OFPFW_NW_DST_SHIFT + if mask > 32: + mask = 32 + if mask != 32 and 'ipv4_dst' not in args: + return False + mask = (0xffffffff << mask) & 0xffffffff + ip_int = int(ipaddress.IPv4Address(args.get('ipv4_dst'))) + if ip_int & mask != flow_ip_int & mask: + return False + if not match_fields.get('wildcards') & FlowWildCards.OFPFW_NW_TOS: + if 'ip_tos' not in args: + return False + if match_fields.get('ip_tos') != int(args.get('ip_tos')): + return False + if not match_fields.get('wildcards') & FlowWildCards.OFPFW_NW_PROTO: + if 'ip_proto' not in args: + return False + if match_fields.get('ip_proto') != int(args.get('ip_proto')): + return False + if not match_fields.get('wildcards') & FlowWildCards.OFPFW_TP_SRC: + if 'tp_src' not in args: + return False + if match_fields.get('tcp_src') != int(args.get('tp_src')): + return False + if not match_fields.get('wildcards') & FlowWildCards.OFPFW_TP_DST: + if 'tp_dst' not in args: + return False + if match_fields.get('tcp_dst') != int(args.get('tp_dst')): + return False + return flow_dict + + # def match13(self, args): + # """Match a packet against this flow (OF1.3).""" + # for name in self.match: + # if name not in args: + # return False + # if name == 'vlan_vid': + # field = args[name][-1] + # else: + # field = args[name] + # if name not in ('ipv4_src', 'ipv4_dst', 'ipv6_src', 'ipv6_dst'): + # if self.match[name].value != field: + # return False + # else: + # packet_ip = int(ipaddress.ip_address(field)) + # ip_addr = self.match[name].value + # if packet_ip & ip_addr.netmask != ip_addr.address: + # return False + # return self + + +def match13_no_strict(flow_to_install, stored_flow_dict): + """Match a packet againts the stored flow (OF 1.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_mask'] & flow_to_install['cookie'] + if cookie == stored_flow_dict['cookie']: + return stored_flow_dict + return False + + if 'match' not in flow_to_install: + return False + + for key, value in flow_to_install.get('match').items(): + if 'match' in stored_flow_dict: + if value == stored_flow_dict['match'].get(key): + return stored_flow_dict + return None + + +def match_flows(flow_to_install, version, stored_flow_dict): + # pylint: disable=bad-staticmethod-argument + """ + Match the packet in request against the flows installed in the switch. + + Try the match with each flow, in other. If many is True, tries the + match with all flows, if False, tries until the first match. + :param args: packet data + :param many: Boolean, indicating whether to continue after matching the + first flow or not + :return: If many, the list of matched flows, or the matched flow + """ + match = do_match(flow_to_install, version, stored_flow_dict) + return match From 376e4115b9dc763f2582f77743bfab8d9776cef2 Mon Sep 17 00:00:00 2001 From: Carlos Magno Date: Tue, 6 Apr 2021 12:49:00 -0300 Subject: [PATCH 03/10] Add support for match packets using IP mask --- match.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/match.py b/match.py index f62c604c..e9671cc1 100644 --- a/match.py +++ b/match.py @@ -155,10 +155,19 @@ def match13_no_strict(flow_to_install, stored_flow_dict): return False for key, value in flow_to_install.get('match').items(): - if 'match' in stored_flow_dict: + 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 - return None + else: + field = flow_to_install.get(key) + packet_ip = int(ipaddress.ip_address(field)) + ip_addr = value + if packet_ip & ip_addr.netmask == ip_addr.address: + return stored_flow_dict + + return False def match_flows(flow_to_install, version, stored_flow_dict): From d0e06e21bb913f8f191ee3dbf7daa245278964cb Mon Sep 17 00:00:00 2001 From: Carlos Magno Date: Mon, 5 Apr 2021 14:25:25 -0300 Subject: [PATCH 04/10] Refactor the non-strict match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Humberto Diógenes --- main.py | 3 +- match.py | 135 ++++++++++++++++++++++------------------------------ settings.py | 2 - 3 files changed, 58 insertions(+), 82 deletions(-) diff --git a/main.py b/main.py index 53a86bfe..ea5bbbe7 100644 --- a/main.py +++ b/main.py @@ -203,8 +203,7 @@ def _store_changed_flows(self, command, flow, switch): version = switch.connection.protocol.version if installed_flow['command'] == 'delete': - # It is necessary to check whether this exclusion is - # strict or not strict + # No strict match if match_flows(flow, version, stored_flow['flow']): deleted_flows.append(stored_flow) diff --git a/match.py b/match.py index e9671cc1..c7ba8307 100644 --- a/match.py +++ b/match.py @@ -4,9 +4,7 @@ from pyof.v0x01.common.flow_match import FlowWildCards -from kytos.core import log - -from .settings import IPV4 +IPV4_ETH_TYPE = 2048 def format_request(request): @@ -23,7 +21,7 @@ def do_match(flow_to_install, version, stored_flow_dict): return match10(stored_flow_dict, flow_to_install) elif version == 0x04: return match13_no_strict(flow_to_install, stored_flow_dict) - return None + raise NotImplementedError(f'Unsupported OpenFlow version {version}') def _get_match_fields(flow_dict): @@ -38,107 +36,88 @@ def _get_match_fields(flow_dict): # pylint: disable=too-many-return-statements, too-many-statements, R0912 def match10(flow_dict, args): """Match a packet against this flow (OF1.0).""" - log.debug('Matching packet') match_fields = _get_match_fields(flow_dict) - if not match_fields.get('wildcards') & FlowWildCards.OFPFW_IN_PORT: + wildcards = match_fields.get('wildcards') + if not wildcards & FlowWildCards.OFPFW_IN_PORT: if 'in_port' not in args: return False if match_fields.get('in_port') != int(args.get('in_port')): return False - if not match_fields.get('wildcards') & FlowWildCards.OFPFW_DL_VLAN_PCP: + if not wildcards & FlowWildCards.OFPFW_DL_VLAN_PCP: if 'vlan_pcp' not in args: return False if match_fields.get('vlan_pcp') != int(args.get('vlan_pcp')): return False - if not match_fields.get('wildcards') & FlowWildCards.OFPFW_DL_VLAN: + if not wildcards & FlowWildCards.OFPFW_DL_VLAN: if 'vlan_vid' not in args: return False if match_fields.get('vlan_vid') != args.get('vlan_vid')[-1]: return False - if not match_fields.get('wildcards') & FlowWildCards.OFPFW_DL_SRC: + if not wildcards & FlowWildCards.OFPFW_DL_SRC: if 'eth_src' not in args: return False if match_fields.get('eth_src') != args.get('eth_src'): return False - if not match_fields.get('wildcards') & FlowWildCards.OFPFW_DL_DST: + if not wildcards & FlowWildCards.OFPFW_DL_DST: if 'eth_dst' not in args: return False if match_fields.get('eth_dst') != args.get('eth_dst'): return False - if not match_fields.get('wildcards') & FlowWildCards.OFPFW_DL_TYPE: + if not wildcards & FlowWildCards.OFPFW_DL_TYPE: if 'eth_type' not in args: return False if match_fields.get('eth_type') != int(args.get('eth_type')): return False - if match_fields['eth_type'] == IPV4: - flow_ip_int = int(ipaddress.IPv4Address(match_fields.get('ipv4_src'))) - if flow_ip_int != 0: - mask = (match_fields.get('wildcards') - & FlowWildCards.OFPFW_NW_SRC_MASK) >> \ - FlowWildCards.OFPFW_NW_SRC_SHIFT - if mask > 32: - mask = 32 - if mask != 32 and 'ipv4_src' not in args: - return False - mask = (0xffffffff << mask) & 0xffffffff - ip_int = int(ipaddress.IPv4Address(args.get('ipv4_src'))) - if ip_int & mask != flow_ip_int & mask: - return False - - flow_ip_int = int(ipaddress.IPv4Address(match_fields['ipv4_dst'])) - if flow_ip_int != 0: - mask = (match_fields.get('wildcards') - & FlowWildCards.OFPFW_NW_DST_MASK) >> \ - FlowWildCards.OFPFW_NW_DST_SHIFT - if mask > 32: - mask = 32 - if mask != 32 and 'ipv4_dst' not in args: - return False - mask = (0xffffffff << mask) & 0xffffffff - ip_int = int(ipaddress.IPv4Address(args.get('ipv4_dst'))) - if ip_int & mask != flow_ip_int & mask: - return False - if not match_fields.get('wildcards') & FlowWildCards.OFPFW_NW_TOS: - if 'ip_tos' not in args: - return False - if match_fields.get('ip_tos') != int(args.get('ip_tos')): - return False - if not match_fields.get('wildcards') & FlowWildCards.OFPFW_NW_PROTO: - if 'ip_proto' not in args: - return False - if match_fields.get('ip_proto') != int(args.get('ip_proto')): - return False - if not match_fields.get('wildcards') & FlowWildCards.OFPFW_TP_SRC: - if 'tp_src' not in args: - return False - if match_fields.get('tcp_src') != int(args.get('tp_src')): - return False - if not match_fields.get('wildcards') & FlowWildCards.OFPFW_TP_DST: - if 'tp_dst' not in args: - return False - if match_fields.get('tcp_dst') != int(args.get('tp_dst')): - return False + if not match_fields['eth_type'] == IPV4_ETH_TYPE: + return False + flow_ip_int = int(ipaddress.IPv4Address(match_fields.get('ipv4_src'))) + if flow_ip_int != 0: + mask = (wildcards + & FlowWildCards.OFPFW_NW_SRC_MASK) >> \ + FlowWildCards.OFPFW_NW_SRC_SHIFT + if mask > 32: + mask = 32 + if mask != 32 and 'ipv4_src' not in args: + return False + mask = (0xffffffff << mask) & 0xffffffff + ip_int = int(ipaddress.IPv4Address(args.get('ipv4_src'))) + if ip_int & mask != flow_ip_int & mask: + return False + flow_ip_int = int(ipaddress.IPv4Address(match_fields['ipv4_dst'])) + if flow_ip_int != 0: + mask = (wildcards + & FlowWildCards.OFPFW_NW_DST_MASK) >> \ + FlowWildCards.OFPFW_NW_DST_SHIFT + if mask > 32: + mask = 32 + if mask != 32 and 'ipv4_dst' not in args: + return False + mask = (0xffffffff << mask) & 0xffffffff + ip_int = int(ipaddress.IPv4Address(args.get('ipv4_dst'))) + if ip_int & mask != flow_ip_int & mask: + return False + if not wildcards & FlowWildCards.OFPFW_NW_TOS: + if 'ip_tos' not in args: + return False + if match_fields.get('ip_tos') != int(args.get('ip_tos')): + return False + if not wildcards & FlowWildCards.OFPFW_NW_PROTO: + if 'ip_proto' not in args: + return False + if match_fields.get('ip_proto') != int(args.get('ip_proto')): + return False + if not wildcards & FlowWildCards.OFPFW_TP_SRC: + if 'tp_src' not in args: + return False + if match_fields.get('tcp_src') != int(args.get('tp_src')): + return False + if not wildcards & FlowWildCards.OFPFW_TP_DST: + if 'tp_dst' not in args: + return False + if match_fields.get('tcp_dst') != int(args.get('tp_dst')): + return False return flow_dict - # def match13(self, args): - # """Match a packet against this flow (OF1.3).""" - # for name in self.match: - # if name not in args: - # return False - # if name == 'vlan_vid': - # field = args[name][-1] - # else: - # field = args[name] - # if name not in ('ipv4_src', 'ipv4_dst', 'ipv6_src', 'ipv6_dst'): - # if self.match[name].value != field: - # return False - # else: - # packet_ip = int(ipaddress.ip_address(field)) - # ip_addr = self.match[name].value - # if packet_ip & ip_addr.netmask != ip_addr.address: - # return False - # return self - def match13_no_strict(flow_to_install, stored_flow_dict): """Match a packet againts the stored flow (OF 1.3). diff --git a/settings.py b/settings.py index 1a3bb2ae..11bc877b 100644 --- a/settings.py +++ b/settings.py @@ -5,5 +5,3 @@ # Time (in seconds) to wait retrieve box from storehouse BOX_RESTORE_TIMER = 0.1 CONSISTENCY_INTERVAL = 60 - -IPV4 = 2048 From 8327d306fb34c7b6945b69bb868fa7e844f642bc Mon Sep 17 00:00:00 2001 From: Carlos Magno Date: Tue, 6 Apr 2021 14:23:17 -0300 Subject: [PATCH 05/10] Refactor the non-strict match (OF1.0) method to improve code quality --- match.py | 76 +++++++++++++++++++++++++++++++------------------------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/match.py b/match.py index c7ba8307..020311d0 100644 --- a/match.py +++ b/match.py @@ -34,40 +34,8 @@ def _get_match_fields(flow_dict): # pylint: disable=too-many-return-statements, too-many-statements, R0912 -def match10(flow_dict, args): - """Match a packet against this flow (OF1.0).""" - match_fields = _get_match_fields(flow_dict) - wildcards = match_fields.get('wildcards') - if not wildcards & FlowWildCards.OFPFW_IN_PORT: - if 'in_port' not in args: - return False - if match_fields.get('in_port') != int(args.get('in_port')): - return False - if not wildcards & FlowWildCards.OFPFW_DL_VLAN_PCP: - if 'vlan_pcp' not in args: - return False - if match_fields.get('vlan_pcp') != int(args.get('vlan_pcp')): - return False - if not wildcards & FlowWildCards.OFPFW_DL_VLAN: - if 'vlan_vid' not in args: - return False - if match_fields.get('vlan_vid') != args.get('vlan_vid')[-1]: - return False - if not wildcards & FlowWildCards.OFPFW_DL_SRC: - if 'eth_src' not in args: - return False - if match_fields.get('eth_src') != args.get('eth_src'): - return False - if not wildcards & FlowWildCards.OFPFW_DL_DST: - if 'eth_dst' not in args: - return False - if match_fields.get('eth_dst') != args.get('eth_dst'): - return False - if not wildcards & FlowWildCards.OFPFW_DL_TYPE: - if 'eth_type' not in args: - return False - if match_fields.get('eth_type') != int(args.get('eth_type')): - return False +def _match_ipv4_10(match_fields, args, wildcards): + """Match IPV4 fields against packet with Flow (OF1.0).""" if not match_fields['eth_type'] == IPV4_ETH_TYPE: return False flow_ip_int = int(ipaddress.IPv4Address(match_fields.get('ipv4_src'))) @@ -116,6 +84,46 @@ def match10(flow_dict, args): return False if match_fields.get('tcp_dst') != int(args.get('tp_dst')): return False + return True + + +# pylint: disable=too-many-return-statements, too-many-statements, R0912 +def match10(flow_dict, args): + """Match a packet against this flow (OF1.0).""" + match_fields = _get_match_fields(flow_dict) + wildcards = match_fields.get('wildcards') + if not wildcards & FlowWildCards.OFPFW_IN_PORT: + if 'in_port' not in args: + return False + if match_fields.get('in_port') != int(args.get('in_port')): + return False + if not wildcards & FlowWildCards.OFPFW_DL_VLAN_PCP: + if 'vlan_pcp' not in args: + return False + if match_fields.get('vlan_pcp') != int(args.get('vlan_pcp')): + return False + if not wildcards & FlowWildCards.OFPFW_DL_VLAN: + if 'vlan_vid' not in args: + return False + if match_fields.get('vlan_vid') != args.get('vlan_vid')[-1]: + return False + if not wildcards & FlowWildCards.OFPFW_DL_SRC: + if 'eth_src' not in args: + return False + if match_fields.get('eth_src') != args.get('eth_src'): + return False + if not wildcards & FlowWildCards.OFPFW_DL_DST: + if 'eth_dst' not in args: + return False + if match_fields.get('eth_dst') != args.get('eth_dst'): + return False + if not wildcards & FlowWildCards.OFPFW_DL_TYPE: + if 'eth_type' not in args: + return False + if match_fields.get('eth_type') != int(args.get('eth_type')): + return False + if not _match_ipv4_10(match_fields, args, wildcards): + return False return flow_dict From 3c32ca4f21f1fd5ef373a27cd20dd52099978cf3 Mon Sep 17 00:00:00 2001 From: Carlos Magno Date: Wed, 7 Apr 2021 20:18:23 -0300 Subject: [PATCH 06/10] Fix the IPv4 and cookie mask used to compare non-strict flows Fix the masks in the non-strict delete operation. Co-authored-by: Antonio Francisco --- match.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/match.py b/match.py index 020311d0..210789f4 100644 --- a/match.py +++ b/match.py @@ -133,11 +133,12 @@ def match13_no_strict(flow_to_install, stored_flow_dict): 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_mask'] & flow_to_install['cookie'] - if cookie == stored_flow_dict['cookie']: + 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: return False @@ -148,12 +149,14 @@ def match13_no_strict(flow_to_install, stored_flow_dict): if value == stored_flow_dict['match'].get(key): return stored_flow_dict else: - field = flow_to_install.get(key) - packet_ip = int(ipaddress.ip_address(field)) - ip_addr = value - if packet_ip & ip_addr.netmask == ip_addr.address: + field = stored_flow_dict['match'].get(key) + if not field: + 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 From 91d699512fc319ab5848aecfaebae30ba6e3d341 Mon Sep 17 00:00:00 2001 From: Carlos Magno Date: Wed, 7 Apr 2021 20:04:13 -0300 Subject: [PATCH 07/10] Add unit tests for non-strict delete in the consistency check Add unit test for non-strict delete using the cookie and IPv4 (OF 1.3) in the consistency check. --- tests/unit/test_main.py | 136 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 87200a7e..4726cf40 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -381,3 +381,139 @@ def test_check_storehouse_consistency(self, *args): self.napp.stored_flows = {dpid: {"flow_list": flow_list}} self.napp.check_storehouse_consistency(switch) mock_install_flows.assert_called() + + @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(self, *args): + """Test the non-strict matching method. + + Test non-strict matching to delete a Flow using a cookie. + """ + (mock_save_flow, _, _) = args + dpid = "00:00:00:00:00:00:00:01" + switch = get_switch_mock(dpid, 0x04) + switch.id = dpid + stored_flow = { + "command": "add", + "flow": { + "actions": [ + { + "action_type": "set_vlan", + "vlan_id": 300 + } + ], + "cookie": 6191162389751548793, + "match": { + "dl_vlan": 300, + "in_port": 1 + } + } + } + stored_flow2 = { + "command": "add", + "flow": { + "actions": [], + "cookie": 4961162389751548787, + "match": { + "in_port": 2 + } + } + } + flow_to_install = { + "cookie": 6191162389751548793, + "cookie_mask": 18446744073709551615 + } + 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), 1) + + @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_with_ipv4(self, *args): + """Test the non-strict matching method. + + Test non-strict matching to delete a Flow using IPv4. + """ + (mock_save_flow, _, _) = args + dpid = "00:00:00:00:00:00:00:01" + switch = get_switch_mock(dpid, 0x04) + switch.id = dpid + stored_flow = { + "command": "add", + "flow": { + "priority": 10, + "cookie": 84114904, + "match": { + "ipv4_src": "192.168.1.120", + "ipv4_dst": "192.168.0.2", + }, + "actions": [] + } + } + stored_flow2 = { + "command": "add", + "flow": { + "actions": [], + "cookie": 4961162389751548787, + "match": { + "in_port": 2 + } + } + } + flow_to_install = {"match": {"ipv4_src": '192.168.1.1/24'}} + 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) + + @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_with_ipv4_fail(self, *args): + """Test the non-strict matching method. + + Test non-strict Fail case matching to delete a Flow using IPv4. + """ + (mock_save_flow, _, _) = args + dpid = "00:00:00:00:00:00:00:01" + switch = get_switch_mock(dpid, 0x04) + switch.id = dpid + stored_flow = { + "command": "add", + "flow": { + "priority": 10, + "cookie": 84114904, + "match": { + "ipv4_src": "192.168.2.1", + "ipv4_dst": "192.168.0.2", + }, + "actions": [] + } + } + stored_flow2 = { + "command": "add", + "flow": { + "actions": [], + "cookie": 4961162389751548787, + "match": { + "in_port": 2 + } + } + } + flow_to_install = {"match": {"ipv4_src": '192.168.1.1/24'}} + 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) From 48ae4718dd8ef889bd238d0b1fe9f5c9f74402b2 Mon Sep 17 00:00:00 2001 From: Carlos Magno Date: Thu, 22 Apr 2021 11:13:40 -0300 Subject: [PATCH 08/10] Fix linter and indentation issues in unit tests --- tests/unit/test_main.py | 122 ++++++++++++++++++---------------------- 1 file changed, 54 insertions(+), 68 deletions(-) diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index 4726cf40..a9afb05f 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -7,7 +7,7 @@ get_test_client) -# pylint: disable=protected-access +# pylint: disable=protected-access, too-many-public-methods class TestMain(TestCase): """Tests for the Main class.""" @@ -395,35 +395,25 @@ def test_no_strict_delete(self, *args): switch = get_switch_mock(dpid, 0x04) switch.id = dpid stored_flow = { - "command": "add", - "flow": { - "actions": [ - { - "action_type": "set_vlan", - "vlan_id": 300 - } - ], - "cookie": 6191162389751548793, - "match": { - "dl_vlan": 300, - "in_port": 1 - } - } - } + "command": "add", + "flow": { + "actions": [{"action_type": "set_vlan", "vlan_id": 300}], + "cookie": 6191162389751548793, + "match": {"dl_vlan": 300, "in_port": 1}, + }, + } stored_flow2 = { - "command": "add", - "flow": { - "actions": [], - "cookie": 4961162389751548787, - "match": { - "in_port": 2 - } - } - } + "command": "add", + "flow": { + "actions": [], + "cookie": 4961162389751548787, + "match": {"in_port": 2}, + }, + } flow_to_install = { - "cookie": 6191162389751548793, - "cookie_mask": 18446744073709551615 - } + "cookie": 6191162389751548793, + "cookie_mask": 18446744073709551615, + } flow_list = {"flow_list": [stored_flow, stored_flow2]} command = "delete" self.napp.stored_flows = {dpid: flow_list} @@ -445,27 +435,25 @@ def test_no_strict_delete_with_ipv4(self, *args): switch = get_switch_mock(dpid, 0x04) switch.id = dpid stored_flow = { - "command": "add", - "flow": { - "priority": 10, - "cookie": 84114904, - "match": { - "ipv4_src": "192.168.1.120", - "ipv4_dst": "192.168.0.2", - }, - "actions": [] - } - } + "command": "add", + "flow": { + "priority": 10, + "cookie": 84114904, + "match": { + "ipv4_src": "192.168.1.120", + "ipv4_dst": "192.168.0.2", + }, + "actions": [], + }, + } stored_flow2 = { - "command": "add", - "flow": { - "actions": [], - "cookie": 4961162389751548787, - "match": { - "in_port": 2 - } - } - } + "command": "add", + "flow": { + "actions": [], + "cookie": 4961162389751548787, + "match": {"in_port": 2}, + }, + } flow_to_install = {"match": {"ipv4_src": '192.168.1.1/24'}} flow_list = {"flow_list": [stored_flow, stored_flow2]} command = "delete" @@ -488,27 +476,25 @@ def test_no_strict_delete_with_ipv4_fail(self, *args): switch = get_switch_mock(dpid, 0x04) switch.id = dpid stored_flow = { - "command": "add", - "flow": { - "priority": 10, - "cookie": 84114904, - "match": { - "ipv4_src": "192.168.2.1", - "ipv4_dst": "192.168.0.2", - }, - "actions": [] - } - } + "command": "add", + "flow": { + "priority": 10, + "cookie": 84114904, + "match": { + "ipv4_src": "192.168.2.1", + "ipv4_dst": "192.168.0.2", + }, + "actions": [], + }, + } stored_flow2 = { - "command": "add", - "flow": { - "actions": [], - "cookie": 4961162389751548787, - "match": { - "in_port": 2 - } - } - } + "command": "add", + "flow": { + "actions": [], + "cookie": 4961162389751548787, + "match": {"in_port": 2}, + }, + } flow_to_install = {"match": {"ipv4_src": '192.168.1.1/24'}} flow_list = {"flow_list": [stored_flow, stored_flow2]} command = "delete" From 4b1644d53e7617efd6ba030ffbdbc33671156e86 Mon Sep 17 00:00:00 2001 From: Carlos Magno Date: Tue, 27 Apr 2021 22:42:14 -0300 Subject: [PATCH 09/10] Refactor non-strict match --- main.py | 4 ++-- match.py | 64 ++++++++++++++++---------------------------------------- 2 files changed, 20 insertions(+), 48 deletions(-) diff --git a/main.py b/main.py index ea5bbbe7..bd11944b 100644 --- a/main.py +++ b/main.py @@ -9,7 +9,7 @@ from kytos.core import KytosEvent, KytosNApp, log, rest from kytos.core.helpers import listen_to -from napps.kytos.flow_manager.match import match_flows +from napps.kytos.flow_manager.match import match_flow from napps.kytos.flow_manager.storehouse import StoreHouse from napps.kytos.of_core.flow import FlowFactory @@ -204,7 +204,7 @@ def _store_changed_flows(self, command, flow, switch): if installed_flow['command'] == 'delete': # No strict match - if match_flows(flow, version, stored_flow['flow']): + if match_flow(flow, version, stored_flow['flow']): deleted_flows.append(stored_flow) elif installed_flow_obj == stored_flow_obj: diff --git a/match.py b/match.py index 210789f4..8e00633c 100644 --- a/match.py +++ b/match.py @@ -15,10 +15,15 @@ def format_request(request): return args -def do_match(flow_to_install, version, stored_flow_dict): - """Match a packet against this flow.""" +def match_flow(flow_to_install, version, stored_flow_dict): + """Check that the flow fields match. + + It has support for (OF 1.0) and (OF 1.3) flows. + If fields match, return the flow, otherwise return False. + Does not require that all fields match. + """ if version == 0x01: - return match10(stored_flow_dict, flow_to_install) + return match10(flow_to_install, stored_flow_dict) elif version == 0x04: return match13_no_strict(flow_to_install, stored_flow_dict) raise NotImplementedError(f'Unsupported OpenFlow version {version}') @@ -36,8 +41,10 @@ def _get_match_fields(flow_dict): # pylint: disable=too-many-return-statements, too-many-statements, R0912 def _match_ipv4_10(match_fields, args, wildcards): """Match IPV4 fields against packet with Flow (OF1.0).""" - if not match_fields['eth_type'] == IPV4_ETH_TYPE: - return False + # if not 'eth_type' in (match_fields, wildcards): + # return False + # if not match_fields['eth_type'] == IPV4_ETH_TYPE: + # return False flow_ip_int = int(ipaddress.IPv4Address(match_fields.get('ipv4_src'))) if flow_ip_int != 0: mask = (wildcards @@ -65,23 +72,15 @@ def _match_ipv4_10(match_fields, args, wildcards): if ip_int & mask != flow_ip_int & mask: return False if not wildcards & FlowWildCards.OFPFW_NW_TOS: - if 'ip_tos' not in args: - return False if match_fields.get('ip_tos') != int(args.get('ip_tos')): return False if not wildcards & FlowWildCards.OFPFW_NW_PROTO: - if 'ip_proto' not in args: - return False if match_fields.get('ip_proto') != int(args.get('ip_proto')): return False if not wildcards & FlowWildCards.OFPFW_TP_SRC: - if 'tp_src' not in args: - return False if match_fields.get('tcp_src') != int(args.get('tp_src')): return False if not wildcards & FlowWildCards.OFPFW_TP_DST: - if 'tp_dst' not in args: - return False if match_fields.get('tcp_dst') != int(args.get('tp_dst')): return False return True @@ -90,37 +89,26 @@ def _match_ipv4_10(match_fields, args, wildcards): # pylint: disable=too-many-return-statements, too-many-statements, R0912 def match10(flow_dict, args): """Match a packet against this flow (OF1.0).""" + args = _get_match_fields(args) match_fields = _get_match_fields(flow_dict) - wildcards = match_fields.get('wildcards') + wildcards = match_fields.get('wildcards', 0) if not wildcards & FlowWildCards.OFPFW_IN_PORT: - if 'in_port' not in args: - return False - if match_fields.get('in_port') != int(args.get('in_port')): + if match_fields.get('in_port') != args.get('in_port'): return False if not wildcards & FlowWildCards.OFPFW_DL_VLAN_PCP: - if 'vlan_pcp' not in args: - return False - if match_fields.get('vlan_pcp') != int(args.get('vlan_pcp')): + if match_fields.get('vlan_pcp') != args.get('vlan_pcp'): return False if not wildcards & FlowWildCards.OFPFW_DL_VLAN: - if 'vlan_vid' not in args: - return False - if match_fields.get('vlan_vid') != args.get('vlan_vid')[-1]: + if match_fields.get('vlan_vid') != args.get('vlan_vid'): return False if not wildcards & FlowWildCards.OFPFW_DL_SRC: - if 'eth_src' not in args: - return False if match_fields.get('eth_src') != args.get('eth_src'): return False if not wildcards & FlowWildCards.OFPFW_DL_DST: - if 'eth_dst' not in args: - return False if match_fields.get('eth_dst') != args.get('eth_dst'): return False if not wildcards & FlowWildCards.OFPFW_DL_TYPE: - if 'eth_type' not in args: - return False - if match_fields.get('eth_type') != int(args.get('eth_type')): + if match_fields.get('eth_type') != args.get('eth_type'): return False if not _match_ipv4_10(match_fields, args, wildcards): return False @@ -158,19 +146,3 @@ def match13_no_strict(flow_to_install, stored_flow_dict): if masked_ip_addr == masked_stored_ip: return stored_flow_dict return False - - -def match_flows(flow_to_install, version, stored_flow_dict): - # pylint: disable=bad-staticmethod-argument - """ - Match the packet in request against the flows installed in the switch. - - Try the match with each flow, in other. If many is True, tries the - match with all flows, if False, tries until the first match. - :param args: packet data - :param many: Boolean, indicating whether to continue after matching the - first flow or not - :return: If many, the list of matched flows, or the matched flow - """ - match = do_match(flow_to_install, version, stored_flow_dict) - return match From 6defa10334890d9b2e9281e51a8d3432f6ccf9c3 Mon Sep 17 00:00:00 2001 From: Carlos Magno Date: Tue, 11 May 2021 17:40:12 -0300 Subject: [PATCH 10/10] Fix non-strict match for OF 1.0 --- match.py | 50 +++++++++++++----------------- tests/unit/test_main.py | 67 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 29 deletions(-) diff --git a/match.py b/match.py index 8e00633c..e1c73015 100644 --- a/match.py +++ b/match.py @@ -7,14 +7,6 @@ IPV4_ETH_TYPE = 2048 -def format_request(request): - """Format user request to match function format.""" - args = {} - for key, value in request.items(): - args[key] = value - return args - - def match_flow(flow_to_install, version, stored_flow_dict): """Check that the flow fields match. @@ -23,7 +15,7 @@ def match_flow(flow_to_install, version, stored_flow_dict): Does not require that all fields match. """ if version == 0x01: - return match10(flow_to_install, stored_flow_dict) + return match10_no_strict(flow_to_install, stored_flow_dict) elif version == 0x04: return match13_no_strict(flow_to_install, stored_flow_dict) raise NotImplementedError(f'Unsupported OpenFlow version {version}') @@ -41,53 +33,53 @@ def _get_match_fields(flow_dict): # pylint: disable=too-many-return-statements, too-many-statements, R0912 def _match_ipv4_10(match_fields, args, wildcards): """Match IPV4 fields against packet with Flow (OF1.0).""" - # if not 'eth_type' in (match_fields, wildcards): - # return False - # if not match_fields['eth_type'] == IPV4_ETH_TYPE: - # return False - flow_ip_int = int(ipaddress.IPv4Address(match_fields.get('ipv4_src'))) + if match_fields.get('dl_type') == IPV4_ETH_TYPE: + return False + flow_ip_int = int(ipaddress.IPv4Address(match_fields.get('nw_src', 0))) if flow_ip_int != 0: mask = (wildcards & FlowWildCards.OFPFW_NW_SRC_MASK) >> \ FlowWildCards.OFPFW_NW_SRC_SHIFT if mask > 32: mask = 32 - if mask != 32 and 'ipv4_src' not in args: + if mask != 32 and 'nw_src' not in args: return False mask = (0xffffffff << mask) & 0xffffffff - ip_int = int(ipaddress.IPv4Address(args.get('ipv4_src'))) + ip_int = int(ipaddress.IPv4Address(args.get('nw_src'))) if ip_int & mask != flow_ip_int & mask: return False - flow_ip_int = int(ipaddress.IPv4Address(match_fields['ipv4_dst'])) + flow_ip_int = int(ipaddress.IPv4Address(match_fields.get('nw_dst', 0))) if flow_ip_int != 0: mask = (wildcards & FlowWildCards.OFPFW_NW_DST_MASK) >> \ FlowWildCards.OFPFW_NW_DST_SHIFT if mask > 32: mask = 32 - if mask != 32 and 'ipv4_dst' not in args: + if mask != 32 and 'nw_dst' not in args: return False mask = (0xffffffff << mask) & 0xffffffff - ip_int = int(ipaddress.IPv4Address(args.get('ipv4_dst'))) + ip_int = int(ipaddress.IPv4Address(args.get('nw_dst'))) if ip_int & mask != flow_ip_int & mask: return False if not wildcards & FlowWildCards.OFPFW_NW_TOS: - if match_fields.get('ip_tos') != int(args.get('ip_tos')): + if ('nw_tos', 'nw_proto', 'tp_src', 'tp_dst') not in args: + return True + if match_fields.get('nw_tos') != int(args.get('nw_tos')): return False if not wildcards & FlowWildCards.OFPFW_NW_PROTO: - if match_fields.get('ip_proto') != int(args.get('ip_proto')): + if match_fields.get('nw_proto') != int(args.get('nw_proto')): return False if not wildcards & FlowWildCards.OFPFW_TP_SRC: - if match_fields.get('tcp_src') != int(args.get('tp_src')): + if match_fields.get('tp_src') != int(args.get('tp_src')): return False if not wildcards & FlowWildCards.OFPFW_TP_DST: - if match_fields.get('tcp_dst') != int(args.get('tp_dst')): + if match_fields.get('tp_dst') != int(args.get('tp_dst')): return False return True # pylint: disable=too-many-return-statements, too-many-statements, R0912 -def match10(flow_dict, args): +def match10_no_strict(flow_dict, args): """Match a packet against this flow (OF1.0).""" args = _get_match_fields(args) match_fields = _get_match_fields(flow_dict) @@ -96,19 +88,19 @@ def match10(flow_dict, args): if match_fields.get('in_port') != args.get('in_port'): return False if not wildcards & FlowWildCards.OFPFW_DL_VLAN_PCP: - if match_fields.get('vlan_pcp') != args.get('vlan_pcp'): + if match_fields.get('dl_vlan_pcp') != args.get('dl_vlan_pcp'): return False if not wildcards & FlowWildCards.OFPFW_DL_VLAN: - if match_fields.get('vlan_vid') != args.get('vlan_vid'): + if match_fields.get('dl_vlan') != args.get('dl_vlan'): return False if not wildcards & FlowWildCards.OFPFW_DL_SRC: - if match_fields.get('eth_src') != args.get('eth_src'): + if match_fields.get('dl_src') != args.get('dl_src'): return False if not wildcards & FlowWildCards.OFPFW_DL_DST: - if match_fields.get('eth_dst') != args.get('eth_dst'): + if match_fields.get('dl_dst') != args.get('dl_dst'): return False if not wildcards & FlowWildCards.OFPFW_DL_TYPE: - if match_fields.get('eth_type') != args.get('eth_type'): + if match_fields.get('dl_type') != args.get('dl_type'): return False if not _match_ipv4_10(match_fields, args, wildcards): return False diff --git a/tests/unit/test_main.py b/tests/unit/test_main.py index a9afb05f..7a039c8c 100644 --- a/tests/unit/test_main.py +++ b/tests/unit/test_main.py @@ -503,3 +503,70 @@ def test_no_strict_delete_with_ipv4_fail(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']), 3) + + @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_of10(self, *args): + """Test the non-strict matching method. + + Test non-strict matching to delete a Flow using OF10. + """ + (mock_save_flow, _, _) = args + dpid = "00:00:00:00:00:00:00:01" + switch = get_switch_mock(dpid, 0x01) + switch.id = dpid + stored_flow = { + "command": "add", + "flow": { + "actions": [{"max_len": 65535, "port": 6}], + "cookie": 4961162389751548787, + "match": { + "in_port": 80, + "dl_src": "00:00:00:00:00:00", + "dl_dst": "f2:0b:a4:7d:f8:ea", + "dl_vlan": 0, + "dl_vlan_pcp": 0, + "dl_type": 0, + "nw_tos": 0, + "nw_proto": 0, + "nw_src": "192.168.0.1", + "nw_dst": "0.0.0.0", + "tp_src": 0, + "tp_dst": 0, + }, + "out_port": 65532, + "priority": 123, + }, + } + stored_flow2 = { + "command": "add", + "flow": { + "actions": [], + "cookie": 4961162389751654, + "match": { + "in_port": 2, + "dl_src": "00:00:00:00:00:00", + "dl_dst": "f2:0b:a4:7d:f8:ea", + "dl_vlan": 0, + "dl_vlan_pcp": 0, + "dl_type": 0, + "nw_tos": 0, + "nw_proto": 0, + "nw_src": "192.168.0.1", + "nw_dst": "0.0.0.0", + "tp_src": 0, + "tp_dst": 0, + }, + "out_port": 655, + "priority": 1, + }, + } + flow_to_install = {"match": {"in_port": 80, "wildcards": 4194303}} + 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']), 1)