acked-by: Andy Zhou <az...@nicira.com>
On Wed, Jul 17, 2013 at 3:58 PM, Ben Pfaff <b...@nicira.com> wrote:
> OpenFlow 1.0, 1.1, and 1.2 all have the same struct size for port and
> queue stats. OpenFlow 1.3 has larger structs, but the ofp-util code didn't
> realize that. This fixes the problem.
>
> It appears that the only consequence of this problem would have been
> printing the wrong count in ofp-print output.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> lib/ofp-util.c | 39 +++++++++++++++++++++++++++++++++------
> tests/ofp-print.at | 31 +++++++++++++++++++++++++++++++
> 2 files changed, 64 insertions(+), 6 deletions(-)
>
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 8921592..f1ba19c 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -5183,6 +5183,21 @@ ofputil_port_stats_from_ofp13(struct
> ofputil_port_stats *ops,
> return error;
> }
>
> +static size_t
> +ofputil_get_port_stats_size(enum ofp_version ofp_version)
> +{
> + switch (ofp_version) {
> + case OFP10_VERSION:
> + return sizeof(struct ofp10_port_stats);
> + case OFP11_VERSION:
> + case OFP12_VERSION:
> + return sizeof(struct ofp11_port_stats);
> + case OFP13_VERSION:
> + return sizeof(struct ofp13_port_stats);
> + default:
> + NOT_REACHED();
> + }
> +}
>
> /* Returns the number of port stats elements in OFPTYPE_PORT_STATS_REPLY
> * message 'oh'. */
> @@ -5194,9 +5209,7 @@ ofputil_count_port_stats(const struct ofp_header *oh)
> ofpbuf_use_const(&b, oh, ntohs(oh->length));
> ofpraw_pull_assert(&b);
>
> - BUILD_ASSERT(sizeof(struct ofp10_port_stats) ==
> - sizeof(struct ofp11_port_stats));
> - return b.size / sizeof(struct ofp10_port_stats);
> + return b.size / ofputil_get_port_stats_size(oh->version);
> }
>
> /* Converts an OFPST_PORT_STATS reply in 'msg' into an abstract
> @@ -5352,6 +5365,22 @@ ofputil_encode_queue_stats_request(enum ofp_version
> ofp_version,
> return request;
> }
>
> +static size_t
> +ofputil_get_queue_stats_size(enum ofp_version ofp_version)
> +{
> + switch (ofp_version) {
> + case OFP10_VERSION:
> + return sizeof(struct ofp10_queue_stats);
> + case OFP11_VERSION:
> + case OFP12_VERSION:
> + return sizeof(struct ofp11_queue_stats);
> + case OFP13_VERSION:
> + return sizeof(struct ofp13_queue_stats);
> + default:
> + NOT_REACHED();
> + }
> +}
> +
> /* Returns the number of queue stats elements in OFPTYPE_QUEUE_STATS_REPLY
> * message 'oh'. */
> size_t
> @@ -5362,9 +5391,7 @@ ofputil_count_queue_stats(const struct ofp_header
> *oh)
> ofpbuf_use_const(&b, oh, ntohs(oh->length));
> ofpraw_pull_assert(&b);
>
> - BUILD_ASSERT(sizeof(struct ofp10_queue_stats) ==
> - sizeof(struct ofp11_queue_stats));
> - return b.size / sizeof(struct ofp10_queue_stats);
> + return b.size / ofputil_get_queue_stats_size(oh->version);
> }
>
> static enum ofperr
> diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> index 22886fc..63a89ec 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -1579,6 +1579,37 @@ OFPST_QUEUE reply (OF1.2) (xid=0x1): 6 queues
> port 2 queue 2: bytes=0, pkts=0, errors=0
> port 1 queue 1: bytes=0, pkts=0, errors=0
> port 1 queue 2: bytes=0, pkts=0, errors=0
> +
> +AT_SETUP([OFPST_QUEUE reply - OF1.3])
> +AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
> +AT_CHECK([ovs-ofctl ofp-print "\
> +04 13 01 00 00 00 00 01 00 05 00 00 00 00 00 00 \
> +00 00 00 03 00 00 00 01 00 00 00 00 00 00 01 2e \
> +00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 00 \
> +00 00 00 64 1d cd 65 00 \
> +00 00 00 03 00 00 00 02 00 00 00 00 00 00 00 00 \
> +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
> +00 00 00 64 1d cd 65 00 \
> +00 00 00 02 00 00 00 01 00 00 00 00 00 00 08 34 \
> +00 00 00 00 00 00 00 14 00 00 00 00 00 00 00 00 \
> +00 00 00 64 1d cd 65 00 \
> +00 00 00 02 00 00 00 02 00 00 00 00 00 00 00 00 \
> +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
> +00 00 00 64 1d cd 65 00 \
> +00 00 00 01 00 00 00 01 00 00 00 00 00 00 00 00 \
> +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
> +00 00 00 64 1d cd 65 00 \
> +00 00 00 01 00 00 00 02 00 00 00 00 00 00 00 00 \
> +00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
> +ff ff ff ff ff ff ff ff \
> +"], [0], [dnl
> +OFPST_QUEUE reply (OF1.3) (xid=0x1): 6 queues
> + port 3 queue 1: bytes=302, pkts=1, errors=0
> + port 3 queue 2: bytes=0, pkts=0, errors=0
> + port 2 queue 1: bytes=2100, pkts=20, errors=0
> + port 2 queue 2: bytes=0, pkts=0, errors=0
> + port 1 queue 1: bytes=0, pkts=0, errors=0
> + port 1 queue 2: bytes=0, pkts=0, errors=0
> ])
> AT_CLEANUP
>
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
