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 <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?
>
> openflow-common.h seems OK to me.
I will make it so.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
