From a4c1f0535d9e1a796c85785e525278a0e4490943 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Wed, 5 Jul 2017 16:08:55 -0300 Subject: [PATCH 01/20] separate actual pack/get_size action (_pack/_get_size) from argument treatment (work_or_pass) It implements work_or_pass for GenericStruct and GenericType and make derivatives (like TypeList) use original pack/get_size funcs. This makes possible to implement _pack/_get_size and leave argument treatment and docstring to original GenericType/GenericStruct pack/get_size method include _ for customTLV and typeList pack and get_size funcs --- pyof/foundation/base.py | 119 ++++++++-------- pyof/foundation/basic_types.py | 241 +++++---------------------------- 2 files changed, 88 insertions(+), 272 deletions(-) diff --git a/pyof/foundation/base.py b/pyof/foundation/base.py index 5864c6b59..4f7922610 100644 --- a/pyof/foundation/base.py +++ b/pyof/foundation/base.py @@ -130,6 +130,18 @@ def value(self): else: return self._value + def _work_or_pass(self, value, work_func): + if value is None: + return getattr(self, work_func)() + elif isinstance(value, type(self)): + return getattr(value, work_func)() + elif 'value' in dir(value): + # if it is enum or bitmask gets only the 'int' value + value = value.value + + new_item = type(self)(value=value, enum_ref=self.enum_ref) + return getattr(new_item, work_func)() + def pack(self, value=None): r"""Pack the value as a binary representation. @@ -158,21 +170,16 @@ def pack(self, value=None): :exc:`~.exceptions.BadValueException`: If the value does not fit the binary format. """ - if isinstance(value, type(self)): - return value.pack() - - if value is None: - value = self.value - elif 'value' in dir(value): - # if it is enum or bitmask gets only the 'int' value - value = value.value + return self._work_or_pass(value, '_pack') + def _pack(self): try: - return struct.pack(self._fmt, value) + return struct.pack(self._fmt, self.value) except struct.error: - msg = '{} could not pack {} = {}.'.format(type(self).__name__, - type(value).__name__, - value) + msg = '{} could not pack {} = {}.'.format( + type(self).__name__, + type(self.value).__name__, + self.value) raise PackException(msg) def unpack(self, buff, offset=0): @@ -197,13 +204,16 @@ def unpack(self, buff, offset=0): buff, offset) raise UnpackException(msg) + def _get_size(self): + return struct.calcsize(self._fmt) + def get_size(self, value=None): """Return the size in bytes of this type. Returns: int: Size in bytes. """ - return struct.calcsize(self._fmt) + return self._work_or_pass(value, '_get_size') def is_valid(self): """Check whether the value fits the binary format. @@ -588,6 +598,20 @@ def _unpack_attribute(self, name, obj, buff, begin): raise UnpackException(msg) return size + def _work_or_pass(self, value, work_func): + if value is None: + return getattr(self, work_func)() + elif isinstance(value, type(self)): + return getattr(value, work_func)() + else: + msg = "{} is not an instance of {}".format(value, + type(self).__name__) + raise PackException(msg) + + def _get_size(self): + return sum(cls_val.get_size(obj_val) + for obj_val, cls_val in self._get_attributes()) + def get_size(self, value=None): """Calculate the total struct size in bytes. @@ -604,15 +628,7 @@ def get_size(self, value=None): Raises: Exception: If the struct is not valid. """ - if value is None: - return sum(cls_val.get_size(obj_val) for obj_val, cls_val in - self._get_attributes()) - elif isinstance(value, type(self)): - return value.get_size() - else: - msg = "{} is not an instance of {}".format(value, - type(self).__name__) - raise PackException(msg) + return self._work_or_pass(value, '_get_size') def pack(self, value=None): """Pack the struct in a binary representation. @@ -627,23 +643,19 @@ def pack(self, value=None): Raises: :exc:`~.exceptions.ValidationError`: If validation fails. """ - if value is None: - if not self.is_valid(): - error_msg = "Error on validation prior to pack() on class " - error_msg += "{}.".format(type(self).__name__) - raise ValidationError(error_msg) - else: - message = b'' - # pylint: disable=no-member - for instance_attr, class_attr in self._get_attributes(): - message += class_attr.pack(instance_attr) - return message - elif isinstance(value, type(self)): - return value.pack() + return self._work_or_pass(value, '_pack') + + def _pack(self): + if not self.is_valid(): + error_msg = "Error on validation prior to pack() on class " + error_msg += "{}.".format(type(self).__name__) + raise ValidationError(error_msg) else: - msg = "{} is not an instance of {}".format(value, - type(self).__name__) - raise PackException(msg) + message = b'' + # pylint: disable=no-member + for instance_attr, class_attr in self._get_attributes(): + message += class_attr.pack(instance_attr) + return message def unpack(self, buff, offset=0): """Unpack a binary struct into this object's attributes. @@ -719,34 +731,9 @@ def is_valid(self): # pylint: disable=unreachable return super().is_valid() and self._validate_message_length() - def pack(self, value=None): - """Pack the message into a binary data. - - One of the basic operations on a Message is the pack operation. During - the packing process, we convert all message attributes to binary - format. - - Since that this is usually used before sending the message to a switch, - here we also call :meth:`update_header_length`. - - .. seealso:: This method call its parent's :meth:`GenericStruct.pack` - after :meth:`update_header_length`. - - Returns: - bytes: A binary data thats represents the Message. - - Raises: - Exception: If there are validation errors. - """ - if value is None: - self.update_header_length() - return super().pack() - elif isinstance(value, type(self)): - return value.pack() - else: - msg = "{} is not an instance of {}".format(value, - type(self).__name__) - raise PackException(msg) + def _pack(self): + self.update_header_length() + return super()._pack() def unpack(self, buff, offset=0): """Unpack a binary message into this object's attributes. diff --git a/pyof/foundation/basic_types.py b/pyof/foundation/basic_types.py index 451b46f2f..e41db8dc0 100644 --- a/pyof/foundation/basic_types.py +++ b/pyof/foundation/basic_types.py @@ -34,17 +34,7 @@ def __repr__(self): def __str__(self): return '0' * self._length - def get_size(self, value=None): - """Return the type size in bytes. - - Args: - value (int): In structs, the user can assign other value instead of - this class' instance. Here, in such cases, ``self`` is a class - attribute of the struct. - - Returns: - int: Size in bytes. - """ + def _get_size(self): return self._length def unpack(self, buff, offset=0): @@ -59,17 +49,7 @@ def unpack(self, buff, offset=0): """ pass - def pack(self, value=None): - """Pack the object. - - Args: - value (int): In structs, the user can assign other value instead of - this class' instance. Here, in such cases, ``self`` is a class - attribute of the struct. - - Returns: - bytes: the byte 0 (zero) *length* times. - """ + def _pack(self): return b'\x00' * self._length @@ -134,20 +114,9 @@ def value(self): """ return self._value - def pack(self, value=None): - """Pack the value as a binary representation. - - Returns: - bytes: The binary representation. - - Raises: - struct.error: If the value does not fit the binary format. - """ - if isinstance(value, type(self)): - return value.pack() - if value is None: - value = self._value - return struct.pack('!8B', *[int(v, 16) for v in value.split(':')]) + def _pack(self): + return struct.pack('!8B', *[int(v, 16) + for v in self.value.split(':')]) def unpack(self, buff, offset=0): """Unpack a binary message into this object's attributes. @@ -185,28 +154,9 @@ def __init__(self, value=None, length=0): self.length = length self._fmt = '!{}{}'.format(self.length, 's') - def pack(self, value=None): - """Pack the value as a binary representation. - - Returns: - bytes: The binary representation. - - Raises: - struct.error: If the value does not fit the binary format. - """ - if isinstance(value, type(self)): - return value.pack() - - try: - if value is None: - value = self.value - packed = struct.pack(self._fmt, bytes(value, 'ascii')) - return packed[:-1] + b'\0' # null-terminated - except struct.error as err: - msg = "Char Pack error. " - msg += "Class: {}, struct error: {} ".format(type(value).__name__, - err) - raise exceptions.PackException(msg) + def _pack(self): + packed = struct.pack(self._fmt, bytes(self.value, 'ascii')) + return packed[:-1] + b'\0' # null-terminated def unpack(self, buff, offset=0): """Unpack a binary message into this object's attributes. @@ -251,29 +201,8 @@ def __init__(self, address="0.0.0.0/32"): super().__init__(address) self.netmask = int(netmask) - def pack(self, value=None): - """Pack the value as a binary representation. - - If the value is None the self._value will be used to pack. - - Args: - value (str): IP Address with ipv4 format. - - Returns: - bytes: The binary representation. - - Raises: - struct.error: If the value does not fit the binary format. - """ - if isinstance(value, type(self)): - return value.pack() - - if value is None: - value = self._value - - if value.find('/') >= 0: - value = value.split('/')[0] - + def _pack(self): + value = self._value try: value = value.split('.') return struct.pack('!4B', *[int(x) for x in value]) @@ -302,17 +231,7 @@ def unpack(self, buff, offset=0): except struct.error as e: raise exceptions.UnpackException('%s; %s: %s' % (e, offset, buff)) - def get_size(self, value=None): - """Return the ip address size in bytes. - - Args: - value: In structs, the user can assign other value instead of - this class' instance. Here, in such cases, ``self`` is a class - attribute of the struct. - - Returns: - int: The address size in bytes. - """ + def _get_size(self): return 4 @@ -326,30 +245,12 @@ def __init__(self, hw_address='00:00:00:00:00:00'): # noqa hw_address (bytes): Hardware address. Defaults to '00:00:00:00:00:00'. """ + if hw_address == 0: + hw_address = '00:00:00:00:00:00' super().__init__(hw_address) - def pack(self, value=None): - """Pack the value as a binary representation. - - If the passed value (or the self._value) is zero (int), then the pack - will assume that the value to be packed is '00:00:00:00:00:00'. - - Returns - bytes: The binary representation. - - Raises: - struct.error: If the value does not fit the binary format. - """ - if isinstance(value, type(self)): - return value.pack() - - if value is None: - value = self._value - - if value == 0: - value = '00:00:00:00:00:00' - - value = value.split(':') + def _pack(self): + value = self._value.split(':') try: return struct.pack('!6B', *[int(x, 16) for x in value]) @@ -383,17 +284,7 @@ def _int2hex(n): transformed_data = ':'.join([_int2hex(x) for x in unpacked_data]) self._value = transformed_data - def get_size(self, value=None): - """Return the address size in bytes. - - Args: - value: In structs, the user can assign other value instead of - this class' instance. Here, in such cases, ``self`` is a class - attribute of the struct. - - Returns: - int: The address size in bytes. - """ + def _get_size(self): return 6 def is_broadcast(self): @@ -423,29 +314,10 @@ def __init__(self, value=b''): # noqa """ if not isinstance(value, bytes): raise ValueError('BinaryData must contain bytes.') - super().__init__(value) - - def pack(self, value=None): - """Pack the value as a binary representation. - - Returns: - bytes: The binary representation. - - Raises: - :exc:`~.exceptions.NotBinaryData`: If value is not :class:`bytes`. - """ - if isinstance(value, type(self)): - return value.pack() - - if value is None: - value = self._value - - if value: - if isinstance(value, bytes): - return value - raise ValueError('BinaryData must contain bytes.') + super().__init__(value, enum_ref=enum_ref) - return b'' + def _pack(self): + return self._value def unpack(self, buff, offset=0): """Unpack a binary message into this object's attributes. @@ -459,23 +331,8 @@ def unpack(self, buff, offset=0): """ self._value = buff[offset:] - def get_size(self, value=None): - """Return the size in bytes. - - Args: - value (bytes): In structs, the user can assign other value instead - of this class' instance. Here, in such cases, ``self`` is a - class attribute of the struct. - - Returns: - int: The address size in bytes. - """ - if value is None: - return len(self._value) - elif hasattr(value, 'get_size'): - return value.get_size() - - return len(value) + def _get_size(self): + return len(self._value) class TypeList(list, GenericStruct): @@ -506,26 +363,11 @@ def extend(self, items): for item in items: self.append(item) - def pack(self, value=None): - """Pack the value as a binary representation. - - Returns: - bytes: The binary representation. - """ - if isinstance(value, type(self)): - return value.pack() - - if value is None: - value = self - else: - container = type(self)(items=None) - container.extend(value) - value = container - + def _pack(self): bin_message = b'' try: - for item in value: - bin_message += item.pack() + for item in self: + bin_message += item._pack() return bin_message except exceptions.PackException as err: msg = "{} pack error: {}".format(type(self).__name__, err) @@ -552,30 +394,17 @@ def unpack(self, buff, item_class, offset=0): self.append(item) begin += item.get_size() - def get_size(self, value=None): - """Return the size in bytes. - - Args: - value: In structs, the user can assign other value instead of - this class' instance. Here, in such cases, ``self`` is a class - attribute of the struct. - - Returns: - int: The size in bytes. - """ - if value is None: - if not self: - # If this is a empty list, then returns zero - return 0 - elif issubclass(type(self[0]), GenericType): - # If the type of the elements is GenericType, then returns the - # length of the list multiplied by the size of the GenericType. - return len(self) * self[0].get_size() - - # Otherwise iter over the list accumulating the sizes. - return sum(item.get_size() for item in self) - - return type(self)(value).get_size() + def _get_size(self): + if not self: + # If this is a empty list, then returns zero + return 0 + elif issubclass(type(self[0]), GenericType): + # If the type of the elements is GenericType, then returns the + # length of the list multiplied by the size of the GenericType. + return len(self) * self[0].get_size() + + # Otherwise iter over the list accumulating the sizes. + return sum(item.get_size() for item in self) def __str__(self): """Human-readable object representantion.""" From 72201adf709eff75bc43fbd22b06f5a46d4f9301 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Fri, 7 Jul 2017 14:35:56 -0300 Subject: [PATCH 02/20] work_or_pass improvement --- pyof/foundation/base.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pyof/foundation/base.py b/pyof/foundation/base.py index 4f7922610..e15f93999 100644 --- a/pyof/foundation/base.py +++ b/pyof/foundation/base.py @@ -139,7 +139,12 @@ def _work_or_pass(self, value, work_func): # if it is enum or bitmask gets only the 'int' value value = value.value - new_item = type(self)(value=value, enum_ref=self.enum_ref) + try: + new_item = type(self)(value=value, enum_ref=self.enum_ref) + except: # noqa - there is no generic Initialization Exception... + msg = "{} is not an instance of {}".format(value, + type(self).__name__) + raise PackException(msg) return getattr(new_item, work_func)() def pack(self, value=None): @@ -604,9 +609,13 @@ def _work_or_pass(self, value, work_func): elif isinstance(value, type(self)): return getattr(value, work_func)() else: - msg = "{} is not an instance of {}".format(value, - type(self).__name__) - raise PackException(msg) + try: + new_item = type(self)(value) + except: # noqa - there is no generic Initialization Exception... + msg = "{} is not an instance of {}".format(value, + type(self).__name__) + raise PackException(msg) + return getattr(new_item, work_func)() def _get_size(self): return sum(cls_val.get_size(obj_val) From ecae384cf9f23e1accd62320370bf97cd99cd61b Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Thu, 13 Jul 2017 14:07:09 -0300 Subject: [PATCH 03/20] genericStruct enum_ref fix --- pyof/foundation/base.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pyof/foundation/base.py b/pyof/foundation/base.py index e15f93999..e8280690e 100644 --- a/pyof/foundation/base.py +++ b/pyof/foundation/base.py @@ -140,8 +140,11 @@ def _work_or_pass(self, value, work_func): value = value.value try: - new_item = type(self)(value=value, enum_ref=self.enum_ref) - except: # noqa - there is no generic Initialization Exception... + new_item = type(self)(value=value) + if hasattr(self, 'enum_ref'): + new_item.enum_ref = self.enum_ref + except Exception as e: # noqa - there is no generic Initialization Exception... + print(e.args) msg = "{} is not an instance of {}".format(value, type(self).__name__) raise PackException(msg) From 5bdfd0bfefee571eb0abf85e1c47bfbc31f64af5 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Wed, 5 Jul 2017 15:52:06 -0300 Subject: [PATCH 04/20] PacketIn and binaryData small fixes --- pyof/foundation/basic_types.py | 4 +++- pyof/v0x04/asynchronous/packet_in.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pyof/foundation/basic_types.py b/pyof/foundation/basic_types.py index e41db8dc0..e3f623ff8 100644 --- a/pyof/foundation/basic_types.py +++ b/pyof/foundation/basic_types.py @@ -303,7 +303,7 @@ class BinaryData(GenericType): return the size of the instance using Python's :func:`len`. """ - def __init__(self, value=b''): # noqa + def __init__(self, value=None, enum_ref=None): # noqa """The constructor takes the parameter below. Args: @@ -312,6 +312,8 @@ def __init__(self, value=b''): # noqa Raises: ValueError: If given value is not bytes. """ + if value is None: + value = b'' if not isinstance(value, bytes): raise ValueError('BinaryData must contain bytes.') super().__init__(value, enum_ref=enum_ref) diff --git a/pyof/v0x04/asynchronous/packet_in.py b/pyof/v0x04/asynchronous/packet_in.py index c1486162e..bc6bed890 100644 --- a/pyof/v0x04/asynchronous/packet_in.py +++ b/pyof/v0x04/asynchronous/packet_in.py @@ -82,4 +82,5 @@ def __init__(self, xid=None, buffer_id=None, total_len=None, reason=None, self.table_id = table_id self.cookie = cookie self.match = match + self.pad = Pad(2) self.data = data From 68dc8fee2da1327d9c278582fde3d65dfcffd144 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Thu, 13 Jul 2017 16:43:22 -0300 Subject: [PATCH 05/20] make BinaryData accept any packable value --- pyof/foundation/basic_types.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/pyof/foundation/basic_types.py b/pyof/foundation/basic_types.py index e3f623ff8..2b0489d9b 100644 --- a/pyof/foundation/basic_types.py +++ b/pyof/foundation/basic_types.py @@ -314,13 +314,29 @@ def __init__(self, value=None, enum_ref=None): # noqa """ if value is None: value = b'' - if not isinstance(value, bytes): - raise ValueError('BinaryData must contain bytes.') + + value = self._pack_if_necessary(value) super().__init__(value, enum_ref=enum_ref) + def _pack_if_necessary(self, value): + if hasattr(value, 'pack') and callable(value.pack): + value = value.pack() + + if not isinstance(value, bytes): + msg = 'BinaryData value must contain bytes or have pack method; ' + msg += 'Received type {} value: {}'.format(type(value), value) + raise ValueError(msg) + + return value + def _pack(self): return self._value + def pack(self, value=None): + if value is not None: + value = self._pack_if_necessary(value) + return super().pack(value) + def unpack(self, buff, offset=0): """Unpack a binary message into this object's attributes. From 63205b7bceb817f523efad3b704c9c1717338d7f Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Wed, 5 Jul 2017 19:07:55 -0300 Subject: [PATCH 06/20] ListOfActions use fix --- pyof/v0x01/controller2switch/flow_mod.py | 2 +- pyof/v0x01/controller2switch/packet_out.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyof/v0x01/controller2switch/flow_mod.py b/pyof/v0x01/controller2switch/flow_mod.py index 5af964a00..dbeea96cf 100644 --- a/pyof/v0x01/controller2switch/flow_mod.py +++ b/pyof/v0x01/controller2switch/flow_mod.py @@ -100,4 +100,4 @@ def __init__(self, xid=None, match=None, cookie=0, command=None, self.buffer_id = buffer_id self.out_port = out_port self.flags = flags - self.actions = [] if actions is None else actions + self.actions = ListOfActions() if actions is None else actions diff --git a/pyof/v0x01/controller2switch/packet_out.py b/pyof/v0x01/controller2switch/packet_out.py index 8a68c3f1a..898c3073c 100644 --- a/pyof/v0x01/controller2switch/packet_out.py +++ b/pyof/v0x01/controller2switch/packet_out.py @@ -47,7 +47,7 @@ def __init__(self, xid=None, buffer_id=NO_BUFFER, in_port=Port.OFPP_NONE, super().__init__(xid) self.buffer_id = buffer_id self.in_port = in_port - self.actions = [] if actions is None else actions + self.actions = ListOfActions() if actions is None else actions self.data = data def validate(self): From 8253d87d5318f43bbc8dda2ab884a7fe2501d88b Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Fri, 7 Jul 2017 14:08:17 -0300 Subject: [PATCH 07/20] packetOut _pack fix --- pyof/v0x04/controller2switch/packet_out.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/pyof/v0x04/controller2switch/packet_out.py b/pyof/v0x04/controller2switch/packet_out.py index bf1c36744..35116ca26 100644 --- a/pyof/v0x04/controller2switch/packet_out.py +++ b/pyof/v0x04/controller2switch/packet_out.py @@ -71,17 +71,9 @@ def is_valid(self): except ValidationError: return False - def pack(self, value=None): - """Update the action_len attribute and call super's pack.""" - if value is None: + def _pack(self, value=None): self._update_actions_len() - return super().pack() - elif isinstance(value, type(self)): - return value.pack() - else: - msg = "{} is not an instance of {}".format(value, - type(self).__name__) - raise PackException(msg) + return super()._pack() def unpack(self, buff, offset=0): """Unpack a binary message into this object's attributes. From 67f6ebc078a84856435c5212b6f5559b1d25a8eb Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Fri, 7 Jul 2017 14:11:57 -0300 Subject: [PATCH 08/20] fix TypeList multiple inheritance order --- pyof/foundation/basic_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyof/foundation/basic_types.py b/pyof/foundation/basic_types.py index 2b0489d9b..1983c36c5 100644 --- a/pyof/foundation/basic_types.py +++ b/pyof/foundation/basic_types.py @@ -353,7 +353,7 @@ def _get_size(self): return len(self._value) -class TypeList(list, GenericStruct): +class TypeList(GenericStruct, list): """Base class for lists that store objects of one single type.""" def __init__(self, items): From f67d2af4468eabf9f3c96e44b2ca1b5e0871c324 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Thu, 13 Jul 2017 14:51:40 -0300 Subject: [PATCH 09/20] make consistent genericType initialization --- pyof/foundation/base.py | 5 ++++- pyof/foundation/basic_types.py | 25 ++++++++++++++----------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/pyof/foundation/base.py b/pyof/foundation/base.py index e8280690e..e626ca7c1 100644 --- a/pyof/foundation/base.py +++ b/pyof/foundation/base.py @@ -130,6 +130,9 @@ def value(self): else: return self._value + def _get_new_instance(self, value): + return type(self)(value) + def _work_or_pass(self, value, work_func): if value is None: return getattr(self, work_func)() @@ -140,7 +143,7 @@ def _work_or_pass(self, value, work_func): value = value.value try: - new_item = type(self)(value=value) + new_item = self._get_new_instance(value) if hasattr(self, 'enum_ref'): new_item.enum_ref = self.enum_ref except Exception as e: # noqa - there is no generic Initialization Exception... diff --git a/pyof/foundation/basic_types.py b/pyof/foundation/basic_types.py index 1983c36c5..c43e2fd7f 100644 --- a/pyof/foundation/basic_types.py +++ b/pyof/foundation/basic_types.py @@ -94,13 +94,13 @@ class DPID(GenericType): _fmt = "!8B" - def __init__(self, dpid=None): + def __init__(self, value=None): """Create an instance and optionally set its dpid value. Args: dpid (str): String with DPID value(e.g. `00:00:00:00:00:00:00:01`). """ - super().__init__(value=dpid) + super().__init__(value=value) def __str__(self): return self._value @@ -154,6 +154,9 @@ def __init__(self, value=None, length=0): self.length = length self._fmt = '!{}{}'.format(self.length, 's') + def _get_new_instance(self, value): + return type(self)(value, length=self.length) + def _pack(self): packed = struct.pack(self._fmt, bytes(self.value, 'ascii')) return packed[:-1] + b'\0' # null-terminated @@ -187,18 +190,18 @@ class IPAddress(GenericType): netmask = UBInt32() max_prefix = UBInt32(32) - def __init__(self, address="0.0.0.0/32"): + def __init__(self, value="0.0.0.0/32"): """The constructor takes the parameters below. Args: address (str): IP Address using ipv4. Defaults to '0.0.0.0/32' """ - if address.find('/') >= 0: - address, netmask = address.split('/') + if value.find('/') >= 0: + value, netmask = value.split('/') else: netmask = 32 - super().__init__(address) + super().__init__(value) self.netmask = int(netmask) def _pack(self): @@ -238,16 +241,16 @@ def _get_size(self): class HWAddress(GenericType): """Defines a hardware address.""" - def __init__(self, hw_address='00:00:00:00:00:00'): # noqa + def __init__(self, value='00:00:00:00:00:00'): # noqa """The constructor takes the parameters below. Args: - hw_address (bytes): Hardware address. Defaults to + value (bytes): Hardware address. Defaults to '00:00:00:00:00:00'. """ - if hw_address == 0: - hw_address = '00:00:00:00:00:00' - super().__init__(hw_address) + if value == 0: + value = '00:00:00:00:00:00' + super().__init__(value) def _pack(self): value = self._value.split(':') From c9f164d62be60013bed9ddbe331e68307623d5e2 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Thu, 6 Jul 2017 16:07:04 -0300 Subject: [PATCH 10/20] linter fixes --- pyof/foundation/basic_types.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyof/foundation/basic_types.py b/pyof/foundation/basic_types.py index c43e2fd7f..ad2c10a30 100644 --- a/pyof/foundation/basic_types.py +++ b/pyof/foundation/basic_types.py @@ -134,7 +134,7 @@ def unpack(self, buff, offset=0): begin = offset hexas = [] while begin < offset + 8: - number = struct.unpack("!B", buff[begin:begin+1])[0] + number = struct.unpack("!B", buff[begin:begin + 1])[0] hexas.append("%.2x" % number) begin += 1 self._value = ':'.join(hexas) @@ -229,7 +229,7 @@ def unpack(self, buff, offset=0): Exception: If there is a struct unpacking error. """ try: - unpacked_data = struct.unpack('!4B', buff[offset:offset+4]) + unpacked_data = struct.unpack('!4B', buff[offset:offset + 4]) self._value = '.'.join([str(x) for x in unpacked_data]) except struct.error as e: raise exceptions.UnpackException('%s; %s: %s' % (e, offset, buff)) @@ -280,7 +280,7 @@ def _int2hex(n): return "{0:0{1}x}".format(n, 2) try: - unpacked_data = struct.unpack('!6B', buff[offset:offset+6]) + unpacked_data = struct.unpack('!6B', buff[offset:offset + 6]) except struct.error as e: raise exceptions.UnpackException('%s; %s: %s' % (e, offset, buff)) From dbf4d5c48c99cbb8219d27cfa9126af6870e7b61 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Wed, 5 Jul 2017 15:39:26 -0300 Subject: [PATCH 11/20] Include UBInt24 and CustomTLV classes --- pyof/foundation/base.py | 44 ++++++++++++++++++++++++-- pyof/foundation/basic_types.py | 57 ++++++++++++++++++++++++++++++++-- 2 files changed, 97 insertions(+), 4 deletions(-) diff --git a/pyof/foundation/base.py b/pyof/foundation/base.py index e626ca7c1..147c0d1a9 100644 --- a/pyof/foundation/base.py +++ b/pyof/foundation/base.py @@ -31,8 +31,8 @@ # This will determine the order on sphinx documentation. -__all__ = ('GenericStruct', 'GenericMessage', 'GenericType', 'GenericBitMask', - 'MetaStruct', 'MetaBitMask') +__all__ = ('GenericStruct', 'GenericMessage', 'GenericType', + 'GenericUBIntType', 'GenericBitMask', 'MetaStruct', 'MetaBitMask') # Classes @@ -115,6 +115,12 @@ def __xor__(self, other): def __rxor__(self, other): return self.value ^ other + def __lshift__(self, other): + return self.value << other + + def __rshift__(self, other): + return self.value >> other + @property def value(self): """Return this type's value. @@ -257,6 +263,40 @@ def is_bitmask(self): return self._value and issubclass(type(self._value), GenericBitMask) +class GenericUBIntType(GenericType): + + _buff_size = 0 + + def _pack(self): + return self.value.to_bytes(self._buff_size, byteorder='big') + + def unpack(self, buff, offset=0): + """Unpack *buff* into this object. + + This method will convert a binary data into a readable value according + to the attribute format. + + Args: + buff (bytes): Binary buffer. + offset (int): Where to begin unpacking. + + Raises: + :exc:`~.exceptions.UnpackException`: If unpack fails. + """ + try: + self._value = int.from_bytes(buff[offset:offset + self._buff_size], + byteorder='big') + if self.enum_ref: + self._value = self.enum_ref(self._value) + except (struct.error, TypeError, ValueError) as e: + msg = '{}; fmt = {}, buff = {}, offset = {}.'.format( + e, 'UBInt' + 8 * self._buff_size, buff, offset) + raise UnpackException(msg) + + def _get_size(self): + return self._buff_size + + class MetaStruct(type): """MetaClass that dinamically handles openflow version of class attributes. diff --git a/pyof/foundation/basic_types.py b/pyof/foundation/basic_types.py index ad2c10a30..954bc1f3a 100644 --- a/pyof/foundation/basic_types.py +++ b/pyof/foundation/basic_types.py @@ -5,13 +5,13 @@ # Local source tree imports from pyof.foundation import exceptions -from pyof.foundation.base import GenericStruct, GenericType +from pyof.foundation.base import GenericStruct, GenericType, GenericUBIntType # Third-party imports __all__ = ('BinaryData', 'Char', 'ConstantTypeList', 'FixedTypeList', 'IPAddress', 'DPID', 'HWAddress', 'Pad', 'UBInt8', 'UBInt16', - 'UBInt32', 'UBInt64') + 'UBInt24', 'UBInt32', 'UBInt64') class Pad(GenericType): @@ -71,6 +71,15 @@ class UBInt16(GenericType): _fmt = "!H" +class UBInt24(GenericUBIntType): + """Format character for an Unsigned Short. + + Class for an 16-bit (2-byte) Unsigned Integer. + """ + + _buff_size = 3 + + class UBInt32(GenericType): """Format character for an Unsigned Int. @@ -556,3 +565,47 @@ def insert(self, index, item): else: raise exceptions.WrongListItemType(item.__class__.__name__, self[0].__class__.__name__) + + +def get_custom_tlv_class(type_size=3, length_size=1): + + size_classes = {1: UBInt8, + 2: UBInt16, + 3: UBInt24, + 4: UBInt32} + + custom_type = size_classes[type_size] + custom_length = size_classes[length_size] + + class CustomTLV(GenericStruct): + tlv_type = custom_type() + tlv_length = custom_length() + tlv_value = BinaryData() + + def __init__(self, tlv_type=None, tlv_value=None): + super().__init__() + self.tlv_type = tlv_type + self.tlv_value = tlv_value + self._update_tlv_length() + + def _update_tlv_length(self): + cls = type(self) + self.tlv_length = (type(cls.tlv_value)(self.tlv_value)).get_size() + + def _pack(self): + self._update_tlv_length() + return super()._pack() + + def unpack(self, buff, offset=0): + begin = offset + for name, value in list(self.get_class_attributes())[:-1]: + size = self._unpack_attribute(name, value, buff, begin) + begin += size + self._unpack_attribute('tlv_value', type(self).tlv_value, + buff[:begin + self.tlv_length], + begin) + + return CustomTLV + + +CustomTLV_24_8 = get_custom_tlv_class(type_size=3, length_size=1) From a68fd5f8567360696a5d204203d97624a760144f Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Wed, 5 Jul 2017 15:47:32 -0300 Subject: [PATCH 12/20] Include OxmTLV class + fix related classes --- pyof/v0x04/common/flow_match.py | 128 ++++++++++++++++++++++++-------- 1 file changed, 97 insertions(+), 31 deletions(-) diff --git a/pyof/v0x04/common/flow_match.py b/pyof/v0x04/common/flow_match.py index df35e3ae4..39155e479 100644 --- a/pyof/v0x04/common/flow_match.py +++ b/pyof/v0x04/common/flow_match.py @@ -8,14 +8,16 @@ # Local source tree imports from pyof.foundation.base import GenericStruct -from pyof.foundation.basic_types import FixedTypeList, UBInt8, UBInt16, UBInt32 +from pyof.foundation.basic_types import (FixedTypeList, UBInt8, UBInt16, + UBInt32, Pad, BinaryData, + CustomTLV_24_8) +from pyof.foundation.exceptions import PackException # Third-party imports -__all__ = ('Ipv6ExtHdrFlags', 'Match', 'MatchField', 'MatchType', - 'OxmExperimenterHeader', 'OxmOfbMatchField', 'VlanId', - 'ListOfOxmHeader') +__all__ = ('Ipv6ExtHdrFlags', 'Match', 'OxmOfbMatchField', 'MatchType', + 'OxmExperimenterHeader', 'MatchField', 'VlanId') class Ipv6ExtHdrFlags(Enum): @@ -41,7 +43,7 @@ class Ipv6ExtHdrFlags(Enum): OFPIEH_UNSEQ = 1 << 8 -class MatchField(IntEnum): +class OxmOfbMatchField(IntEnum): """OXM Flow match field types for OpenFlow basic class. A switch is not required to support all match field types, just those @@ -148,7 +150,7 @@ class MatchType(IntEnum): OFPMT_OXM = 1 -class OxmOfbMatchField(IntEnum): +class OxmClass(IntEnum): """OpenFlow Extensible Match (OXM) Class IDs. The high order bit differentiate reserved classes from member classes. @@ -182,17 +184,66 @@ class VlanId(IntEnum): # Classes - -class OxmHeader(GenericStruct): +class OxmTLV(GenericStruct): + + oxm_class = UBInt16(enum_ref=OxmClass) + oxm_field = UBInt8() + oxm_hasmask = UBInt8() + oxm_length = UBInt8() + oxm_value = BinaryData() + + def __init__(self, oxm_class=None, oxm_field=None, + oxm_hasmask=None, oxm_value=None): + self.oxm_class = oxm_class + self.oxm_field = oxm_field + self.oxm_hasmask = oxm_hasmask if oxm_hasmask else 0 + self.oxm_length = None + self.oxm_value = oxm_value + self.tlv_class = CustomTLV_24_8 + + def unpack(self, buff, offset=0): + tlv = self.tlv_class() + tlv.unpack(buff, offset) + # print('tlv unpack values:') + # print(type(tlv.tlv_type), tlv.tlv_type) + # print(type(tlv.tlv_length), tlv.tlv_length) + # print(type(tlv.tlv_value), tlv.tlv_value) + self.oxm_class = tlv.tlv_type >> 8 + self.oxm_field = (tlv.tlv_type & 0xFF) >> 1 + self.oxm_hasmask = tlv.tlv_type & 1 + self.oxm_length = tlv.tlv_length + self.oxm_value = tlv.tlv_value + + def _pack(self): + tlv_type = ((self.oxm_class << 8) + + (self.oxm_field << 1) + + self.oxm_hasmask) + + tlv_value = self.oxm_value + tlv = self.tlv_class(tlv_type, tlv_value) + return tlv._pack() + + def _get_size(self): + size = super()._get_size() - 1 + return size + + +class OxmMatchFields(FixedTypeList): """Generic Openflow EXtensible Match header. Abstract class that can be instanciated as Match or OxmExperimenterHeader. """ - pass + def __init__(self, items=None): + """The constructor just assings parameters to object attributes. + + Args: + items (OxmHeader): Instance or a list of instances. + """ + super().__init__(pyof_class=OxmTLV, items=items) -class Match(OxmHeader): +class Match(GenericStruct): """Describes the flow match header structure. These are the fields to match against flows. @@ -207,13 +258,9 @@ class Match(OxmHeader): match_type = UBInt16(enum_ref=MatchType) #: Length of Match (excluding padding) length = UBInt16() - oxm_field1 = UBInt8(enum_ref=OxmOfbMatchField) - oxm_field2 = UBInt8(enum_ref=OxmOfbMatchField) - oxm_field3 = UBInt8(enum_ref=OxmOfbMatchField) - oxm_field4 = UBInt8(enum_ref=OxmOfbMatchField) + oxm_match_fields = OxmMatchFields() - def __init__(self, match_type=None, length=None, oxm_field1=None, - oxm_field2=None, oxm_field3=None, oxm_field4=None): + def __init__(self, match_type=None, oxm_match_fields=None): """Describe the flow match header structure. Args: @@ -222,26 +269,45 @@ def __init__(self, match_type=None, length=None, oxm_field1=None, Exactly (length - 4) (possibly 0) bytes containing OXM TLVs, then exactly ((length + 7)/8*8 - length) (between 0 and 7) bytes of all-zero bytes. - oxm_field1 (OXMClass): Sample description. - oxm_field2 (OXMClass): Sample description. - oxm_field3 (OXMClass): Sample description. - oxm_field4 (OXMClass): Sample description. + oxm_fields (OxmMatchFields): Sample description. """ super().__init__() self.match_type = match_type - self.length = length - self.oxm_field1 = oxm_field1 - self.oxm_field2 = oxm_field2 - self.oxm_field3 = oxm_field3 - self.oxm_field4 = oxm_field4 - - -class OxmExperimenterHeader(OxmHeader): + self.oxm_match_fields = oxm_match_fields + self._update_match_length() + + def _update_match_length(self): + self.length = super()._get_size() + + def _pack(self): + self._update_match_length() + packet = super()._pack() + super_size = len(packet) + lacking_bytes = (8 - (super_size % 8)) % 8 + if lacking_bytes != 0: + packet += Pad(lacking_bytes).pack() + return packet + + def _get_size(self): + super_size = super()._get_size() + return super_size + (8 - (super_size % 8)) % 8 + + def unpack(self, buff, offset=0): + begin = offset + for name, value in list(self.get_class_attributes())[:-1]: + size = self._unpack_attribute(name, value, buff, begin) + begin += size + self._unpack_attribute('oxm_match_fields', type(self).oxm_match_fields, + buff[:offset + self.length], + begin) + + +class OxmExperimenterHeader(GenericStruct): """Header for OXM experimenter match fields.""" #: oxm_class = OFPXMC_EXPERIMENTER - oxm_header = UBInt32(OxmOfbMatchField.OFPXMC_EXPERIMENTER, - enum_ref=OxmOfbMatchField) + oxm_header = UBInt32(OxmClass.OFPXMC_EXPERIMENTER, + enum_ref=OxmClass) #: Experimenter ID which takes the same form as in struct #: ofp_experimenter_header experimenter = UBInt32() @@ -269,4 +335,4 @@ def __init__(self, items=None): Args: items (OxmHeader): Instance or a list of instances. """ - super().__init__(pyof_class=OxmHeader, items=items) + super().__init__(pyof_class=OxmTLV, items=items) From ce42a7d152558e0128f7797791bc47d1d5e3c731 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Wed, 5 Jul 2017 16:46:31 -0300 Subject: [PATCH 13/20] OxmType utility class --- pyof/v0x04/common/flow_match.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pyof/v0x04/common/flow_match.py b/pyof/v0x04/common/flow_match.py index 39155e479..3e5ae7eff 100644 --- a/pyof/v0x04/common/flow_match.py +++ b/pyof/v0x04/common/flow_match.py @@ -184,6 +184,16 @@ class VlanId(IntEnum): # Classes +class OxmType(GenericStruct): + oxm_class = UBInt16(enum_ref=OxmClass) + oxm_field = UBInt8() + + def __init__(self, oxm_class, oxm_field): + cls = type(self) + self.oxm_class = type(cls.oxm_class)(oxm_class) + self.oxm_field = oxm_field + + class OxmTLV(GenericStruct): oxm_class = UBInt16(enum_ref=OxmClass) From 861b912b5b86598d7aa420342df3f173a1c5e5d1 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Thu, 6 Jul 2017 13:59:21 -0300 Subject: [PATCH 14/20] docstrings updates --- pyof/foundation/basic_types.py | 31 +++++++++++++++++++++++++++++-- pyof/v0x04/common/flow_match.py | 24 ++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/pyof/foundation/basic_types.py b/pyof/foundation/basic_types.py index 954bc1f3a..80c0e3aac 100644 --- a/pyof/foundation/basic_types.py +++ b/pyof/foundation/basic_types.py @@ -42,6 +42,7 @@ def unpack(self, buff, offset=0): Do nothing, since the _length is already defined and it is just a Pad. Keep buff and offset just for compability with other unpack methods. + [this will check if bytes are 0 for validity in the future] Args: buff: Buffer where data is located. @@ -355,6 +356,10 @@ def unpack(self, buff, offset=0): Unpack the binary value *buff* and update this object attributes based on the results. Since the *buff* is binary data, no conversion is done. + All the data in the buffer from the offset forward will be used, + so the buffer must be truncated using the desired size before passing + it to BinaryData. + Args: buff (bytes): Binary data package to be unpacked. offset (int): Where to begin unpacking. @@ -406,9 +411,9 @@ def _pack(self): def unpack(self, buff, item_class, offset=0): """Unpack the elements of the list. - This unpack method considers that all elements have the same size. + This unpack method considers that all elements are of the same type. To use this class with a pyof_class that accepts elements with - different sizes, you must reimplement the unpack method. + different classes, you must reimplement the unpack method. Args: buff (bytes): The binary data to be unpacked. @@ -568,6 +573,15 @@ def insert(self, index, item): def get_custom_tlv_class(type_size=3, length_size=1): + """ Generate a CUstomTLV class + + Create a CustomTLV class with the defined number of bytes for type and + length fields. + + Args: + type_size (int): length in bytes for the type field of the TLV + length_size (int): length in bytes for the length field of the TLV + """ size_classes = {1: UBInt8, 2: UBInt16, @@ -578,6 +592,12 @@ def get_custom_tlv_class(type_size=3, length_size=1): custom_length = size_classes[length_size] class CustomTLV(GenericStruct): + """ a compact TLV class + + Args: + tlv_type: type field of the TLV + tlv_value: length field of the TLV + """ tlv_type = custom_type() tlv_length = custom_length() tlv_value = BinaryData() @@ -597,6 +617,13 @@ def _pack(self): return super()._pack() def unpack(self, buff, offset=0): + """Unpack the buffer into a custom TLV + + Args: + buff (bytes): The binary data to be unpacked. + offset (int): If we need to shift the beginning of the data. + """ + begin = offset for name, value in list(self.get_class_attributes())[:-1]: size = self._unpack_attribute(name, value, buff, begin) diff --git a/pyof/v0x04/common/flow_match.py b/pyof/v0x04/common/flow_match.py index 3e5ae7eff..790489bad 100644 --- a/pyof/v0x04/common/flow_match.py +++ b/pyof/v0x04/common/flow_match.py @@ -185,6 +185,15 @@ class VlanId(IntEnum): # Classes class OxmType(GenericStruct): + """ Oxm TLV `type` metafield. + + OxmType is defined by the combination of a OxmClass and a OxmField, + + Args: + oxm_class (:class:`OxmClass`, int): The oxm TLV defined class. + oxm_field (:class:`OxmOfbMatchField`, Oxm*MatchField, int): the oxm + TLV defined field of the correspondent class + """ oxm_class = UBInt16(enum_ref=OxmClass) oxm_field = UBInt8() @@ -195,6 +204,15 @@ def __init__(self, oxm_class, oxm_field): class OxmTLV(GenericStruct): + """ Oxm (Openflow Extensible Match) TLV. + + Args: + oxm_class (:class:`OxmClass`, int): The oxm TLV defined class. + oxm_field (:class:`OxmOfbMatchField`, Oxm*MatchField, int): the oxm + TLV defined field of the correspondent oxm_class. + oxm_hasmask (bool): + oxm_value (:class:`BinaryData`, bytes): + """ oxm_class = UBInt16(enum_ref=OxmClass) oxm_field = UBInt8() @@ -212,6 +230,12 @@ def __init__(self, oxm_class=None, oxm_field=None, self.tlv_class = CustomTLV_24_8 def unpack(self, buff, offset=0): + """Unpack the buffer into a OxmTLV. + + Args: + buff (bytes): The binary data to be unpacked. + offset (int): If we need to shift the beginning of the data. + """ tlv = self.tlv_class() tlv.unpack(buff, offset) # print('tlv unpack values:') From c3cb36c34ecec5252d8f5446809d4628e65b9b62 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Thu, 13 Jul 2017 14:16:04 -0300 Subject: [PATCH 15/20] multipart test binary fix --- tests/v0x04/test_controller2switch/test_multipart_reply.py | 4 ++-- tests/v0x04/test_controller2switch/test_multipart_request.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/v0x04/test_controller2switch/test_multipart_reply.py b/tests/v0x04/test_controller2switch/test_multipart_reply.py index 68a3c6ef6..325092644 100644 --- a/tests/v0x04/test_controller2switch/test_multipart_reply.py +++ b/tests/v0x04/test_controller2switch/test_multipart_reply.py @@ -16,13 +16,13 @@ def setUpClass(cls): super().set_message(MultipartReply, xid=16, multipart_type=MultipartTypes.OFPMP_METER_CONFIG, flags=MultipartReplyFlags.OFPMPF_REPLY_MORE, - body='') + body=b'') super().set_minimum_size(16) @staticmethod def get_attributes(multipart_type=MultipartTypes.OFPMP_DESC, flags=MultipartReplyFlags.OFPMPF_REPLY_MORE, - body=''): + body=b''): """Method used to return a dict with instance paramenters.""" return {'xid': 32, 'multipart_type': multipart_type, 'flags': flags, 'body': body} diff --git a/tests/v0x04/test_controller2switch/test_multipart_request.py b/tests/v0x04/test_controller2switch/test_multipart_request.py index 2ed29e4e5..664712548 100644 --- a/tests/v0x04/test_controller2switch/test_multipart_request.py +++ b/tests/v0x04/test_controller2switch/test_multipart_request.py @@ -22,7 +22,7 @@ def setUpClass(cls): @staticmethod def get_attributes(multipart_type=MultipartTypes.OFPMP_DESC, flags=MultipartRequestFlags.OFPMPF_REQ_MORE, - body=''): + body=b''): """Method used to return a dict with instance paramenters.""" return {'xid': 32, 'multipart_type': multipart_type, 'flags': flags, 'body': body} From 3ab02e12442ef3832b7ea5f7aa3606ab38672953 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Tue, 18 Jul 2017 11:38:14 -0300 Subject: [PATCH 16/20] minor fix in stats tests --- tests/v0x01/test_controller2switch/test_stats_reply.py | 2 +- tests/v0x01/test_controller2switch/test_stats_request.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/v0x01/test_controller2switch/test_stats_reply.py b/tests/v0x01/test_controller2switch/test_stats_reply.py index ce2260a3d..9c93c4563 100644 --- a/tests/v0x01/test_controller2switch/test_stats_reply.py +++ b/tests/v0x01/test_controller2switch/test_stats_reply.py @@ -14,5 +14,5 @@ def setUpClass(cls): super().set_raw_dump_file('v0x01', 'ofpt_stats_reply') super().set_raw_dump_object(StatsReply, xid=1, body_type=StatsTypes.OFPST_FLOW, - flags=0x0001, body=[]) + flags=0x0001, body=b'') super().set_minimum_size(12) diff --git a/tests/v0x01/test_controller2switch/test_stats_request.py b/tests/v0x01/test_controller2switch/test_stats_request.py index c18704c97..f50e51825 100644 --- a/tests/v0x01/test_controller2switch/test_stats_request.py +++ b/tests/v0x01/test_controller2switch/test_stats_request.py @@ -14,5 +14,5 @@ def setUpClass(cls): super().set_raw_dump_file('v0x01', 'ofpt_stats_request') super().set_raw_dump_object(StatsRequest, xid=1, body_type=StatsTypes.OFPST_FLOW, - flags=1, body=[]) + flags=1, body=b'') super().set_minimum_size(12) From 7ba4f75b4128df227d760e5bf6f48d1b66490a5d Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Tue, 18 Jul 2017 11:57:52 -0300 Subject: [PATCH 17/20] linter fixes --- pyof/foundation/base.py | 30 ++++++++++++++-------- pyof/foundation/basic_types.py | 23 +++++++++-------- pyof/v0x04/common/flow_match.py | 18 +++++++------ pyof/v0x04/controller2switch/packet_out.py | 8 +++--- 4 files changed, 45 insertions(+), 34 deletions(-) diff --git a/pyof/foundation/base.py b/pyof/foundation/base.py index 147c0d1a9..f376d7810 100644 --- a/pyof/foundation/base.py +++ b/pyof/foundation/base.py @@ -264,6 +264,15 @@ def is_bitmask(self): class GenericUBIntType(GenericType): + r"""Base class for a generic Uint type. + + example: + class Uint8(GenericUBIntType): + _buff_size = 1 + + Uint8.pack(255) + b'\xff' + """ _buff_size = 0 @@ -523,10 +532,10 @@ class GenericStruct(object, metaclass=MetaStruct): too. """ - def __init__(self): + def __init__(self, value=None): """Contructor takes no argument and stores attributes' deep copies.""" - for name, value in self.get_class_attributes(): - setattr(self, name, deepcopy(value)) + for name, att_value in self.get_class_attributes(): + setattr(self, name, deepcopy(att_value)) def __eq__(self, other): """Check whether two structures have the same structure and values. @@ -654,14 +663,13 @@ def _work_or_pass(self, value, work_func): return getattr(self, work_func)() elif isinstance(value, type(self)): return getattr(value, work_func)() - else: - try: - new_item = type(self)(value) - except: # noqa - there is no generic Initialization Exception... - msg = "{} is not an instance of {}".format(value, - type(self).__name__) - raise PackException(msg) - return getattr(new_item, work_func)() + try: + new_item = type(self)(value) + except: # noqa - there is no generic Initialization Exception... + msg = "{} is not an instance of {}".format(value, + type(self).__name__) + raise PackException(msg) + return getattr(new_item, work_func)() def _get_size(self): return sum(cls_val.get_size(obj_val) diff --git a/pyof/foundation/basic_types.py b/pyof/foundation/basic_types.py index 80c0e3aac..ae6f3ad9e 100644 --- a/pyof/foundation/basic_types.py +++ b/pyof/foundation/basic_types.py @@ -331,7 +331,8 @@ def __init__(self, value=None, enum_ref=None): # noqa value = self._pack_if_necessary(value) super().__init__(value, enum_ref=enum_ref) - def _pack_if_necessary(self, value): + @staticmethod + def _pack_if_necessary(value): if hasattr(value, 'pack') and callable(value.pack): value = value.pack() @@ -346,6 +347,7 @@ def _pack(self): return self._value def pack(self, value=None): + """Pack the struct in a binary representation.""" if value is not None: value = self._pack_if_necessary(value) return super().pack(value) @@ -402,7 +404,7 @@ def _pack(self): bin_message = b'' try: for item in self: - bin_message += item._pack() + bin_message += item._pack() # noqa return bin_message except exceptions.PackException as err: msg = "{} pack error: {}".format(type(self).__name__, err) @@ -573,16 +575,15 @@ def insert(self, index, item): def get_custom_tlv_class(type_size=3, length_size=1): - """ Generate a CUstomTLV class + """Generate a CUstomTLV class. Create a CustomTLV class with the defined number of bytes for type and length fields. Args: - type_size (int): length in bytes for the type field of the TLV - length_size (int): length in bytes for the length field of the TLV + type_size (int): length in bytes for the type field of the TLV. + length_size (int): length in bytes for the length field of the TLV. """ - size_classes = {1: UBInt8, 2: UBInt16, 3: UBInt24, @@ -592,12 +593,13 @@ def get_custom_tlv_class(type_size=3, length_size=1): custom_length = size_classes[length_size] class CustomTLV(GenericStruct): - """ a compact TLV class + """A compact TLV class. Args: - tlv_type: type field of the TLV - tlv_value: length field of the TLV + tlv_type: type field of the TLV. + tlv_value: length field of the TLV. """ + tlv_type = custom_type() tlv_length = custom_length() tlv_value = BinaryData() @@ -617,13 +619,12 @@ def _pack(self): return super()._pack() def unpack(self, buff, offset=0): - """Unpack the buffer into a custom TLV + """Unpack the buffer into a custom TLV. Args: buff (bytes): The binary data to be unpacked. offset (int): If we need to shift the beginning of the data. """ - begin = offset for name, value in list(self.get_class_attributes())[:-1]: size = self._unpack_attribute(name, value, buff, begin) diff --git a/pyof/v0x04/common/flow_match.py b/pyof/v0x04/common/flow_match.py index 790489bad..1b12ecfbc 100644 --- a/pyof/v0x04/common/flow_match.py +++ b/pyof/v0x04/common/flow_match.py @@ -8,16 +8,14 @@ # Local source tree imports from pyof.foundation.base import GenericStruct -from pyof.foundation.basic_types import (FixedTypeList, UBInt8, UBInt16, - UBInt32, Pad, BinaryData, - CustomTLV_24_8) -from pyof.foundation.exceptions import PackException +from pyof.foundation.basic_types import ( + BinaryData, CustomTLV_24_8, FixedTypeList, Pad, UBInt8, UBInt16, UBInt32) # Third-party imports __all__ = ('Ipv6ExtHdrFlags', 'Match', 'OxmOfbMatchField', 'MatchType', - 'OxmExperimenterHeader', 'MatchField', 'VlanId') + 'OxmExperimenterHeader', 'OxmMatchFields', 'VlanId') class Ipv6ExtHdrFlags(Enum): @@ -185,7 +183,7 @@ class VlanId(IntEnum): # Classes class OxmType(GenericStruct): - """ Oxm TLV `type` metafield. + """Oxm TLV `type` metafield. OxmType is defined by the combination of a OxmClass and a OxmField, @@ -194,17 +192,19 @@ class OxmType(GenericStruct): oxm_field (:class:`OxmOfbMatchField`, Oxm*MatchField, int): the oxm TLV defined field of the correspondent class """ + oxm_class = UBInt16(enum_ref=OxmClass) oxm_field = UBInt8() def __init__(self, oxm_class, oxm_field): + super().__init__() cls = type(self) self.oxm_class = type(cls.oxm_class)(oxm_class) self.oxm_field = oxm_field class OxmTLV(GenericStruct): - """ Oxm (Openflow Extensible Match) TLV. + """Oxm (Openflow Extensible Match) TLV. Args: oxm_class (:class:`OxmClass`, int): The oxm TLV defined class. @@ -222,6 +222,7 @@ class OxmTLV(GenericStruct): def __init__(self, oxm_class=None, oxm_field=None, oxm_hasmask=None, oxm_value=None): + super().__init__() self.oxm_class = oxm_class self.oxm_field = oxm_field self.oxm_hasmask = oxm_hasmask if oxm_hasmask else 0 @@ -255,7 +256,7 @@ def _pack(self): tlv_value = self.oxm_value tlv = self.tlv_class(tlv_type, tlv_value) - return tlv._pack() + return tlv._pack() # noqa def _get_size(self): size = super()._get_size() - 1 @@ -327,6 +328,7 @@ def _get_size(self): return super_size + (8 - (super_size % 8)) % 8 def unpack(self, buff, offset=0): + """Unpack bytes buffer into this Instance.""" begin = offset for name, value in list(self.get_class_attributes())[:-1]: size = self._unpack_attribute(name, value, buff, begin) diff --git a/pyof/v0x04/controller2switch/packet_out.py b/pyof/v0x04/controller2switch/packet_out.py index 35116ca26..76455a880 100644 --- a/pyof/v0x04/controller2switch/packet_out.py +++ b/pyof/v0x04/controller2switch/packet_out.py @@ -3,7 +3,7 @@ from pyof.foundation.base import GenericMessage from pyof.foundation.basic_types import BinaryData, Pad, UBInt16, UBInt32 -from pyof.foundation.exceptions import PackException, ValidationError +from pyof.foundation.exceptions import ValidationError from pyof.v0x04.common.action import ListOfActions from pyof.v0x04.common.header import Header, Type from pyof.v0x04.common.port import Port, PortNo @@ -72,8 +72,8 @@ def is_valid(self): return False def _pack(self, value=None): - self._update_actions_len() - return super()._pack() + self._update_actions_len() + return super()._pack() def unpack(self, buff, offset=0): """Unpack a binary message into this object's attributes. @@ -97,7 +97,7 @@ def unpack(self, buff, offset=0): attribute = deepcopy(class_attribute) if attribute_name == 'actions': length = self.actions_len.value - attribute.unpack(buff[begin:begin+length]) + attribute.unpack(buff[begin:begin + length]) else: attribute.unpack(buff, begin) setattr(self, attribute_name, attribute) From 7a2c1006582921c85654568f533ca4f6af88fe8a Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Thu, 20 Jul 2017 10:43:12 -0300 Subject: [PATCH 18/20] Fix ActionSetField and make it use OxmTLV --- pyof/v0x04/common/action.py | 36 ++++++++++++++++++++------------- pyof/v0x04/common/flow_match.py | 2 +- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/pyof/v0x04/common/action.py b/pyof/v0x04/common/action.py index 7af360cfd..de678030f 100644 --- a/pyof/v0x04/common/action.py +++ b/pyof/v0x04/common/action.py @@ -6,6 +6,7 @@ from pyof.foundation.base import GenericStruct from pyof.foundation.basic_types import ( FixedTypeList, Pad, UBInt8, UBInt16, UBInt32) +from pyof.v0x04.common.flow_match import OxmTLV # Third-party imports @@ -278,14 +279,11 @@ class ActionSetField(GenericStruct): action_type = UBInt16(ActionType.OFPAT_SET_FIELD, enum_ref=ActionType) #: Length is padded to 64 bits. length = UBInt16() + pad = Pad(length=4) #: OXM TLV - Make compiler happy - field1 = UBInt8() - field2 = UBInt8() - field3 = UBInt8() - field4 = UBInt8() + field = OxmTLV() - def __init__(self, length=None, field1=None, field2=None, field3=None, - field4=None): + def __init__(self, length=None, field=None): """The constructor just assigns parameters to object attributes. Args: @@ -293,17 +291,27 @@ def __init__(self, length=None, field1=None, field2=None, field3=None, oxm_len bytes containing a single OXM TLV, then exactly ((oxm_len + 4) + 7)/8*8 - (oxm_len + 4) (between 0 and 7) bytes of all-zero bytes - field1 (int): OXM field. - field2 (int): OXM field. - field3 (int): OXM field. - field4 (int): OXM field. + field (OcmTLV): OXM field. """ super().__init__() self.length = length - self.field1 = field1 - self.field2 = field2 - self.field3 = field3 - self.field4 = field4 + self.field = field + + def _get_size(self): + super_size = super()._get_size() + return super_size + (8 - (super_size % 8)) % 8 + + def _update_length(self): + self.length = self._get_size() + + def _pack(self): + self._update_length() + packet = super()._pack() + super_size = len(packet) + lacking_bytes = self._get_size() - super_size + if lacking_bytes != 0: + packet += Pad(lacking_bytes).pack() + return packet class ActionSetQueue(GenericStruct): diff --git a/pyof/v0x04/common/flow_match.py b/pyof/v0x04/common/flow_match.py index 1b12ecfbc..87e9db156 100644 --- a/pyof/v0x04/common/flow_match.py +++ b/pyof/v0x04/common/flow_match.py @@ -15,7 +15,7 @@ __all__ = ('Ipv6ExtHdrFlags', 'Match', 'OxmOfbMatchField', 'MatchType', - 'OxmExperimenterHeader', 'OxmMatchFields', 'VlanId') + 'OxmExperimenterHeader', 'OxmMatchFields', 'VlanId', 'OxmTLV') class Ipv6ExtHdrFlags(Enum): From 24f44a4eb5d7cb52934d2d61e84b0884b63beccd Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Thu, 20 Jul 2017 11:03:02 -0300 Subject: [PATCH 19/20] linter fix --- pyof/v0x04/common/flow_match.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pyof/v0x04/common/flow_match.py b/pyof/v0x04/common/flow_match.py index 87e9db156..2d444fd28 100644 --- a/pyof/v0x04/common/flow_match.py +++ b/pyof/v0x04/common/flow_match.py @@ -222,6 +222,14 @@ class OxmTLV(GenericStruct): def __init__(self, oxm_class=None, oxm_field=None, oxm_hasmask=None, oxm_value=None): + """Init OxmTLV with oxm class, field and value. + + Args: + oxm_class (OxmClass): The oxm_class field. + oxm_field (int): The oxm_field field. + oxm_hasmask (int): The oxm_hasmask field. + oxm_value (int/bytes): The oxm_value field. + """ super().__init__() self.oxm_class = oxm_class self.oxm_field = oxm_field From 8292fbccda241c47b3acb57aa4c09387f01b94e1 Mon Sep 17 00:00:00 2001 From: Erick Vermot Date: Thu, 6 Jul 2017 17:58:35 -0300 Subject: [PATCH 20/20] tests for PacketIn, Match and OxmTLV structures --- .../v0x04/test_asynchronous/test_packet_in.py | 48 ++++++++++++++----- tests/v0x04/test_common/test_flow_match.py | 48 +++++++++++++++++++ 2 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 tests/v0x04/test_common/test_flow_match.py diff --git a/tests/v0x04/test_asynchronous/test_packet_in.py b/tests/v0x04/test_asynchronous/test_packet_in.py index 12a27dfb9..e2cea2e0f 100644 --- a/tests/v0x04/test_asynchronous/test_packet_in.py +++ b/tests/v0x04/test_asynchronous/test_packet_in.py @@ -1,19 +1,41 @@ """Packet in message tests.""" from pyof.v0x04.asynchronous.packet_in import PacketIn, PacketInReason -from tests.test_struct import TestStruct +from pyof.v0x04.common.flow_match import OxmTLV, Match +from tests.test_struct import TestMsgDump, TestMsgDumpFile -class TestPacketIn(TestStruct): +oxmtlv = OxmTLV(oxm_class=32768, + oxm_field=0, + oxm_hasmask=0, + oxm_value=b'\x00\x00\x00\x16') + +match = Match(match_type=1, + oxm_match_fields=[oxmtlv]) + +packetin = PacketIn(xid=0, + buffer_id=257, + total_len=81, + reason=1, + table_id=0, + cookie=18446744073709551615, + match=match, + data=81 * b'\x00') + +dump = b'\x04\n\x00{\x00\x00\x00\x00\x00\x00\x01\x01\x00Q\x01\x00\xff\xff' +dump += b'\xff\xff\xff\xff\xff\xff\x00\x01\x00\x0c\x80\x00\x00\x04\x00\x00' +dump += b'\x00\x16\x00\x00\x00\x00\x00\x00' +dump += 81 * b'\x00' + + +class TestPacketInDump(TestMsgDump): """Packet in message tests (also those in :class:`.TestDump`).""" + obj = packetin + dump = dump + - @classmethod - def setUpClass(cls): - """Configure raw file and its object in parent class (TestDump).""" - super().setUpClass() - super().set_raw_dump_file('v0x04', 'ofpt_packet_in') - super().set_raw_dump_object(PacketIn, xid=1, buffer_id=1, total_len=1, - reason=PacketInReason.OFPR_ACTION, - table_id=1, cookie=1, data=b'') - # Different from the specification, the minimum size of this class is - # 18, not 20. - super().set_minimum_size(34) +class TestPacketInDumpFile(TestMsgDumpFile): + """Packet in message tests (also those in :class:`.TestDump`).""" + dumpfile = 'v0x04/ofpt_packet_in.dat' + obj = PacketIn(xid=1, buffer_id=1, total_len=1, + reason=PacketInReason.OFPR_ACTION, + table_id=1, cookie=1, data=b'') diff --git a/tests/v0x04/test_common/test_flow_match.py b/tests/v0x04/test_common/test_flow_match.py new file mode 100644 index 000000000..a72e2d360 --- /dev/null +++ b/tests/v0x04/test_common/test_flow_match.py @@ -0,0 +1,48 @@ +"""Testing FlowMatch structure.""" +from pyof.v0x04.common import flow_match as fm +from tests.test_struct import TestStructDump + + +class TestOxmTLV1(TestStructDump): + dump = b'\x80\x00\x00\x03abc' + obj = fm.OxmTLV(fm.OxmClass.OFPXMC_OPENFLOW_BASIC, # class + fm.OxmOfbMatchField.OFPXMT_OFB_IN_PORT, # field + 0, # mask + b'abc') # value + + +class TestOxmTLV2(TestStructDump): + dump = b'\x80\x00\x00\x02ab' + obj = fm.OxmTLV(fm.OxmClass.OFPXMC_EXPERIMENTER, + 2, + 0, + b'de') + + +class TestOxmTLV3(TestStructDump): + dump = b'\x80\x00\x00\x04fghi' + obj = fm.OxmTLV(fm.OxmClass.OFPXMC_OPENFLOW_BASIC, + fm.OxmOfbMatchField.OFPXMT_OFB_IN_PHY_PORT, + 0, + b'fghi') + + +class TestMatch(TestStructDump): + oxm1_dump = TestOxmTLV1.dump + oxm2_dump = TestOxmTLV2.dump + oxm3_dump = TestOxmTLV3.dump + + oxm1_obj = TestOxmTLV1.obj + oxm2_obj = TestOxmTLV2.obj + oxm3_obj = TestOxmTLV3.obj + + dump = (b'\x00\x01\x00\x19' + + oxm1_dump + + oxm2_dump + + oxm3_dump) + + obj = fm.Match( + match_type=fm.MatchType.OFPMT_OXM, + oxm_match_fields=fm.OxmMatchFields([oxm1_obj, + oxm2_obj, + oxm3_obj]))