Skip to content
This repository was archived by the owner on Apr 22, 2024. It is now read-only.

fix packetOut validation and in_port enum reference #403

Closed
wants to merge 2 commits into from
Closed

fix packetOut validation and in_port enum reference #403

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cff57f
fix packetOut validation and in_port enum reference
erickvermot Jul 11, 2017
1190214
requested changes: remove space between operator in slice.
erickvermot Jul 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions pyof/v0x01/controller2switch/packet_out.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,13 +112,17 @@ def _update_actions_len(self):
self.actions_len = ListOfActions(self.actions).get_size()

def _validate_in_port(self):
port = self.in_port
valid = True
if isinstance(port, Port):
if port not in _VIRT_IN_PORTS:
if (isinstance(self.in_port, int) and
(self.in_port > 0 and self.in_port < Port.OFPP_MAX.value)):
Copy link
Contributor

Choose a reason for hiding this comment

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

Port is a IntEnum, thus you do not need to use the .value here, it's elements can be directly compared to integers out of the box.
Also, why comparing if the in_port is less then Port.OFPP_MAX instead of less or equal to ?

In [4]: Port.OFPP_MAX
Out[4]: <Port.OFPP_MAX: 65280>

In [5]: 65280 == Port.OFPP_MAX
Out[5]: True

In [6]: 65280 >= Port.OFPP_MAX
Out[6]: True

In [7]: 65280 > Port.OFPP_MAX
Out[7]: False

In [8]: 65280 < Port.OFPP_MAX
Out[8]: False

In [9]: 65281 <= Port.OFPP_MAX
Out[9]: False

In [10]: 65279 <= Port.OFPP_MAX
Out[10]: True

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lines 116 to 118 treats valid ports that are not in Port class. the function returns so they don't get to be parsed to a instance of Port.
lines 119 to 125 treats valid ports that are in the Port class.
Since OFPP_MAX is in the Port class, we should let it be parsed.

I left the .value there because it was already there. (commit 7c7faf7)

return
if not isinstance(self.in_port, Port):
try:
self.in_port = Port(self.in_port)
except ValueError:
valid = False
elif isinstance(port, int) and (port < 1 or port >=
Port.OFPP_MAX.value):
if self.in_port not in _VIRT_IN_PORTS:
valid = False
if not valid:
raise ValidationError('{} is not a valid input port.'.format(port))
raise ValidationError(
'{} is not a valid input port.'.format(self.in_port))