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 <ho...@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?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to