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

implement general unpack method (moved from of_core.utils) #397

Closed
wants to merge 2 commits into from
Closed

implement general unpack method (moved from of_core.utils) #397

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9842dc
introduce general unpack method (comes from of_core.utils)
erickvermot Jul 7, 2017
a2d4480
separate packet validation from unpack
erickvermot Jul 14, 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
50 changes: 50 additions & 0 deletions pyof/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
"""General Unpack utils for python-openflow."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, add a comment stating that you are copying the code from of_core, specifying the filename from its repo root. Also write that this is because you want to have the exact same behavior for testing purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is already clear in the corresponding issues mentioned here:
#396 and #381

import pyof.v0x01.common.header
import pyof.v0x01.common.utils
import pyof.v0x04.common.header
import pyof.v0x04.common.utils
Copy link
Contributor

Choose a reason for hiding this comment

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

@erickvermot you are not using this imported modules anywhere, so you should not import them here.
Considering your code, I would do:

from pyof import v0x01
from pyof import v0x04

pyof_version_libs = {0x01: v0x01,
                     0x04: v0x04}

Note that I'm using the from statement, instead of the import statement.
This is because on some tests I've performed related to the python importing system, this import statement appeared to be less error prone than using the from ... import statement, so I would give preference to use the latter.

Copy link
Contributor

Choose a reason for hiding this comment

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

Even better:
from pyof import v0x01, v0x04

Copy link
Contributor Author

Choose a reason for hiding this comment

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

anywhere = line 25 for example. Those imports are needed since importing pyof does not import the child modules by default.

The from statement is better suited when one tries to import a Class, function or variable from a given module, or when one wants to place the imported module in the current namespace.

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked this out and, unfortunately, Diraol suggestion doesn't work because this is a special case like Erick said. Linters can't detect unused import statements like these, but I can't think of a better way to do it right now.

Copy link
Contributor Author

@erickvermot erickvermot Jul 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

  • "unused import statements like these", That's because they are not unused, as I said before.

  • This is not a special case. Unless we make the imports inside the init s, those imports are needed.

from pyof.foundation.exceptions import UnpackException

pyof_version_libs = {0x01: pyof.v0x01,
0x04: pyof.v0x04}


def validate_packet(packet):
"""Check if packet is valid OF packet."""
if not isinstance(packet, bytes):
raise UnpackException('invalid packet')

packet_length = len(packet)

if packet_length < 8 or packet_length > 2**16:
raise UnpackException('invalid packet')

if packet_length != int.from_bytes(packet[2:4],
byteorder='big'):
raise UnpackException('invalid packet')

version = packet[0]
if version == 0 or version >= 128:
raise UnpackException('invalid packet')


def unpack(packet):
"""Unpack the OpenFlow Packet and returns a message."""
validate_packet(packet)

version = packet[0]
try:
pyof_lib = pyof_version_libs[version]
except KeyError:
raise UnpackException('Version not supported')

try:
header = pyof_lib.common.header.Header()
header.unpack(packet[:8])
Copy link
Contributor

Choose a reason for hiding this comment

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

@erickvermot the header will always have length 8 [just to make sure that on OF 1.3.5 and newer they still keep this behavior]?
Or should we use pyof_lib.common.header.Header.get_size() instead of 8?

If my comment make any sense, then we should "apply" it below also.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

headers always have length 8

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok then!

message = pyof_lib.common.utils.new_message_from_header(header)
binary_data = packet[8:]
if binary_data:
message.unpack(binary_data)
return message
except (UnpackException, ValueError) as e:
raise UnpackException(e)