[ovs-dev] [PATCH 25/41] ofp-util: Fix OF1.4+ version of ofputil_decode_set_async_config().

[Jarno Rajahalme]

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

> On Jan 18, 2016, at 11:27 PM, Ben Pfaff <blp at ovn.org> wrote:
> 
> The OF1.0 through OF1.3 "set async config" set the whole configuration,
> OF1.4+ only update parts of it piecemeal, but the decoding function always
> set the whole configuration.  This commit fixes the problem by changing the
> interface to require the caller to provide an initial state.  (It would
> be possible to simply make it mutate existing state in-place, but that
> interface seems a little more error-prone.)
> 
> Found by inspection.
> 
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
> OPENFLOW-1.1+.md  | 5 +----
> lib/ofp-print.c   | 7 +++++--
> lib/ofp-util.c    | 8 ++++++++
> lib/ofp-util.h    | 1 +
> ofproto/ofproto.c | 5 +++--
> 5 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md
> index 537f660..62ebddc 100644
> --- a/OPENFLOW-1.1+.md
> +++ b/OPENFLOW-1.1+.md
> @@ -192,10 +192,7 @@ OpenFlow 1.4 features are listed in the previous section.
> 
>   * More extensible wire protocol
>     Many on-wire structures got TLVs.
> -    Already implemented: port desc properties, port mod properties,
> -                         port stats properties, table mod properties,
> -                         queue stats, unified property errors, queue desc.
> -    Remaining required: set-async
> +    All required features are now supported.
>     Remaining optional: table desc, table-status
>     [EXT-262]
>     [required for OF1.4+]
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index accde8e..f36335b 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -2130,9 +2130,12 @@ ofp_print_nxt_set_async_config(struct ds *string,
>         }
>     } else if (raw == OFPRAW_OFPT14_SET_ASYNC ||
>                raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) {
> -        struct ofputil_async_cfg ac = OFPUTIL_ASYNC_CFG_INIT;
> +        struct ofputil_async_cfg basis = OFPUTIL_ASYNC_CFG_INIT;
> +        struct ofputil_async_cfg ac;
> +
>         bool is_reply = raw == OFPRAW_OFPT14_GET_ASYNC_REPLY;
> -        enum ofperr error = ofputil_decode_set_async_config(oh, is_reply, &ac);
> +        enum ofperr error = ofputil_decode_set_async_config(oh, is_reply,
> +                                                            &basis, &ac);
>         if (error) {
>             ofp_print_error(string, error);
>             return;
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 19d3775..5bb0b74 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -9570,6 +9570,11 @@ decode_legacy_async_masks(const ovs_be32 masks[2],
> /* Decodes the OpenFlow "set async config" request and "get async config
>  * reply" message in '*oh' into an abstract form in 'ac'.
>  *
> + * Some versions of the "set async config" request change only some of the
> + * settings and leave the others alone.  This function uses 'basis' as the
> + * initial state for decoding these.  Other versions of the request change all
> + * the settings; this function ignores 'basis' when decoding these.
> + *
>  * If 'loose' is true, this function ignores properties and values that it does
>  * not understand, as a controller would want to do when interpreting
>  * capabilities provided by a switch.  If 'loose' is false, this function
> @@ -9585,6 +9590,7 @@ decode_legacy_async_masks(const ovs_be32 masks[2],
>  * supported.*/
> enum ofperr
> ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose,
> +                                const struct ofputil_async_cfg *basis,
>                                 struct ofputil_async_cfg *ac)
> {
>     enum ofpraw raw;
> @@ -9598,6 +9604,7 @@ ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose,
>         raw == OFPRAW_OFPT13_GET_ASYNC_REPLY) {
>         const struct nx_async_config *msg = ofpmsg_body(oh);
> 
> +        *ac = OFPUTIL_ASYNC_CFG_INIT;
>         decode_legacy_async_masks(msg->packet_in_mask, OAM_PACKET_IN,
>                                   oh->version, ac);
>         decode_legacy_async_masks(msg->port_status_mask, OAM_PORT_STATUS,
> @@ -9606,6 +9613,7 @@ ofputil_decode_set_async_config(const struct ofp_header *oh, bool loose,
>                                   oh->version, ac);
>     } else if (raw == OFPRAW_OFPT14_SET_ASYNC ||
>                raw == OFPRAW_OFPT14_GET_ASYNC_REPLY) {
> +        *ac = *basis;
>         while (b.size > 0) {
>             struct ofpbuf property;
>             enum ofperr error;
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index 52268d8..88c67f9 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -1317,6 +1317,7 @@ struct ofputil_async_cfg {
> 
> enum ofperr ofputil_decode_set_async_config(const struct ofp_header *,
>                                             bool loose,
> +                                            const struct ofputil_async_cfg *,
>                                             struct ofputil_async_cfg *);
> 
> struct ofpbuf *ofputil_encode_get_async_config(
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index fbaf7dd..957e323 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -5403,10 +5403,11 @@ handle_nxt_set_packet_in_format(struct ofconn *ofconn,
> static enum ofperr
> handle_nxt_set_async_config(struct ofconn *ofconn, const struct ofp_header *oh)
> {
> -    struct ofputil_async_cfg ac = OFPUTIL_ASYNC_CFG_INIT;
> +    struct ofputil_async_cfg basis = ofconn_get_async_config(ofconn);
> +    struct ofputil_async_cfg ac;
>     enum ofperr error;
> 
> -    error = ofputil_decode_set_async_config(oh, false, &ac);
> +    error = ofputil_decode_set_async_config(oh, false, &basis, &ac);
>     if (error) {
>         return error;
>     }
> -- 
> 2.1.3
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev