[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