ofproto: Add support for OF1.3 port description multipart message.

OpenFlow 1.0 is limited to displaying 1364 ports in the Features Reply
message, and there is no other way to get consolidated port information.
OpenFlow 1.3 adds a new port description multipart message
(OFPMP_PORT_DESC) that is not limited by size.  This commit adds support
through the OpenFlow 1.0 stats mechanism, since they have complimentary
enum values.

Bug #11040

Signed-off-by: Justin Pettit <jpettit@nicira.com>

11 files changed, 210 insertions, 65 deletions

diff --git a/NEWS b/NEWS
index 723c256c..8485587c 100644
--- a/NEWS
+++ b/NEWS
@@ -10,6 +10,11 @@ post-v1.6.0
       interface.
     - OpenFlow:
         - Added support to mask nd_target for ICMPv6 neighbor discovery flows.
+        - Added support for OpenFlow 1.3 port description (OFPMP_PORT_DESC)
+          multipart messages.
+    - ovs-ofctl:
+        - Added the "dump-ports-desc" command to retrieve port
+          information using the new port description multipart messages.
     - ovs-test:
         - Added support for spawning ovs-test server from the client.
         - Now ovs-test is able to automatically create test bridges and ports.
diff --git a/include/openflow/openflow-common.h b/include/openflow/openflow-common.h
index c3cf2750..3dc76cc2 100644
--- a/include/openflow/openflow-common.h
+++ b/include/openflow/openflow-common.h
@@ -273,6 +273,13 @@ enum ofp_stats_types {
      * The OF1.0 reply body is an array of struct ofp_queue_stats. */
     OFPST_QUEUE = 5,
 
+    /* Port description. (OFPMP_PORT_DESC)
+     * This was introduced as part of OF1.3, but is useful for bridges
+     * with many ports, so we support it with OF1.0, too.
+     * The OF1.0 request is struct ofp_stats_msg.
+     * The OF1.0 reply body is an array of struct ofp10_phy_port. */
+    OFPST_PORT_DESC = 13,
+
     /* Vendor extension.
      * The OF1.0 request and reply begin with struct ofp_vendor_stats. */
     OFPST_VENDOR = 0xffff
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 74f51fea..4e7cedaf 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -278,12 +278,14 @@ lswitch_process_packet(struct lswitch *sw, struct rconn *rconn,
     case OFPUTIL_OFPST_TABLE_REQUEST:
     case OFPUTIL_OFPST_PORT_REQUEST:
     case OFPUTIL_OFPST_QUEUE_REQUEST:
+    case OFPUTIL_OFPST_PORT_DESC_REQUEST:
     case OFPUTIL_OFPST_DESC_REPLY:
     case OFPUTIL_OFPST_FLOW_REPLY:
     case OFPUTIL_OFPST_QUEUE_REPLY:
     case OFPUTIL_OFPST_PORT_REPLY:
     case OFPUTIL_OFPST_TABLE_REPLY:
     case OFPUTIL_OFPST_AGGREGATE_REPLY:
+    case OFPUTIL_OFPST_PORT_DESC_REPLY:
     case OFPUTIL_NXT_ROLE_REQUEST:
     case OFPUTIL_NXT_ROLE_REPLY:
     case OFPUTIL_NXT_FLOW_MOD_TABLE_ID:
@@ -363,7 +365,7 @@ process_switch_features(struct lswitch *sw, struct ofp_switch_features *osf)
 
     sw->datapath_id = features.datapath_id;
 
-    while (!ofputil_pull_switch_features_port(&b, &port)) {
+    while (!ofputil_pull_phy_port(osf->header.version, &b, &port)) {
         struct lswitch_port *lp = shash_find_data(&sw->queue_names, port.name);
         if (lp && hmap_node_is_null(&lp->hmap_node)) {
             lp->port_no = port.port_no;
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 5b840982..7479bf22 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -642,6 +642,37 @@ ofp_print_phy_port(struct ds *string, const struct ofputil_phy_port *port)
                   port->max_speed / UINT32_C(1000));
 }
 
+/* Given a buffer 'b' that contains an array of OpenFlow ports of type
+ * 'ofp_version', writes a detailed description of each port into
+ * 'string'. */
+static void
+ofp_print_phy_ports(struct ds *string, uint8_t ofp_version,
+                    struct ofpbuf *b)
+{
+    size_t n_ports;
+    struct ofputil_phy_port *ports;
+    enum ofperr error;
+    size_t i;
+
+    n_ports = ofputil_count_phy_ports(ofp_version, b);
+
+    ports = xmalloc(n_ports * sizeof *ports);
+    for (i = 0; i < n_ports; i++) {
+        error = ofputil_pull_phy_port(ofp_version, b, &ports[i]);
+        if (error) {
+            ofp_print_error(string, error);
+            goto exit;
+        }
+    }
+    qsort(ports, n_ports, sizeof *ports, compare_ports);
+    for (i = 0; i < n_ports; i++) {
+        ofp_print_phy_port(string, &ports[i]);
+    }
+
+exit:
+    free(ports);
+}
+
 static const char *
 ofputil_capabilities_to_name(uint32_t bit)
 {
@@ -704,11 +735,8 @@ ofp_print_switch_features(struct ds *string,
                           const struct ofp_switch_features *osf)
 {
     struct ofputil_switch_features features;
-    struct ofputil_phy_port *ports;
     enum ofperr error;
     struct ofpbuf b;
-    size_t n_ports;
-    size_t i;
 
     error = ofputil_decode_switch_features(osf, &features, &b);
     if (error) {
@@ -730,21 +758,7 @@ ofp_print_switch_features(struct ds *string,
                         ofputil_action_bitmap_to_name);
     ds_put_char(string, '\n');
 
-    n_ports = ofputil_count_phy_ports(osf);
-
-    ports = xmalloc(n_ports * sizeof *ports);
-    for (i = 0; i < n_ports; i++) {
-        error = ofputil_pull_switch_features_port(&b, &ports[i]);
-        if (error) {
-            ofp_print_error(string, error);
-            return;
-        }
-    }
-    qsort(ports, n_ports, sizeof *ports, compare_ports);
-    for (i = 0; i < n_ports; i++) {
-        ofp_print_phy_port(string, &ports[i]);
-    }
-    free(ports);
+    ofp_print_phy_ports(string, osf->header.version, &b);
 }
 
 static void
@@ -1393,6 +1407,18 @@ ofp_print_ofpst_queue_reply(struct ds *string, const struct ofp_header *oh,
 }
 
 static void
+ofp_print_ofpst_port_desc_reply(struct ds *string,
+                                const struct ofp_header *oh)
+{
+    struct ofpbuf b;
+
+    ofpbuf_use_const(&b, oh, ntohs(oh->length));
+    ofpbuf_pull(&b, sizeof(struct ofp_stats_msg));
+    ds_put_char(string, '\n');
+    ofp_print_phy_ports(string, oh->version, &b);
+}
+
+static void
 ofp_print_stats_request(struct ds *string, const struct ofp_header *oh)
 {
     const struct ofp_stats_msg *srq = (const struct ofp_stats_msg *) oh;
@@ -1656,6 +1682,7 @@ ofp_to_string__(const struct ofp_header *oh,
         break;
 
     case OFPUTIL_OFPST_DESC_REQUEST:
+    case OFPUTIL_OFPST_PORT_DESC_REQUEST:
         ofp_print_stats_request(string, oh);
         break;
 
@@ -1712,6 +1739,11 @@ ofp_to_string__(const struct ofp_header *oh,
         ofp_print_ofpst_aggregate_reply(string, msg);
         break;
 
+    case OFPUTIL_OFPST_PORT_DESC_REPLY:
+        ofp_print_stats_reply(string, oh);
+        ofp_print_ofpst_port_desc_reply(string, oh);
+        break;
+
     case OFPUTIL_NXT_ROLE_REQUEST:
     case OFPUTIL_NXT_ROLE_REPLY:
         ofp_print_nxt_role_message(string, msg);
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 14006f9e..60071477 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -587,6 +587,10 @@ ofputil_decode_ofpst_request(const struct ofp_header *oh, size_t length,
           OFPST_QUEUE, "OFPST_QUEUE request",
           sizeof(struct ofp_queue_stats_request), 0 },
 
+        { OFPUTIL_OFPST_PORT_DESC_REQUEST, OFP10_VERSION,
+          OFPST_PORT_DESC, "OFPST_PORT_DESC request",
+          sizeof(struct ofp_stats_msg), 0 },
+
         { 0, 0,
           OFPST_VENDOR, "OFPST_VENDOR request",
           sizeof(struct ofp_vendor_stats_msg), 1 },
@@ -644,6 +648,10 @@ ofputil_decode_ofpst_reply(const struct ofp_header *oh, size_t length,
           OFPST_QUEUE, "OFPST_QUEUE reply",
           sizeof(struct ofp_stats_msg), sizeof(struct ofp_queue_stats) },
 
+        { OFPUTIL_OFPST_PORT_DESC_REPLY, OFP10_VERSION,
+          OFPST_PORT_DESC, "OFPST_PORT_DESC reply",
+          sizeof(struct ofp_stats_msg), sizeof(struct ofp10_phy_port) },
+
         { 0, 0,
           OFPST_VENDOR, "OFPST_VENDOR reply",
           sizeof(struct ofp_vendor_stats_msg), 1 },
@@ -2363,19 +2371,6 @@ ofputil_decode_ofp11_port(struct ofputil_phy_port *pp,
     return 0;
 }
 
-static int
-ofputil_pull_phy_port(uint8_t ofp_version, struct ofpbuf *b,
-                      struct ofputil_phy_port *pp)
-{
-    if (ofp_version == OFP10_VERSION) {
-        const struct ofp10_phy_port *opp = ofpbuf_try_pull(b, sizeof *opp);
-        return opp ? ofputil_decode_ofp10_phy_port(pp, opp) : EOF;
-    } else {
-        const struct ofp11_port *op = ofpbuf_try_pull(b, sizeof *op);
-        return op ? ofputil_decode_ofp11_port(pp, op) : EOF;
-    }
-}
-
 static void
 ofputil_encode_ofp10_phy_port(const struct ofputil_phy_port *pp,
                               struct ofp10_phy_port *opp)
@@ -2435,6 +2430,24 @@ ofputil_put_phy_port(uint8_t ofp_version, const struct ofputil_phy_port *pp,
         }
     }
 }
+
+void
+ofputil_append_port_desc_stats_reply(uint8_t ofp_version,
+                                     const struct ofputil_phy_port *pp,
+                                     struct list *replies)
+{
+    if (ofp_version == OFP10_VERSION) {
+        struct ofp10_phy_port *opp;
+
+        opp = ofputil_append_stats_reply(sizeof *opp, replies);
+        ofputil_encode_ofp10_phy_port(pp, opp);
+    } else {
+        struct ofp11_port *op;
+
+        op = ofputil_append_stats_reply(sizeof *op, replies);
+        ofputil_encode_ofp11_port(pp, op);
+    }
+}
 
 /* ofputil_switch_features */
 
@@ -2515,7 +2528,7 @@ decode_action_bits(ovs_be32 of_actions,
 /* Decodes an OpenFlow 1.0 or 1.1 "switch_features" structure 'osf' into an
  * abstract representation in '*features'.  Initializes '*b' to iterate over
  * the OpenFlow port structures following 'osf' with later calls to
- * ofputil_pull_switch_features_port().  Returns 0 if successful, otherwise an
+ * ofputil_pull_phy_port().  Returns 0 if successful, otherwise an
  * OFPERR_* value.  */
 enum ofperr
 ofputil_decode_switch_features(const struct ofp_switch_features *osf,
@@ -2524,7 +2537,6 @@ ofputil_decode_switch_features(const struct ofp_switch_features *osf,
 {
     ofpbuf_use_const(b, osf, ntohs(osf->header.length));
     ofpbuf_pull(b, sizeof *osf);
-    b->l2 = (struct ofputil_switch_features *) osf;
 
     features->datapath_id = ntohll(osf->datapath_id);
     features->n_buffers = ntohl(osf->n_buffers);
@@ -2556,33 +2568,6 @@ ofputil_decode_switch_features(const struct ofp_switch_features *osf,
     return 0;
 }
 
-/* Given a buffer 'b' that was initialized by a previous successful call to
- * ofputil_decode_switch_features(), tries to decode an OpenFlow port structure
- * following the main switch features information.  If successful, initializes
- * '*pp' with an abstract representation of the port and returns 0.  If no
- * ports remained to be decoded, returns EOF.  On an error, returns a positive
- * OFPERR_* value.  */
-int
-ofputil_pull_switch_features_port(struct ofpbuf *b,
-                                  struct ofputil_phy_port *pp)
-{
-    const struct ofp_switch_features *osf = b->l2;
-    return ofputil_pull_phy_port(osf->header.version, b, pp);
-}
-
-/* Returns the number of OpenFlow port structures that follow the main switch
- * features information in '*osf'.  The return value is only guaranteed to be
- * accurate if '*osf' is well-formed, that is, if
- * ofputil_decode_switch_features() can process '*osf' successfully. */
-size_t
-ofputil_count_phy_ports(const struct ofp_switch_features *osf)
-{
-    size_t ports_len = ntohs(osf->header.length) - sizeof *osf;
-    return (osf->header.version == OFP10_VERSION
-            ? ports_len / sizeof(struct ofp10_phy_port)
-            : ports_len / sizeof(struct ofp11_port));
-}
-
 static ovs_be32
 encode_action_bits(enum ofputil_action_bitmap ofputil_actions,
                    const struct ofputil_action_bit_translation *x)
@@ -3362,6 +3347,33 @@ ofputil_format_port(uint16_t port, struct ds *s)
     ds_put_cstr(s, name);
 }
 
+/* Given a buffer 'b' that contains an array of OpenFlow ports of type
+ * 'ofp_version', tries to pull the first element from the array.  If
+ * successful, initializes '*pp' with an abstract representation of the
+ * port and returns 0.  If no ports remain to be decoded, returns EOF.
+ * On an error, returns a positive OFPERR_* value. */
+int
+ofputil_pull_phy_port(uint8_t ofp_version, struct ofpbuf *b,
+                      struct ofputil_phy_port *pp)
+{
+    if (ofp_version == OFP10_VERSION) {
+        const struct ofp10_phy_port *opp = ofpbuf_try_pull(b, sizeof *opp);
+        return opp ? ofputil_decode_ofp10_phy_port(pp, opp) : EOF;
+    } else {
+        const struct ofp11_port *op = ofpbuf_try_pull(b, sizeof *op);
+        return op ? ofputil_decode_ofp11_port(pp, op) : EOF;
+    }
+}
+
+/* Given a buffer 'b' that contains an array of OpenFlow ports of type
+ * 'ofp_version', returns the number of elements. */
+size_t ofputil_count_phy_ports(uint8_t ofp_version, struct ofpbuf *b)
+{
+    return (ofp_version == OFP10_VERSION
+            ? b->size / sizeof(struct ofp10_phy_port)
+            : b->size / sizeof(struct ofp11_port));
+}
+
 static enum ofperr
 check_resubmit_table(const struct nx_action_resubmit *nar)
 {
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 2e3fb3a4..a3c12fc7 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -62,6 +62,7 @@ enum ofputil_msg_code {
     OFPUTIL_OFPST_TABLE_REQUEST,
     OFPUTIL_OFPST_PORT_REQUEST,
     OFPUTIL_OFPST_QUEUE_REQUEST,
+    OFPUTIL_OFPST_PORT_DESC_REQUEST,
 
     /* OFPST_* stat replies. */
     OFPUTIL_OFPST_DESC_REPLY,
@@ -70,6 +71,7 @@ enum ofputil_msg_code {
     OFPUTIL_OFPST_PORT_REPLY,
     OFPUTIL_OFPST_TABLE_REPLY,
     OFPUTIL_OFPST_AGGREGATE_REPLY,
+    OFPUTIL_OFPST_PORT_DESC_REPLY,
 
     /* NXT_* messages. */
     OFPUTIL_NXT_ROLE_REQUEST,
@@ -435,9 +437,6 @@ struct ofputil_switch_features {
 enum ofperr ofputil_decode_switch_features(const struct ofp_switch_features *,
                                            struct ofputil_switch_features *,
                                            struct ofpbuf *);
-int ofputil_pull_switch_features_port(struct ofpbuf *,
-                                      struct ofputil_phy_port *);
-size_t ofputil_count_phy_ports(const struct ofp_switch_features *);
 
 struct ofpbuf *ofputil_encode_switch_features(
     const struct ofputil_switch_features *, enum ofputil_protocol,
@@ -445,6 +444,11 @@ struct ofpbuf *ofputil_encode_switch_features(
 void ofputil_put_switch_features_port(const struct ofputil_phy_port *,
                                       struct ofpbuf *);
 
+/* phy_port helper functions. */
+int ofputil_pull_phy_port(uint8_t ofp_version, struct ofpbuf *,
+                          struct ofputil_phy_port *);
+size_t ofputil_count_phy_ports(uint8_t ofp_version, struct ofpbuf *);
+
 /* Abstract ofp_port_status. */
 struct ofputil_port_status {
     enum ofp_port_reason reason;
@@ -500,6 +504,10 @@ void ofputil_start_stats_reply(const struct ofp_stats_msg *request,
 struct ofpbuf *ofputil_reserve_stats_reply(size_t len, struct list *);
 void *ofputil_append_stats_reply(size_t len, struct list *);
 
+void ofputil_append_port_desc_stats_reply(uint8_t ofp_version,
+                                          const struct ofputil_phy_port *pp,
+                                          struct list *replies);
+
 const void *ofputil_stats_body(const struct ofp_header *);
 size_t ofputil_stats_body_len(const struct ofp_header *);
 
@@ -524,6 +532,7 @@ struct ofpbuf *ofputil_encode_barrier_request(void);
 
 const char *ofputil_frag_handling_to_string(enum ofp_config_flags);
 bool ofputil_frag_handling_from_string(const char *, enum ofp_config_flags *);
+
 
 /* Actions. */
 
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 4b2cbc90..806e56bf 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2187,6 +2187,25 @@ handle_port_stats_request(struct ofconn *ofconn,
     return 0;
 }
 
+static enum ofperr
+handle_port_desc_stats_request(struct ofconn *ofconn,
+                               const struct ofp_stats_msg *osm)
+{
+    struct ofproto *p = ofconn_get_ofproto(ofconn);
+    struct ofport *port;
+    struct list replies;
+
+    ofputil_start_stats_reply(osm, &replies);
+
+    HMAP_FOR_EACH (port, hmap_node, &p->ports) {
+        ofputil_append_port_desc_stats_reply(ofconn_get_protocol(ofconn),
+                                             &port->pp, &replies);
+    }
+
+    ofconn_send_replies(ofconn, &replies);
+    return 0;
+}
+
 static void
 calc_flow_duration__(long long int start, long long int now,
                      uint32_t *sec, uint32_t *nsec)
@@ -3309,6 +3328,9 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
     case OFPUTIL_OFPST_QUEUE_REQUEST:
         return handle_queue_stats_request(ofconn, msg->data);
 
+    case OFPUTIL_OFPST_PORT_DESC_REQUEST:
+        return handle_port_desc_stats_request(ofconn, msg->data);
+
     case OFPUTIL_MSG_INVALID:
     case OFPUTIL_OFPT_HELLO:
     case OFPUTIL_OFPT_ERROR:
@@ -3326,6 +3348,7 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
     case OFPUTIL_OFPST_PORT_REPLY:
     case OFPUTIL_OFPST_TABLE_REPLY:
     case OFPUTIL_OFPST_AGGREGATE_REPLY:
+    case OFPUTIL_OFPST_PORT_DESC_REPLY:
     case OFPUTIL_NXT_ROLE_REPLY:
     case OFPUTIL_NXT_FLOW_REMOVED:
     case OFPUTIL_NXT_PACKET_IN:
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 2b172d49..4b94fb4b 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -688,6 +688,32 @@ OFPST_QUEUE reply (xid=0x1): 6 queues
 ])
 AT_CLEANUP
 
+AT_SETUP([OFPST_PORT_DESC request - OF1.0])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "0110000c00000001000d0000"], [0], [dnl
+OFPST_PORT_DESC request (xid=0x1):
+])
+AT_CLEANUP
+
+AT_SETUP([OFPST_PORT_DESC reply - OF1.0])
+AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
+AT_CHECK([ovs-ofctl ofp-print "\
+01 11 00 3c 00 00 00 00 00 0d 00 00 00 03 50 54 \
+00 00 00 01 65 74 68 30 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 01 00 00 00 01 00 00 02 08 \
+00 00 02 8f 00 00 02 8f 00 00 00 00 \
+"], [0], [dnl
+OFPST_PORT_DESC reply (xid=0x0):
+ 3(eth0): addr:50:54:00:00:00:01
+     config:     PORT_DOWN
+     state:      LINK_DOWN
+     current:    100MB-FD AUTO_NEG
+     advertised: 10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+     supported:  10MB-HD 10MB-FD 100MB-HD 100MB-FD COPPER AUTO_NEG
+     speed: 100 Mbps now, 100 Mbps max
+])
+AT_CLEANUP
+
 AT_SETUP([OFPT_BARRIER_REQUEST])
 AT_KEYWORDS([ofp-print])
 AT_CHECK([ovs-ofctl ofp-print '01 12 00 08 00 00 00 01'], [0], [dnl
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 89264275..9009b915 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -38,6 +38,21 @@ AT_CLEANUP
 
 dnl This is really bare-bones.
 dnl It at least checks request and reply serialization and deserialization.
+AT_SETUP([ofproto - port-desc stats])
+OVS_VSWITCHD_START
+AT_CHECK([ovs-ofctl -vwarn dump-ports-desc br0], [0], [stdout])
+AT_CHECK([STRIP_XIDS stdout], [0], [dnl
+OFPST_PORT_DESC reply:
+ LOCAL(br0): addr:aa:55:aa:55:00:00
+     config:     PORT_DOWN
+     state:      LINK_DOWN
+     speed: 100 Mbps now, 100 Mbps max
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
+dnl This is really bare-bones.
+dnl It at least checks request and reply serialization and deserialization.
 AT_SETUP([ofproto - queue stats])
 OVS_VSWITCHD_START
 AT_CHECK([ovs-ofctl -vANY:ANY:WARN queue-stats br0], [0], [stdout])
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index fdaa037d..4f54208a 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -66,6 +66,12 @@ associated with that device will be printed.  \fInetdev\fR can be an
 OpenFlow assigned port number or device name, e.g. \fBeth0\fR.
 .
 .TP
+\fBdump\-ports\-desc \fIswitch\fR
+Prints to the console detailed information about network devices
+associated with \fIswitch\fR (version 1.7 or later).  This is a subset
+of the information provided by the \fBshow\fR command.
+.
+.TP
 \fBmod\-port \fIswitch\fR \fInetdev\fR \fIaction\fR
 Modify characteristics of an interface monitored by \fIswitch\fR.  
 \fInetdev\fR can be referred to by its OpenFlow assigned port number or 
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 86c0a859..4a37067f 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -210,6 +210,7 @@ usage(void)
            "  get-frags SWITCH            print fragment handling behavior\n"
            "  set-frags SWITCH FRAG_MODE  set fragment handling behavior\n"
            "  dump-ports SWITCH [PORT]    print port statistics\n"
+           "  dump-ports-desc SWITCH      print port descriptions\n"
            "  dump-flows SWITCH           print all flow entries\n"
            "  dump-flows SWITCH FLOW      print matching FLOWs\n"
            "  dump-aggregate SWITCH       print aggregate flow statistics\n"
@@ -547,7 +548,7 @@ fetch_ofputil_phy_port(const char *vconn_name, const char *port_name,
                   vconn_name, ofperr_to_string(error));
     }
 
-    while (!ofputil_pull_switch_features_port(&b, pp)) {
+    while (!ofputil_pull_phy_port(osf->header.version, &b, pp)) {
         if (port_no != UINT_MAX
             ? port_no == pp->port_no
             : !strcmp(pp->name, port_name)) {
@@ -1085,6 +1086,12 @@ do_dump_ports(int argc, char *argv[])
 }
 
 static void
+do_dump_ports_desc(int argc OVS_UNUSED, char *argv[])
+{
+    dump_trivial_stats_transaction(argv[1], OFPST_PORT_DESC);
+}
+
+static void
 do_probe(int argc OVS_UNUSED, char *argv[])
 {
     struct ofpbuf *request;
@@ -1896,6 +1903,7 @@ static const struct command all_commands[] = {
     { "diff-flows", 2, 2, do_diff_flows },
     { "packet-out", 4, INT_MAX, do_packet_out },
     { "dump-ports", 1, 2, do_dump_ports },
+    { "dump-ports-desc", 1, 1, do_dump_ports_desc },
     { "mod-port", 3, 3, do_mod_port },
     { "get-frags", 1, 1, do_get_frags },
     { "set-frags", 2, 2, do_set_frags },