[ovs-dev] [of1.1 rollup 14/20] openflow: Separate flow stats headers from their bodies.

[Ben Pfaff]

On Wed, Jun 13, 2012 at 09:44:11AM +0900, Simon Horman wrote:
> On Tue, Jun 12, 2012 at 12:32:18AM -0700, Ben Pfaff wrote:
> > @@ -2208,11 +2208,10 @@ ofputil_encode_aggregate_stats_reply(
> >          put_32aligned_be64(&asr->byte_count,
> >                             htonll(unknown_to_zero(stats->byte_count)));
> >          asr->flow_count = htonl(stats->flow_count);
> > -    } else if (request->type == htons(OFPST_VENDOR)) {
> > +    } else if (code == OFPUTIL_NXST_AGGREGATE_REQUEST) {
> >          struct nx_aggregate_stats_reply *nasr;
> >  
> >          nasr = ofputil_make_stats_reply(sizeof *nasr, request, &msg);
> > -        assert(nasr->nsm.subtype == htonl(NXST_AGGREGATE));
> 
> Its unclear to me why the above assert is now less useful than before.

"struct nx_aggregate_stats_reply" no longer has a member of the
appropriate type.  That's the main purpose of the patch: to separate
stats headers from their bodies.

> >  void *
> > -ofputil_make_stats_request(size_t openflow_len, uint16_t ofpst_type,
> > +ofputil_make_stats_request(size_t body_len, uint16_t ofpst_type,
> >                             uint32_t nxst_subtype, struct ofpbuf **bufferp)
> >  {
> >      struct ofpbuf *msg;
> >  
> > -    msg = *bufferp = ofpbuf_new(openflow_len);
> > +    msg = *bufferp = ofpbuf_new(24 + body_len);
> 
> Perhaps 24 could be a #define or similar?

Yeah, I should have done that to begin with.

I added:

    enum {
        HEADER_LEN = MAX(sizeof(struct ofp_stats_msg),
                         sizeof(struct nicira_stats_msg));
    }

and used it in place of 24.

Thanks,

Ben.