Implement OpenFlow 1.3 queue stats duration feature.

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/OPENFLOW-1.1+ b/OPENFLOW-1.1+
index d08a46e..aea689e 100644 (file)
--- 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 ab4feae..301a754 100644 (file)
--- 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
@@ -1998,9 +1999,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);
     }
 }
 
@@ -2807,6 +2810,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);
     }
 
@@ -3040,6 +3044,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 (file)
--- 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 (file)
--- 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 1d8b98c..1a4dd9c 100644 (file)
--- 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 e328585..d1bcf9c 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -5401,9 +5401,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;
 }
@@ -5420,9 +5421,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;
 }
@@ -5431,11 +5433,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;
@@ -5507,9 +5508,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
@@ -5518,9 +5519,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
@@ -5528,10 +5529,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 (file)
--- 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 9119235..432aef3 100644 (file)
--- 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 (file)
--- 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