Create an OutputPort data type for use in Output actions

[seliopou]

Currently the PseudoPort data type contains a variant for the special controller port. That constructor takes an integer which is the maximum number of bytes of the packet that should be sent to the controller. The only place where the controller PseudoPort should be used is in an Output action, and that's also the only place that handles the length parameter properly. That's mentioned in the code here, and is also clear from the OpenFlow standard, § A.2.5 Action Structures, as the max_length is part of the ofp_action_output struct, not the port representation.

PseudoPort should not include the Controller variant. That should be removed, and a new data type OutputPort should be introduced which is either a PseudoPort or a controller with an int parameter. That should only be used in the Output action.

The comment in fa51a3f is relevant to this issue.

[jnfoster]

So the tradeoff is that we diverge a bit from the structure implied by the
(informal) specification but eliminate run-time checks?

We've not been pedantic about adhering to the spec, but on the whole it's
nice if people can read

https://www.opennetworking.org/images/stories/downloads/sdn-resources/onf-specifications/openflow/openflow-spec-v1.0.0.pdf

and quickly grok what we're doing.

-N

On Mon, Oct 14, 2013 at 10:53 AM, Spiros Eliopoulos <
[email protected]> wrote:

Currently the PseudoPort data type contains a variant for the special
controller port. That constructor takes an integer which is the maximum
number of bytes of the packet that should be sent to the controller. The
only place where the controller PseudoPort should be used is in an Outputaction, and that's also the only place that handles the length parameter
properly. That's mentioned in the code herehttps://github.com/frenetic-lang/ocaml-openflow/blob/master/lib/OpenFlow0x01.ml#L335,
and is also clear from the OpenFlow standard, § A.2.5 Action Structures, as
the max_length is part of the ofp_action_output struct, not the port
representation.

PseudoPort should not include the Controller variant. That should be
removed, and a new data type OutputPort should be introduced which is
either a PseudoPort or a controller with an int parameter. That should
only be used in the Output action.

The comment in fa51a3fhttps://github.com/frenetic-lang/ocaml-openflow/commit/fa51a3f29a186is relevant to this issue.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/5
.

[arjunguha]

The informal spec is actually confusing on this matter. I think creating a type distinction is valuable. If you understand the spec, it should be obvious why we have it. If you don't understand the spec, you'll look closely and realize the types make more sense.

[jnfoster]

Okay, I'm convinced.

-N

On Tue, Oct 15, 2013 at 11:00 AM, Arjun Guha [email protected]:

The informal spec is actually confusing on this matter. I think creating a
type distinction is valuable. If you understand the spec, it should be
obvious why we have it. If you don't understand the spec, you'll look
closely and realize the types make more sense.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-26341978
.

[seliopou]

Ok sounds good. I gave the refactor a crack and it'd touch some high-level APIs that I'd rather discuss before going ahead and committing. Gonna put this on ice for now.