diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3fcc6d13..1bfe54aa 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,6 +13,12 @@ Fixed - Fix ``NextTableProperty.next_table_ids`` to correctly export only the Table IDs - Fix table features ``Property`` to add padding bytes when size is not multiple of 8 +Added +===== + +- Added classes for Table Feature Properties: ``InstructionId``, ``ActionId`` and ``OxmId`` for better handling the + packing and unpacking routines + [2025.1.0] - 2025-04-14 *********************** diff --git a/pyof/__init__.py b/pyof/__init__.py index d1fb2b7a..33aca242 100644 --- a/pyof/__init__.py +++ b/pyof/__init__.py @@ -3,4 +3,4 @@ This package is a library that parses and creates OpenFlow Messages. It contains all implemented versions of OpenFlow protocol """ -__version__ = '2025.1.0' +__version__ = '2025.2.0rc1' diff --git a/pyof/v0x04/common/flow_match.py b/pyof/v0x04/common/flow_match.py index 20083b28..16a7fead 100644 --- a/pyof/v0x04/common/flow_match.py +++ b/pyof/v0x04/common/flow_match.py @@ -192,14 +192,8 @@ class OxmTLV(GenericStruct): oxm_length = UBInt8() oxm_value = BinaryData() - def __init__( - self, - oxm_class=OxmClass.OFPXMC_OPENFLOW_BASIC, - oxm_field=None, - oxm_hasmask=False, - oxm_value=None, - oxm_length=None, - ): + def __init__(self, oxm_class=OxmClass.OFPXMC_OPENFLOW_BASIC, + oxm_field=None, oxm_hasmask=False, oxm_value=None): """Create an OXM TLV struct with the optional parameters below. Args: @@ -213,7 +207,7 @@ def __init__( super().__init__() self.oxm_class = oxm_class self.oxm_field_and_mask = None - self.oxm_length = oxm_length + self.oxm_length = None self.oxm_value = oxm_value # Attributes that are not packed self.oxm_field = oxm_field @@ -265,8 +259,6 @@ def _update_length(self): Update the oxm_length field with the packed payload length. """ - if self.oxm_length: - return payload = type(self).oxm_value.pack(self.oxm_value) self.oxm_length = len(payload) diff --git a/pyof/v0x04/controller2switch/common.py b/pyof/v0x04/controller2switch/common.py index 41e7f8e2..240feee0 100644 --- a/pyof/v0x04/controller2switch/common.py +++ b/pyof/v0x04/controller2switch/common.py @@ -8,22 +8,23 @@ from pyof.foundation.basic_types import ( Char, FixedTypeList, Pad, UBInt8, UBInt16, UBInt32, UBInt64) from pyof.foundation.constants import OFP_MAX_TABLE_NAME_LEN +from pyof.foundation.exceptions import UnpackException from pyof.v0x04.asynchronous.flow_removed import FlowRemovedReason from pyof.v0x04.asynchronous.packet_in import PacketInReason from pyof.v0x04.asynchronous.port_status import PortReason # Local source tree imports -from pyof.v0x04.common.action import ( - ActionHeader, ControllerMaxLen, ListOfActions) -from pyof.v0x04.common.flow_instructions import ListOfInstruction -from pyof.v0x04.common.flow_match import ListOfOxmHeader +from pyof.v0x04.common.action import ActionHeader, ActionType, ControllerMaxLen +from pyof.v0x04.common.flow_instructions import InstructionType +from pyof.v0x04.common.flow_match import OxmClass, OxmTLV from pyof.v0x04.common.header import Header from pyof.v0x04.controller2switch.table_mod import Table __all__ = ('ConfigFlag', 'ControllerRole', 'TableFeaturePropType', 'MultipartType', 'Bucket', 'BucketCounter', 'ListOfBucketCounter', 'ExperimenterMultipartHeader', 'Property', 'InstructionsProperty', - 'NextTablesProperty', 'ActionsProperty', 'OxmProperty', - 'ListOfTableId', 'ListOfProperty', 'TableFeatures') + 'NextTablesProperty', 'ActionsProperty', 'OxmProperty', 'ActionId', + 'ListOfActionId', 'InstructionId', 'ListOfInstructionId', 'OxmId', + 'ListOfOxmId', 'ListOfTableId', 'ListOfProperty', 'TableFeatures') # Enum @@ -497,6 +498,80 @@ def _complete_last_byte(self, packet): return packet +class InstructionId(GenericStruct): + """Generic InstructionId class. + + This class represents a InstructionId that can be used with TableFeatures. + While OpenFlow 1.3.2 shares the same ofp_instruction for flow_mod and + table_features, we followed same approach as Ryu and OF 1.4: we have + separate classes. + """ + + instruction_type = UBInt16(enum_ref=InstructionType) + length = UBInt16() + exp_data = FixedTypeList(UBInt8) + + def __init__(self, instruction_type=None, length=None, exp_data=None): + """Create a Instruction with the optional parameters below. + + Args: + instruction_type(InstructionType): Type of instruction. + length (int): Length of the InstructionId, including this header. + exp_data (Union[list[UBInt8], int]): + Optional experimenter id + data. Can be a list of unsigned 8bit + integers or an integer (which will be converted accoding to the + length - 4). + """ + super().__init__() + self.instruction_type = instruction_type + self.length = length or 4 + if isinstance(exp_data, FixedTypeList): + self.exp_data = exp_data + elif isinstance(exp_data, int) and length > 4: + self.exp_data = FixedTypeList(UBInt8, [ + UBInt8(b) + for b in exp_data.to_bytes(length - 4, 'big', signed=False) + ]) + else: + self.exp_data = FixedTypeList(UBInt8) + + def unpack(self, buff, offset=0): + """Discard padding bytes using the unpacked length attribute.""" + 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('exp_data', type(self).exp_data, + buff[:offset+self.length], begin) + + def get_size(self, value=None): + """ + Return the InstructionId length. + + If the object length is None, returns the minimum size. + """ + if self.length is None: + return super().get_size(value) + + return self.length + + +class ListOfInstructionId(FixedTypeList): + """List of InstructionIds. + + Represented by instances of InstructionId. + """ + + def __init__(self, items=None): + """Create ListOfInstructionId with the optional parameters below. + + Args: + items (~pyof.v0x04.controller2switch.common.InstructionId): + Instance or a list of instances. + """ + super().__init__(pyof_class=InstructionId, items=items) + + class InstructionsProperty(Property): """Instructions property. @@ -505,21 +580,21 @@ class InstructionsProperty(Property): OFPTFPT_INSTRUCTIONS_MISS """ - instruction_ids = ListOfInstruction() + instruction_ids = ListOfInstructionId() def __init__(self, property_type=TableFeaturePropType.OFPTFPT_INSTRUCTIONS, instruction_ids=None): """Create a InstructionsProperty with the optional parameters below. Args: - type(|TableFeaturePropType_v0x04|): + property_type(|TableFeaturePropType_v0x04|): Property Type value of this instance. - next_table_ids(|ListOfInstruction_v0x04|): - List of InstructionGotoTable instances. + instruction_ids(|ListOfInstruction_v0x04|): + List of InstructionId instances. """ super().__init__(property_type=property_type) - self.instruction_ids = instruction_ids if instruction_ids else [] + self.instruction_ids = instruction_ids or ListOfInstructionId() self.update_length() @@ -572,6 +647,81 @@ def __init__(self, property_type=TableFeaturePropType.OFPTFPT_NEXT_TABLES, self.update_length() +class ActionId(GenericStruct): + """Generic ActionId class. + + This class represents an ActionId that can be used with TableFeatures. + While OpenFlow 1.3.2 shares the same ofp_action for flow_mod and + table_features, we followed same approach as Ryu and OF 1.4: we have + separate classes. + """ + + action_type = UBInt16(enum_ref=ActionType) + length = UBInt16() + exp_data = FixedTypeList(UBInt8) + + def __init__(self, action_type=None, length=None, exp_data=None): + """Create a ActionId with the optional parameters below. + + Args: + action_type (~pyof.v0x04.common.action.ActionType): + The type of the action (One of OFPAT_*). + length (int): Length of the ActionId, including this header. + exp_data (Union[list[UBInt8], int]): + Optional experimenter id + data. Can be a list of unsigned 8bit + integers or an integer (which will be converted accoding to the + length). + """ + super().__init__() + self.action_type = action_type + self.length = length or 4 + if isinstance(exp_data, FixedTypeList): + self.exp_data = exp_data + elif isinstance(exp_data, int) and length > 4: + self.exp_data = FixedTypeList(UBInt8, [ + UBInt8(b) + for b in exp_data.to_bytes(length - 4, 'big', signed=False) + ]) + else: + self.exp_data = FixedTypeList(UBInt8) + + def unpack(self, buff, offset=0): + """Discard padding bytes using the unpacked length attribute.""" + 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('exp_data', type(self).exp_data, + buff[:offset+self.length], begin) + + def get_size(self, value=None): + """ + Return the ActionId length. + + If the object length is None, returns the minimum size. + """ + if self.length is None: + return super().get_size(value) + + return self.length + + +class ListOfActionId(FixedTypeList): + """List of ActionIds. + + Represented by instances of ActionId. + """ + + def __init__(self, items=None): + """Create ListOfActionId with the optional parameters below. + + Args: + items (:class:`~pyof.v0x04.controller2switch.common.ActionId`): + Instance or a list of instances. + """ + super().__init__(pyof_class=ActionId, items=items) + + class ActionsProperty(Property): """Actions Property. @@ -582,7 +732,7 @@ class ActionsProperty(Property): OFPTFPT_APPLY_ACTIONS_MISS """ - action_ids = ListOfActions() + action_ids = ListOfActionId() def __init__(self, property_type=TableFeaturePropType.OFPTFPT_WRITE_ACTIONS, @@ -597,10 +747,80 @@ def __init__(self, """ super().__init__(property_type) - self.action_ids = action_ids if action_ids else ListOfActions() + self.action_ids = action_ids or ListOfActionId() self.update_length() +class OxmId(OxmTLV): + """Generic OxmId class for Oxm Property table features. + + This class represents an OxmId that can be used with TableFeatures. + OpenFlow spec only specify that elements are 32-bit OXM headers or 64-bit + OXM headers for experimenter OXM fields. Since no oxm_value is expected + for non-experimenter OXM fields, we implemented this wrapper around OxmTLV + to avoid unpacking more than bytes than necessary. + """ + + def __init__(self, *args, oxm_length=None, **kwargs): + """Create a OXM TLV with fixed length for non-experimenter OxmIds.""" + super().__init__(*args, **kwargs) + self.oxm_length = oxm_length + + def _update_length(self): + """OxmId for table feature properties are fixed on 32 or 64bits.""" + if self.oxm_length: + return + super()._update_length() + + def unpack(self, buff, offset=0): + """Unpack the buffer into a OxmId. Different from OxmTLV we dont create + actual Match fields with their attributes (just field, mask and value). + + Args: + buff (bytes): The binary data to be unpacked. + offset (int): If we need to shift the beginning of the data. + + """ + # oxm (32 bit) == class (16) | field (7) | hasmask (1) | length (8) + # in case of experimenter OXMs, another 32 bit value + # (experimenter id) follows. + self.oxm_class = UBInt16(enum_ref=OxmClass) + self.oxm_class.unpack(buff, offset) + self.oxm_field_and_mask = UBInt8() + self.oxm_field_and_mask.unpack(buff, offset+2) + self.oxm_length = UBInt8() + self.oxm_length.unpack(buff, offset+3) + + # Recover field from field_and_hasmask. + try: + self.oxm_field = self._unpack_oxm_field() + except ValueError as exception: + raise UnpackException(exception) + + # The last bit of field_and_mask is oxm_hasmask + self.oxm_hasmask = (self.oxm_field_and_mask & 1) == 1 # as boolean + + # Unpack the remaining 32bit in case of experimenter id + if self.oxm_class == OxmClass.OFPXMC_EXPERIMENTER: + self.oxm_value = buff[offset+4:offset+8] + + +class ListOfOxmId(FixedTypeList): + """List of OxmIds. + + Represented by instances of OxmId. + """ + + def __init__(self, items=None): + """Create ListOfOxmId with the optional parameters below. + + Args: + items (:class:`~pyof.v0x04.controller2switch.common.OxmId`): + Instance or a list of instances. + """ + super().__init__(pyof_class=OxmId, items=items) + + class OxmProperty(Property): """Match, Wildcard or Set-Field property. @@ -613,7 +833,7 @@ class OxmProperty(Property): OFPTFPT_APPLY_SETFIELD_MISS """ - oxm_ids = ListOfOxmHeader() + oxm_ids = ListOfOxmId() def __init__(self, property_type=TableFeaturePropType.OFPTFPT_MATCH, oxm_ids=None): @@ -622,12 +842,12 @@ def __init__(self, property_type=TableFeaturePropType.OFPTFPT_MATCH, Args: type(|TableFeaturePropType_v0x04|): Property Type value of this instance. - oxm_ids(|ListOfOxmHeader_v0x04|): - List of OxmHeader instances. + oxm_ids(|ListOfOxmId|): + List of OxmId instances. """ super().__init__(property_type) - self.oxm_ids = ListOfOxmHeader() if oxm_ids is None else oxm_ids + self.oxm_ids = ListOfOxmId() if oxm_ids is None else oxm_ids self.update_length() diff --git a/tests/unit/v0x04/test_controller2switch/test_table_features.py b/tests/unit/v0x04/test_controller2switch/test_table_features.py index ccdee780..d0e62291 100644 --- a/tests/unit/v0x04/test_controller2switch/test_table_features.py +++ b/tests/unit/v0x04/test_controller2switch/test_table_features.py @@ -1,29 +1,20 @@ """TableFeatures message tests.""" -from pyof.v0x04.common.action import ActionExperimenter, ActionHeader, ActionType -from pyof.v0x04.common.flow_instructions import Instruction, InstructionType -from pyof.v0x04.common.flow_match import OxmOfbMatchField, OxmTLV +from pyof.v0x04.common.action import ActionType +from pyof.v0x04.common.flow_instructions import InstructionType +from pyof.v0x04.common.flow_match import OxmOfbMatchField from pyof.v0x04.controller2switch.common import ( + ActionId, ActionsProperty, + InstructionId, InstructionsProperty, NextTablesProperty, + OxmId, OxmProperty, TableFeaturePropType, TableFeatures, ) -from tests.unit.test_struct import StructTest as PyofStructTest - - -# TODO: remove this wrapper class once we fix issue #108 -class StructTest(PyofStructTest): - """Wrapper class to temporary skip unpack tests.""" - - def test_raw_dump_file(self): - """Overwrite test_raw_dump_file to skip unpack test.""" - file_bytes = self.get_raw_dump().read() - pyof_obj = self.get_raw_object() - self._test_pack(pyof_obj, file_bytes) - # self._test_unpack(pyof_obj, file_bytes) +from tests.unit.test_struct import StructTest class TestTableFeatures(StructTest): @@ -53,7 +44,7 @@ def _prepare_table_features(self): ] inst_ids = [] for inst in insts: - inst_ids.append(Instruction(instruction_type=inst)) + inst_ids.append(InstructionId(instruction_type=inst)) inst_prop_types = [ TableFeaturePropType.OFPTFPT_INSTRUCTIONS, TableFeaturePropType.OFPTFPT_INSTRUCTIONS_MISS, @@ -103,8 +94,12 @@ def _prepare_table_features(self): ] action_ids = [] for action in actions: - action_ids.append(ActionHeader(action_type=action, length=4)) - action_ids.append(ActionExperimenter(length=8, experimenter=0xFF000002)) + action_ids.append(ActionId(action_type=action)) + action_ids.append( + ActionId( + action_type=ActionType.OFPAT_EXPERIMENTER, length=8, exp_data=0xFF000002 + ) + ) action_prop_types = [ TableFeaturePropType.OFPTFPT_WRITE_ACTIONS, TableFeaturePropType.OFPTFPT_WRITE_ACTIONS_MISS, @@ -164,7 +159,7 @@ def _prepare_table_features(self): oxm_ids = [] for oxm in oxms: s, m = match_fields_size_mask[oxm.value] - oxm_ids.append(OxmTLV(oxm_field=oxm, oxm_hasmask=m, oxm_length=s)) + oxm_ids.append(OxmId(oxm_field=oxm, oxm_hasmask=m, oxm_length=s)) tb_props.append( OxmProperty( property_type=TableFeaturePropType.OFPTFPT_MATCH, oxm_ids=oxm_ids @@ -173,7 +168,7 @@ def _prepare_table_features(self): oxm_ids = [] for oxm in oxms: s, m = match_fields_size_mask[oxm.value] - oxm_ids.append(OxmTLV(oxm_field=oxm, oxm_hasmask=False, oxm_length=s)) + oxm_ids.append(OxmId(oxm_field=oxm, oxm_hasmask=False, oxm_length=s)) tb_props.append( OxmProperty( property_type=TableFeaturePropType.OFPTFPT_WILDCARDS, oxm_ids=oxm_ids @@ -229,7 +224,7 @@ def _prepare_table_features(self): ] oxm_ids = [] for oxm in setfield_oxms: - oxm_ids.append(OxmTLV(oxm_field=oxm, oxm_hasmask=False, oxm_length=0)) + oxm_ids.append(OxmId(oxm_field=oxm, oxm_hasmask=False, oxm_length=0)) for oxm_prop_type in setfield_oxm_prop_types: tb_oxm_ids = oxm_ids tb_props.append(