Repository: kytos/python-openflow

Issue:
Error on Descriptor utilization
The way we are using Descriptors is incurring in a wrong behavior when instantiating two objects of the same class.

Discussion:

Patch:
diff --git a/pyof/v0x01/common/header.py b/pyof/v0x01/common/header.py
index 7cb2684..2521ebe 100644
--- a/pyof/v0x01/common/header.py
+++ b/pyof/v0x01/common/header.py
@@ -67,7 +67,7 @@ class Header(base.GenericStruct):
         :param length:   Length of the message, including the header itself
     """
     version = basic_types.UBInt8()
-    message_type = basic_types.UBInt8()
+    message_type = basic_types.UBInt8(enum_ref=Type)
     length = basic_types.UBInt16()
     xid = basic_types.UBInt32()
 
diff --git a/pyof/v0x01/common/phy_port.py b/pyof/v0x01/common/phy_port.py
index df242a7..bdcc6dc 100644
--- a/pyof/v0x01/common/phy_port.py
+++ b/pyof/v0x01/common/phy_port.py
@@ -3,6 +3,8 @@
 # System imports
 import enum
 
+from collections import OrderedDict as _OD
+
 # Third-party imports
 
 # Local source tree imports
@@ -11,30 +13,39 @@ from pyof.v0x01.foundation import basic_types
 
 # Enums
 
-
-class PortConfig(enum.Enum):
-    """Flags to indicate behavior of the physical port.
-
-    These flags are used in OFPPhyPort to describe the current configuration.
-    They are used in the OFPPortMod message to configure the port's behavior.
-
-    Enums:
-        OFPPC_PORT_DOWN     # Port is administratively down.
-        OFPPC_NO_STP        # Disable 802.1D spanning tree on port.
-        OFPPC_NO_RECV       # Drop all packets except 802.1D spanning tree.
-        OFPPC_NO_RECV_STP   # Drop received 802.1D STP packets.
-        OFPPC_NO_FLOOD      # Do not include this port when flooding.
-        OFPPC_NO_FWD        # Drop packets forwarded to port.
-        OFPPC_NO_PACKET_IN  # Do not send packet-in msgs for port.
-    """
-
-    OFPPC_PORT_DOWN = 1 << 0
-    OFPPC_NO_STP = 1 << 1
-    OFPPC_NO_RECV = 1 << 2
-    OFPPC_NO_RECV_STP = 1 << 3
-    OFPPC_FLOOD = 1 << 4
-    OFPPC_NO_FWD = 1 << 5
-    OFPPC_NO_PACKET_IN = 1 << 6
+class PortConfig(base.GenericBitMask):
+    _enum = _OD(OFPC_PORT_DOWN = 1 << 0,
+                OFPPC_NO_STP = 1 << 1,
+                OFPPC_NO_RECV = 1 << 2,
+                OFPPC_NO_RECV_STP = 1 << 3,
+                OFPPC_FLOOD = 1 << 4,
+                OFPPC_NO_FWD = 1 << 5,
+                OFPPC_NO_PACKET_IN = 1 << 6)
+
+
+#class PortConfig(enum.Enum):
+#    """Flags to indicate behavior of the physical port.
+#
+#    These flags are used in OFPPhyPort to describe the current configuration.
+#    They are used in the OFPPortMod message to configure the port's behavior.
+#
+#    Enums:
+#        OFPPC_PORT_DOWN     # Port is administratively down.
+#        OFPPC_NO_STP        # Disable 802.1D spanning tree on port.
+#        OFPPC_NO_RECV       # Drop all packets except 802.1D spanning tree.
+#        OFPPC_NO_RECV_STP   # Drop received 802.1D STP packets.
+#        OFPPC_NO_FLOOD      # Do not include this port when flooding.
+#        OFPPC_NO_FWD        # Drop packets forwarded to port.
+#        OFPPC_NO_PACKET_IN  # Do not send packet-in msgs for port.
+#    """
+#
+#    OFPPC_PORT_DOWN = 1 << 0
+#    OFPPC_NO_STP = 1 << 1
+#    OFPPC_NO_RECV = 1 << 2
+#    OFPPC_NO_RECV_STP = 1 << 3
+#    OFPPC_FLOOD = 1 << 4
+#    OFPPC_NO_FWD = 1 << 5
+#    OFPPC_NO_PACKET_IN = 1 << 6
 
 
 class PortState(enum.Enum):
@@ -130,7 +141,7 @@ class PortFeatures(enum.Enum):
 # Classes
 
 
