[ovs-dev] [PATCH 3/4] Add some missing Open Flow 1.1 definitions

[Ben Pfaff]

On Wed, Mar 28, 2012 at 09:44:16AM +0900, Simon Horman wrote:
> Signed-off-by: Simon Horman <horms at verge.net.au>

The comment here is wrong, since it talks about OXM but OXM was
introduced in OpenFlow 1.2:
> +/* The match type indicates the match structure (set of fields that compose the
> + * match) in use. The match type is placed in the type field at the beginning
> + * of all match structures. The "OpenFlow Extensible Match" type corresponds
> + * to OXM TLV format described below and must be supported by all OpenFlow
> + * switches. Extensions that define other match types may be published on the
> + * ONF wiki. Support for extensions is optional.
> + */
> +enum ofp11_match_type {
> +    OFPMT11_STANDARD = 0,       /* The match fields defined in the ofp11_match
> +                                   structure apply */
> +};

I'm actually thinking that maybe we should just have "enum
ofp_match_type" in openflow-common.h, with all the possible values (so
far only two).  Sure, enum ofp_match_type was introduced in OpenFlow
1.1, but the intention, at least, is to maintain the meaning of each
value over time.

I think I feel the same way about ofp_packet_in_reason and
specifically about OFPR_INVALID_TTL.  No values have been removed or
had their meanings changed from OF1.0 to OF1.2, and in fact OVS
supports OFPR_INVALID_TTL value as a Nicira extension to OF1.0, so
there's not much value in breaking it apart across header files.

Also for ofp_flow_removed_reason.

For stats messages with fixed length, we've found over time that it's
more convenient to include the ofp_header and stats message header in
the structure, rather than having separate headers and bodies.  For
example, look at struct ofp_flow_stats_request in openflow-1.0.h.  I
think that we should retain this pattern in openflow-1.1.h where it is
applicable, that is, for ofp11_flow_stats_request,
ofp11_queue_stats_request, and ofp11_group_stats_request, I think.

I'd remove ofp11_aggregate_stats_request since it is identical to
ofp11_flow_stats_request, just updating the comment to note that.

Thanks,

Ben.