[ovs-dev] [PATCH 21/41] openflow: Implement OF1.4+ OFPMP_QUEUE_DESC multipart message.

Ben Pfaff blp at ovn.org
Tue Jan 19 07:27:08 UTC 2016


OpenFlow 1.0 through 1.3 have a message OFPT_QUEUE_GET_CONFIG_REQUEST and
its corresponding reply, for fetching a description of the queues
configured on a given port.  OpenFlow 1.4 changes this message to a
multipart message OFPMP_QUEUE_DESC, which Open vSwitch has not until now
implemented.  This commit adds an implemntation of that message.  Because
the message is a replacement for the former one, this commit implements it
using the same ofp-util functions as the former message, so that the client
code doesn't have to distinguish a difference between versions.

The ovs-ofctl command queue-get-config was previously undocumented (due
only to an oversight).  This commit corrects that and documents the new
feature available with OpenFlow 1.4.

Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 NEWS                            |   4 +-
 OPENFLOW-1.1+.md                |   4 +-
 include/openflow/openflow-1.4.h |  25 ++++
 lib/ofp-errors.h                |   3 +
 lib/ofp-msgs.h                  |  15 +-
 lib/ofp-print.c                 |  32 +++--
 lib/ofp-util.c                  | 305 +++++++++++++++++++++++++++-------------
 lib/ofp-util.h                  |  16 +--
 ofproto/ofproto.c               | 116 +++++++++------
 tests/ofp-print.at              |  44 ++++++
 tests/ofproto.at                |  46 ++++--
 utilities/ovs-ofctl.8.in        |  12 +-
 utilities/ovs-ofctl.c           |  20 +--
 13 files changed, 453 insertions(+), 189 deletions(-)

diff --git a/NEWS b/NEWS
index 4433329..5c18867 100644
--- a/NEWS
+++ b/NEWS
@@ -4,7 +4,9 @@ Post-v2.5.0
      * New "monitor2" and "update2" extensions to RFC 7047.
    - OpenFlow:
      * OpenFlow 1.1+ OFPT_QUEUE_GET_CONFIG_REQUEST now supports OFPP_ANY.
-
+     * OpenFlow 1.4+ OFPMP_QUEUE_DESC is now supported.
+   - ovs-ofctl:
+     * queue-get-config command now allows a queue ID to be specified.
 
 v2.5.0 - xx xxx xxxx
 ---------------------
diff --git a/OPENFLOW-1.1+.md b/OPENFLOW-1.1+.md
index 0b2f0f9..537f660 100644
--- a/OPENFLOW-1.1+.md
+++ b/OPENFLOW-1.1+.md
@@ -194,8 +194,8 @@ OpenFlow 1.4 features are listed in the previous section.
     Many on-wire structures got TLVs.
     Already implemented: port desc properties, port mod properties,
                          port stats properties, table mod properties,
-                         queue stats, unified property errors.
-    Remaining required: set-async, queue desc
+                         queue stats, unified property errors, queue desc.
+    Remaining required: set-async
     Remaining optional: table desc, table-status
     [EXT-262]
     [required for OF1.4+]
diff --git a/include/openflow/openflow-1.4.h b/include/openflow/openflow-1.4.h
index bbdd54f..bce1598 100644
--- a/include/openflow/openflow-1.4.h
+++ b/include/openflow/openflow-1.4.h
@@ -219,6 +219,31 @@ struct ofp14_queue_stats {
 OFP_ASSERT(sizeof(struct ofp14_queue_stats) == 48);
 
 
+/* ## ---------------- ## */
+/* ## ofp14_queue_desc ## */
+/* ## ---------------- ## */
+
+struct ofp14_queue_desc_request {
+    ovs_be32 port;              /* All ports if OFPP_ANY. */
+    ovs_be32 queue;             /* All queues if OFPQ_ALL. */
+};
+OFP_ASSERT(sizeof(struct ofp14_queue_desc_request) == 8);
+
+/* Body of reply to OFPMP_QUEUE_DESC request. */
+struct ofp14_queue_desc {
+    ovs_be32 port_no;           /* Port this queue is attached to. */
+    ovs_be32 queue_id;          /* ID for the specific queue. */
+    ovs_be16 len;               /* Length in bytes of this queue desc. */
+    uint8_t pad[6];             /* 64-bit alignment. */
+};
+OFP_ASSERT(sizeof(struct ofp14_queue_desc) == 16);
+
+enum ofp14_queue_desc_prop_type {
+    OFPQDPT14_MIN_RATE = 1,
+    OFPQDPT14_MAX_RATE = 2,
+    OFPQDPT14_EXPERIMENTER = 0xffff
+};
+
 /* ## -------------- ## */
 /* ## Miscellaneous. ## */
 /* ## -------------- ## */
diff --git a/lib/ofp-errors.h b/lib/ofp-errors.h
index 4f59acf..8e13873 100644
--- a/lib/ofp-errors.h
+++ b/lib/ofp-errors.h
@@ -504,6 +504,9 @@ enum ofperr {
     /* OF1.0(5,2), OF1.1+(9,2).  Permissions error. */
     OFPERR_OFPQOFC_EPERM,
 
+    /* NX1.4+(23).  System error retrieving queue details. */
+    OFPERR_NXQOFC_QUEUE_ERROR,
+
 /* ## -------------------------- ## */
 /* ## OFPET_SWITCH_CONFIG_FAILED ## */
 /* ## -------------------------- ## */
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 2c4a916..54018b4 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -208,12 +208,12 @@ enum ofpraw {
 
     /* OFPT 1.0 (20): struct ofp10_queue_get_config_request. */
     OFPRAW_OFPT10_QUEUE_GET_CONFIG_REQUEST,
-    /* OFPT 1.1+ (22): struct ofp11_queue_get_config_request. */
+    /* OFPT 1.1-1.3 (22): struct ofp11_queue_get_config_request. */
     OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST,
 
     /* OFPT 1.0 (21): struct ofp10_queue_get_config_reply, uint8_t[8][]. */
     OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY,
-    /* OFPT 1.1+ (23): struct ofp11_queue_get_config_reply, uint8_t[8][]. */
+    /* OFPT 1.1-1.3 (23): struct ofp11_queue_get_config_reply, uint8_t[8][]. */
     OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY,
 
     /* OFPT 1.2+ (24): struct ofp12_role_request. */
@@ -396,6 +396,11 @@ enum ofpraw {
     /* OFPST 1.4+ (13): uint8_t[8][]. */
     OFPRAW_OFPST14_PORT_DESC_REPLY,
 
+    /* OFPST 1.4+ (15): struct ofp14_queue_desc_request. */
+    OFPRAW_OFPST14_QUEUE_DESC_REQUEST,
+    /* OFPST 1.4+ (15): uint8_t[8][]. */
+    OFPRAW_OFPST14_QUEUE_DESC_REPLY,
+
     /* OFPST 1.4+ (16): uint8_t[8][]. */
     OFPRAW_OFPST14_FLOW_MONITOR_REQUEST,
     /* NXST 1.0 (2): uint8_t[8][]. */
@@ -537,9 +542,11 @@ enum ofptype {
 
     /* Queue Configuration messages. */
     OFPTYPE_QUEUE_GET_CONFIG_REQUEST, /* OFPRAW_OFPT10_QUEUE_GET_CONFIG_REQUEST.
-                                       * OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST. */
+                                       * OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST.
+                                       * OFPRAW_OFPST14_QUEUE_DESC_REQUEST. */
     OFPTYPE_QUEUE_GET_CONFIG_REPLY, /* OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY.
-                                     * OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY. */
+                                     * OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY.
+                                     * OFPRAW_OFPST14_QUEUE_DESC_REPLY. */
 
     /* Controller role change request messages. */
     OFPTYPE_ROLE_REQUEST,         /* OFPRAW_OFPT12_ROLE_REQUEST.
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index af56e9b..42e822b 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1086,8 +1086,9 @@ ofp_print_queue_get_config_request(struct ds *string,
 {
     enum ofperr error;
     ofp_port_t port;
+    uint32_t queue;
 
-    error = ofputil_decode_queue_get_config_request(oh, &port);
+    error = ofputil_decode_queue_get_config_request(oh, &port, &queue);
     if (error) {
         ofp_print_error(string, error);
         return;
@@ -1095,6 +1096,11 @@ ofp_print_queue_get_config_request(struct ds *string,
 
     ds_put_cstr(string, " port=");
     ofputil_format_port(port, string);
+
+    if (queue != OFPQ_ALL) {
+        ds_put_cstr(string, " queue=");
+        ofp_print_queue_name(string, queue);
+    }
 }
 
 static void
@@ -1111,21 +1117,12 @@ static void
 ofp_print_queue_get_config_reply(struct ds *string,
                                  const struct ofp_header *oh)
 {
-    enum ofperr error;
     struct ofpbuf b;
-    ofp_port_t port;
+    ofp_port_t port = 0;
 
     ofpbuf_use_const(&b, oh, ntohs(oh->length));
-    error = ofputil_decode_queue_get_config_reply(&b, &port);
-    if (error) {
-        ofp_print_error(string, error);
-        return;
-    }
-
-    ds_put_cstr(string, " port=");
-    ofputil_format_port(port, string);
-    ds_put_char(string, '\n');
 
+    ds_put_char(string, ' ');
     for (;;) {
         struct ofputil_queue_config queue;
         int retval;
@@ -1135,10 +1132,19 @@ ofp_print_queue_get_config_reply(struct ds *string,
             if (retval != EOF) {
                 ofp_print_error(string, retval);
             }
+            ds_chomp(string, ' ');
             break;
         }
 
-        ds_put_format(string, "queue %"PRIu32":", queue.queue_id);
+        if (queue.port != port) {
+            port = queue.port;
+
+            ds_put_cstr(string, "port=");
+            ofputil_format_port(port, string);
+            ds_put_char(string, '\n');
+        }
+
+        ds_put_format(string, "queue %"PRIu32":", queue.queue);
         print_queue_rate(string, "min_rate", queue.min_rate);
         print_queue_rate(string, "max_rate", queue.max_rate);
         ds_put_char(string, '\n');
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 1225664..6f9aeb7 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2343,10 +2343,14 @@ ofputil_decode_nxst_flow_request(struct ofputil_flow_stats_request *fsr,
 }
 
 /* Constructs and returns an OFPT_QUEUE_GET_CONFIG request for the specified
- * 'port', suitable for OpenFlow version 'version'. */
+ * 'port' and 'queue', suitable for OpenFlow version 'version'.
+ *
+ * 'queue' is honored only for OpenFlow 1.4 and later; older versions always
+ * request all queues. */
 struct ofpbuf *
 ofputil_encode_queue_get_config_request(enum ofp_version version,
-                                        ofp_port_t port)
+                                        ofp_port_t port,
+                                        uint32_t queue)
 {
     struct ofpbuf *request;
 
@@ -2357,13 +2361,21 @@ ofputil_encode_queue_get_config_request(enum ofp_version version,
                                version, 0);
         qgcr10 = ofpbuf_put_zeros(request, sizeof *qgcr10);
         qgcr10->port = htons(ofp_to_u16(port));
-    } else {
+    } else if (version < OFP14_VERSION) {
         struct ofp11_queue_get_config_request *qgcr11;
 
         request = ofpraw_alloc(OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST,
                                version, 0);
         qgcr11 = ofpbuf_put_zeros(request, sizeof *qgcr11);
         qgcr11->port = ofputil_port_to_ofp11(port);
+    } else {
+        struct ofp14_queue_desc_request *qdr14;
+
+        request = ofpraw_alloc(OFPRAW_OFPST14_QUEUE_DESC_REQUEST,
+                               version, 0);
+        qdr14 = ofpbuf_put_zeros(request, sizeof *qdr14);
+        qdr14->port = ofputil_port_to_ofp11(port);
+        qdr14->queue = htonl(queue);
     }
 
     return request;
@@ -2374,10 +2386,11 @@ ofputil_encode_queue_get_config_request(enum ofp_version version,
  * code. */
 enum ofperr
 ofputil_decode_queue_get_config_request(const struct ofp_header *oh,
-                                        ofp_port_t *port)
+                                        ofp_port_t *port, uint32_t *queue)
 {
     const struct ofp10_queue_get_config_request *qgcr10;
     const struct ofp11_queue_get_config_request *qgcr11;
+    const struct ofp14_queue_desc_request *qdr14;
     enum ofpraw raw;
     struct ofpbuf b;
 
@@ -2388,16 +2401,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));
+        *queue = OFPQ_ALL;
         break;
 
     case OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST:
         qgcr11 = b.data;
+        *queue = OFPQ_ALL;
         enum ofperr error = ofputil_port_from_ofp11(qgcr11->port, port);
         if (error || *port == OFPP_ANY) {
             return error;
         }
         break;
 
+    case OFPRAW_OFPST14_QUEUE_DESC_REQUEST:
+        qdr14 = b.data;
+        *queue = ntohl(qdr14->queue);
+        return ofputil_port_from_ofp11(qdr14->port, port);
+
     default:
         OVS_NOT_REACHED();
     }
@@ -2408,45 +2428,49 @@ ofputil_decode_queue_get_config_request(const struct ofp_header *oh,
 }
 
 /* Constructs and returns the beginning of a reply to
- * OFPT_QUEUE_GET_CONFIG_REQUEST 'oh'.  The caller may append information about
- * individual queues with ofputil_append_queue_get_config_reply(). */
-struct ofpbuf *
-ofputil_encode_queue_get_config_reply(const struct ofp_header *oh)
+ * OFPT_QUEUE_GET_CONFIG_REQUEST or OFPMP_QUEUE_DESC request 'oh'.  The caller
+ * may append information about individual queues with
+ * ofputil_append_queue_get_config_reply(). */
+void
+ofputil_start_queue_get_config_reply(const struct ofp_header *request,
+                                     struct ovs_list *replies)
 {
-    struct ofp10_queue_get_config_reply *qgcr10;
-    struct ofp11_queue_get_config_reply *qgcr11;
     struct ofpbuf *reply;
     enum ofperr error;
-    struct ofpbuf b;
-    enum ofpraw raw;
     ofp_port_t port;
+    uint32_t queue;
 
-    error = ofputil_decode_queue_get_config_request(oh, &port);
+    error = ofputil_decode_queue_get_config_request(request, &port, &queue);
     ovs_assert(!error);
 
-    ofpbuf_use_const(&b, oh, ntohs(oh->length));
-    raw = ofpraw_pull_assert(&b);
-
+    enum ofpraw raw = ofpraw_decode_assert(request);
     switch ((int) raw) {
     case OFPRAW_OFPT10_QUEUE_GET_CONFIG_REQUEST:
         reply = ofpraw_alloc_reply(OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY,
-                                   oh, 0);
-        qgcr10 = ofpbuf_put_zeros(reply, sizeof *qgcr10);
+                                   request, 0);
+        struct ofp10_queue_get_config_reply *qgcr10
+            = ofpbuf_put_zeros(reply, sizeof *qgcr10);
         qgcr10->port = htons(ofp_to_u16(port));
         break;
 
     case OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST:
         reply = ofpraw_alloc_reply(OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY,
-                                   oh, 0);
-        qgcr11 = ofpbuf_put_zeros(reply, sizeof *qgcr11);
+                                   request, 0);
+        struct ofp11_queue_get_config_reply *qgcr11
+            = ofpbuf_put_zeros(reply, sizeof *qgcr11);
         qgcr11->port = ofputil_port_to_ofp11(port);
         break;
 
+    case OFPRAW_OFPST14_QUEUE_DESC_REQUEST:
+        reply = ofpraw_alloc_stats_reply(request, 0);
+        break;
+
     default:
         OVS_NOT_REACHED();
     }
 
-    return reply;
+    list_init(replies);
+    list_push_back(replies, &reply->list_node);
 }
 
 static void
