> On Feb 19, 2016, at 3:56 PM, Ben Pfaff <b...@ovn.org> wrote:
>
> 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));
Yes, thanks!
Jarno
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