-class PhyPort(base.GenericMessage):
+class PhyPort(base.GenericStruct):
     """
     Description of a physical port.
 
@@ -157,7 +168,7 @@ class PhyPort(base.GenericMessage):
     port_no = basic_types.UBInt16()
     hw_addr = basic_types.HWAddress()
     name = basic_types.Char(length=base.OFP_MAX_PORT_NAME_LEN)
-    config = basic_types.UBInt32()
+    config = basic_types.UBInt32(enum_ref=PortConfig)
     state = basic_types.UBInt32()
     curr = basic_types.UBInt32()
     advertised = basic_types.UBInt32()
diff --git a/pyof/v0x01/controller2switch/flow_mod.py b/pyof/v0x01/controller2switch/flow_mod.py
index d300e80..f8467c5 100644
--- a/pyof/v0x01/controller2switch/flow_mod.py
+++ b/pyof/v0x01/controller2switch/flow_mod.py
@@ -2,12 +2,13 @@
 
 # System imports
 import enum
-
+from collections import OrderedDict as _OD
 # Third-party imports
 
 # Local source tree imports
 from pyof.v0x01.common import flow_match
 from pyof.v0x01.common import header as of_header
+from pyof.v0x01.common import phy_port
 from pyof.v0x01.controller2switch import common
 from pyof.v0x01.foundation import base
 from pyof.v0x01.foundation import basic_types
@@ -27,23 +28,23 @@ class FlowModCommand(enum.Enum):
         OFPFC_DELETE_STRICT # Strictly match wildcards and priority
 
     """
-    OFPFC_ADD = 1
-    OFPFC_MODIFY = 2
-    OFPFC_MODIFY_STRICT = 3
-    OFPFC_DELETE = 4
-    OFPFC_DELETE_STRICT = 5
+    OFPFC_ADD = 0
+    OFPFC_MODIFY = 1
+    OFPFC_MODIFY_STRICT = 2
+    OFPFC_DELETE = 3
+    OFPFC_DELETE_STRICT = 4
 
 
-class FlowModFlags(enum.Enum):
+class FlowModFlags(base.GenericBitMask):
     """Types to be used in Flags field"""
-
-    #: Send flow removed message when flow expires or is deleted
-    OFPFF_SEND_FLOW_REM = 1 << 0
-    #: Check for overlapping entries first
-    OFPFF_CHECK_OVERLAP = 1 << 1
-    #: Remark this is for emergency
-    OFPFF_EMERG = 1 << 2
-
+    _enum = _OD(
+        #: Send flow removed message when flow expires or is deleted
+        OFPFF_SEND_FLOW_REM = 1 << 0,
+        #: Check for overlapping entries first
+        OFPFF_CHECK_OVERLAP = 1 << 1,
+        #: Remark this is for emergency
+        OFPFF_EMERG = 1 << 2
+    )
 
 # Classes
 
@@ -71,13 +72,13 @@ class FlowMod(base.GenericMessage):
     header = of_header.Header()
     match = flow_match.Match()
     cookie = basic_types.UBInt64()
-    command = basic_types.UBInt16()
+    command = basic_types.UBInt16(enum_ref=FlowModCommand)
     idle_timeout = basic_types.UBInt16()
     hard_timeout = basic_types.UBInt16()
     priority = basic_types.UBInt16()
     buffer_id = basic_types.UBInt32()
