ofproto: Implement OFPT_QUEUE_GET_CONFIG_REQUEST for OFPP_ANY in OF1.1+.

I was not previously aware that this feature was missing.

Reported-by: Minoru TAKAHASHI <takahashi.minoru7@gmail.com>
Reported-at: http://openvswitch.org/pipermail/discuss/2015-October/019229.html
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Andy Zhou <azhou@ovn.org>

NEWS

@@ -2,6 +2,9 @@ Post-v2.5.0
 ---------------------
    - ovsdb-server:
      * New "monitor2" and "update2" extensions to RFC 7047.
+   - OpenFlow:
+     * OpenFlow 1.1+ OFPT_QUEUE_GET_CONFIG_REQUEST now supports OFPP_ANY.
+
 
 v2.5.0 - xx xxx xxxx
 ---------------------

lib/ofp-util.c

@@ -2492,14 +2492,23 @@ ofputil_decode_queue_get_config_request(const struct ofp_header *oh,
     case OFPRAW_OFPT10_QUEUE_GET_CONFIG_REQUEST:
         qgcr10 = b.data;
         *port = u16_to_ofp(ntohs(qgcr10->port));
-        return 0;
+        break;
 
     case OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST:
         qgcr11 = b.data;
-        return ofputil_port_from_ofp11(qgcr11->port, port);
+        enum ofperr error = ofputil_port_from_ofp11(qgcr11->port, port);
+        if (error || *port == OFPP_ANY) {
+            return error;
+        }
+        break;
+
+    default:
+        OVS_NOT_REACHED();
     }
 
-    OVS_NOT_REACHED();
+    return (ofp_to_u16(*port) < ofp_to_u16(OFPP_MAX)
+            ? 0
+            : OFPERR_OFPQOFC_BAD_PORT);
 }
 
 /* Constructs and returns the beginning of a reply to
@@ -2576,15 +2585,10 @@ ofputil_append_queue_get_config_reply(struct ofpbuf *reply,
         opq10->queue_id = htonl(oqc->queue_id);
         len_ofs = (char *) &opq10->len - (char *) reply->data;
     } else {
-        struct ofp11_queue_get_config_reply *qgcr11;
         struct ofp12_packet_queue *opq12;
-        ovs_be32 port;
-
-        qgcr11 = reply->msg;
-        port = qgcr11->port;
 
         opq12 = ofpbuf_put_zeros(reply, sizeof *opq12);
-        opq12->port = port;
+        opq12->port = ofputil_port_to_ofp11(oqc->port);
         opq12->queue_id = htonl(oqc->queue_id);
         len_ofs = (char *) &opq12->len - (char *) reply->data;
     }

lib/ofp-util.h

@@ -957,6 +957,7 @@ enum ofperr ofputil_decode_queue_get_config_request(const struct ofp_header *,
 
 /* Queue configuration reply. */
 struct ofputil_queue_config {
+    ofp_port_t port;
     uint32_t queue_id;
 
     /* Each of these optional values is expressed in tenths of a percent.

ofproto/ofproto.c

@@ -6230,30 +6230,12 @@ handle_group_features_stats_request(struct ofconn *ofconn,
     return 0;
 }
 
-static enum ofperr
+static void
-handle_queue_get_config_request(struct ofconn *ofconn,
+put_queue_config(struct ofport *ofport, struct ofpbuf *reply)
-                                const struct ofp_header *oh)
 {
-   struct ofproto *p = ofconn_get_ofproto(ofconn);
    struct netdev_queue_dump queue_dump;
-   struct ofport *ofport;
    unsigned int queue_id;
-   struct ofpbuf *reply;
    struct smap details;
-   ofp_port_t request;
-   enum ofperr error;
-
-   error = ofputil_decode_queue_get_config_request(oh, &request);
-   if (error) {
-       return error;
-   }
-
-   ofport = ofproto_get_port(p, request);
-   if (!ofport) {
-      return OFPERR_OFPQOFC_BAD_PORT;
-   }
-
-   reply = ofputil_encode_queue_get_config_reply(oh);
 
    smap_init(&details);
    NETDEV_QUEUE_FOR_EACH (&queue_id, &details, &queue_dump, ofport->netdev) {
@@ -6261,13 +6243,42 @@ handle_queue_get_config_request(struct ofconn *ofconn,
 
        /* None of the existing queues have compatible properties, so we
         * hard-code omitting min_rate and max_rate. */
+       queue.port = ofport->ofp_port;
        queue.queue_id = queue_id;
        queue.min_rate = UINT16_MAX;
        queue.max_rate = UINT16_MAX;
        ofputil_append_queue_get_config_reply(reply, &queue);
    }
    smap_destroy(&details);
