On Thu, Oct 04, 2012 at 10:18:26AM -0700, Ben Pfaff wrote:
> On Thu, Sep 20, 2012 at 01:10:41PM +0900, Simon Horman wrote:
> > This allows for encoding and decoding Open Flow 1.1 and 1.2 Port Stats
> > Request and Reply message
> > 
> > Signed-off-by: Simon Horman <ho...@verge.net.au>
> 
> I see a few careless errors here.  print_port_stat() should have its
> return type on a separate line.  ofputil_parse_key_value() shouldn't
> be changed at all.  ofputlil_dump_ports() has a typo in its name.  So
> does ofptutil_decode_one_port_reply().  So does
> ofptutil_decode_port_stats_request().
> 
> I see some incorrect uses of assert, that could fire in production if
> an unsupported OF1.1 port number is used.  They would also drop the
> port numbers from the output if NDEBUG was defined.
> 
> I'm not really fond of the use of ofp11_port_stats as the common
> format.  I think it would be better to define a host-byte-order
> structure.  It could have the same fields as ofp_port_stats, or it
> could have a uint16_t port number and a "struct netdev_stats".  (The
> latter would be convenient for ofproto.)

Hi Ben,

I believe that the revised patch below addresses the concerns that
you raised above. I have taken the uint16_t + struct netdev_stats
approach that you suggested.


From: Simon Horman <ho...@verge.net.au>

ofp-msgs: Open Flow 1.1 and 1.2 Port Status Messages

This allows for encoding and decoding Open Flow 1.1 and 1.2 Port Stats
Request and Reply message

Signed-off-by: Simon Horman <ho...@verge.net.au>

---

v14.1
* Add and make use of struct ofputil_port_stats,
  a generic host-byte-order representation of port status replies
* Refactor and do not add new calls to abort()
* Fix typos in several helper function names
* Fix code-style problems
* Remove unnecessary 'noise' changes

v14
* Manual rebase

v13
* Merge the following patches
  - ofp-msgs: Split OFPRAW_OFPST_PORT_{REQUEST,REPLY}
  - ovs-ofctl: Teach dump-ports about Open Flow 1.1 & 1.2
  - ofp-util: Allow encoding of Open Flow 1.1 & 1.2 Port Statistics Reply 
Messages
  - ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port Reply Messages
  - ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Port Status Request Messages
  - ofp-print: Allow printing of Open Flow 1.1 & 1.2 Port Status Request 
Messages
* Move encoding and decoding into a helper functions

v12
* No change

v11
* No change

v10
* No change

v9
* Update description - this only prepares for both encoding and decoding
* Add ofp-print test
* Make use of enum ofp_version
* Correct printing of port number for Open Flow 1.0
* Replace UINT64_MAX with htonll(UINT64_MAX) in ofp_print_port_stat()
  to correct byte-order problem detected by sparse

v8
* Manual rebase
* Make use of enum ofp_version
* Add ofp-print test

v7
* Omitted

v6
* No change

v5
* No change

v4
* Initial post
---
 lib/ofp-msgs.h        |   14 ++-
 lib/ofp-print.c       |   71 ++++++++++------
 lib/ofp-util.c        |  225 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/ofp-util.h        |   16 ++++
 ofproto/ofproto.c     |   35 +++-----
 tests/ofp-print.at    |   59 ++++++++++++-
 utilities/ovs-ofctl.c |    6 +-
 7 files changed, 366 insertions(+), 60 deletions(-)

diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index 752d12c..6420c5d 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -216,10 +216,14 @@ enum ofpraw {
     OFPRAW_OFPST12_TABLE_REPLY,
 
     /* OFPST 1.0 (4): struct ofp10_port_stats_request. */
-    OFPRAW_OFPST_PORT_REQUEST,
+    OFPRAW_OFPST10_PORT_REQUEST,
+    /* OFPST 1.1+ (4): struct ofp11_port_stats_request. */
+    OFPRAW_OFPST11_PORT_REQUEST,
 
     /* OFPST 1.0 (4): struct ofp10_port_stats[]. */
-    OFPRAW_OFPST_PORT_REPLY,
+    OFPRAW_OFPST10_PORT_REPLY,
+    /* OFPST 1.1+ (4): struct ofp11_port_stats[]. */
+    OFPRAW_OFPST11_PORT_REPLY,
 
     /* OFPST 1.0 (5): struct ofp10_queue_stats_request. */
     OFPRAW_OFPST_QUEUE_REQUEST,
@@ -382,8 +386,10 @@ enum ofptype {
     OFPTYPE_TABLE_STATS_REPLY,       /* OFPRAW_OFPST10_TABLE_REPLY.
                                       * OFPRAW_OFPST11_TABLE_REPLY.
                                       * OFPRAW_OFPST12_TABLE_REPLY. */
-    OFPTYPE_PORT_STATS_REQUEST,      /* OFPRAW_OFPST_PORT_REQUEST. */
-    OFPTYPE_PORT_STATS_REPLY,        /* OFPRAW_OFPST_PORT_REPLY. */
+    OFPTYPE_PORT_STATS_REQUEST,      /* OFPRAW_OFPST10_PORT_REQUEST.
+                                      * OFPRAW_OFPST11_PORT_REQUEST. */
+    OFPTYPE_PORT_STATS_REPLY,        /* OFPRAW_OFPST10_PORT_REPLY.
+                                      * OFPRAW_OFPST11_PORT_REPLY. */
     OFPTYPE_QUEUE_STATS_REQUEST,     /* OFPRAW_OFPST_QUEUE_REQUEST. */
     OFPTYPE_QUEUE_STATS_REPLY,       /* OFPRAW_OFPST_QUEUE_REPLY. */
     OFPTYPE_PORT_DESC_STATS_REQUEST, /* OFPRAW_OFPST_PORT_DESC_REQUEST. */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 707699e..88d5d8e 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1065,11 +1065,9 @@ ofp_print_aggregate_stats_reply(struct ds *string, const 
struct ofp_header *oh)
     ds_put_format(string, " flow_count=%"PRIu32, as.flow_count);
 }
 
-static void print_port_stat(struct ds *string, const char *leader,
-                            const ovs_32aligned_be64 *statp, int more)
+static void
+print_port_stat(struct ds *string, const char *leader, uint64_t stat, int more)
 {
-    uint64_t stat = ntohll(get_32aligned_be64(statp));
-
     ds_put_cstr(string, leader);
     if (stat != UINT64_MAX) {
         ds_put_format(string, "%"PRIu64, stat);
@@ -1084,52 +1082,71 @@ static void print_port_stat(struct ds *string, const 
char *leader,
 }
 
 static void
+print_port_stat_32aligned(struct ds *string, const char *leader,
+                          const ovs_32aligned_be64 *statp, int more)
+{
+    print_port_stat(string, leader, ntohll(get_32aligned_be64(statp)), more);
+}
+
+static void
 ofp_print_ofpst_port_request(struct ds *string, const struct ofp_header *oh)
 {
-    const struct ofp10_port_stats_request *psr = ofpmsg_body(oh);
-    ds_put_format(string, " port_no=%"PRIu16, ntohs(psr->port_no));
+    uint16_t ofp10_port;
+    enum ofperr error;
+
+    error = ofputil_decode_port_stats_request(oh, &ofp10_port);
+    if (error) {
+        ofp_print_error(string, error);
+        return;
+    }
+
+    ds_put_format(string, " port_no=%2"PRIu16, ofp10_port);
 }
 
 static void
 ofp_print_ofpst_port_reply(struct ds *string, const struct ofp_header *oh,
                            int verbosity)
 {
-    struct ofp10_port_stats *ps;
     struct ofpbuf b;
     size_t n;
 
     ofpbuf_use_const(&b, oh, ntohs(oh->length));
     ofpraw_pull_assert(&b);
 
-    n = b.size / sizeof *ps;
+    /* This calculation is correct because struct ofp10_port_stats and
+     * struct ofp11_port_stats are the same size. */
+    n = b.size / sizeof(struct ofp10_port_stats);
     ds_put_format(string, " %zu ports\n", n);
     if (verbosity < 1) {
         return;
     }
 
     for (;;) {
-        ps = ofpbuf_try_pull(&b, sizeof *ps);
-        if (!ps) {
+        struct ofputil_port_stats ops;
+        enum ofperr error;
+
+        error = ofputil_decode_port_stats(&ops, oh->version, &b);
+        if (error) {
             return;
         }
 
-        ds_put_format(string, "  port %2"PRIu16": ", ntohs(ps->port_no));
+        ds_put_format(string, "  port %2"PRIu16, ops.port_no);
 
-        ds_put_cstr(string, "rx ");
-        print_port_stat(string, "pkts=", &ps->rx_packets, 1);
-        print_port_stat(string, "bytes=", &ps->rx_bytes, 1);
-        print_port_stat(string, "drop=", &ps->rx_dropped, 1);
-        print_port_stat(string, "errs=", &ps->rx_errors, 1);
-        print_port_stat(string, "frame=", &ps->rx_frame_err, 1);
-        print_port_stat(string, "over=", &ps->rx_over_err, 1);
-        print_port_stat(string, "crc=", &ps->rx_crc_err, 0);
+        ds_put_cstr(string, ": rx ");
+        print_port_stat(string, "pkts=", ops.stats.rx_packets, 1);
+        print_port_stat(string, "bytes=", ops.stats.rx_bytes, 1);
+        print_port_stat(string, "drop=", ops.stats.rx_dropped, 1);
+        print_port_stat(string, "errs=", ops.stats.rx_errors, 1);
+        print_port_stat(string, "frame=", ops.stats.rx_frame_errors, 1);
+        print_port_stat(string, "over=", ops.stats.rx_over_errors, 1);
+        print_port_stat(string, "crc=", ops.stats.rx_crc_errors, 0);
 
         ds_put_cstr(string, "           tx ");
-        print_port_stat(string, "pkts=", &ps->tx_packets, 1);
-        print_port_stat(string, "bytes=", &ps->tx_bytes, 1);
-        print_port_stat(string, "drop=", &ps->tx_dropped, 1);
-        print_port_stat(string, "errs=", &ps->tx_errors, 1);
-        print_port_stat(string, "coll=", &ps->collisions, 0);
+        print_port_stat(string, "pkts=", ops.stats.tx_packets, 1);
+        print_port_stat(string, "bytes=", ops.stats.tx_bytes, 1);
+        print_port_stat(string, "drop=", ops.stats.tx_dropped, 1);
+        print_port_stat(string, "errs=", ops.stats.tx_errors, 1);
+        print_port_stat(string, "coll=", ops.stats.collisions, 0);
     }
 }
 
@@ -1357,9 +1374,9 @@ ofp_print_ofpst_queue_reply(struct ds *string, const 
struct ofp_header *oh,
         ofp_print_queue_name(string, ntohl(qs->queue_id));
         ds_put_cstr(string, ": ");
 
-        print_port_stat(string, "bytes=", &qs->tx_bytes, 1);
-        print_port_stat(string, "pkts=", &qs->tx_packets, 1);
-        print_port_stat(string, "errors=", &qs->tx_errors, 0);
+        print_port_stat_32aligned(string, "bytes=", &qs->tx_bytes, 1);
+        print_port_stat_32aligned(string, "pkts=", &qs->tx_packets, 1);
+        print_port_stat_32aligned(string, "errors=", &qs->tx_errors, 0);
     }
 }
 
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 00dfea3..68f2b25 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3938,3 +3938,228 @@ ofputil_parse_key_value(char **stringp, char **keyp, 
char **valuep)
     *valuep = value;
     return true;
 }
+
+/* Encode a dump ports request for 'port', the encoded message
+ * will be fore Open Flow version 'ofp_version'. Returns message
+ * as a struct ofpbuf. Returns encoded message on success, NULL on error */
+struct ofpbuf *
+ofputil_encode_dump_ports_request(enum ofp_version ofp_version, int16_t port)
+{
+    struct ofpbuf *request;
+    enum ofpraw raw;
+
+    switch (ofp_version) {
+    case OFP10_VERSION:
+        raw = OFPRAW_OFPST10_PORT_REQUEST;
+        break;
+    case OFP11_VERSION:
+    case OFP12_VERSION:
+        raw = OFPRAW_OFPST11_PORT_REQUEST;
+        break;
+    default:
+        NOT_REACHED();
+    }
+
+    request = ofpraw_alloc(raw, ofp_version, 0);
+
+    switch (ofp_version) {
+    case OFP10_VERSION: {
+        struct ofp10_port_stats_request *req;
+        req = ofpbuf_put_zeros(request, sizeof *req);
+        req->port_no = htons(port);
+        break;
+    }
+    case OFP11_VERSION:
+    case OFP12_VERSION: {
+        struct ofp11_port_stats_request *req;
+        req = ofpbuf_put_zeros(request, sizeof *req);
+        req->port_no = ofputil_port_to_ofp11(port);
+        break;
+    }
+    default:
+        NOT_REACHED();
+    }
+
+    return request;
+}
+
+static void
+ofputil_port_stats_to_ofp10(const struct ofputil_port_stats *ops,
+                            struct ofp10_port_stats *ps10)
+{
+    ps10->port_no = htons(ops->port_no);
+    memset(ps10->pad, 0, sizeof ps10->pad);
+    put_32aligned_be64(&ps10->rx_packets, htonll(ops->stats.rx_packets));
+    put_32aligned_be64(&ps10->tx_packets, htonll(ops->stats.tx_packets));
+    put_32aligned_be64(&ps10->rx_bytes, htonll(ops->stats.rx_bytes));
+    put_32aligned_be64(&ps10->tx_bytes, htonll(ops->stats.tx_bytes));
+    put_32aligned_be64(&ps10->rx_dropped, htonll(ops->stats.rx_dropped));
+    put_32aligned_be64(&ps10->tx_dropped, htonll(ops->stats.tx_dropped));
+    put_32aligned_be64(&ps10->rx_errors, htonll(ops->stats.rx_errors));
+    put_32aligned_be64(&ps10->tx_errors, htonll(ops->stats.tx_errors));
+    put_32aligned_be64(&ps10->rx_frame_err, 
htonll(ops->stats.rx_frame_errors));
+    put_32aligned_be64(&ps10->rx_over_err, htonll(ops->stats.rx_over_errors));
+    put_32aligned_be64(&ps10->rx_crc_err, htonll(ops->stats.rx_crc_errors));
+    put_32aligned_be64(&ps10->collisions, htonll(ops->stats.collisions));
+}
+
+static void
+ofputil_port_stats_to_ofp11(const struct ofputil_port_stats *ops,
+                            struct ofp11_port_stats *ps11)
+{
+    ps11->port_no = ofputil_port_to_ofp11(ops->port_no);
+    memset(ps11->pad, 0, sizeof ps11->pad);
+    ps11->rx_packets = htonll(ops->stats.rx_packets);
+    ps11->tx_packets = htonll(ops->stats.tx_packets);
+    ps11->rx_bytes = htonll(ops->stats.rx_bytes);
+    ps11->tx_bytes = htonll(ops->stats.tx_bytes);
+    ps11->rx_dropped = htonll(ops->stats.rx_dropped);
+    ps11->tx_dropped = htonll(ops->stats.tx_dropped);
+    ps11->rx_errors = htonll(ops->stats.rx_errors);
+    ps11->tx_errors = htonll(ops->stats.tx_errors);
+    ps11->rx_frame_err = htonll(ops->stats.rx_frame_errors);
+    ps11->rx_over_err = htonll(ops->stats.rx_over_errors);
+    ps11->rx_crc_err = htonll(ops->stats.rx_crc_errors);
+    ps11->collisions = htonll(ops->stats.collisions);
+}
+
+/* Encode a ports stat for 'opes' and append it to 'replies'.
+ * The encoded message will be for Open Flow version 'ofp_version'. */
+void
+ofputil_append_port_stat(struct list *replies,
+                         const struct ofputil_port_stats *ops)
+{
+    struct ofpbuf *msg = ofpbuf_from_list(list_back(replies));
+    struct ofp_header *oh = msg->data;
+
+    switch ((enum ofp_version)oh->version) {
+    case OFP12_VERSION:
+    case OFP11_VERSION: {
+        struct ofp11_port_stats *reply = ofpmp_append(replies, sizeof *reply);
+        ofputil_port_stats_to_ofp11(ops, reply);
+        break;
+    }
+
+    case OFP10_VERSION: {
+        struct ofp10_port_stats *reply = ofpmp_append(replies, sizeof *reply);
+        ofputil_port_stats_to_ofp10(ops, reply);
+        break;
+    }
+
+    default:
+        NOT_REACHED();
+    }
+}
+
+static enum ofperr
+ofputil_port_stats_from_ofp10(struct ofputil_port_stats *ops,
+                              const struct ofp10_port_stats *ps10)
+{
+    memset(ops, 0, sizeof *ops);
+
+    ops->port_no = ntohs(ps10->port_no);
+    ops->stats.rx_packets = ntohll(get_32aligned_be64(&ps10->rx_packets));
+    ops->stats.tx_packets = ntohll(get_32aligned_be64(&ps10->tx_packets));
+    ops->stats.rx_bytes = ntohll(get_32aligned_be64(&ps10->rx_bytes));
+    ops->stats.tx_bytes = ntohll(get_32aligned_be64(&ps10->tx_bytes));
+    ops->stats.rx_dropped = ntohll(get_32aligned_be64(&ps10->rx_dropped));
+    ops->stats.tx_dropped = ntohll(get_32aligned_be64(&ps10->tx_dropped));
+    ops->stats.rx_errors = ntohll(get_32aligned_be64(&ps10->rx_errors));
+    ops->stats.tx_errors = ntohll(get_32aligned_be64(&ps10->tx_errors));
+    ops->stats.rx_frame_errors =
+        ntohll(get_32aligned_be64(&ps10->rx_frame_err));
+    ops->stats.rx_over_errors = ntohll(get_32aligned_be64(&ps10->rx_over_err));
+    ops->stats.rx_crc_errors = ntohll(get_32aligned_be64(&ps10->rx_crc_err));
+    ops->stats.collisions = ntohll(get_32aligned_be64(&ps10->collisions));
+
+    return 0;
+}
+
+static enum ofperr
+ofputil_port_stats_from_ofp11(struct ofputil_port_stats *ops,
+                              const struct ofp11_port_stats *ps11)
+{
+    enum ofperr error;
+
+    memset(ops, 0, sizeof *ops);
+    error = ofputil_port_from_ofp11(ps11->port_no, &ops->port_no);
+    if (error) {
+        return error;
+    }
+
+    ops->stats.rx_packets = ntohll(ps11->rx_packets);
+    ops->stats.tx_packets = ntohll(ps11->tx_packets);
+    ops->stats.rx_bytes = ntohll(ps11->rx_bytes);
+    ops->stats.tx_bytes = ntohll(ps11->tx_bytes);
+    ops->stats.rx_dropped = ntohll(ps11->rx_dropped);
+    ops->stats.tx_dropped = ntohll(ps11->tx_dropped);
+    ops->stats.rx_errors = ntohll(ps11->rx_errors);
+    ops->stats.tx_errors = ntohll(ps11->tx_errors);
+    ops->stats.rx_frame_errors = ntohll(ps11->rx_frame_err);
+    ops->stats.rx_over_errors = ntohll(ps11->rx_over_err);
+    ops->stats.rx_crc_errors = ntohll(ps11->rx_crc_err);
+    ops->stats.collisions = ntohll(ps11->collisions);
+
+    return 0;
+}
+
+/* Parse a struct one element of a port stats reply message into a struct
+ * ofputil_port_stats.  'ps' should point to memory for a struct
+ * ofputil_port_stats.  Returns 0 if successful, otherwise an OFPERR_*
+ * value. */
+enum ofperr
+ofputil_decode_port_stats(struct ofputil_port_stats *ops,
+                          enum ofp_version ofp_version,
+                          struct ofpbuf *openflow)
+{
+    switch (ofp_version) {
+    case OFP12_VERSION:
+    case OFP11_VERSION: {
+        const struct ofp11_port_stats *ps11;
+
+        ps11 = ofpbuf_try_pull(openflow, sizeof *ps11);
+        if (!ps11) {
+            return OFPERR_OFPBRC_BAD_LEN;
+        }
+        return ofputil_port_stats_from_ofp11(ops, ps11);
+    }
+
+    case OFP10_VERSION: {
+        const struct ofp10_port_stats *ps10;
+
+        ps10 = ofpbuf_try_pull(openflow, sizeof *ps10);
+        if (!ps10) {
+            return OFPERR_OFPBRC_BAD_LEN;
+        }
+        return ofputil_port_stats_from_ofp10(ops, ps10);
+    }
+
+    default:
+        NOT_REACHED();
+    }
+}
+
+/* Parse a port status request message into a 16 bit OpenFlow 1.0
+ * port number and stores the latter in '*ofp10_port'.
+ * Returns 0 if successful, otherwise an OFPERR_* number. */
+enum ofperr
+ofputil_decode_port_stats_request(const struct ofp_header *request,
+                                  uint16_t *ofp10_port)
+{
+    switch ((enum ofp_version)request->version) {
+    case OFP12_VERSION:
+    case OFP11_VERSION: {
+        const struct ofp11_port_stats_request *psr11 = ofpmsg_body(request);
+        return ofputil_port_from_ofp11(psr11->port_no, ofp10_port);
+    }
+
+    case OFP10_VERSION: {
+        const struct ofp10_port_stats_request *psr10 = ofpmsg_body(request);
+        *ofp10_port = ntohs(psr10->port_no);
+        return 0;
+    }
+
+    default:
+        NOT_REACHED();
+    }
+}
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index c5812c6..2d25a67 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -620,4 +620,20 @@ union ofp_action *ofputil_actions_clone(const union 
ofp_action *, size_t n);
 /* Handy utility for parsing flows and actions. */
 bool ofputil_parse_key_value(char **stringp, char **keyp, char **valuep);
 
+struct ofpbuf *ofputlil_dump_ports(enum ofp_version ofp_version, int16_t port);
+
+struct ofputil_port_stats {
+    uint16_t port_no;
+    struct netdev_stats stats;
+};
+
+struct ofpbuf * ofputil_encode_dump_ports_request(enum ofp_version ofp_version,
+                                                  int16_t port);
+void ofputil_append_port_stat(struct list *replies,
+                              const struct ofputil_port_stats *ops);
+enum ofperr ofputil_decode_port_stats(struct ofputil_port_stats *ops,
+                                      enum ofp_version ofp_version,
+                                      struct ofpbuf *openflow);
+enum ofperr ofputil_decode_port_stats_request(const struct ofp_header *request,
+                                              uint16_t *ofp10_port);
 #endif /* ofp-util.h */
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 764202f..97e00ab 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2295,29 +2295,14 @@ handle_table_stats_request(struct ofconn *ofconn,
 static void
 append_port_stat(struct ofport *port, struct list *replies)
 {
-    struct netdev_stats stats;
-    struct ofp10_port_stats *ops;
+    struct ofputil_port_stats ops = { .port_no = port->pp.port_no };
 
     /* Intentionally ignore return value, since errors will set
      * 'stats' to all-1s, which is correct for OpenFlow, and
      * netdev_get_stats() will log errors. */
-    ofproto_port_get_stats(port, &stats);
-
-    ops = ofpmp_append(replies, sizeof *ops);
-    ops->port_no = htons(port->pp.port_no);
-    memset(ops->pad, 0, sizeof ops->pad);
-    put_32aligned_be64(&ops->rx_packets, htonll(stats.rx_packets));
-    put_32aligned_be64(&ops->tx_packets, htonll(stats.tx_packets));
-    put_32aligned_be64(&ops->rx_bytes, htonll(stats.rx_bytes));
-    put_32aligned_be64(&ops->tx_bytes, htonll(stats.tx_bytes));
-    put_32aligned_be64(&ops->rx_dropped, htonll(stats.rx_dropped));
-    put_32aligned_be64(&ops->tx_dropped, htonll(stats.tx_dropped));
-    put_32aligned_be64(&ops->rx_errors, htonll(stats.rx_errors));
-    put_32aligned_be64(&ops->tx_errors, htonll(stats.tx_errors));
-    put_32aligned_be64(&ops->rx_frame_err, htonll(stats.rx_frame_errors));
-    put_32aligned_be64(&ops->rx_over_err, htonll(stats.rx_over_errors));
-    put_32aligned_be64(&ops->rx_crc_err, htonll(stats.rx_crc_errors));
-    put_32aligned_be64(&ops->collisions, htonll(stats.collisions));
+    ofproto_port_get_stats(port, &ops.stats);
+
+    ofputil_append_port_stat(replies, &ops);
 }
 
 static enum ofperr
@@ -2325,13 +2310,19 @@ handle_port_stats_request(struct ofconn *ofconn,
                           const struct ofp_header *request)
 {
     struct ofproto *p = ofconn_get_ofproto(ofconn);
-    const struct ofp10_port_stats_request *psr = ofpmsg_body(request);
     struct ofport *port;
     struct list replies;
+    uint16_t port_no;
+    enum ofperr error;
+
+    error = ofputil_decode_port_stats_request(request, &port_no);
+    if (error) {
+        return error;
+    }
 
     ofpmp_init(&replies, request);
-    if (psr->port_no != htons(OFPP_NONE)) {
-        port = ofproto_get_port(p, ntohs(psr->port_no));
+    if (port_no != OFPP_NONE) {
+        port = ofproto_get_port(p, port_no);
         if (port) {
             append_port_stat(port, &replies);
         }
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 5a64ff2..ad36617 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -949,7 +949,7 @@ AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
 AT_CHECK([ovs-ofctl ofp-print "$(cat in)"], [0], [expout])
 AT_CLEANUP
 
-AT_SETUP([OFPST_PORT request])
+AT_SETUP([OFPST_PORT request - 1.0])
 AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
 AT_CHECK([ovs-ofctl ofp-print "\
 01 10 00 14 00 00 00 01 00 04 00 00 ff ff 00 00 \
@@ -959,7 +959,27 @@ OFPST_PORT request (xid=0x1): port_no=65535
 ])
 AT_CLEANUP
 
-AT_SETUP([OFPST_PORT reply])
+AT_SETUP([OFPST_PORT request - 1.1])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "\
+02 12 00 18 00 00 00 02 00 04 00 00 00 00 00 00 \
+ff ff ff ff 00 00 00 00 \
+"], [0], [dnl
+OFPST_PORT request (OF1.1) (xid=0x2): port_no=65535
+])
+AT_CLEANUP
+
+AT_SETUP([OFPST_PORT request - 1.2])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "\
+03 12 00 18 00 00 00 02 00 04 00 00 00 00 00 00 \
+ff ff ff ff 00 00 00 00 \
+"], [0], [dnl
+OFPST_PORT request (OF1.2) (xid=0x2): port_no=65535
+])
+AT_CLEANUP
+
+AT_SETUP([OFPST_PORT reply - OF1.0])
 AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
 AT_CHECK([ovs-ofctl ofp-print "\
 01 11 01 ac 00 00 00 01 00 04 00 00 00 03 00 00 \
@@ -1002,6 +1022,41 @@ OFPST_PORT reply (xid=0x1): 4 ports
 ])
 AT_CLEANUP
 
+AT_SETUP([OFPST_PORT reply - OF1.2])
+AT_KEYWORDS([ofp-print OFPT_STATS_REPLY])
+AT_CHECK([ovs-ofctl ofp-print "\
+03 13 01 48 00 00 00 02 00 04 00 00 00 00 00 00 \
+00 00 00 02 00 00 00 00 00 00 00 00 00 01 95 56 \
+00 00 00 00 00 00 00 88 00 00 00 00 02 5d 08 98 \
+00 00 00 00 00 00 2c f8 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 ff ff ff fe 00 00 00 00 \
+00 00 00 00 00 00 00 44 00 00 00 00 00 00 9d 2c \
+00 00 00 00 00 00 16 7c 00 00 00 00 01 1e 36 44 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 44 \
+00 00 00 00 00 00 9d 2c 00 00 00 00 00 00 16 7c \
+00 00 00 00 01 1e 36 44 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 \
+00 00 00 00 00 00 00 00 \
+"], [0], [dnl
+OFPST_PORT reply (OF1.2) (xid=0x2): 3 ports
+  port  2: rx pkts=103766, bytes=39651480, drop=0, errs=0, frame=0, over=0, 
crc=0
+           tx pkts=136, bytes=11512, drop=0, errs=0, coll=0
+  port 65534: rx pkts=68, bytes=5756, drop=0, errs=0, frame=0, over=0, crc=0
+           tx pkts=40236, bytes=18757188, drop=0, errs=0, coll=0
+  port  1: rx pkts=68, bytes=5756, drop=0, errs=0, frame=0, over=0, crc=0
+           tx pkts=40236, bytes=18757188, drop=0, errs=0, coll=0
+])
+AT_CLEANUP
+
 AT_SETUP([OFPST_QUEUE request])
 AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
 AT_CHECK([ovs-ofctl ofp-print "\
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 39aed86..5a1f106 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1418,17 +1418,13 @@ ofctl_snoop(int argc OVS_UNUSED, char *argv[])
 static void
 ofctl_dump_ports(int argc, char *argv[])
 {
-    struct ofp10_port_stats_request *req;
     struct ofpbuf *request;
     struct vconn *vconn;
     uint16_t port;
 
     open_vconn(argv[1], &vconn);
-    request = ofpraw_alloc(OFPRAW_OFPST_PORT_REQUEST,
-                           vconn_get_version(vconn), 0);
-    req = ofpbuf_put_zeros(request, sizeof *req);
     port = argc > 2 ? str_to_port_no(argv[1], argv[2]) : OFPP_NONE;
-    req->port_no = htons(port);
+    request = ofputil_encode_dump_ports_request(vconn_get_version(vconn), 
port);
     dump_stats_transaction(vconn, request);
     vconn_close(vconn);
 }
-- 
1.7.10.4

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to