Skip to content

Comments

Create the ListOfTableId and use it on NextTableProperty#107

Merged
italovalcy merged 1 commit intomasterfrom
fix/issue_101
Dec 10, 2025
Merged

Create the ListOfTableId and use it on NextTableProperty#107
italovalcy merged 1 commit intomasterfrom
fix/issue_101

Conversation

@italovalcy
Copy link

@italovalcy italovalcy commented Nov 21, 2025

Closes #101

Summary

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

Local Tests

Without the changes proposed here (i.e., using ListOfInstruction with InstructionGotoTable as suggested in the code):

> python3
Python 3.13.2 (main, Feb  4 2025, 14:51:09) [Clang 16.0.0 (clang-1600.0.26.6)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from pyof.v0x04.common.flow_instructions import ListOfInstruction, InstructionGotoTable
>>> from pyof.v0x04.controller2switch.common import NextTablesProperty, TableFeaturePropType
>>> loi = ListOfInstruction([InstructionGotoTable(table_id=1), InstructionGotoTable(table_id=2), InstructionGotoTable(table_id=3)])
>>> next_tb_prop = NextTablesProperty(property_type=TableFeaturePropType.OFPTFPT_NEXT_TABLES, next_table_ids=loi)
>>> next_tb_prop.pack()
b'\x00\x02\x00\x1c\x00\x01\x00\x08\x01\x00\x00\x00\x00\x01\x00\x08\x02\x00\x00\x00\x00\x01\x00\x08\x03\x00\x00\x00'
>>> len(next_tb_prop.pack())
28
>>>

As you can see above, the length of the NextTablesProperty became much bigger than expected (28 bytes, while it was expected to be 7 bytes). We can also check a packet capture from Noviflow Switch:

Screenshot 2025-11-21 at 17 34 58

Now, after fixing this issue:

> python3
Python 3.13.2 (main, Feb  4 2025, 14:51:09) [Clang 16.0.0 (clang-1600.0.26.6)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from pyof.v0x04.controller2switch.common import NextTablesProperty, TableFeaturePropType
>>> next_tb_prop = NextTablesProperty(property_type=TableFeaturePropType.OFPTFPT_NEXT_TABLES, next_table_ids=[1,2,3])
>>> next_tb_prop.pack()
b'\x00\x02\x00\x07\x01\x02\x03'
>>> len(next_tb_prop.pack())
7

Packing and unpacking works as expected:

>>> from pyof.v0x04.controller2switch.common import NextTablesProperty, TableFeaturePropType
>>> packed = NextTablesProperty(property_type=TableFeaturePropType.OFPTFPT_NEXT_TABLES, next_table_ids=[1,2,3])
>>> unpacked = NextTablesProperty().unpack(packed.pack())
>>> unpacked = NextTablesProperty()
>>> unpacked.unpack(packed.pack())
>>> packed == unpacked
True
>>>

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 21, 2025 21:13
@italovalcy italovalcy self-assigned this Nov 21, 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.

Well done @italovalcy, great to have ListOfTableId supported and fixed. I'll leave pre-approved, before merging make sure to also include unit tests, the examples you provided as local tests for instance would be great to have as unit tests, thanks.

@italovalcy italovalcy mentioned this pull request Dec 3, 2025
Base automatically changed from fix/issue_103 to master December 10, 2025 14:27
@italovalcy italovalcy merged commit d2adc9d into master Dec 10, 2025
1 check passed
@italovalcy italovalcy deleted the fix/issue_101 branch December 10, 2025 15:46
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.

NextTablesProperty next_table_ids is using ListOfInstruction instead of a array of uint8_t

2 participants