+}
 
+static enum ofperr
+handle_queue_get_config_request(struct ofconn *ofconn,
+                                const struct ofp_header *oh)
+{
+   struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
+   ofp_port_t port;
+   enum ofperr error;
+
+   error = ofputil_decode_queue_get_config_request(oh, &port);
+   if (error) {
+       return error;
+   }
+
+   struct ofpbuf *reply = ofputil_encode_queue_get_config_reply(oh);
+   struct ofport *ofport;
+   if (port == OFPP_ANY) {
+       HMAP_FOR_EACH (ofport, hmap_node, &ofproto->ports) {
+           put_queue_config(ofport, reply);
+       }
+   } else {
+       ofport = ofproto_get_port(ofproto, port);
+       if (!ofport) {
+           ofpbuf_delete(reply);
+           return OFPERR_OFPQOFC_BAD_PORT;
+       }
+       put_queue_config(ofport, reply);
+   }
    ofconn_send_reply(ofconn, reply);
 
    return 0;

tests/ofproto.at

@@ -240,6 +240,11 @@ AT_CHECK([ovs-ofctl queue-get-config br0 1], [0], [stdout])
 AT_CHECK([STRIP_XIDS stdout], [0], [dnl
 OFPT_QUEUE_GET_CONFIG_REPLY: port=1
 ])
+AT_CHECK([ovs-ofctl queue-get-config br0], [0], [stdout])
+AT_CHECK([STRIP_XIDS stdout | sort], [0], [dnl
+OFPT_QUEUE_GET_CONFIG_REPLY: port=1
+OFPT_QUEUE_GET_CONFIG_REPLY: port=2
+])
 AT_CHECK([ovs-ofctl queue-get-config br0 10], [0],
   [OFPT_ERROR (xid=0x2): OFPQOFC_BAD_PORT
 OFPT_QUEUE_GET_CONFIG_REQUEST (xid=0x2): port=10
@@ -254,6 +259,10 @@ AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 1], [0], [stdout])
 AT_CHECK([STRIP_XIDS stdout], [0], [dnl
 OFPT_QUEUE_GET_CONFIG_REPLY (OF1.2): port=1
 ])
+AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 ANY], [0], [stdout])
+AT_CHECK([STRIP_XIDS stdout], [0], [dnl
+OFPT_QUEUE_GET_CONFIG_REPLY (OF1.2): port=ANY
+])
 AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 10], [0],
   [OFPT_ERROR (OF1.2) (xid=0x2): OFPQOFC_BAD_PORT
 OFPT_QUEUE_GET_CONFIG_REQUEST (OF1.2) (xid=0x2): port=10

utilities/ovs-ofctl.8.in

@@ -244,6 +244,17 @@ statistics are printed for all queues on \fIport\fR; if only
 \fIport\fR is omitted, then statistics are printed for \fIqueue\fR on
 every port where it exists.
 .
+.IP "\fBqueue\-get\-config \fIswitch \fR[\fIport\fR]"
+Prints to the console information about all of the queues configured
+on \fIport\fR within \fIswitch\fR.  If \fIport\fR is \fBANY\fR or if
+it is omitted, prints information about queues on every port.  The
+OpenFlow specification says that only physical ports have queues; in
+particular, \fBLOCAL\fR is not valid for \fIport\fR.
+.IP
+This command has limited usefulness, because ports often have no
+configured queues and because the OpenFlow protocol provides only very
+limited information about the configuration of a queue.
+.
 .SS "OpenFlow 1.1+ Group Table Commands"
 .
 The following commands work only with switches that support OpenFlow

utilities/ovs-ofctl.c

@@ -378,7 +378,7 @@ usage(void)
            "  dump-group-features SWITCH  print group features\n"
            "  dump-groups SWITCH [GROUP]  print group description\n"
            "  dump-group-stats SWITCH [GROUP]  print group statistics\n"
-           "  queue-get-config SWITCH PORT  print queue information for port\n"
+           "  queue-get-config SWITCH [PORT]  print queue config for PORT\n"
            "  add-meter SWITCH METER      add meter described by METER\n"
            "  mod-meter SWITCH METER      modify specific METER\n"
            "  del-meter SWITCH METER      delete METER\n"
@@ -1252,19 +1252,40 @@ static void
 ofctl_queue_get_config(struct ovs_cmdl_context *ctx)
 {
     const char *vconn_name = ctx->argv[1];
-    const char *port_name = ctx->argv[2];
+    const char *port_name = ctx->argc >= 3 ? ctx->argv[2] : NULL;
-    enum ofputil_protocol protocol;
+    ofp_port_t port = (port_name
-    enum ofp_version version;
+                       ? str_to_port_no(vconn_name, port_name)
-    struct ofpbuf *request;
+                       : OFPP_ANY);
+
     struct vconn *vconn;
-    ofp_port_t port;
+    enum ofputil_protocol protocol = open_vconn(vconn_name, &vconn);
+    enum ofp_version version = ofputil_protocol_to_ofp_version(protocol);
+    if (port == OFPP_ANY && version == OFP10_VERSION) {
+        /* The user requested all queues on all ports.  OpenFlow 1.0 only
+         * supports getting queues for an individual port, so to implement the
+         * user's request we have to get a list of all the ports.
+         *
+         * We use a second vconn to avoid having to accumulate a list of all of
+         * the ports. */
+        struct vconn *vconn2;
+        enum ofputil_protocol protocol2 = open_vconn(vconn_name, &vconn2);
+        enum ofp_version version2 = ofputil_protocol_to_ofp_version(protocol2);
 
-    port = str_to_port_no(vconn_name, port_name);
+        struct port_iterator pi;
-
+        struct ofputil_phy_port pp;
-    protocol = open_vconn(vconn_name, &vconn);
+        for (port_iterator_init(&pi, vconn); port_iterator_next(&pi, &pp); ) {
-    version = ofputil_protocol_to_ofp_version(protocol);
+            if (ofp_to_u16(pp.port_no) < ofp_to_u16(OFPP_MAX)) {
-    request = ofputil_encode_queue_get_config_request(version, port);
+                dump_transaction(vconn2,
-    dump_transaction(vconn, request);
+                                 ofputil_encode_queue_get_config_request(
+                                     version2, pp.port_no));
+            }
+        }
+        port_iterator_destroy(&pi);
+        vconn_close(vconn2);
+    } else {
+        dump_transaction(vconn, ofputil_encode_queue_get_config_request(
+                             version, port));
+    }
     vconn_close(vconn);
 }
 
@@ -3870,8 +3891,8 @@ static const struct ovs_cmdl_command all_commands[] = {
       1, 2, ofctl_dump_aggregate },
     { "queue-stats", "switch [port [queue]]",
       1, 3, ofctl_queue_stats },
-    { "queue-get-config", "switch port",
+    { "queue-get-config", "switch [port]",
-      2, 2, ofctl_queue_get_config },
+      1, 2, ofctl_queue_get_config },
     { "add-flow", "switch flow",
       2, 2, ofctl_add_flow },
     { "add-flows", "switch file",