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 <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
---
 AUTHORS           |    1 +
 NEWS              |    7 ++++++-
 ofproto/ofproto.c |   28 ++++++++++++++++++----------
 tests/ofproto.at  |    8 ++++++++
 4 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index 10a01ab..801d31a 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -137,6 +137,7 @@ Paul Ingram             [email protected]
 Paulo Cravero           [email protected]
 Peter Balland           [email protected]
 Peter Phaal             [email protected]
+Prabina Pattnaik        [email protected]
 Ram Jothikumar          [email protected]
 Ramana Reddy            [email protected]
 Rob Sherwood            [email protected]
diff --git a/NEWS b/NEWS
index 8cd38dd..b96ced8 100644
--- a/NEWS
+++ b/NEWS
@@ -2,7 +2,12 @@ post-v1.7.0
 ------------------------
     - ovs-ofctl:
         - "mod-port" command can now control all OpenFlow config flags.
-    - Added support for arbitrary ethernet masks
+    - OpenFlow:
+        - Added support for arbitrary ethernet masks
+        - 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 dee2cbc..8f7d05c 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2644,7 +2644,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)
 {
@@ -2657,8 +2657,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
@@ -2667,9 +2670,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);
 
@@ -2678,21 +2682,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 dbe31c4..a193490 100644
--- 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
 
-- 
1.7.2.5

_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to