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

Simon Horman horms at verge.net.au
Thu Mar 29 00:33:31 UTC 2012


On Wed, Mar 28, 2012 at 05:20:30PM -0700, Ben Pfaff wrote:
> On Thu, Mar 29, 2012 at 09:18:57AM +0900, Simon Horman wrote:
> > On Thu, Mar 29, 2012 at 09:06:36AM +0900, Simon Horman wrote:
> > > On Wed, Mar 28, 2012 at 12:21:18PM -0700, Ben Pfaff wrote:
> > > > 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.
> > > 
> > > That sounds good to me, its probably easier all around to have
> > > the definitions centralised where possible.
> > 
> > Would it be appropriate to move these enums into
> > include/openflow/openflow-common.h ? Or would you prefer
> > that they appeared in the header corresponding to the first Open Flow
> > version they appear in?
> 
> openflow-common.h seems OK to me.

I will make it so.



More information about the dev mailing list