ofproto: Report nonexistent ports and queues as errors in queue stats.

Until now, Open vSwitch has ignored missing ports and queues in most cases
in queue stats requests, simply returning an empty set of statistics.
It seems that it is better to report an error, so this commit does this.

Reported-by: Prabina Pattnaik <Prabina.Pattnaik@nechclst.in>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/AUTHORS b/AUTHORS
index bdfcb33ad4ae05a2c9ad7b29325b3b7d0f8cb34d..dc98ebe173ee0907d4a2afdf8646a28aee8fc9e8 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -140,6 +140,7 @@ Paul Ingram             paul@nicira.com
 Paulo Cravero           pcravero@as2594.net
 Peter Balland           peter@nicira.com
 Peter Phaal             peter.phaal@inmon.com
+Prabina Pattnaik        Prabina.Pattnaik@nechclst.in
 Ralf Heiringhoff        ralf@frosty-geek.net
 Ram Jothikumar          rjothikumar@nicira.com
 Ramana Reddy            gtvrreddy@gmail.com

diff --git a/NEWS b/NEWS
index c2ec9539dc06aefce949a80cf0dda7cf3c0490fd..f0b24907ea8bbff0558747df6637f18bc6b15dd2 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,10 @@ post-v1.7.0
         the multicast bit in the destination address could be individually
        masked.)
       - New field OXM_OF_METADATA, to align with OpenFlow 1.1.
+      - The OFPST_QUEUE request now reports an error if a specified port or
+        queue does not exist, or for requests for a specific queue on all
+        ports, if the specified queue does not exist on any port.  (Previous
+        versions generally reported an empty set of results.)
     - Additional protocols are not mirrored and dropped when forward-bpdu is
       false.  For a full list, see the ovs-vswitchd.conf.db man page.
     - Open vSwitch now sends RARP packets in situations where it previously

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 1a2f71257ecb50bf3278c1fa7e2ef6faa08c9037..ce4da9d35d65924d5400415f7b115787d11026a5 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2684,7 +2684,7 @@ handle_queue_stats_dump_cb(uint32_t queue_id,
     put_queue_stats(cbdata, queue_id, stats);
 }
 
-static void
+static enum ofperr
 handle_queue_stats_for_port(struct ofport *port, uint32_t queue_id,
                             struct queue_stats_cbdata *cbdata)
 {
@@ -2697,8 +2697,11 @@ handle_queue_stats_for_port(struct ofport *port, uint32_t queue_id,
 
         if (!netdev_get_queue_stats(port->netdev, queue_id, &stats)) {
             put_queue_stats(cbdata, queue_id, &stats);
+        } else {
+            return OFPERR_OFPQOFC_BAD_QUEUE;
         }
     }
+    return 0;
 }
 
 static enum ofperr
@@ -2707,9 +2710,10 @@ handle_queue_stats_request(struct ofconn *ofconn,
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     struct queue_stats_cbdata cbdata;
-    struct ofport *port;
     unsigned int port_no;
+    struct ofport *port;
     uint32_t queue_id;
+    enum ofperr error;
 
     COVERAGE_INC(ofproto_queue_req);
 
@@ -2718,21 +2722,25 @@ handle_queue_stats_request(struct ofconn *ofconn,
     port_no = ntohs(qsr->port_no);
     queue_id = ntohl(qsr->queue_id);
     if (port_no == OFPP_ALL) {
+        error = OFPERR_OFPQOFC_BAD_QUEUE;
         HMAP_FOR_EACH (port, hmap_node, &ofproto->ports) {
-            handle_queue_stats_for_port(port, queue_id, &cbdata);
+            if (!handle_queue_stats_for_port(port, queue_id, &cbdata)) {
+                error = 0;
+            }
         }
-    } else if (port_no < OFPP_MAX) {
+    } else {
         port = ofproto_get_port(ofproto, port_no);
-        if (port) {
-            handle_queue_stats_for_port(port, queue_id, &cbdata);
-        }
+        error = (port
+                 ? handle_queue_stats_for_port(port, queue_id, &cbdata)
+                 : OFPERR_OFPQOFC_BAD_PORT);
+    }
+    if (!error) {
+        ofconn_send_replies(ofconn, &cbdata.replies);
     } else {
         ofpbuf_list_delete(&cbdata.replies);
-        return OFPERR_OFPQOFC_BAD_PORT;
     }
-    ofconn_send_replies(ofconn, &cbdata.replies);
 
-    return 0;
+    return error;
 }
 
 static bool

diff --git a/tests/ofproto.at b/tests/ofproto.at
index 28adf74dd7cae19de454aaeeb33f79bb8ec0968d..d703fa839cd8ab3d0c26adadcd15afda4396c6a6 100644 (file)
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -59,6 +59,14 @@ AT_CHECK([ovs-ofctl -vwarn queue-stats br0], [0], [stdout])
 AT_CHECK([STRIP_XIDS stdout], [0], [dnl
 OFPST_QUEUE reply: 0 queues
 ])
+AT_CHECK([ovs-ofctl -vwarn queue-stats br0 ALL 5], [0],
+  [OFPT_ERROR (xid=0x1): OFPQOFC_BAD_QUEUE
+OFPST_QUEUE request (xid=0x1):port=ALL queue=5
+])
+AT_CHECK([ovs-ofctl -vwarn queue-stats br0 10], [0],
+  [OFPT_ERROR (xid=0x1): OFPQOFC_BAD_PORT
+OFPST_QUEUE request (xid=0x1):port=10 queue=ALL
+])
 OVS_VSWITCHD_STOP
 AT_CLEANUP