Looks Good.

Ethan

On Thu, Jul 21, 2011 at 14:45, Ben Pfaff <[email protected]> wrote:
> These ovs-ofctl commands have been sending malformed stats requests since
> commit 63f2140a553 "openflow: Make stats replies more like other OpenFlow
> messages."  This commit fixes the problem and adds basic unit tests that
> should prevent similar regressions.
>
> Reported-by: Hao Zheng <[email protected]>
> ---
>  tests/ofproto.at      |   24 ++++++++++++++++++++++++
>  utilities/ovs-ofctl.c |    9 +++++----
>  2 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index e6def3e..227efe0 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -19,6 +19,30 @@ OFPT_GET_CONFIG_REPLY: frags=normal miss_send_len=0
>  OFPROTO_STOP
>  AT_CLEANUP
>
> +dnl This is really bare-bones.
> +dnl It at least checks request and reply serialization and deserialization.
> +AT_SETUP([ofproto - port stats])
> +OFPROTO_START
> +AT_CHECK([ovs-ofctl -vANY:ANY:WARN dump-ports br0], [0], [stdout])
> +AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> +OFPST_PORT reply: 1 ports
> +  port 65534: rx pkts=0, bytes=0, drop=0, errs=0, frame=0, over=0, crc=0
> +           tx pkts=0, bytes=0, drop=0, errs=0, coll=0
> +])
> +OFPROTO_STOP
> +AT_CLEANUP
> +
> +dnl This is really bare-bones.
> +dnl It at least checks request and reply serialization and deserialization.
> +AT_SETUP([ofproto - queue stats])
> +OFPROTO_START
> +AT_CHECK([ovs-ofctl -vANY:ANY:WARN queue-stats br0], [0], [stdout])
> +AT_CHECK([STRIP_XIDS stdout], [0], [dnl
> +OFPST_QUEUE reply: 0 queues
> +])
> +OFPROTO_STOP
> +AT_CLEANUP
> +
>  AT_SETUP([ofproto - mod-port])
>  OFPROTO_START
>  for command_config_state in \
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 7cdf42d..d3fccce 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -261,13 +261,14 @@ open_vconn(const char *name, struct vconn **vconnp)
>  }
>
>  static void *
> -alloc_stats_request(size_t body_len, uint16_t type, struct ofpbuf **bufferp)
> +alloc_stats_request(size_t rq_len, uint16_t type, struct ofpbuf **bufferp)
>  {
>     struct ofp_stats_msg *rq;
> -    rq = make_openflow(sizeof *rq + body_len, OFPT_STATS_REQUEST, bufferp);
> +
> +    rq = make_openflow(rq_len, OFPT_STATS_REQUEST, bufferp);
>     rq->type = htons(type);
>     rq->flags = htons(0);
> -    return rq + 1;
> +    return rq;
>  }
>
>  static void
> @@ -333,7 +334,7 @@ static void
>  dump_trivial_stats_transaction(const char *vconn_name, uint8_t stats_type)
>  {
>     struct ofpbuf *request;
> -    alloc_stats_request(0, stats_type, &request);
> +    alloc_stats_request(sizeof(struct ofp_stats_msg), stats_type, &request);
>     dump_stats_transaction(vconn_name, request);
>  }
>
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to