Subject: Re: [forces] More comments on draft-haleplidis-forces-openflow-lib-00

From: "Haleplidis Evangelos" <[email-masked]>
Date: Wed, 11 Jul 2012 23:08:00 +0300

Greetings,

Thank you again for your comments.

Please see inline for responses for comments requiring answer.

The new version have all these comments addressed.

Regards,

Evangelos Haleplidis.

From: Zoltán Lajos Kis [mailto:[email-masked]] 
Sent: Thursday, June 07, 2012 4:51 PM
To: Haleplidis Evangelos
Cc: [email-masked]
Subject: More comments on draft-haleplidis-forces-openflow-lib-00

Hi,

I went through the whole draft - not too thoroughly on the XML part -  and
would like to clarify / comment on a few things.

I'm still working on understanding the complete data model of the described
architecture, so don't take this as an

exthaustive list of issues. (Btw, is there a tool available for ForCES,
which can convert the XML description to a more digestive

form? ER-diagrams, or similar....)

Regards,

Zoltan.

Generic comments:

1) I think the Action Set description - section 4.7 in OpenFlowSpec1.1 - is
misinterpreted. In OpenFlow

there is only one set of actions defined (25 + experimenter). Those actions
are used in ApplyAction,

WriteAction instructions and PacketOut messages. The 9-element listing in
the section merely defined

To discuss the ordering in which actions in the ActionSet must be executed
(regardless of in what order

they have been written to it). These 9 elements represents different
"classes" of actions. For example,

"Setting a field" refers to all the action types for manipulating header
fields in the frame.

Actions of the same class are not concurrent, i.e., they can be executed in
arbitrary order, without

affecting the overall result of executing the ActionSet. Actions of
different classes however can have

effect on each other, so their execution order must be specified. 

2) QoS handling is misinterpreted. In the OpenFlow 1.1 model QoS queues are
attached to (the output side of)

ports. The identifiers of these queues are port-specific; i.e. both Port 1
and Port 2 can have a queue with an ID of 2.

If the controller is to send a frame to the default (or no-)queue, it simply
uses the output action. If it

wants to target a specific queue of the port, it first has to set the
port-local queue ID (with set_queue action),

and then use the output action. The output action will forward the frame to
the appropriate port, and the port will

use the queue previously appointed by the set_queue call.

(Note that beginning with OpenFlow 1.3 a new Meter entity is introduced,
which can be used for QoS between

tables, and before outputting to ports.)

3) Besides the pieces of OpenFlow metadata identified, there are additional
metadata that needs to be attached

to the frame - due to implicit requirements in OpenFlow, and due to the
architecture of the ForCES description.

queue_id: as described above in 2), the id of the queue which is to be used
on output is set by the set_queue

action. Therefore the set queue_id must be passed along with the frame as
metadata for the output action

to process it.

of_phy_port: The OpenFlow specification allows logical ports over physical
ports. In this case a packet_in

message is supposed to tell the controller both the logical and the physical
port of the packet. Thus the

physical port id must also be sent along with the packet (given a logical
port was used).

4) The description of how outputting a frame is handled is not clear. It
would seem logical that it is

done by the OFOutputActionLFB only, i.e. only that LFB's output is connected
to the ports' input - but not those

Of OFFlowTableLFBs.

Furthermore a special ToControllerLFB should be added, which would send the
frame to the controller.

This LFB could be used by both the FlowTableLFBs and the OutputActionLFB.
Also, this LFB could address

the buffering of packets sent to the controller.

5) OpenFlow allows the controller to construct and send frames to the
datapath for processing in the

OFPT_PACKET_OUT message type. I believe this functionality is missing from
the current description.

6) OpenFlow 1.1 added extensibility features to the datapath. One can define
custom experimenter messages,

matches, instructions, actions. While not necessarily the duty of this text,
it would be nice to see how

that can be incorporated into the ForCES description. I.e., how an
experimenter action is mapped into

an (experimenter) ForCES LFB, etc.

Comments on the text:

Page 5:

- at the end: it should at least mention group entries, "the controller can
add, update and delete flow and group entries"

Page 7:

- Even though it is fairly trivial to tell them apart, I think it would make
sense to add a separator between

ForCES and OpenFlow definitions. Also, I think further OpenFlow concept
should be explained, and the order of elements

should be changed.

Suggested list of elements: Match fields, Actions, Flow Entry, Instruction,
Flow Table, Pipeline, Action Set,

Groups (incl. GroupEntry and GroupTable), Ports.

(I'm willing to provide definition for these terms.)

[ΕΗ] I have made an attempt, however if you’ll find the definitions
inadequate please provide me with alternatives. Thanks.

Page 9:

- Fig. 1., and the text below suggests that one piece of metadata (the
Action Set) is available in the packet before

entering Flow Table 0, while another piece (the Metadata field) is only
available after leaving the table.

I believe a more convenient description would be if the Metadata is already
available when entering Flow Table 0,

with a default value of all zero.

1) A table not necessarily touches Metadata, so in the current method
FlowTable 0 would have the special function

of "create if not exists" the Metadata. 2) The OpenFlowSpec1.1 in theory
allows modifying Metadata with bitmasks used

- let alone matching on it.

[ΕΗ] Both Metadata and ActionSet Metadata are now invisible to the model
since all Flow Tables reside within one OFFlowTable LFB. 

- "Write Metadata action" should be "Write Metadata instruction"

- "Forward to the OpenFlow controllers" should be changed to singular
"controller". OF 1.1 does not define multi-controller

support. It is only added in OF 1.2

- "The list of actions a Flow Table may perform..." should be "The list of
instructions..."

Page 10:

- "Write a List of actions to the action set" would be more correct as "set
of actions", and should refer to the method

of writing the action set (which should probably be described among the
concepts of Page 7.

- "The list of actions the Flow Table can perform  or write in the Action
Set is". Similar to generic comment 1), this

is not a list of possible actions, but rather the list of the different
action "classes".

- "Additionally a Flow Table may drop the packet as an action. The drop
action is implicit based on the Flow Table's

configuration.". This sentence is confusing. The configuration is only
consulted in case of no match. As this listing

is for the list of actions in write actions, there is no drop action. On
match a flow entry can drop a flow by

clearing the action set, and not specifying a goto.

- "An Action Set contains a maximum of one action of the following
types...". Should be "...one action of each type...".

(See generic comment 1)

- "1. Copy TTL Outwards" should be "1. Copy TTL Inwards"

Page 11:

- "The Group Table contains a set of actions": the Group Table contains a
set of Group Entries, each of which contains

  a set of "Action Buckets", each of which contains a set of actions.

Page 11-13 (Fig 2-4.):

- At this point the basic architectural decisions (i.e. ActionLFBs
maintaining the action parameters) are not described,

so the use of ActionIndex is quite confusing at this point.

- Also it is not clear (and remains unclear to me), that after the
FlowTableLFB sent the packet for processing to an

ActionLFB, when the packet returns from processing, how will the
FlowTableLFB know the processing state of the packet?

Particularly, how does it know which was the matching flow entry, and which
action in the action list is being executed?

[ΕΗ] A new metadata has been added, the PacketID which must uniquely
identify a packet inside the switch. This is not implementation specific,
but rather internal info as how the packet can be identified. This metadata
can be used to track a packets progress within the Flow or Group Table and
when it returns it can continue from where it stopped. 

Thank you for this catch.

Page 15:

- "Additionally each OFFlowTable can output a packet to a specific port." As
in generic comment 4), this should not

be the case. Tables can only output to the controller (without OutputAction
involvment).

[ΕΗ] Text has been changed (as per the figure).

- On Fig 5. a number of arrows are missing from the lines pointing to
OFPorts.

[ΕΗ] Fig 5 has been completely renovated.

Page 16:

- The "OpenFlow Atomic Types" table: it should be made clear that there are
more atomic types used in the OpenFlow

description, but those types (e.g., IEEEMAC) are defined elsewhere - I guess
in the LFB Library.

 [ΕΗ] Yes, the types are defined in the LFB library. Added necessary text.

- ActionSetType: see the generic comment on action sets

[ΕΗ] The ActionSet model has been changed.

Page 20:

- "... the maximum number of bytes of new flow that datapath should send ...
". "new flow" is not a correct a term here.

"unmatched / invalid frame" should be better. The controller does not
neccesarily ant to convert it to a flow.

Page 22:

- first paragraph: same comment as for Page 9 / Fig 1. regarding Action Set
vs. Metadata field

- sencond paragraph: as Dave Meyer commented already, the text should
elaborat on how matching is done. I would suggest

that it is described among the description of concepts, and this paragraph
refers to that.

[ΕΗ] The text has been changed.

Page 24:

- It it intentional that "buffer" is made a component of the FlowTable, and
not a separate LFB? In OpenFlow a packet

is buffered when sent to the controller, regardless of which table it came
on. With packet out, the controller can

instruct the switch to execute "on the fly defined" actions on the packet
and/or send it through the pipeline starting

at Table 0. There is no need for keeping track of which packet was buffered
in which table.

 [ΕΗ] BufferID is a metadata and will be removed from the FlowTable as
well. As the BufferLFB would hold no information whatsoever, there is no
need to have it as a separate LFB. The FlowTable will produce the bufferID
metadata and send it to the controller.

Page 25:

- OFPortLFB / Data Handling: The OpenFlow spec. allows a port to be a
"logical port" defined over the physical ports.

So the OFPort is not necessarily directly connected to the phyisical medium
ports.

Page 26:

- OFQueueLFB: see generic comment 2) on how QoS is defined.

- "The Length component" : I think this is a misunderstanding. The queue
does not have a length. The only length we

have in the Queue context is the length of the queue properties, which is a
wire protocol feature (for the TLV structure),

and so should not be reflected in the data model.

Page 27:

- "length of the property" : see above comment.

- "maximum 9 actions": see general comment 1"

- "maximum size 9 rows": see general comment 1"

[ΕΗ] The length is removed, the ActionSetLFB has been removed. 

Page 28:

- "As none of the OFActionLFBs have no capabilities..." - there no is not
needed

Page 29:

- "Only valid in the action set of a packet-out message" should be "action
list of a packet-out message".

Page 33:

- 5.8.18.2: the Push VLAN action's argument is an Ethertype, not a VLAN
header value.

Page 35:

- 5.8.25.2: "SetIPTTLTable" is probably wrong, something like "

 [ΕΗ] Why? There is a Set IPv4 TTL action..

Page 36:

- "C:\Workspace\Forces...\" - I guess this should be something else

[ΕΗ] Indeed! J I use the schema to validate the xml, but the location is
locally to my pc.

Page 37:

- MPLSLabelValue: 1048576 should be 1048575

- IPv4ToSBits: 64 should be 63

- "Ethermet frame typ Wildcard" - e missing

Page 38:

- IngressPort: certain values are invalid for the match type (those >
OFPP_MAX)

Page 39:

- ArpOpcode: As of OpenFlow 1.1, this field is "overloaded" into the IP
protocol field. Is it intentional

that it is separately defined?

[ΕΗ] This is the IPProtocol field (with the ARP opcode) Text is fixed.

Page 48:

- "Buffer Reason" should be "Packet-in reason"

Page 50:

- "multiple ows" -> "multiple flows"

- "speciffic" -> "specific"

Page 52:

- "VLNA" -> "VLAN"

Page 53:

- "administatevely" - "administratively"

Page 57:

- "trasmittion" - "transmission"

Page 58:

- "Length of property" : same as for Page 26. This is a wire protocol TLV
feature, should be not part of the data model.

- ActionsetLFB dataTypeDef: see generic comment 1