Skip to content

Comments

Fix/issue 103#106

Merged
italovalcy merged 2 commits intomasterfrom
fix/issue_103
Dec 10, 2025
Merged

Fix/issue 103#106
italovalcy merged 2 commits intomasterfrom
fix/issue_103

Conversation

@italovalcy
Copy link

@italovalcy italovalcy commented Nov 21, 2025

Closes #103

Summary

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

Local Tests

With the proposed change, we only fix the get_size fo ActionHeader objs, the children objs will keep their size with multiple of 8:

> 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.action import ActionExperimenter, ActionGroup, ActionHeader, ActionCopyTTLIn, ActionCopyTTLOut, ActionDecMPLSTTL, ActionSetMPLSTTL, ActionDecNWTTL, ActionSetNWTTL, ActionOutput, ActionPopMPLS, ActionPopPBB, ActionPopVLAN, Ac\
tionPush, ActionSetField, ActionSetQueue, ActionType, ActionHeader
>>> accs_ids = [ActionOutput(port=0), ActionCopyTTLOut(), ActionCopyTTLIn(), ActionSetMPLSTTL(mpls_ttl=0), ActionDecMPLSTTL(), ActionPush(action_type=ActionType.OFPAT_PUSH_VLAN, ethertype=0), ActionPopVLAN(), ActionPush(action_type=ActionType.OFPAT_PU\
SH_MPLS, ethertype=0), ActionPopMPLS(ethertype=0), ActionSetQueue(queue_id=0), ActionGroup(group_id=0), ActionSetNWTTL(nw_ttl=0), ActionDecNWTTL(), ActionPush(action_type=ActionType.OFPAT_PUSH_PBB, ethertype=0), ActionPopPBB(), ActionExperimenter(leng\
th=0, experimenter=0)]
>>>
>>> for acc in accs_ids:
...     print(acc.get_size(), acc)
...
16 ActionOutput(port=0)
8 <pyof.v0x04.common.action.ActionCopyTTLOut object at 0x10bc05400>
8 <pyof.v0x04.common.action.ActionCopyTTLIn object at 0x10bc05010>
8 <pyof.v0x04.common.action.ActionSetMPLSTTL object at 0x10bc052b0>
8 <pyof.v0x04.common.action.ActionDecMPLSTTL object at 0x10bc06cf0>
8 <pyof.v0x04.common.action.ActionPush object at 0x10bc06e40>
8 <pyof.v0x04.common.action.ActionPopVLAN object at 0x10bc06f90>
8 <pyof.v0x04.common.action.ActionPush object at 0x10bc956d0>
8 <pyof.v0x04.common.action.ActionPopMPLS object at 0x10bc070e0>
8 <pyof.v0x04.common.action.ActionSetQueue object at 0x10bc07230>
8 <pyof.v0x04.common.action.ActionGroup object at 0x10bc07380>
8 <pyof.v0x04.common.action.ActionSetNWTTL object at 0x10bc074d0>
8 <pyof.v0x04.common.action.ActionDecNWTTL object at 0x10bc07620>
8 <pyof.v0x04.common.action.ActionPush object at 0x10bc95950>
8 <pyof.v0x04.common.action.ActionPopPBB object at 0x10bc07770>
8 <pyof.v0x04.common.action.ActionExperimenter object at 0x10bc078c0>
>>>

The ActionHeader will be its correct size:

>>> a = ActionHeader(action_type=ActionType.OFPAT_OUTPUT, length=4)
>>> a.pack()
b'\x00\x00\x00\x04'
>>> a.get_size()
4
>>>

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 19:49
@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.

Nicely done @italovalcy, thanks for finding this and patching it. Looks good to me, I'll leave pre-approved. Before merging, make sure to push unit tests too though, we need coverage for this new added part (the examples you provided on local tests are great)

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

Wrong get_size fo ActionHeader objects

2 participants