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

[Jarno Rajahalme]

With one comment below:

Acked-by: Jarno Rajahalme <jarno at ovn.org>

> 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>
> ---
> lib/ofp-prop.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> lib/ofp-prop.h |  4 ++++
> 2 files changed, 53 insertions(+)
> 
> diff --git a/lib/ofp-prop.c b/lib/ofp-prop.c
> index d6fd3a6..b6395b6 100644
> --- a/lib/ofp-prop.c
> +++ b/lib/ofp-prop.c
> @@ -265,6 +265,27 @@ ofpprop_parse_uuid(const struct ofpbuf *property, struct uuid *uuid)
>     return 0;
> }
> 
> +/* Attempts to parse 'property' as a property that contains nested properties.
> + * If successful, stores the nested data into '*nested' and returns 0;
> + * otherwise returns an OpenFlow error.
> + *
> + * The only thing special about nested properties is that the property header
> + * is followed by 4 bytes of padding, so that the nested properties begin at an
> + * 8-byte aligned offset.  This function can be used in other situations where
> + * this is the case. */
> +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;
> +}
> +
> /* Adds a property with the given 'type' and 'len'-byte contents 'value' to
>  * 'msg', padding the property out to a multiple of 8 bytes. */
> void
> @@ -392,6 +413,18 @@ ofpprop_put_uuid(struct ofpbuf *msg, uint64_t type, const struct uuid *uuid)
>     ofpprop_put(msg, type, uuid, sizeof *uuid);
> }
> 
> +/* Appends a property of type 'type' to 'msg' whose contents are padding to
> + * 8-byte alignment followed by 'nested'.  This is a suitable way to add nested
> + * properties to 'msg'. */
> +void
> +ofpprop_put_nested(struct ofpbuf *msg, uint64_t type,
> +                   const struct ofpbuf *nested)
> +{
> +    size_t start = ofpprop_start_nested(msg, type);
> +    ofpbuf_put(msg, nested->data, nested->size);
> +    ofpprop_end(msg, start);
> +}
> +
> /* Appends a header for a property of type 'type' to 'msg'.  The caller should
>  * add the contents of the property to 'msg', then finish it by calling
>  * ofpprop_end().  Returns the offset of the beginning of the property (to pass
> @@ -429,6 +462,22 @@ ofpprop_end(struct ofpbuf *msg, size_t start_ofs)
>     ofpbuf_padto(msg, ROUND_UP(msg->size, 8));
> }
> 
> +/* 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?

> +    return start_ofs;
> +}
> +
> enum ofperr
> ofpprop_unknown(struct vlog_module *module, bool loose, const char *msg,
>                 uint64_t type)
> diff --git a/lib/ofp-prop.h b/lib/ofp-prop.h
> index 921b6c1..4f8e78d 100644
> --- a/lib/ofp-prop.h
> +++ b/lib/ofp-prop.h
> @@ -85,6 +85,7 @@ enum ofperr ofpprop_parse_u16(const struct ofpbuf *, uint16_t *value);
> enum ofperr ofpprop_parse_u32(const struct ofpbuf *, uint32_t *value);
> enum ofperr ofpprop_parse_u64(const struct ofpbuf *, uint64_t *value);
> enum ofperr ofpprop_parse_uuid(const struct ofpbuf *, struct uuid *);
> +enum ofperr ofpprop_parse_nested(const struct ofpbuf *, struct ofpbuf *);
> 
> /* Serializing properties. */
> void ofpprop_put(struct ofpbuf *, uint64_t type,
> @@ -100,10 +101,13 @@ void ofpprop_put_u64(struct ofpbuf *, uint64_t type, uint64_t value);
> void ofpprop_put_bitmap(struct ofpbuf *, uint64_t type, uint64_t bitmap);
> void ofpprop_put_flag(struct ofpbuf *, uint64_t type);
> void ofpprop_put_uuid(struct ofpbuf *, uint64_t type, const struct uuid *);
> +void ofpprop_put_nested(struct ofpbuf *, uint64_t type, const struct ofpbuf *);
> 
> size_t ofpprop_start(struct ofpbuf *, uint64_t type);
> void ofpprop_end(struct ofpbuf *, size_t start_ofs);
> 
> +size_t ofpprop_start_nested(struct ofpbuf *, uint64_t type);
> +
> /* Logging errors while deserializing properties.
>  *
>  * The attitude that a piece of code should take when it deserializes an
> -- 
> 2.1.3
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev