[ovs-dev] [pre-async 8/8] ofproto: Better abstract flow stats encoding.

Ben Pfaff blp at nicira.com
Tue May 31 23:57:23 UTC 2011


On Tue, May 31, 2011 at 01:13:08AM -0700, Justin Pettit wrote:
> On May 24, 2011, at 4:25 PM, Ben Pfaff wrote:
> 
> > +/* Appends an OFPST_FLOW or NXST_FLOW reply that contains the data in 'fs' to
> > + * those already present in the list of ofpbufs in 'replies'.  'replies' should
> > + * have been initialized with ofputil_start_stats_reply(). */
> > +void
> > +ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs,
> > +                                struct list *replies)
> > +{
> > ...
> > +}
> 
> This is really minor, but in both forms of the reply, the "cookie"
> is the only element that's out of order for the "ofp_flow_stats" and
> "nx_flow_stats" structures.  Having them be in the same order makes
> sanity-checking them easier.

I also like to keep them in the same order.  Fixed.

> > -    ofconn_send_replies(ofconn, &replies);
> > +
> > +    LIST_FOR_EACH_SAFE (msg, next, list_node, &replies) {
> > +        list_remove(&msg->list_node);
> > +        ofconn_send_reply(ofconn, msg);
> > +    }
> 
> Is there a reason you replaced ofconn_send_replies() with this?
> Aren't they the same?

I think this was some kind of rebasing error.  ofconn_send_replies()
is the same, so I put it back.

> Thanks for re-abstracting everything in this series.  It's much cleaner now.

Thanks for the review.



More information about the dev mailing list