Skip to content

Comments

Adding optional padding into table features Property to make it multiple of 8#109

Merged
italovalcy merged 6 commits intomasterfrom
fix/issue_102
Dec 10, 2025
Merged

Adding optional padding into table features Property to make it multiple of 8#109
italovalcy merged 6 commits intomasterfrom
fix/issue_102

Conversation

@italovalcy
Copy link

@italovalcy italovalcy commented Nov 25, 2025

Closes #102

Summary

See updated changelog file and/or add any other summarized helpful information for reviewers

Local Tests

Without the proposed changes:

>>> from pyof.v0x04.common.flow_instructions import InstructionType, Instruction
... from pyof.v0x04.controller2switch.common import InstructionsProperty, TableFeaturePropType
... insts = [
...     InstructionType.OFPIT_GOTO_TABLE,
...     InstructionType.OFPIT_WRITE_METADATA,
...     InstructionType.OFPIT_WRITE_ACTIONS,
...     InstructionType.OFPIT_APPLY_ACTIONS,
...     InstructionType.OFPIT_CLEAR_ACTIONS,
...     InstructionType.OFPIT_METER,
... ]
... inst_ids = []
... for inst in insts:
...     inst_ids.append(Instruction(instruction_type=inst))
...
... inst_prop = InstructionsProperty(property_type=TableFeaturePropType.OFPTFPT_INSTRUCTIONS, instruction_ids=inst_ids)
...
>>> inst_prop.pack()
b'\x00\x00\x00\x1c\x00\x01\x00\x04\x00\x02\x00\x04\x00\x03\x00\x04\x00\x04\x00\x04\x00\x05\x00\x04\x00\x06\x00\x04'
>>> len(inst_prop.pack())
28
>>>

With the changes:

>>> from pyof.v0x04.common.flow_instructions import InstructionType, Instruction
... from pyof.v0x04.controller2switch.common import InstructionsProperty, TableFeaturePropType
... insts = [
...     InstructionType.OFPIT_GOTO_TABLE,
...     InstructionType.OFPIT_WRITE_METADATA,
...     InstructionType.OFPIT_WRITE_ACTIONS,
...     InstructionType.OFPIT_APPLY_ACTIONS,
...     InstructionType.OFPIT_CLEAR_ACTIONS,
...     InstructionType.OFPIT_METER,
... ]
... inst_ids = []
... for inst in insts:
...     inst_ids.append(Instruction(instruction_type=inst))
...
... inst_prop = InstructionsProperty(property_type=TableFeaturePropType.OFPTFPT_INSTRUCTIONS, instruction_ids=inst_ids)
...
>>> inst_prop.pack()
b'\x00\x00\x00\x1c\x00\x01\x00\x04\x00\x02\x00\x04\x00\x03\x00\x04\x00\x04\x00\x04\x00\x05\x00\x04\x00\x06\x00\x04\x00\x00\x00\x00'
>>> len(inst_prop.pack())
32
>>>

See that, new size is multiple of 8 by padding with 4 zero bytes, as suggested by the spec.

I've also added unit tests from packet capture obtained from a passive openflow connection from a Noviflow switch (I preferred to use Noviflow instead of OpenVswitch because the Table Features from OpenVSwitch is huge.. has many fields from NXM -- Nicira Extended Match and others).

End-to-End Tests

Please refer to the results presented here: #110

The end-to-end tests were executed with all PRs stacked

@italovalcy italovalcy requested a review from a team as a code owner November 25, 2025 00:07
@italovalcy italovalcy self-assigned this Nov 25, 2025
Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just left a trivial comment about documenting a TODO

Thanks, Italo. Great to have table features Property packing fixed and improved.

Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the updates, @italovalcy

@italovalcy italovalcy mentioned this pull request Dec 3, 2025
@viniarck viniarck self-requested a review December 6, 2025 17:00
Base automatically changed from fix/issue_101 to master December 10, 2025 15:46
@italovalcy italovalcy merged commit b4378f6 into master Dec 10, 2025
1 check passed
@italovalcy italovalcy deleted the fix/issue_102 branch December 10, 2025 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Table Features Properties are missing to complete the padding bytes

2 participants