[ovs-dev] [PATCH v4 06/14] ofp-prop: Add support for putting and parsing nested properties.

Jarno Rajahalme jarno at ovn.org
Sat Feb 20 00:23:26 UTC 2016


> On Feb 19, 2016, at 3:56 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> On Fri, Feb 19, 2016 at 02:47:59PM -0800, Jarno Rajahalme wrote:
>> With one comment below:
>> 
>> Acked-by: Jarno Rajahalme <jarno at ovn.org>
> 
> Thanks for the review.
> 
>>> On Feb 19, 2016, at 12:34 AM, Ben Pfaff <blp at 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 <blp at 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





More information about the dev mailing list