On Fri, Feb 19, 2016 at 02:47:59PM -0800, Jarno Rajahalme wrote:
> With one comment below:
>
> Acked-by: Jarno Rajahalme <ja...@ovn.org>
Thanks for the review.
> > On Feb 19, 2016, at 12:34 AM, Ben Pfaff <b...@ovn.org> wrote:
> >
> > It hadn't occurred to me before that any special support was actually
> > necessary or useful for nested properties, but the functions introduced in
> > this commit are nice wrappers to deal with the extra 4-byte padding that
> > ensures that the nested properties begin on 8-byte boundaries just like
> > the outer properties.
> >
> > Signed-off-by: Ben Pfaff <b...@ovn.org>
...
> > +enum ofperr
> > +ofpprop_parse_nested(const struct ofpbuf *property, struct ofpbuf *nested)
> > +{
> > + size_t nested_offset = ROUND_UP(ofpbuf_headersize(property), 8);
> > + if (property->size < nested_offset) {
> > + return OFPERR_OFPBPC_BAD_LEN;
> > + }
> > +
> > + ofpbuf_use_const(nested, property->data, property->size);
> > + ofpbuf_pull(nested, nested_offset);
> > + return 0;
> > +}
...
> > +/* Appends a header for a property of type 'type' to 'msg', followed by
> > padding
> > + * suitable for putting nested properties into the property; that is,
> > padding
> > + * to an 8-byte alignment.
> > + *
> > + * This otherwise works like ofpprop_start().
> > + *
> > + * There's no need for ofpprop_end_nested(), because ofpprop_end() works
> > fine
> > + * for this case. */
> > +size_t
> > +ofpprop_start_nested(struct ofpbuf *msg, uint64_t type)
> > +{
> > + size_t start_ofs = ofpprop_start(msg, type);
> > + ofpbuf_put_zeros(msg, 4);
>
> Somehow I find the ‘4’ here asymmetric to the
> ROUND_UP(ofpbuf_headersize(…)..) in ofpprop_parse_nested(). Maybe it
> would be better to simply pull ‘8’ there (with the comment that both
> the outer property and the padding are bypassed?
Hmm, 8 would be wrong sometimes there, because experimenter property
headers are 12 bytes.
Do you like this better:
ofpbuf_padto(msg, ROUND_UP(msg->size, 8));
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