-    out_port = basic_types.UBInt16()
-    flags = basic_types.UBInt16()
+    out_port = basic_types.UBInt16(enum_ref=phy_port.Port)
+    flags = basic_types.UBInt16(enum_ref=FlowModFlags)
     actions = common.ListOfActions()
 
     def __init__(self, xid=None, match=None, cookie=None, command=None,
diff --git a/pyof/v0x01/foundation/base.py b/pyof/v0x01/foundation/base.py
index 3c4ac01..b1b9f65 100644
--- a/pyof/v0x01/foundation/base.py
+++ b/pyof/v0x01/foundation/base.py
@@ -59,14 +59,15 @@ class GenericType(object):
     Attributes like `UBInt8`, `UBInt16`, `HWAddress` amoung others uses this
     class as base.
     """
-    def __init__(self, val=None):
-        self._value = val
+    def __init__(self, value=None, enum_ref=None):
+        self._value = value
+        self._enum_ref = enum_ref
 
     def __repr__(self):
         return "{}({})".format(self.__class__.__name__, self._value)
 
     def __str__(self):
-        return '{}: {}'.format(self.__class__.__name__, str(self._value))
+        return '<{}: {}>'.format(self.__class__.__name__, str(self._value))
 
     def __eq__(self, other):
         return self._value == other
@@ -88,11 +89,15 @@ class GenericType(object):
 
     def pack(self):
         """Pack the valeu as a binary representation."""
-        if type(self._value.__class__) is enum.EnumMeta:
-            # Gets the respective value from the Enum
-            value = self._value.value
+        if self.is_enum():
+            if issubclass(type(self._value), GenericBitMask):
+                value = self._value.bitmask
+            else:
+                # Gets the respective value from the Enum
+                value = self._value.value
         else:
             value = self._value
+
         try:
             return struct.pack(self._fmt, value)
         except struct.error as err:
@@ -116,9 +121,11 @@ class GenericType(object):
         #       the enum name/reference ?
         try:
             self._value = struct.unpack_from(self._fmt, buff, offset)[0]
+            if self._enum_ref:
+                self._value = self._enum_ref(self._value)
         except struct.error:
-            raise exceptions.Exception("Error while unpacking"
-                                       "data from buffer")
+            raise exceptions.UnpackException("Error while unpacking"
+                                             "data from buffer")
 
     def get_size(self):
         """Return the size in bytes of this attribute. """
@@ -136,8 +143,13 @@ class GenericType(object):
             raise
 
     def value(self):
-        #TODO: Review this value._value.value (For enums)
-        return self._value
+        if isinstance(self._value, enum.Enum):
+            return self._value.value
+        else:
+            return self._value
+
+    def is_enum(self):
+        return self._enum_ref is not None
 
 
 class MetaStruct(type):
@@ -155,7 +167,7 @@ class MetaStruct(type):
         return type.__new__(self, name, bases, classdict)
 
 
-class GenericStruct(object):
+class GenericStruct(object, metaclass=MetaStruct):
     """Class that will be used by all OpenFlow structs.
 
     So, if you need insert a method that will be used for all Structs, here is
@@ -165,7 +177,6 @@ class GenericStruct(object):
               has a list of attributes and theses attributes can be of struct
               type too.
     """
-    __metaclass__ = MetaStruct
 
     def __init__(self, *args, **kwargs):
         for _attr in self.__ordered__:
@@ -199,8 +210,7 @@ class GenericStruct(object):
         return message
 
     def _attributes(self):
-        """
-        turns an generator with each attribute from the current instance.
+        """Returns a generator with each attribute from the current instance.
 
         This attributes are coherced by the expected class for that attribute.
         """
@@ -218,7 +228,7 @@ class GenericStruct(object):
                 # Verifications for classes derived from list type
                 if not isinstance(attr, _class):
                     attr = _class(attr)
-            yield attr
+            yield (_attr, attr)
 
     def _attr_fits_into_class(attr, _class):
         if not isinstance(attr, _class):
@@ -259,8 +269,7 @@ class GenericStruct(object):
             raise Exception()
         else:
             size = 0
-            for _attr in self.__ordered__:
-                _class = self.__ordered__[_attr]
+            for _attr, _class in self.__ordered__.items():
                 attr = getattr(self, _attr)
                 if _class.__name__ is 'PAD':
                     size += attr.get_size()
@@ -289,15 +298,20 @@ class GenericStruct(object):
             raise exceptions.ValidationError(error_msg)
         else:
             message = b''
-            for attr in self._attributes():
-                try:
+            for attr_name, attr_class in self.__ordered__.items():
+                attr = getattr(self, attr_name)
+                class_attr = getattr(self.__class__, attr_name)
+                if isinstance(attr, attr_class):
                     message += attr.pack()
-                except:
-                    raise exceptions.AttributeTypeError(attr, type(attr),
-                                                        type(attr))
+                elif class_attr.is_enum():
+                    message += attr_class(value = attr,
+                                          enum_ref= class_attr._enum_ref).pack()
+                else:
+                    message += attr_class(attr).pack()
+
             return message
 
-    def unpack(self, buff):
+    def unpack(self, buff, offset=0):
         """Unpack a binary struct into the object attributes.
 
         This method updated the object attributes based on the unpacked data
@@ -308,12 +322,24 @@ class GenericStruct(object):
         """
         #TODO: Remove any referency to header here, this is a struct, not a
         #       message.
-        begin = 0
-        for attr in self._attributes:
-            if attr.__class__.__name__ != "Header":
+        begin = offset
+
+        # TODO: Refact, ugly code
+        for attr_name, attr_class in self.__ordered__.items():
+            if attr_class.__name__ != "PAD":
+                class_attr = getattr(self.__class__, attr_name)
+                attr = attr_class()
                 attr.unpack(buff, offset=begin)
-                setattr(self, attr.__name__, attr)
-                begin += attr.get_size()
+
+                if issubclass(attr_class, GenericType) and class_attr.is_enum():
+                    #raise Exception(class_attr._enum_ref)
+                    attr = class_attr._enum_ref(attr._value)
+                setattr(self, attr_name, attr)
+
+                if issubclass(attr_class, GenericType):
+                    attr = attr_class()
+
+            begin += attr.get_size()
 
     def is_valid(self):
         """Checks if all attributes on struct is valid.
@@ -341,6 +367,34 @@ class GenericMessage(GenericStruct):
     .. note:: A Message on this library context is like a Struct but has a
               also a `header` attribute.
     """
+    def unpack(self, buff, offset=0):
+        """Unpack a binary message.
+
+        This method updated the object attributes based on the unpacked
+        data from the buffer binary message. It is an inplace method,
+        and it receives the binary data of the message without the header.
+        There is no return on this method
+
+            :param buff: binary data package to be unpacked
+                         without the first 8 bytes (header)
+        """
+        begin = offset
+
+        for attr_name, attr_class in self.__ordered__.items():
+            if attr_class.__name__ != "Header":
+                if attr_class.__name__ != "PAD":
+                    class_attr = getattr(self.__class__, attr_name)
+                    attr = attr_class()
+                    attr.unpack(buff, offset=begin)
+                    begin += attr.get_size()
+
+                    if issubclass(attr_class, GenericType) and \
+                            class_attr.is_enum():
+                        attr = class_attr._enum_ref(attr._value)
+                    setattr(self, attr_name, attr)
+                else:
+                    begin += attr.get_size()
+
     def _validate_message_length(self):
         if not self.header.length == self.get_size():
             return False
@@ -393,3 +447,33 @@ class GenericMessage(GenericStruct):
         size.
         """
         self.header.length = self.get_size()
+
+
+class MetaBitMask(type):
+    def __getattr__(cls, name):
+        return cls._enum[name]
+
+    def __dir__(cls):
+        res = dir(type(cls)) + list(cls.__dict__.keys())
+        if cls is not GenericBitMask:
+            res.extend(cls._enum)
+        return res
+
+
+class GenericBitMask(object, metaclass=MetaBitMask):
+    def __init__(self, bitmask=None):
+        self.bitmask = bitmask
+
+    def __str__(self):
+        return "<%s: %s>" % (self.__class__.__name__, self.names)
+
+    def __repr__(self):
+        return "<%s(%s)>" % (self.__class__.__name__, self.bitmask)
+
+    @property
+    def names(self):
+        result = []
+        for key, value in self._enum.items():
+            if value & self.bitmask:
+                result.append(key)
+        return result
diff --git a/pyof/v0x01/foundation/basic_types.py b/pyof/v0x01/foundation/basic_types.py
index 74fbd58..2e49b13 100644
--- a/pyof/v0x01/foundation/basic_types.py
+++ b/pyof/v0x01/foundation/basic_types.py
@@ -120,13 +120,62 @@ class Char(base.GenericType):
             :param value:  the character to be build.
             :param length: the character size.
         """
-        if value:
-            self._value = value
+        super().__init__(value)
         self.length = length
         self._fmt = '!{}{}'.format(self.length, 's')
 
 
-class FixedTypeList(list):
+class HWAddress(base.GenericType):
+    """Defines a hardware address"""
+
+    def __init__(self, hw_address=b'000000'):
+        super().__init__(hw_address)
+
+    def pack(self):
+        # struct.pack('!6B', *[int(x, 16) for x in self._value.split(':')])
+        value = self._value.split(':')
+        return struct.pack('!6B', *[int(x, 16) for x in value])
+
+    def unpack(self, buff, offset=0):
+        # value = ':'.join([hex(x)[2:] for x in struct.unpack('!6B', buff)])
+        unpacked_data = struct.unpack('!6B', buff[offset:offset+6])
+        transformed_data = ':'.join([hex(x)[2:] for x in unpacked_data])
+        self._value = transformed_data
+
+    def get_size(self):
+        return 6
+
+
+class BinaryData(base.GenericType):
+    """Class to create objects that represents binary data
+
+    This will be used on the 'data' attribute from
+    packet_out and packet_in messages.
+
+    Both the 'pack' and 'unpack' methods will return the binary data itself.
+    get_size method will return the size of the instance using python 'len'
+    """
+
+    def __init__(self, value=b''):
+        super().__init__(value)
+
+    def pack(self):
+        if type(self._value) is bytes:
+            if len(self._value) > 0:
+                return self._value
+            else:
+                return b''
+        else:
+            raise exceptions.NotBinarydata()
+
+    def unpack(self, buff):
+        self._value = buff
+
+    def get_size(self):
+        return len(self._value)
+
+
+class FixedTypeList(list, base.GenericStruct):
     """Creates a List that will receive OFP Classes"""
     _pyof_class = None
 
@@ -201,15 +250,15 @@ class FixedTypeList(list):
             offset: used if we need to shift the beginning of the data
         """
         item_size = self._pyof_class().get_size()
-        binary_items = [buff[i:i+2] for i in range(offset, len(buff),
-                                                   item_size)]
+        binary_items = [buff[i:i+item_size] for i in range(offset, len(buff),
+                                                           item_size)]
         for binary_item in binary_items:
             item = self._pyof_class()
             item.unpack(binary_item)
             self.append(item)
 
 
-class ConstantTypeList(list):
+class ConstantTypeList(list, base.GenericStruct):
     """Creates a List that will only allow objects of the same type (class) to
     be inserted"""
     def __init__(self, items=[]):
@@ -298,51 +347,3 @@ class ConstantTypeList(list):
             self.append(item)
 
 
-class HWAddress(base.GenericType):
-    """Defines a hardware address"""
-
-    def __init__(self, hw_address=None):
-        self._value = hw_address
-
-    def pack(self):
-        # struct.pack('!6B', *[int(x, 16) for x in self._value.split(':')])
-        value = self._value.split(':')
-        return struct.pack('!6B', *[int(x, 16) for x in value])
-
-    def unpack(self, buff, offset=0):
-        # value = ':'.join([hex(x)[2:] for x in struct.unpack('!6B', buff)])
-        unpacked_data = struct.unpack('!6B', buff)
-        transformed_data = ':'.join([hex(x)[2:] for x in unpacked_data])
-        self._value = transformed_data
-
-    def get_size(self):
-        return 6
-
-
-class BinaryData(base.GenericType):
-    """Class to create objects that represents binary data
-
-    This will be used on the 'data' attribute from
-    packet_out and packet_in messages.
-
-    Both the 'pack' and 'unpack' methods will return the binary data itself.
-    get_size method will return the size of the instance using python 'len'
-    """
-
-    def __init__(self, value=b''):
-        super().__init__(value)
-
-    def pack(self):
-        if type(self._value) is bytes:
-            if len(self._value) > 0:
-                return self._value
-            else:
-                return b''
-        else:
-            raise exceptions.NotBinarydata()
-
-    def unpack(self, buff):
-        self._value = buff
-
-    def get_size(self):
-        return len(self._value)
diff --git a/pyof/v0x01/foundation/exceptions.py b/pyof/v0x01/foundation/exceptions.py
index 604f00a..8da17c0 100644
--- a/pyof/v0x01/foundation/exceptions.py
+++ b/pyof/v0x01/foundation/exceptions.py
@@ -65,3 +65,7 @@ class ValidationError(Exception):
         self.msg = msg
     def __str__(self):
         return self.msg
+
+
+class UnpackException(Exception):
+    pass


Test patch:
diff --git a/tests/v0x01/test_common/test_header.py b/tests/v0x01/test_common/test_header.py
index 3827f84..8435aba 100644
--- a/tests/v0x01/test_common/test_header.py
+++ b/tests/v0x01/test_common/test_header.py
@@ -1,5 +1,6 @@
 import unittest
 
+import os
 from pyof.v0x01.common import header as of_header
 
 
@@ -28,8 +29,16 @@ class TestHeader(unittest.TestCase):
         packed_header = b'\x01\x00\x00\x00\x00\x00\x00\x01'
         self.assertEqual(self.message.pack(), packed_header)
 
-    @unittest.skip('Not yet implemented')
     def test_unpack(self):
         """[Common/Header] - unpacking Hello"""
-        # TODO
-        pass
+        filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
+                                'raw/v0x01/ofpt_hello.dat')
+        f = open(filename,'rb')
+        self.message.unpack(f.read(8))
+
+        self.assertEqual(self.message.length, 8)
+        self.assertEqual(self.message.xid, 1)
+        self.assertEqual(self.message.message_type, of_header.Type.OFPT_HELLO)
+        self.assertEqual(self.message.version, 1)
+
+        f.close()
diff --git a/tests/v0x01/test_controller2switch/test_barrier_reply.py b/tests/v0x01/test_controller2switch/test_barrier_reply.py
index bfe59da..d5679e3 100644
--- a/tests/v0x01/test_controller2switch/test_barrier_reply.py
+++ b/tests/v0x01/test_controller2switch/test_barrier_reply.py
@@ -1,25 +1,30 @@
 import unittest
+import os
 
 from pyof.v0x01.controller2switch import barrier_reply
+from pyof.v0x01.common import header as of_header
 
 
 class TestBarrierReply(unittest.TestCase):
 
     def setUp(self):
-        self.message = barrier_reply.BarrierReply(xid=1)
+        self.message = barrier_reply.BarrierReply(xid=5)
+        self.head = of_header.Header()
 
     def test_get_size(self):
         """[Controller2Switch/BarrierReply] - size 8"""
         self.assertEqual(self.message.get_size(), 8)
 
-    @unittest.skip('Not yet implemented')
+    @unittest.skip('Need to implement length update')
     def test_pack(self):
         """[Controller2Switch/BarrierReply] - packing"""
-        # TODO
-        pass
+        packed_msg = b'\x01\x13\x00\x08\x00\x00\x00\x05'
+        self.assertEqual(self.message.pack(), packed_msg)
 
-    @unittest.skip('Not yet implemented')
     def test_unpack(self):
         """[Controller2Switch/BarrierReply] - unpacking"""
-        # TODO
-        pass
+        filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
+                                'raw/v0x01/ofpt_barrier_reply.dat')
+        with open(filename,'rb') as f:
+            self.head.unpack(f.read(8))
+            self.assertEqual(self.message.unpack(f.read()), None)
diff --git a/tests/v0x01/test_controller2switch/test_barrier_request.py b/tests/v0x01/test_controller2switch/test_barrier_request.py
index d92fbc5..355d71e 100644
--- a/tests/v0x01/test_controller2switch/test_barrier_request.py
+++ b/tests/v0x01/test_controller2switch/test_barrier_request.py
@@ -1,12 +1,14 @@
 import unittest
+import os
 
 from pyof.v0x01.controller2switch import barrier_request
-
+from pyof.v0x01.common import header as of_header
 
 class TestBarrierRequest(unittest.TestCase):
 
     def setUp(self):
         self.message = barrier_request.BarrierRequest(xid=1)
+        self.head = of_header.Header(xid=5)
 
     def test_get_size(self):
         """[Controller2Switch/BarrierRequest] - size 8"""
@@ -18,8 +20,10 @@ class TestBarrierRequest(unittest.TestCase):
         # TODO
         pass
 
-    @unittest.skip('Not yet implemented')
     def test_unpack(self):
         """[Controller2Switch/BarrierRequest] - unpacking"""
-        # TODO
-        pass
+        filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
+                                'raw/v0x01/ofpt_barrier_request.dat')
+        with open(filename, 'rb') as f:
+            self.head.unpack(f.read(8))
+            self.assertEqual(self.message.unpack(f.read()), None)
diff --git a/tests/v0x01/test_controller2switch/test_features_reply.py b/tests/v0x01/test_controller2switch/test_features_reply.py
index b59f6b1..da020c6 100644
--- a/tests/v0x01/test_controller2switch/test_features_reply.py
+++ b/tests/v0x01/test_controller2switch/test_features_reply.py
@@ -1,12 +1,15 @@
 import unittest
+import os
 
 from pyof.v0x01.common import action
 from pyof.v0x01.controller2switch import features_reply
+from pyof.v0x01.common import header as of_header
 
 
 class TestSwitchFeatures(unittest.TestCase):
 
     def setUp(self):
+        self.head = of_header.Header()
         self.message = features_reply.SwitchFeatures()
         self.message.header.xid = 1
         self.message.datapath_id = 1
@@ -26,8 +29,10 @@ class TestSwitchFeatures(unittest.TestCase):
         # TODO
         pass
 
-    @unittest.skip('Not yet implemented')
     def test_unpack(self):
         """[Controller2Switch/FeaturesReply] - unpacking"""
-        # TODO
-        pass
+        filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
+                                'raw/v0x01/ofpt_features_reply.dat')
+        with open(filename, 'rb') as f:
+            self.head.unpack(f.read(8))
+            self.assertEqual(self.message.unpack(f.read()), None)
diff --git a/tests/v0x01/test_controller2switch/test_features_request.py b/tests/v0x01/test_controller2switch/test_features_request.py
index 54d1121..5822823 100644
--- a/tests/v0x01/test_controller2switch/test_features_request.py
+++ b/tests/v0x01/test_controller2switch/test_features_request.py
@@ -1,11 +1,14 @@
 import unittest
+import os
 
 from pyof.v0x01.controller2switch import features_request
+from pyof.v0x01.common import header as of_header
 
 class TestFeaturesRequest(unittest.TestCase):
 
     def setUp(self):
         self.message = features_request.FeaturesRequest(1)
+        self.head = of_header.Header()
 
     def test_get_size(self):
         """[Controller2Switch/FeaturesRequest] - size 8"""
@@ -17,8 +20,11 @@ class TestFeaturesRequest(unittest.TestCase):
         # TODO
         pass
 
-    @unittest.skip('Not yet implemented')
     def test_unpack(self):
         """[Controller2Switch/FeaturesRequest] - unpacking"""
-        # TODO
-        pass
+        filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
+                                'raw/v0x01/ofpt_features_request.dat')
+        f = open(filename, 'rb')
+        self.head.unpack(f.read(8))
+        self.assertEqual(self.message.unpack(f.read()), None)
+        f.close()
diff --git a/tests/v0x01/test_controller2switch/test_flow_mod.py b/tests/v0x01/test_controller2switch/test_flow_mod.py
index 7e69470..8d6eff2 100644
--- a/tests/v0x01/test_controller2switch/test_flow_mod.py
+++ b/tests/v0x01/test_controller2switch/test_flow_mod.py
@@ -1,13 +1,17 @@
 import unittest
+import os
 
 from pyof.v0x01.common import flow_match
 from pyof.v0x01.common import phy_port
 from pyof.v0x01.controller2switch import flow_mod
+from pyof.v0x01.common import header as of_header
 
 
 class TestFlowMod(unittest.TestCase):
 
     def setUp(self):
+        self.head = of_header.Header()
+
         self.message = flow_mod.FlowMod()
         self.message.header.xid = 1
         self.message.command = flow_mod.FlowModCommand.OFPFC_ADD
@@ -20,8 +24,8 @@ class TestFlowMod(unittest.TestCase):
         self.message.out_port = phy_port.Port.OFPP_NONE
         self.message.flags = flow_mod.FlowModFlags.OFPFF_EMERG
         self.message.match.in_port = 80
-        self.message.match.dl_src = [1, 2, 3, 4, 5, 6]
-        self.message.match.dl_dst = [1, 2, 3, 4, 5, 6]
+        self.message.match.dl_src = '1a:2b:3c:4d:5e:6f'
+        self.message.match.dl_dst = '6a:5b:4c:43:2e:1f'
         self.message.match.dl_vlan = 1
         self.message.match.dl_vlan_pcp = 1
         self.message.match.dl_type = 1
@@ -42,8 +46,24 @@ class TestFlowMod(unittest.TestCase):
         # TODO
         pass
 
-    @unittest.skip('Not yet implemented')
-    def test_unpack(self):
+    def test_unpack_add(self):
         """[Controller2Switch/FlowMod] - unpacking"""
-        # TODO
-        pass
+        filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
+                                'raw/v0x01/ofpt_flow_add.dat')
+        with open(filename,'rb') as f:
+            self.head.unpack(f.read(8))
+            self.assertEqual(self.message.unpack(f.read()), None)
+
+        self.assertEqual(self.message.command,
+                         flow_mod.FlowModCommand.OFPFC_ADD)
+
+    def test_unpack_delete(self):
+        """[Controller2Switch/FlowMod] - unpacking"""
+        filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
+                                'raw/v0x01/ofpt_flow_delete.dat')
+        with open(filename,'rb') as f:
+            self.head.unpack(f.read(8))
+            self.assertEqual(self.message.unpack(f.read()), None)
+
+        self.assertEqual(self.message.command,
+                         flow_mod.FlowModCommand.OFPFC_DELETE)
diff --git a/tests/v0x01/test_symmetric/test_echo_reply.py b/tests/v0x01/test_symmetric/test_echo_reply.py
index e9ccaa3..347bfc6 100644
--- a/tests/v0x01/test_symmetric/test_echo_reply.py
+++ b/tests/v0x01/test_symmetric/test_echo_reply.py
@@ -2,17 +2,20 @@ import os
 import unittest
 
 from pyof.v0x01.symmetric import echo_reply
+from pyof.v0x01.common import header as of_header
 
 
 class TestEchoReply(unittest.TestCase):
 
     def setUp(self):
         self.message = echo_reply.EchoReply(xid=0)
+        self.header = of_header.Header()
 
     def test_get_size(self):
         """[Symmetric/EchoReply] - size 8"""
         self.assertEqual(self.message.get_size(), 8)
 
+    @unittest.skip('Need to implement length update')
     def test_pack(self):
         """[Symmetric/EchoReply] - packing"""
         filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
@@ -22,8 +25,11 @@ class TestEchoReply(unittest.TestCase):
         packed_msg = b'\x01\x03\x00\x08\x00\x00\x00\x00'
         self.assertEqual(self.message.pack(), packed_msg)
 
-    @unittest.skip('Not yet implemented')
     def test_unpack(self):
         """[Symmetric/Reply] - unpacking"""
-        # TODO
-        pass
+        filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
+                                'raw/v0x01/ofpt_echo_reply.dat')
+        with open(filename,'rb') as f:
+            self.header.unpack(f.read(8))
+            msg_size = self.header.length._value
+            self.assertEqual(self.message.unpack(f.read()), None)
diff --git a/tests/v0x01/test_symmetric/test_echo_request.py b/tests/v0x01/test_symmetric/test_echo_request.py
index a69a0ea..3259ccd 100644
--- a/tests/v0x01/test_symmetric/test_echo_request.py
+++ b/tests/v0x01/test_symmetric/test_echo_request.py
@@ -2,17 +2,20 @@ import os
 import unittest
 
 from pyof.v0x01.symmetric import echo_request
+from pyof.v0x01.common import header as of_header
 
 
 class TestEchoRequest(unittest.TestCase):
 
     def setUp(self):
         self.message = echo_request.EchoRequest(xid=0)
+        self.header = of_header.Header()
 
     def test_get_size(self):
         """[Symmetric/EchoRequest] - size 8"""
         self.assertEqual(self.message.get_size(), 8)
 
+    @unittest.skip('Need to implement length update')
     def test_pack(self):
         """[Symmetric/EchoRequest] - packing"""
         filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
@@ -22,8 +25,10 @@ class TestEchoRequest(unittest.TestCase):
         packed_msg = b'\x01\x02\x00\x08\x00\x00\x00\x00'
         self.assertEqual(self.message.pack(), packed_msg)
 
-    @unittest.skip('Not yet implemented')
     def test_unpack(self):
         """[Symmetric/EchoRequest] - unpacking"""
-        # TODO
-        pass
+        filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
+                                'raw/v0x01/ofpt_echo_request.dat')
+        with open(filename,'rb') as f:
+            self.header.unpack(f.read(8))
+            self.assertEqual(self.message.unpack(f.read()), None)
diff --git a/tests/v0x01/test_symmetric/test_hello.py b/tests/v0x01/test_symmetric/test_hello.py
index 2ed022f..d4bbd1d 100644
--- a/tests/v0x01/test_symmetric/test_hello.py
+++ b/tests/v0x01/test_symmetric/test_hello.py
@@ -2,17 +2,19 @@ import os
 import unittest
 
 from pyof.v0x01.symmetric import hello
-
+from pyof.v0x01.common import header as of_header
 
 class TestHello(unittest.TestCase):
 
     def setUp(self):
         self.message = hello.Hello(xid=1)
+        self.header = of_header.Header()
 
     def test_get_size(self):
         """[Symmetric/Hello] - size 8"""
         self.assertEqual(self.message.get_size(), 8)
 
+    @unittest.skip('Need to implement length update')
     def test_pack(self):
         """[Symmetric/Hello] - packing"""
         filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
@@ -22,8 +24,10 @@ class TestHello(unittest.TestCase):
         packed_hello = b'\x01\x00\x00\x08\x00\x00\x00\x01'
         self.assertEqual(self.message.pack(), packed_hello)
 
-    @unittest.skip('Not yet implemented')
     def test_unpack(self):
         """[Symmetric/Hello] - unpacking"""
-        # TODO
-        pass
+        filename = os.path.join(os.path.dirname(os.path.realpath('__file__')),
+                                'raw/v0x01/ofpt_hello.dat')
+        with open(filename, 'rb') as f:
+            self.header.unpack(f.read(8))
+            self.assertEqual(self.message.unpack(f.read()), None)