acked-by: Andy Zhou <az...@nicira.com>
On Wed, Jul 17, 2013 at 3:58 PM, Ben Pfaff <b...@nicira.com> wrote:
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> OPENFLOW-1.1+ | 3 ---
> lib/netdev-linux.c | 11 ++++++++---
> lib/netdev.c | 8 ++++++--
> lib/netdev.h | 3 +++
> lib/ofp-print.c | 14 +++++++++++---
> lib/ofp-util.c | 44 ++++++++++++++++++++++++--------------------
> lib/ofp-util.h | 10 +++++++++-
> ofproto/ofproto.c | 19 ++++++++++++++-----
> tests/ofp-print.at | 50
> ++++++++++++++++++++++++++------------------------
> 9 files changed, 101 insertions(+), 61 deletions(-)
>
> diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
> index d08a46e..aea689e 100644
> --- a/OPENFLOW-1.1+
> +++ b/OPENFLOW-1.1+
> @@ -143,9 +143,6 @@ didn't compare the specs carefully yet.)
> * Rework tag order. I'm not sure whether we need to do anything
> for this.
>
> - * Duration for queue stats. (Duration for port stats is already
> - implemented.)
> -
> * On-demand flow counters. I think this might be a real
> optimization in some cases for the software switch.
>
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 8790f14..83d3d67 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -148,6 +148,7 @@ struct tc {
> struct tc_queue {
> struct hmap_node hmap_node; /* In struct tc's "queues" hmap. */
> unsigned int queue_id; /* OpenFlow queue ID. */
> + long long int created; /* Time queue was created, in msecs. */
> };
>
> /* A particular kind of traffic control. Each implementation generally
> maps to
> @@ -2010,9 +2011,11 @@ netdev_linux_get_queue_stats(const struct netdev
> *netdev_,
> return EOPNOTSUPP;
> } else {
> const struct tc_queue *queue = tc_find_queue(netdev_, queue_id);
> - return (queue
> - ? netdev->tc->ops->class_get_stats(netdev_, queue, stats)
> - : ENOENT);
> + if (!queue) {
> + return ENOENT;
> + }
> + stats->created = queue->created;
> + return netdev->tc->ops->class_get_stats(netdev_, queue, stats);
> }
> }
>
> @@ -2819,6 +2822,7 @@ htb_update_queue__(struct netdev *netdev, unsigned
> int queue_id,
> hcp = xmalloc(sizeof *hcp);
> queue = &hcp->tc_queue;
> queue->queue_id = queue_id;
> + queue->created = time_msec();
> hmap_insert(&htb->tc.queues, &queue->hmap_node, hash);
> }
>
> @@ -3052,6 +3056,7 @@ hfsc_update_queue__(struct netdev *netdev, unsigned
> int queue_id,
> hcp = xmalloc(sizeof *hcp);
> queue = &hcp->tc_queue;
> queue->queue_id = queue_id;
> + queue->created = time_msec();
> hmap_insert(&hfsc->tc.queues, &queue->hmap_node, hash);
> }
>
> diff --git a/lib/netdev.c b/lib/netdev.c
> index 727c538..6e2a92f 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -1246,7 +1246,8 @@ netdev_delete_queue(struct netdev *netdev, unsigned
> int queue_id)
> /* Obtains statistics about 'queue_id' on 'netdev'. On success, returns
> 0 and
> * fills 'stats' with the queue's statistics; individual members of
> 'stats' may
> * be set to all-1-bits if the statistic is unavailable. On failure,
> returns a
> - * positive errno value and fills 'stats' with all-1-bits. */
> + * positive errno value and fills 'stats' with values indicating
> unsupported
> + * statistics. */
> int
> netdev_get_queue_stats(const struct netdev *netdev, unsigned int queue_id,
> struct netdev_queue_stats *stats)
> @@ -1258,7 +1259,10 @@ netdev_get_queue_stats(const struct netdev *netdev,
> unsigned int queue_id,
> ? class->get_queue_stats(netdev, queue_id, stats)
> : EOPNOTSUPP);
> if (retval) {
> - memset(stats, 0xff, sizeof *stats);
> + stats->tx_bytes = UINT64_MAX;
> + stats->tx_packets = UINT64_MAX;
> + stats->tx_errors = UINT64_MAX;
> + stats->created = LLONG_MIN;
> }
> return retval;
> }
> diff --git a/lib/netdev.h b/lib/netdev.h
> index b1cc319..eb1870b 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -227,6 +227,9 @@ struct netdev_queue_stats {
> uint64_t tx_bytes;
> uint64_t tx_packets;
> uint64_t tx_errors;
> +
> + /* Time at which the queue was created, in msecs, LLONG_MIN if
> unknown. */
> + long long int created;
> };
>
> int netdev_set_policing(struct netdev *, uint32_t kbits_rate,
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index 76bd09c..aabb168 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -1737,9 +1737,17 @@ ofp_print_ofpst_queue_reply(struct ds *string,
> const struct ofp_header *oh,
> ofp_print_queue_name(string, qs.queue_id);
> ds_put_cstr(string, ": ");
>
> - print_port_stat(string, "bytes=", qs.stats.tx_bytes, 1);
> - print_port_stat(string, "pkts=", qs.stats.tx_packets, 1);
> - print_port_stat(string, "errors=", qs.stats.tx_errors, 0);
> + print_port_stat(string, "bytes=", qs.tx_bytes, 1);
> + print_port_stat(string, "pkts=", qs.tx_packets, 1);
> + print_port_stat(string, "errors=", qs.tx_errors, 1);
> +
> + ds_put_cstr(string, "duration=");
> + if (qs.duration_sec != UINT32_MAX) {
> + ofp_print_duration(string, qs.duration_sec, qs.duration_nsec);
> + } else {
> + ds_put_char(string, '?');
> + }
> + ds_put_char(string, '\n');
> }
> }
>
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index f1ba19c..95eb3e0 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -5400,9 +5400,10 @@ ofputil_queue_stats_from_ofp10(struct
> ofputil_queue_stats *oqs,
> {
> oqs->port_no = u16_to_ofp(ntohs(qs10->port_no));
> oqs->queue_id = ntohl(qs10->queue_id);
> - oqs->stats.tx_bytes = ntohll(get_32aligned_be64(&qs10->tx_bytes));
> - oqs->stats.tx_packets = ntohll(get_32aligned_be64(&qs10->tx_packets));
> - oqs->stats.tx_errors = ntohll(get_32aligned_be64(&qs10->tx_errors));
> + oqs->tx_bytes = ntohll(get_32aligned_be64(&qs10->tx_bytes));
> + oqs->tx_packets = ntohll(get_32aligned_be64(&qs10->tx_packets));
> + oqs->tx_errors = ntohll(get_32aligned_be64(&qs10->tx_errors));
> + oqs->duration_sec = oqs->duration_nsec = UINT32_MAX;
>
> return 0;
> }
> @@ -5419,9 +5420,10 @@ ofputil_queue_stats_from_ofp11(struct
> ofputil_queue_stats *oqs,
> }
>
> oqs->queue_id = ntohl(qs11->queue_id);
> - oqs->stats.tx_bytes = ntohll(qs11->tx_bytes);
> - oqs->stats.tx_packets = ntohll(qs11->tx_packets);
> - oqs->stats.tx_errors = ntohll(qs11->tx_errors);
> + oqs->tx_bytes = ntohll(qs11->tx_bytes);
> + oqs->tx_packets = ntohll(qs11->tx_packets);
> + oqs->tx_errors = ntohll(qs11->tx_errors);
> + oqs->duration_sec = oqs->duration_nsec = UINT32_MAX;
>
> return 0;
> }
> @@ -5430,11 +5432,10 @@ static enum ofperr
> ofputil_queue_stats_from_ofp13(struct ofputil_queue_stats *oqs,
> const struct ofp13_queue_stats *qs13)
> {
> - enum ofperr error
> - = ofputil_queue_stats_from_ofp11(oqs, &qs13->qs);
> + enum ofperr error = ofputil_queue_stats_from_ofp11(oqs, &qs13->qs);
> if (!error) {
> - /* FIXME: Get qs13->duration_sec and qs13->duration_nsec,
> - * Add to netdev_queue_stats? */
> + oqs->duration_sec = ntohl(qs13->duration_sec);
> + oqs->duration_nsec = ntohl(qs13->duration_nsec);
> }
>
> return error;
> @@ -5506,9 +5507,9 @@ ofputil_queue_stats_to_ofp10(const struct
> ofputil_queue_stats *oqs,
> qs10->port_no = htons(ofp_to_u16(oqs->port_no));
> memset(qs10->pad, 0, sizeof qs10->pad);
> qs10->queue_id = htonl(oqs->queue_id);
> - put_32aligned_be64(&qs10->tx_bytes, htonll(oqs->stats.tx_bytes));
> - put_32aligned_be64(&qs10->tx_packets, htonll(oqs->stats.tx_packets));
> - put_32aligned_be64(&qs10->tx_errors, htonll(oqs->stats.tx_errors));
> + put_32aligned_be64(&qs10->tx_bytes, htonll(oqs->tx_bytes));
> + put_32aligned_be64(&qs10->tx_packets, htonll(oqs->tx_packets));
> + put_32aligned_be64(&qs10->tx_errors, htonll(oqs->tx_errors));
> }
>
> static void
> @@ -5517,9 +5518,9 @@ ofputil_queue_stats_to_ofp11(const struct
> ofputil_queue_stats *oqs,
> {
> qs11->port_no = ofputil_port_to_ofp11(oqs->port_no);
> qs11->queue_id = htonl(oqs->queue_id);
> - qs11->tx_bytes = htonll(oqs->stats.tx_bytes);
> - qs11->tx_packets = htonll(oqs->stats.tx_packets);
> - qs11->tx_errors = htonll(oqs->stats.tx_errors);
> + qs11->tx_bytes = htonll(oqs->tx_bytes);
> + qs11->tx_packets = htonll(oqs->tx_packets);
> + qs11->tx_errors = htonll(oqs->tx_errors);
> }
>
> static void
> @@ -5527,10 +5528,13 @@ ofputil_queue_stats_to_ofp13(const struct
> ofputil_queue_stats *oqs,
> struct ofp13_queue_stats *qs13)
> {
> ofputil_queue_stats_to_ofp11(oqs, &qs13->qs);
> - /* OF 1.3 adds duration fields */
> - /* FIXME: Need to implement queue alive duration (sec + nsec) */
> - qs13->duration_sec = htonl(~0);
> - qs13->duration_nsec = htonl(~0);
> + if (oqs->duration_sec != UINT32_MAX) {
> + qs13->duration_sec = htonl(oqs->duration_sec);
> + qs13->duration_nsec = htonl(oqs->duration_nsec);
> + } else {
> + qs13->duration_sec = htonl(UINT32_MAX);
> + qs13->duration_nsec = htonl(UINT32_MAX);
> + }
> }
>
> /* Encode a queue stat for 'oqs' and append it to 'replies'. */
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index 0385a57..f94982d 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -832,7 +832,15 @@ ofputil_encode_queue_stats_request(enum ofp_version
> ofp_version,
> struct ofputil_queue_stats {
> ofp_port_t port_no;
> uint32_t queue_id;
> - struct netdev_queue_stats stats;
> +
> + /* Values of unsupported statistics are set to all-1-bits
> (UINT64_MAX). */
> + uint64_t tx_bytes;
> + uint64_t tx_packets;
> + uint64_t tx_errors;
> +
> + /* UINT32_MAX if unknown. */
> + uint32_t duration_sec;
> + uint32_t duration_nsec;
> };
>
> size_t ofputil_count_queue_stats(const struct ofp_header *);
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 3f4b8a7..98155ac 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -3170,18 +3170,26 @@ handle_aggregate_stats_request(struct ofconn
> *ofconn,
> struct queue_stats_cbdata {
> struct ofport *ofport;
> struct list replies;
> + long long int now;
> };
>
> static void
> put_queue_stats(struct queue_stats_cbdata *cbdata, uint32_t queue_id,
> const struct netdev_queue_stats *stats)
> {
> + struct ofputil_queue_stats oqs;
>
> - struct ofputil_queue_stats oqs = {
> - .port_no = cbdata->ofport->pp.port_no,
> - .queue_id = queue_id,
> - .stats = *stats,
> - };
> + oqs.port_no = cbdata->ofport->pp.port_no;
> + oqs.queue_id = queue_id;
> + oqs.tx_bytes = stats->tx_bytes;
> + oqs.tx_packets = stats->tx_packets;
> + oqs.tx_errors = stats->tx_errors;
> + if (stats->created != LLONG_MIN) {
> + calc_duration(stats->created, cbdata->now,
> + &oqs.duration_sec, &oqs.duration_nsec);
> + } else {
> + oqs.duration_sec = oqs.duration_nsec = UINT32_MAX;
> + }
> ofputil_append_queue_stat(&cbdata->replies, &oqs);
> }
>
> @@ -3228,6 +3236,7 @@ handle_queue_stats_request(struct ofconn *ofconn,
> COVERAGE_INC(ofproto_queue_req);
>
> ofpmp_init(&cbdata.replies, rq);
> + cbdata.now = time_msec();
>
> error = ofputil_decode_queue_stats_request(rq, &oqsr);
> if (error) {
> diff --git a/tests/ofp-print.at b/tests/ofp-print.at
> index 63a89ec..986b931 100644
> --- a/tests/ofp-print.at
> +++ b/tests/ofp-print.at
> @@ -1512,12 +1512,12 @@ AT_CHECK([ovs-ofctl ofp-print "\
> 00 00 00 00 00 00 00 00 00 00 00 00 \
> "], [0], [dnl
> OFPST_QUEUE reply (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
> + port 3 queue 1: bytes=302, pkts=1, errors=0, duration=?
> + port 3 queue 2: bytes=0, pkts=0, errors=0, duration=?
> + port 2 queue 1: bytes=2100, pkts=20, errors=0, duration=?
> + port 2 queue 2: bytes=0, pkts=0, errors=0, duration=?
> + port 1 queue 1: bytes=0, pkts=0, errors=0, duration=?
> + port 1 queue 2: bytes=0, pkts=0, errors=0, duration=?
> ])
> AT_CLEANUP
>
> @@ -1546,12 +1546,12 @@ AT_CHECK([ovs-ofctl ofp-print "\
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
> "], [0], [dnl
> OFPST_QUEUE reply (OF1.1) (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
> + port 3 queue 1: bytes=302, pkts=1, errors=0, duration=?
> + port 3 queue 2: bytes=0, pkts=0, errors=0, duration=?
> + port 2 queue 1: bytes=2100, pkts=20, errors=0, duration=?
> + port 2 queue 2: bytes=0, pkts=0, errors=0, duration=?
> + port 1 queue 1: bytes=0, pkts=0, errors=0, duration=?
> + port 1 queue 2: bytes=0, pkts=0, errors=0, duration=?
> ])
> AT_CLEANUP
>
> @@ -1573,12 +1573,14 @@ AT_CHECK([ovs-ofctl ofp-print "\
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
> "], [0], [dnl
> OFPST_QUEUE reply (OF1.2) (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
> + port 3 queue 1: bytes=302, pkts=1, errors=0, duration=?
> + port 3 queue 2: bytes=0, pkts=0, errors=0, duration=?
> + port 2 queue 1: bytes=2100, pkts=20, errors=0, duration=?
> + port 2 queue 2: bytes=0, pkts=0, errors=0, duration=?
> + port 1 queue 1: bytes=0, pkts=0, errors=0, duration=?
> + port 1 queue 2: bytes=0, pkts=0, errors=0, duration=?
> +])
> +AT_CLEANUP
>
> AT_SETUP([OFPST_QUEUE reply - OF1.3])
> AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
> @@ -1604,12 +1606,12 @@ AT_CHECK([ovs-ofctl ofp-print "\
> 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
> + port 3 queue 1: bytes=302, pkts=1, errors=0, duration=100.5s
> + port 3 queue 2: bytes=0, pkts=0, errors=0, duration=100.5s
> + port 2 queue 1: bytes=2100, pkts=20, errors=0, duration=100.5s
> + port 2 queue 2: bytes=0, pkts=0, errors=0, duration=100.5s
> + port 1 queue 1: bytes=0, pkts=0, errors=0, duration=100.5s
> + port 1 queue 2: bytes=0, pkts=0, errors=0, duration=?
> ])
> 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
