On Fri, Oct 25, 2013 at 03:02:00PM -0700, Joe Stringer wrote:
> On Fri, Oct 25, 2013 at 1:12 PM, Ben Pfaff <[email protected]> wrote:
>
> > From: Venkitachalam Gopalakrishnan <[email protected]>
> >
> > Open vSwitch has never implemented this request and reply, even though they
> > have been in OpenFlow since version 1.0. This commit adds an
> > implementation.
> >
> > Signed-off: Venkitachalam Gopalakrishnan <[email protected]>
> > Co-authored-by: Ben Pfaff <[email protected]>
> > Signed-off-by: Ben Pfaff <[email protected]>
Please consider trimming context more aggressively, it makes the
comments easier to see.
> > --- a/include/openflow/openflow-1.2.h
> > +++ b/include/openflow/openflow-1.2.h
> > @@ -251,13 +251,16 @@ enum ofp12_capabilities {
> > OFPC12_PORT_BLOCKED = 1 << 8 /* Switch will block looping ports.
> > */
> > };
> >
> > -/* OpenFlow 1.2 specific properties
> > - * (struct ofp_queue_prop_header member property). */
> > -enum ofp12_queue_properties {
> > - OFPQT12_MIN_RATE = 1, /* Minimum datarate guaranteed. */
> > - OFPQT12_MAX_RATE = 2, /* Maximum datarate. */
> > - OFPQT12_EXPERIMENTER = 0xffff /* Experimenter defined property. */
...
> > enum ofp_queue_properties {
> > - OFPQT_NONE = 0, /* No property defined for queue (default). */
> > - OFPQT_MIN_RATE, /* Minimum datarate guaranteed. */
> > - /* Other types should be added here
> > - * (i.e. max rate, precedence, etc). */
> > + OFPQT_MIN_RATE = 1, /* Minimum datarate guaranteed. */
> > + OFPQT_MAX_RATE = 2, /* Maximum guaranteed rate. */
> > };
>
> Is there a particular reason why OFPQT_NONE and OFPQT_EXPERIMENTER are
> dropped?
OFPQT_NONE was removed from later standards because it was never defined
and it didn't make any sense to define it (why would you define a
property to say there are no properties?).
I can leave OFPQT_EXPERIMENTER defined if you like, it doesn't hurt anything.
> > --- a/lib/ofp-util.h
> > +++ b/lib/ofp-util.h
> > @@ -26,6 +26,7 @@
> > #include "match.h"
> > #include "netdev.h"
> > #include "openflow/nicira-ext.h"
> > +#include "openflow/openflow-1.1.h"
> >
>
> Is this needed?
Doesn't look like it, I'll remove it.
> Acked-by: Joe Stringer <[email protected]>
Thanks!
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