@@ -2463,64 +2487,58 @@ put_ofp11_queue_rate(struct ofpbuf *reply,
     }
 }
 
-/* Appends a queue description for 'queue_id' to the
- * OFPT_QUEUE_GET_CONFIG_REPLY already in 'oh'. */
+static void
+put_ofp14_queue_rate(struct ofpbuf *reply,
+                     enum ofp14_queue_desc_prop_type type, uint16_t rate)
+{
+    if (rate != UINT16_MAX) {
+        ofpprop_put_u16(reply, type, rate);
+    }
+}
+
 void
-ofputil_append_queue_get_config_reply(struct ofpbuf *reply,
-                                      const struct ofputil_queue_config *oqc)
+ofputil_append_queue_get_config_reply(const struct ofputil_queue_config *qc,
+                                      struct ovs_list *replies)
 {
-    const struct ofp_header *oh = reply->data;
-    size_t start_ofs, len_ofs;
+    enum ofp_version ofp_version = ofpmp_version(replies);
+    struct ofpbuf *reply = ofpbuf_from_list(list_back(replies));
+    size_t start_ofs = reply->size;
+    size_t len_ofs;
     ovs_be16 *len;
 
-    start_ofs = reply->size;
-    if (oh->version < OFP12_VERSION) {
-        struct ofp10_packet_queue *opq10;
+    if (ofp_version < OFP14_VERSION) {
+        if (ofp_version < OFP12_VERSION) {
+            struct ofp10_packet_queue *opq10;
 
-        opq10 = ofpbuf_put_zeros(reply, sizeof *opq10);
-        opq10->queue_id = htonl(oqc->queue_id);
-        len_ofs = (char *) &opq10->len - (char *) reply->data;
-    } else {
-        struct ofp12_packet_queue *opq12;
+            opq10 = ofpbuf_put_zeros(reply, sizeof *opq10);
+            opq10->queue_id = htonl(qc->queue);
+            len_ofs = (char *) &opq10->len - (char *) reply->data;
+        } else {
+            struct ofp12_packet_queue *opq12;
 
-        opq12 = ofpbuf_put_zeros(reply, sizeof *opq12);
-        opq12->port = ofputil_port_to_ofp11(oqc->port);
-        opq12->queue_id = htonl(oqc->queue_id);
-        len_ofs = (char *) &opq12->len - (char *) reply->data;
-    }
+            opq12 = ofpbuf_put_zeros(reply, sizeof *opq12);
+            opq12->port = ofputil_port_to_ofp11(qc->port);
+            opq12->queue_id = htonl(qc->queue);
+            len_ofs = (char *) &opq12->len - (char *) reply->data;
+        }
 
-    put_ofp11_queue_rate(reply, OFPQT10_MIN_RATE, oqc->min_rate);
-    put_ofp11_queue_rate(reply, OFPQT11_MAX_RATE, oqc->max_rate);
+        put_ofp11_queue_rate(reply, OFPQT10_MIN_RATE, qc->min_rate);
+        put_ofp11_queue_rate(reply, OFPQT11_MAX_RATE, qc->max_rate);
+    } else {
+        struct ofp14_queue_desc *oqd = ofpbuf_put_zeros(reply, sizeof *oqd);
+        oqd->port_no = ofputil_port_to_ofp11(qc->port);
+        oqd->queue_id = htonl(qc->queue);
+        len_ofs = (char *) &oqd->len - (char *) reply->data;
+        put_ofp14_queue_rate(reply, OFPQDPT14_MIN_RATE, qc->min_rate);
+        put_ofp14_queue_rate(reply, OFPQDPT14_MAX_RATE, qc->max_rate);
+    }
 
     len = ofpbuf_at(reply, len_ofs, sizeof *len);
     *len = htons(reply->size - start_ofs);
-}
-
-/* Decodes the initial part of an OFPT_QUEUE_GET_CONFIG_REPLY from 'reply' and
- * stores in '*port' the port that the reply is about.  The caller may call
- * ofputil_pull_queue_get_config_reply() to obtain information about individual
- * queues included in the reply.  Returns 0 if successful, otherwise an
- * ofperr.*/
-enum ofperr
-ofputil_decode_queue_get_config_reply(struct ofpbuf *reply, ofp_port_t *port)
-{
-    const struct ofp10_queue_get_config_reply *qgcr10;
-    const struct ofp11_queue_get_config_reply *qgcr11;
-    enum ofpraw raw;
-
-    raw = ofpraw_pull_assert(reply);
-    switch ((int) raw) {
-    case OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY:
-        qgcr10 = ofpbuf_pull(reply, sizeof *qgcr10);
-        *port = u16_to_ofp(ntohs(qgcr10->port));
-        return 0;
 
-    case OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY:
-        qgcr11 = ofpbuf_pull(reply, sizeof *qgcr11);
-        return ofputil_port_from_ofp11(qgcr11->port, port);
+    if (ofp_version >= OFP14_VERSION) {
+        ofpmp_postappend(replies, start_ofs);
     }
-
-    OVS_NOT_REACHED();
 }
 
 static enum ofperr
@@ -2538,65 +2556,65 @@ parse_ofp11_queue_rate(const struct ofp10_queue_prop_header *hdr,
     }
 }
 
-/* Decodes information about a queue from the OFPT_QUEUE_GET_CONFIG_REPLY in
- * 'reply' and stores it in '*queue'.  ofputil_decode_queue_get_config_reply()
- * must already have pulled off the main header.
- *
- * This function returns EOF if the last queue has already been decoded, 0 if a
- * queue was successfully decoded into '*queue', or an ofperr if there was a
- * problem decoding 'reply'. */
-int
-ofputil_pull_queue_get_config_reply(struct ofpbuf *reply,
-                                    struct ofputil_queue_config *queue)
+static int
+ofputil_pull_queue_get_config_reply10(struct ofpbuf *msg,
+                                      struct ofputil_queue_config *queue)
 {
-    const struct ofp_header *oh;
-    unsigned int opq_len;
-    unsigned int len;
+    const struct ofp_header *oh = msg->header;
+    unsigned int opq_len;       /* Length of protocol-specific queue header. */
+    unsigned int len;           /* Total length of queue + properties. */
 
-    if (!reply->size) {
-        return EOF;
+    /* Obtain the port number from the message header. */
+    if (oh->version == OFP10_VERSION) {
+        const struct ofp10_queue_get_config_reply *oqgcr10 = msg->msg;
+        queue->port = u16_to_ofp(ntohs(oqgcr10->port));
+    } else {
+        const struct ofp11_queue_get_config_reply *oqgcr11 = msg->msg;
+        enum ofperr error = ofputil_port_from_ofp11(oqgcr11->port,
+                                                    &queue->port);
+        if (error) {
+            return error;
+        }
     }
 
-    queue->min_rate = UINT16_MAX;
-    queue->max_rate = UINT16_MAX;
-
-    oh = reply->header;
+    /* Pull off the queue header and get the queue number and length. */
     if (oh->version < OFP12_VERSION) {
         const struct ofp10_packet_queue *opq10;
-
-        opq10 = ofpbuf_try_pull(reply, sizeof *opq10);
+        opq10 = ofpbuf_try_pull(msg, sizeof *opq10);
         if (!opq10) {
             return OFPERR_OFPBRC_BAD_LEN;
         }
-        queue->queue_id = ntohl(opq10->queue_id);
+        queue->queue = ntohl(opq10->queue_id);
         len = ntohs(opq10->len);
         opq_len = sizeof *opq10;
     } else {
         const struct ofp12_packet_queue *opq12;
-
-        opq12 = ofpbuf_try_pull(reply, sizeof *opq12);
+        opq12 = ofpbuf_try_pull(msg, sizeof *opq12);
         if (!opq12) {
             return OFPERR_OFPBRC_BAD_LEN;
         }
-        queue->queue_id = ntohl(opq12->queue_id);
+        queue->queue = ntohl(opq12->queue_id);
         len = ntohs(opq12->len);
         opq_len = sizeof *opq12;
     }
 
-    if (len < opq_len || len > reply->size + opq_len || len % 8) {
+    /* Length check. */
+    if (len < opq_len || len > msg->size + opq_len || len % 8) {
         return OFPERR_OFPBRC_BAD_LEN;
     }
     len -= opq_len;
 
+    /* Pull properties.  The format of these properties differs from used in
+     * OF1.4+ so we can't use the common property functions. */
     while (len > 0) {
         const struct ofp10_queue_prop_header *hdr;
         unsigned int property;
         unsigned int prop_len;
         enum ofperr error = 0;
 
-        hdr = ofpbuf_at_assert(reply, 0, sizeof *hdr);
+        hdr = ofpbuf_at_assert(msg, 0, sizeof *hdr);
         prop_len = ntohs(hdr->len);
-        if (prop_len < sizeof *hdr || prop_len > reply->size || prop_len % 8) {
+        if (prop_len < sizeof *hdr || prop_len > msg->size || prop_len % 8) {
             return OFPERR_OFPBRC_BAD_LEN;
         }
 
@@ -2618,12 +2636,107 @@ ofputil_pull_queue_get_config_reply(struct ofpbuf *reply,
             return error;
         }
 
-        ofpbuf_pull(reply, prop_len);
+        ofpbuf_pull(msg, prop_len);
         len -= prop_len;
     }
     return 0;
 }
 
+static int
+ofputil_pull_queue_get_config_reply14(struct ofpbuf *msg,
+                                      struct ofputil_queue_config *queue)
+{
+    struct ofp14_queue_desc *oqd14 = ofpbuf_try_pull(msg, sizeof *oqd14);
+    if (!oqd14) {
+        return OFPERR_OFPBRC_BAD_LEN;
+    }
+    enum ofperr error = ofputil_port_from_ofp11(oqd14->port_no, &queue->port);
+    if (error) {
+        return error;
+    }
+    queue->queue = ntohl(oqd14->queue_id);
+
+    /* Length check. */
+    unsigned int len = ntohs(oqd14->len);
+    if (len < sizeof *oqd14 || len > msg->size + sizeof *oqd14 || len % 8) {
+        return OFPERR_OFPBRC_BAD_LEN;
+    }
+    len -= sizeof *oqd14;
+
+    struct ofpbuf properties;
+    ofpbuf_use_const(&properties, ofpbuf_pull(msg, len), len);
+    while (properties.size > 0) {
+        struct ofpbuf payload;
+        uint64_t type;
+
+        error = ofpprop_pull(&properties, &payload, &type);
+        if (error) {
+            return error;
+        }
+
+        switch (type) {
+        case OFPQDPT14_MIN_RATE:
+            error = ofpprop_parse_u16(&payload, &queue->min_rate);
+            break;
+
+        case OFPQDPT14_MAX_RATE:
+            error = ofpprop_parse_u16(&payload, &queue->max_rate);
+            break;
+
+        default:
+            error = OFPPROP_UNKNOWN(true, "queue desc", type);
+            break;
+        }
+
+        if (error) {
+            return error;
+        }
+    }
+
+    return 0;
+}
+
+/* Decodes information about a queue from the OFPT_QUEUE_GET_CONFIG_REPLY in
+ * 'reply' and stores it in '*queue'.  ofputil_decode_queue_get_config_reply()
+ * must already have pulled off the main header.
+ *
+ * This function returns EOF if the last queue has already been decoded, 0 if a
+ * queue was successfully decoded into '*queue', or an ofperr if there was a
+ * problem decoding 'reply'. */
+int
+ofputil_pull_queue_get_config_reply(struct ofpbuf *msg,
+                                    struct ofputil_queue_config *queue)
+{
+    enum ofpraw raw;
+    if (!msg->header) {
+        /* Pull OpenFlow header. */
+        raw = ofpraw_pull_assert(msg);
+
+        /* Pull protocol-specific ofp_queue_get_config_reply header (OF1.4
+         * doesn't have one at all). */
+        if (raw == OFPRAW_OFPT10_QUEUE_GET_CONFIG_REPLY) {
+            ofpbuf_pull(msg, sizeof(struct ofp10_queue_get_config_reply));
+        } else if (raw == OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY) {
+            ofpbuf_pull(msg, sizeof(struct ofp11_queue_get_config_reply));
+        } else {
+            ovs_assert(raw == OFPRAW_OFPST14_QUEUE_DESC_REPLY);
+        }
+    } else {
+        raw = ofpraw_decode_assert(msg->header);
+    }
+
+    queue->min_rate = UINT16_MAX;
+    queue->max_rate = UINT16_MAX;
+
+    if (!msg->size) {
+        return EOF;
+    } else if (raw == OFPRAW_OFPST14_QUEUE_DESC_REPLY) {
+        return ofputil_pull_queue_get_config_reply14(msg, queue);
+    } else {
+        return ofputil_pull_queue_get_config_reply10(msg, queue);
+    }
+}
+
 /* Converts an OFPST_FLOW, OFPST_AGGREGATE, NXST_FLOW, or NXST_AGGREGATE
  * request 'oh', into an abstract flow_stats_request in 'fsr'.  Returns 0 if
  * successful, otherwise an OpenFlow error code. */
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index f411651..89f3d95 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -986,14 +986,16 @@ int ofputil_decode_table_stats_reply(struct ofpbuf *reply,
 
 /* Queue configuration request. */
 struct ofpbuf *ofputil_encode_queue_get_config_request(enum ofp_version,
-                                                       ofp_port_t port);
+                                                       ofp_port_t port,
+                                                       uint32_t queue);
 enum ofperr ofputil_decode_queue_get_config_request(const struct ofp_header *,
-                                                    ofp_port_t *port);
+                                                    ofp_port_t *port,
+                                                    uint32_t *queue);
 
 /* Queue configuration reply. */
 struct ofputil_queue_config {
     ofp_port_t port;
-    uint32_t queue_id;
+    uint32_t queue;
 
     /* Each of these optional values is expressed in tenths of a percent.
      * Values greater than 1000 indicate that the feature is disabled.
@@ -1002,13 +1004,11 @@ struct ofputil_queue_config {
     uint16_t max_rate;
 };
 
-struct ofpbuf *ofputil_encode_queue_get_config_reply(
-    const struct ofp_header *request);
+void ofputil_start_queue_get_config_reply(const struct ofp_header *request,
+                                          struct ovs_list *replies);
 void ofputil_append_queue_get_config_reply(
-    struct ofpbuf *reply, const struct ofputil_queue_config *);
+    const struct ofputil_queue_config *, struct ovs_list *replies);
 
-enum ofperr ofputil_decode_queue_get_config_reply(struct ofpbuf *reply,
-                                                  ofp_port_t *);
 int ofputil_pull_queue_get_config_reply(struct ofpbuf *reply,
                                         struct ofputil_queue_config *);
 
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 386009e..8dcfafe 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -6227,57 +6227,89 @@ handle_group_features_stats_request(struct ofconn *ofconn,
 }
 
 static void
-put_queue_config(struct ofport *ofport, struct ofpbuf *reply)
+put_queue_get_config_reply(struct ofport *port, uint32_t queue,
+                           struct ovs_list *replies)
 {
-   struct netdev_queue_dump queue_dump;
-   unsigned int queue_id;
-   struct smap details;
+    struct ofputil_queue_config qc;
 
-   smap_init(&details);
-   NETDEV_QUEUE_FOR_EACH (&queue_id, &details, &queue_dump, ofport->netdev) {
-       struct ofputil_queue_config queue;
+    /* None of the existing queues have compatible properties, so we hard-code
+     * omitting min_rate and max_rate. */
+    qc.port = port->ofp_port;
+    qc.queue = queue;
+    qc.min_rate = UINT16_MAX;
+    qc.max_rate = UINT16_MAX;
+    ofputil_append_queue_get_config_reply(&qc, replies);
+}
 
-       /* 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 int
+handle_queue_get_config_request_for_port(struct ofport *port, uint32_t queue,
+                                         struct ovs_list *replies)
+{
+    struct smap details = SMAP_INITIALIZER(&details);
+    if (queue != OFPQ_ALL) {
+        int error = netdev_get_queue(port->netdev, queue, &details);
+        switch (error) {
+        case 0:
+            put_queue_get_config_reply(port, queue, replies);
+            break;
+        case EOPNOTSUPP:
+        case EINVAL:
+            return OFPERR_OFPQOFC_BAD_QUEUE;
+        default:
+            return OFPERR_NXQOFC_QUEUE_ERROR;
+        }
+    } else {
+        struct netdev_queue_dump queue_dump;
+        uint32_t queue_id;
+
+        NETDEV_QUEUE_FOR_EACH (&queue_id, &details, &queue_dump,
+                               port->netdev) {
+            put_queue_get_config_reply(port, queue_id, replies);
+        }
+    }
+    smap_destroy(&details);
+    return 0;
 }
 
 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;
+    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
+    struct ovs_list replies;
+    struct ofport *port;
+    ofp_port_t req_port;
+    uint32_t req_queue;
+    enum ofperr error;
+
+    error = ofputil_decode_queue_get_config_request(oh, &req_port, &req_queue);
+    if (error) {
+        return error;
+    }
+
+    ofputil_start_queue_get_config_reply(oh, &replies);
+    if (req_port == OFPP_ANY) {
+        error = OFPERR_OFPQOFC_BAD_QUEUE;
+        HMAP_FOR_EACH (port, hmap_node, &ofproto->ports) {
+            if (!handle_queue_get_config_request_for_port(port, req_queue,
+                                                          &replies)) {
+                error = 0;
+            }
+        }
+    } else {
+        port = ofproto_get_port(ofproto, req_port);
+        error = (port
+                 ? handle_queue_get_config_request_for_port(port, req_queue,
+                                                            &replies)
+                 : OFPERR_OFPQOFC_BAD_PORT);
+    }
+    if (!error) {
+        ofconn_send_replies(ofconn, &replies);
+    } else {
+        ofpbuf_list_delete(&replies);
+    }
+
+    return error;
 }
 
 static enum ofperr
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 24e602e..6f20fac 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -2546,6 +2546,15 @@ OFPT_QUEUE_GET_CONFIG_REQUEST (OF1.2) (xid=0x1): port=1
 ])
 AT_CLEANUP
 
+AT_SETUP([OFPST_QUEUE_DESC request - OF1.4])
+AT_KEYWORDS([ofp-print OFPT_QUEUE_GET_CONFIG_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "\
+05 12 00 18 00 00 00 01 00 0f 00 00 00 00 00 00 \
+00 00 00 01 00 00 00 02"], [0],
+  [OFPST_QUEUE_DESC request (OF1.4) (xid=0x1): port=1 queue=2
+])
+AT_CLEANUP
+
 AT_SETUP([OFPT_QUEUE_GET_CONFIG_REPLY - OF1.0])
 AT_KEYWORDS([ofp-print])
 AT_CHECK([ovs-ofctl ofp-print "01 15 00 40 00 00 00 01 \
@@ -2591,6 +2600,41 @@ queue 17476:
 ])
 AT_CLEANUP
 
+AT_SETUP([OFPT_QUEUE_GET_CONFIG_REPLY - OF1.3])
+AT_KEYWORDS([ofp-print])
+AT_CHECK([ovs-ofctl ofp-print "04 17 00 50 00 00 00 01 \
+00 00 00 01 00 00 00 00 \
+00 00 55 55 00 00 00 01 00 30 00 00 00 00 00 00 \
+00 01 00 10 00 00 00 00 01 f4 00 00 00 00 00 00 \
+00 02 00 10 00 00 00 00 02 ee 00 00 00 00 00 00 \
+00 00 44 44 00 08 00 01 00 10 00 00 00 00 00 00 \
+"], [0], [dnl
+OFPT_QUEUE_GET_CONFIG_REPLY (OF1.3) (xid=0x1): port=1
+queue 21845: min_rate:50.0% max_rate:75.0%
+queue 17476:
+])
+AT_CLEANUP
+
+# OF1.4 renamed OFPT_QUEUE_GET_CONFIG_REPLY to OFPST_QUEUE_DESC.
+AT_SETUP([OFPST_QUEUE_DESC reply - OF1.4])
+AT_KEYWORDS([ofp-print OFPT_QUEUE_GET_CONFIG_REPLY])
+AT_CHECK([ovs-ofctl ofp-print "\
+05 13 00 48 00 00 00 01 00 0f 00 00 00 00 00 00 \
+
+00 00 00 01 00 00 55 55 00 20 00 00 00 00 00 00 \
+00 01 00 08 01 f4 00 00 \
+00 02 00 08 02 ee 00 00 \
+
+00 00 00 02 00 00 44 44 00 18 00 00 00 00 00 00 \
+00 02 00 08 00 64 00 00 \
+"], [0], [dnl
+OFPST_QUEUE_DESC reply (OF1.4) (xid=0x1): port=1
+queue 21845: min_rate:50.0% max_rate:75.0%
+port=2
+queue 17476: max_rate:10.0%
+])
+AT_CLEANUP
+
 AT_SETUP([OFPT_SET_ASYNC - OF1.3])
 AT_KEYWORDS([ofp-print])
 AT_CHECK([ovs-ofctl ofp-print "\
diff --git a/tests/ofproto.at b/tests/ofproto.at
index a4064d4..ab4d254 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -253,14 +253,14 @@ AT_CLEANUP
 AT_SETUP([ofproto - queue configuration - (OpenFlow 1.1)])
 OVS_VSWITCHD_START
 ADD_OF_PORTS([br0], [1], [2])
-AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 1], [0], [stdout])
+AT_CHECK([ovs-ofctl -O OpenFlow11 queue-get-config br0 1], [0], [stdout])
 AT_CHECK([STRIP_XIDS stdout], [0], [dnl
-OFPT_QUEUE_GET_CONFIG_REPLY (OF1.2): port=1
+OFPT_QUEUE_GET_CONFIG_REPLY (OF1.1): port=1
 queue 0:
 ])
-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
+AT_CHECK([ovs-ofctl -O OpenFlow11 queue-get-config br0 10 | STRIP_XIDS], [0],
+  [OFPT_ERROR (OF1.1): OFPQOFC_BAD_PORT
+OFPT_QUEUE_GET_CONFIG_REQUEST (OF1.1): port=10
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
@@ -280,9 +280,39 @@ queue 0:
 queue 0:
 queue 0:
 ])
-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
+AT_CHECK([ovs-ofctl -O OpenFlow12 queue-get-config br0 10 | STRIP_XIDS], [0],
+  [OFPT_ERROR (OF1.2): OFPQOFC_BAD_PORT
+OFPT_QUEUE_GET_CONFIG_REQUEST (OF1.2): port=10
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+AT_SETUP([ofproto - queue configuration - (OpenFlow 1.4)])
+OVS_VSWITCHD_START
+ADD_OF_PORTS([br0], [1], [2])
+
+AT_CHECK([ovs-ofctl -O OpenFlow14 queue-get-config br0 any | STRIP_XIDS], [0],
+  [OFPST_QUEUE_DESC reply (OF1.4): port=1
+queue 0:
+port=LOCAL
+queue 0:
+port=2
+queue 0:
+])
+
+AT_CHECK([ovs-ofctl -O OpenFlow14 queue-get-config br0 1 | STRIP_XIDS], [0],
+  [OFPST_QUEUE_DESC reply (OF1.4): port=1
+queue 0:
+])
+
+AT_CHECK([ovs-ofctl -O OpenFlow14 queue-get-config br0 10 | STRIP_XIDS], [0],
+  [OFPT_ERROR (OF1.4): OFPQOFC_BAD_PORT
+OFPST_QUEUE_DESC request (OF1.4): port=10
+])
+
+AT_CHECK([ovs-ofctl -O OpenFlow14 queue-get-config br0 1 2 | STRIP_XIDS], [0],
+  [OFPT_ERROR (OF1.4): OFPQOFC_BAD_QUEUE
+OFPST_QUEUE_DESC request (OF1.4): port=1 queue=2
 ])
 OVS_VSWITCHD_STOP
 AT_CLEANUP
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 454f6d5..d2fbf49 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -244,12 +244,12 @@ 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 "\fBqueue\-get\-config \fIswitch [\fIport \fR[\fIqueue\fR]]"
+Prints to the console the configuration of \fIqueue\fR on \fIport\fR
+in \fIswitch\fR.  If \fIport\fR is omitted or \fBANY\fR, reports
+queues for all port.  If \fIqueue\fR is omitted or \fBANY\fR, reports
+all queues.  For OpenFlow 1.3 and earlier, the output always includes
+all queues, ignoring \fIqueue\fR if specified.
 .IP
 This command has limited usefulness, because ports often have no
 configured queues and because the OpenFlow protocol provides only very
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 4dbd4ed..1ad48c3 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1232,12 +1232,14 @@ static void
 ofctl_queue_get_config(struct ovs_cmdl_context *ctx)
 {
     const char *vconn_name = ctx->argv[1];
-    const char *port_name = ctx->argc >= 3 ? ctx->argv[2] : NULL;
-    ofp_port_t port = (port_name
-                       ? str_to_port_no(vconn_name, port_name)
-                       : OFPP_ANY);
-
+    const char *port_name = ctx->argc > 2 ? ctx->argv[2] : "any";
+    ofp_port_t port = str_to_port_no(vconn_name, port_name);
+    const char *queue_name = ctx->argc > 3 ? ctx->argv[3] : "all";
+    uint32_t queue = (!strcasecmp(queue_name, "all")
+                      ? OFPQ_ALL
+                      : atoi(queue_name));
     struct vconn *vconn;
+
     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) {
@@ -1257,14 +1259,14 @@ ofctl_queue_get_config(struct ovs_cmdl_context *ctx)
             if (ofp_to_u16(pp.port_no) < ofp_to_u16(OFPP_MAX)) {
                 dump_transaction(vconn2,
                                  ofputil_encode_queue_get_config_request(
-                                     version2, pp.port_no));
+                                     version2, pp.port_no, queue));
             }
         }
         port_iterator_destroy(&pi);
         vconn_close(vconn2);
     } else {
         dump_transaction(vconn, ofputil_encode_queue_get_config_request(
-                             version, port));
+                             version, port, queue));
     }
     vconn_close(vconn);
 }
@@ -3883,8 +3885,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]",
-      1, 2, ofctl_queue_get_config },
+    { "queue-get-config", "switch [port [queue]]",
+      1, 3, ofctl_queue_get_config },
     { "add-flow", "switch flow",
       2, 2, ofctl_add_flow },
     { "add-flows", "switch file",
-- 
2.1.3





More information about the dev mailing list