openflow: Merge ofp_stats_request and ofp_stats_reply.

These structures for OpenFlow stats requests and replies have identical
memebers, but until now they have been separate structures.  Since in some
cases we actually want to treat both of them the same way, this has led
to various kinds of awkwardness.  This commit merges them into a new
"struct ofp_stats_msg" and fixes up the users.

diff --git a/include/openflow/openflow.h b/include/openflow/openflow.h
index f53954336da94d8ef4180fad2ce10aa2e5ef9c06..9213be6401eb3936efc7b7054c289f4f32079c1c 100644 (file)
--- a/include/openflow/openflow.h
+++ b/include/openflow/openflow.h
@@ -630,9 +630,9 @@ enum ofp_hello_failed_code {
 enum ofp_bad_request_code {
     OFPBRC_BAD_VERSION,         /* ofp_header.version not supported. */
     OFPBRC_BAD_TYPE,            /* ofp_header.type not supported. */
-    OFPBRC_BAD_STAT,            /* ofp_stats_request.type not supported. */
+    OFPBRC_BAD_STAT,            /* ofp_stats_msg.type not supported. */
     OFPBRC_BAD_VENDOR,          /* Vendor not supported (in ofp_vendor_header
-                                 * or ofp_stats_request or ofp_stats_reply). */
+                                 * or ofp_stats_msg). */
     OFPBRC_BAD_SUBTYPE,         /* Vendor subtype not supported. */
     OFPBRC_EPERM,               /* Permissions error. */
     OFPBRC_BAD_LEN,             /* Wrong request length for type. */
@@ -732,26 +732,20 @@ enum ofp_stats_types {
     OFPST_VENDOR = 0xffff
 };
 
-struct ofp_stats_request {
+/* Statistics request or reply message. */
+struct ofp_stats_msg {
     struct ofp_header header;
     ovs_be16 type;              /* One of the OFPST_* constants. */
-    ovs_be16 flags;             /* OFPSF_REQ_* flags (none yet defined). */
+    ovs_be16 flags;             /* Requests: always 0.
+                                 * Replies: 0 or OFPSF_REPLY_MORE. */
     uint8_t body[0];            /* Body of the request. */
 };
-OFP_ASSERT(sizeof(struct ofp_stats_request) == 12);
+OFP_ASSERT(sizeof(struct ofp_stats_msg) == 12);
 
 enum ofp_stats_reply_flags {
     OFPSF_REPLY_MORE  = 1 << 0  /* More replies to follow. */
 };
 
-struct ofp_stats_reply {
-    struct ofp_header header;
-    ovs_be16 type;              /* One of the OFPST_* constants. */
-    ovs_be16 flags;             /* OFPSF_REPLY_* flags. */
-    uint8_t body[0];            /* Body of the reply. */
-};
-OFP_ASSERT(sizeof(struct ofp_stats_reply) == 12);
-
 #define DESC_STR_LEN   256
 #define SERIAL_NUM_LEN 32
 /* Body of reply to OFPST_DESC request.  Each entry is a NULL-terminated
@@ -766,7 +760,7 @@ struct ofp_desc_stats {
 };
 OFP_ASSERT(sizeof(struct ofp_desc_stats) == 1056);
 
-/* Body for ofp_stats_request of type OFPST_FLOW. */
+/* Body for stats request of type OFPST_FLOW. */
 struct ofp_flow_stats_request {
     struct ofp_match match;   /* Fields to match. */
     uint8_t table_id;         /* ID of table to read (from ofp_table_stats)
@@ -799,7 +793,7 @@ struct ofp_flow_stats {
 };
 OFP_ASSERT(sizeof(struct ofp_flow_stats) == 88);
 
-/* Body for ofp_stats_request of type OFPST_AGGREGATE. */
+/* Body for stats request of type OFPST_AGGREGATE. */
 struct ofp_aggregate_stats_request {
     struct ofp_match match;   /* Fields to match. */
     uint8_t table_id;         /* ID of table to read (from ofp_table_stats)
@@ -835,7 +829,7 @@ struct ofp_table_stats {
 };
 OFP_ASSERT(sizeof(struct ofp_table_stats) == 64);
 
-/* Body for ofp_stats_request of type OFPST_PORT. */
+/* Body for stats request of type OFPST_PORT. */
 struct ofp_port_stats_request {
     ovs_be16 port_no;        /* OFPST_PORT message may request statistics
                                 for a single port (specified with port_no)
@@ -871,7 +865,7 @@ OFP_ASSERT(sizeof(struct ofp_port_stats) == 104);
 /* All ones is used to indicate all queues in a port (for stats retrieval). */
 #define OFPQ_ALL      0xffffffff
 
-/* Body for ofp_stats_request of type OFPST_QUEUE. */
+/* Body for stats request of type OFPST_QUEUE. */
 struct ofp_queue_stats_request {
     ovs_be16 port_no;        /* All ports if OFPP_ALL. */
     uint8_t pad[2];          /* Align to 32-bits. */
@@ -879,7 +873,7 @@ struct ofp_queue_stats_request {
 };
 OFP_ASSERT(sizeof(struct ofp_queue_stats_request) == 8);
 
-/* Body for ofp_stats_reply of type OFPST_QUEUE consists of an array of this
+/* Body for stats reply of type OFPST_QUEUE consists of an array of this
  * structure type. */
 struct ofp_queue_stats {
     ovs_be16 port_no;

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 0c3bea1d03699af63ad5586e06167d7a4aeef0fb..90273edfd645634ad0e43d9c96d693db8b6335f9 100644 (file)
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1287,8 +1287,7 @@ ofp_print_ofpst_queue_reply(struct ds *string, const struct ofp_header *oh,
 static void
 ofp_print_stats_request(struct ds *string, const struct ofp_header *oh)
 {
-    const struct ofp_stats_request *srq
-        = (const struct ofp_stats_request *) oh;
+    const struct ofp_stats_msg *srq = (const struct ofp_stats_msg *) oh;
 
     if (srq->flags) {
         ds_put_format(string, " ***unknown flags 0x%04"PRIx16"***",
@@ -1299,7 +1298,7 @@ ofp_print_stats_request(struct ds *string, const struct ofp_header *oh)
 static void
 ofp_print_stats_reply(struct ds *string, const struct ofp_header *oh)
 {
-    const struct ofp_stats_reply *srp = (const struct ofp_stats_reply *) oh;
+    const struct ofp_stats_msg *srp = (const struct ofp_stats_msg *) oh;
 
     if (srp->flags) {
         uint16_t flags = ntohs(srp->flags);

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index dfbbf1b66bd6e0c389b217fe4b0b94a9a1855f67..2c6afbe0164a6e6a42920b7236049b303bd1ef23 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -410,19 +410,17 @@ ofputil_decode_vendor(const struct ofp_header *oh,
 static int
 check_nxstats_msg(const struct ofp_header *oh)
 {
-    const struct ofp_stats_request *osr;
+    const struct ofp_stats_msg *osm = (const struct ofp_stats_msg *) oh;
     ovs_be32 vendor;
 
-    osr = (const struct ofp_stats_request *) oh;
-
-    memcpy(&vendor, osr->body, sizeof vendor);
+    memcpy(&vendor, osm->body, sizeof vendor);
     if (vendor != htonl(NX_VENDOR_ID)) {
         VLOG_WARN_RL(&bad_ofmsg_rl, "received vendor stats message for "
                      "unknown vendor %"PRIx32, ntohl(vendor));
         return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_VENDOR);
     }
 
-    if (ntohs(osr->header.length) < sizeof(struct nicira_stats_msg)) {
+    if (ntohs(osm->header.length) < sizeof(struct nicira_stats_msg)) {
         VLOG_WARN_RL(&bad_ofmsg_rl, "truncated Nicira stats message");
         return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN);
     }
@@ -502,35 +500,35 @@ static int
 ofputil_decode_ofpst_request(const struct ofp_header *oh,
                              const struct ofputil_msg_type **typep)
 {
-    enum { OSR_SIZE = sizeof(struct ofp_stats_request) };
+    enum { OSM_SIZE = sizeof(struct ofp_stats_msg) };
     static const struct ofputil_msg_type ofpst_requests[] = {
         { OFPUTIL_OFPST_DESC_REQUEST,
           OFPST_DESC, "OFPST_DESC request",
-          OSR_SIZE, 0 },
+          OSM_SIZE, 0 },
 
         { OFPUTIL_OFPST_FLOW_REQUEST,
           OFPST_FLOW, "OFPST_FLOW request",
-          OSR_SIZE + sizeof(struct ofp_flow_stats_request), 0 },
+          OSM_SIZE + sizeof(struct ofp_flow_stats_request), 0 },
 
         { OFPUTIL_OFPST_AGGREGATE_REQUEST,
           OFPST_AGGREGATE, "OFPST_AGGREGATE request",
-          OSR_SIZE + sizeof(struct ofp_aggregate_stats_request), 0 },
+          OSM_SIZE + sizeof(struct ofp_aggregate_stats_request), 0 },
 
         { OFPUTIL_OFPST_TABLE_REQUEST,
           OFPST_TABLE, "OFPST_TABLE request",
-          OSR_SIZE, 0 },
+          OSM_SIZE, 0 },
 
         { OFPUTIL_OFPST_PORT_REQUEST,
           OFPST_PORT, "OFPST_PORT request",
-          OSR_SIZE + sizeof(struct ofp_port_stats_request), 0 },
+          OSM_SIZE + sizeof(struct ofp_port_stats_request), 0 },
 
         { OFPUTIL_OFPST_QUEUE_REQUEST,
           OFPST_QUEUE, "OFPST_QUEUE request",
-          OSR_SIZE + sizeof(struct ofp_queue_stats_request), 0 },
+          OSM_SIZE + sizeof(struct ofp_queue_stats_request), 0 },
 
         { 0,
           OFPST_VENDOR, "OFPST_VENDOR request",
-          OSR_SIZE + sizeof(uint32_t), 1 },
+          OSM_SIZE + sizeof(uint32_t), 1 },
     };
 
     static const struct ofputil_msg_category ofpst_request_category = {
@@ -539,14 +537,13 @@ ofputil_decode_ofpst_request(const struct ofp_header *oh,
         OFP_MKERR(OFPET_BAD_REQUEST, OFPBRC_BAD_STAT)
     };
 
-    const struct ofp_stats_request *osr;
+    const struct ofp_stats_msg *request = (const struct ofp_stats_msg *) oh;
     int error;
 
-    osr = (const struct ofp_stats_request *) oh;
     error = ofputil_lookup_openflow_message(&ofpst_request_category,
-                                            ntohs(osr->type),
+                                            ntohs(request->type),
                                             ntohs(oh->length), typep);
-    if (!error && osr->type == htons(OFPST_VENDOR)) {
+    if (!error && request->type == htons(OFPST_VENDOR)) {
         error = ofputil_decode_nxst_request(oh, typep);
     }
     return error;
@@ -556,35 +553,35 @@ static int
 ofputil_decode_ofpst_reply(const struct ofp_header *oh,
                            const struct ofputil_msg_type **typep)
 {
-    enum { OSR_SIZE = sizeof(struct ofp_stats_reply) };
+    enum { OSM_SIZE = sizeof(struct ofp_stats_msg) };
     static const struct ofputil_msg_type ofpst_replies[] = {
         { OFPUTIL_OFPST_DESC_REPLY,
           OFPST_DESC, "OFPST_DESC reply",
-          OSR_SIZE + sizeof(struct ofp_desc_stats), 0 },
+          OSM_SIZE + sizeof(struct ofp_desc_stats), 0 },
 
         { OFPUTIL_OFPST_FLOW_REPLY,
           OFPST_FLOW, "OFPST_FLOW reply",
-          OSR_SIZE, 1 },
+          OSM_SIZE, 1 },
 
         { OFPUTIL_OFPST_AGGREGATE_REPLY,
           OFPST_AGGREGATE, "OFPST_AGGREGATE reply",
-          OSR_SIZE + sizeof(struct ofp_aggregate_stats_reply), 0 },
+          OSM_SIZE + sizeof(struct ofp_aggregate_stats_reply), 0 },
 
         { OFPUTIL_OFPST_TABLE_REPLY,
           OFPST_TABLE, "OFPST_TABLE reply",
-          OSR_SIZE, sizeof(struct ofp_table_stats) },
+          OSM_SIZE, sizeof(struct ofp_table_stats) },
 
         { OFPUTIL_OFPST_PORT_REPLY,
           OFPST_PORT, "OFPST_PORT reply",
-          OSR_SIZE, sizeof(struct ofp_port_stats) },
+          OSM_SIZE, sizeof(struct ofp_port_stats) },
 
         { OFPUTIL_OFPST_QUEUE_REPLY,
           OFPST_QUEUE, "OFPST_QUEUE reply",
-          OSR_SIZE, sizeof(struct ofp_queue_stats) },
+          OSM_SIZE, sizeof(struct ofp_queue_stats) },
 
         { 0,
           OFPST_VENDOR, "OFPST_VENDOR reply",
-          OSR_SIZE + sizeof(uint32_t), 1 },
+          OSM_SIZE + sizeof(uint32_t), 1 },
     };
 
     static const struct ofputil_msg_category ofpst_reply_category = {
@@ -593,13 +590,13 @@ ofputil_decode_ofpst_reply(const struct ofp_header *oh,
         OFP_MKERR(OFPET_BAD_REQUEST, OFPBRC_BAD_STAT)
     };
 
-    const struct ofp_stats_reply *osr = (const struct ofp_stats_reply *) oh;
+    const struct ofp_stats_msg *reply = (const struct ofp_stats_msg *) oh;
     int error;
 
     error = ofputil_lookup_openflow_message(&ofpst_reply_category,
-                                           ntohs(osr->type),
+                                           ntohs(reply->type),
                                            ntohs(oh->length), typep);
-    if (!error && osr->type == htons(OFPST_VENDOR)) {
+    if (!error && reply->type == htons(OFPST_VENDOR)) {
         error = ofputil_decode_nxst_reply(oh, typep);
     }
     return error;
@@ -682,11 +679,11 @@ ofputil_decode_msg_type(const struct ofp_header *oh,
 
         { 0,
           OFPT_STATS_REQUEST, "OFPT_STATS_REQUEST",
-          sizeof(struct ofp_stats_request), 1 },
+          sizeof(struct ofp_stats_msg), 1 },
 
         { 0,
           OFPT_STATS_REPLY, "OFPT_STATS_REPLY",
-          sizeof(struct ofp_stats_reply), 1 },
+          sizeof(struct ofp_stats_msg), 1 },
 
         { OFPUTIL_OFPT_BARRIER_REQUEST,
           OFPT_BARRIER_REQUEST, "OFPT_BARRIER_REQUEST",
@@ -1152,7 +1149,7 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
     if (!msg->l2) {
         msg->l2 = msg->data;
         if (code == OFPUTIL_OFPST_FLOW_REPLY) {
-            ofpbuf_pull(msg, sizeof(struct ofp_stats_reply));
+            ofpbuf_pull(msg, sizeof(struct ofp_stats_msg));
         } else if (code == OFPUTIL_NXST_FLOW_REPLY) {
             ofpbuf_pull(msg, sizeof(struct nicira_stats_msg));
         } else {
@@ -1526,19 +1523,19 @@ update_openflow_length(struct ofpbuf *buffer)
     oh->length = htons(buffer->size);
 }
 
-/* Creates an ofp_stats_request with the given 'type' and 'body_len' bytes of
- * space allocated for the 'body' member.  Returns the first byte of the 'body'
+/* Creates an ofp_stats_msg with the given 'type' and 'body_len' bytes of space
+ * allocated for the 'body' member.  Returns the first byte of the 'body'
  * member. */
 void *
 ofputil_make_stats_request(size_t body_len, uint16_t type,
                            struct ofpbuf **bufferp)
 {
-    struct ofp_stats_request *osr;
-    osr = make_openflow((offsetof(struct ofp_stats_request, body)
-                        + body_len), OFPT_STATS_REQUEST, bufferp);
-    osr->type = htons(type);
-    osr->flags = htons(0);
-    return osr->body;
+    struct ofp_stats_msg *request;
+    request = make_openflow(offsetof(struct ofp_stats_msg, body)
+                            + body_len, OFPT_STATS_REQUEST, bufferp);
+    request->type = htons(type);
+    request->flags = htons(0);
+    return request + 1;
 }
 
 /* Creates a stats request message with Nicira as vendor and the given
@@ -1557,22 +1554,20 @@ ofputil_make_nxstats_request(size_t openflow_len, uint32_t subtype,
     return nsm;
 }
 
-/* Returns the first byte of the 'body' member of the ofp_stats_request or
- * ofp_stats_reply in 'oh'. */
+/* Returns the first byte of the body of the ofp_stats_msg in 'oh'. */
 const void *
 ofputil_stats_body(const struct ofp_header *oh)
 {
     assert(oh->type == OFPT_STATS_REQUEST || oh->type == OFPT_STATS_REPLY);
-    return ((const struct ofp_stats_request *) oh)->body;
+    return (const struct ofp_stats_msg *) oh + 1;
 }
 
-/* Returns the length of the 'body' member of the ofp_stats_request or
- * ofp_stats_reply in 'oh'. */
+/* Returns the length of the body of the ofp_stats_msg in 'oh'. */
 size_t
 ofputil_stats_body_len(const struct ofp_header *oh)
 {
     assert(oh->type == OFPT_STATS_REQUEST || oh->type == OFPT_STATS_REPLY);
-    return ntohs(oh->length) - sizeof(struct ofp_stats_request);
+    return ntohs(oh->length) - sizeof(struct ofp_stats_msg);
 }
 
 /* Returns the first byte of the body of the nicira_stats_msg in 'oh'. */

diff --git a/lib/vconn.h b/lib/vconn.h
index 357fc4ee413ce36eb6f91e1c23af069e85ed6758..3f8bc47b87876128e51b81e6128bb33b1c960cc3 100644 (file)
--- a/lib/vconn.h
+++ b/lib/vconn.h
@@ -29,7 +29,7 @@ struct ofpbuf;
 struct ofp_action_header;
 struct ofp_header;
 struct ofp_match;
-struct ofp_stats_reply;
+struct ofp_stats_msg;
 struct pvconn;
 struct vconn;
 

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 141aefbeeb6286d54d2106cac2dbd7747c223bc3..553680dc65f57818746d2e6fade6ee3d1010fcf6 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1585,22 +1585,21 @@ handle_port_mod(struct ofconn *ofconn, const struct ofp_header *oh)
 static struct ofpbuf *
 make_ofp_stats_reply(ovs_be32 xid, ovs_be16 type, size_t body_len)
 {
-    struct ofp_stats_reply *osr;
+    struct ofp_stats_msg *reply;
     struct ofpbuf *msg;
 
-    msg = ofpbuf_new(MIN(sizeof *osr + body_len, UINT16_MAX));
-    osr = put_openflow_xid(sizeof *osr, OFPT_STATS_REPLY, xid, msg);
-    osr->type = type;
-    osr->flags = htons(0);
+    msg = ofpbuf_new(MIN(sizeof *reply + body_len, UINT16_MAX));
+    reply = put_openflow_xid(sizeof *reply, OFPT_STATS_REPLY, xid, msg);
+    reply->type = type;
+    reply->flags = htons(0);
     return msg;
 }
 
 static struct ofpbuf *
 start_ofp_stats_reply(const struct ofp_header *request, size_t body_len)
 {
-    const struct ofp_stats_request *osr
-        = (const struct ofp_stats_request *) request;
-    return make_ofp_stats_reply(osr->header.xid, osr->type, body_len);
+    const struct ofp_stats_msg *osm = (const struct ofp_stats_msg *) request;
+    return make_ofp_stats_reply(osm->header.xid, osm->type, body_len);
 }
 
 static void *
@@ -1608,9 +1607,9 @@ append_ofp_stats_reply(size_t nbytes, struct ofconn *ofconn,
                        struct ofpbuf **msgp)
 {
     struct ofpbuf *msg = *msgp;
-    assert(nbytes <= UINT16_MAX - sizeof(struct ofp_stats_reply));
+    assert(nbytes <= UINT16_MAX - sizeof(struct ofp_stats_msg));
     if (nbytes + msg->size > UINT16_MAX) {
-        struct ofp_stats_reply *reply = msg->data;
+        struct ofp_stats_msg *reply = msg->data;
         reply->flags = htons(OFPSF_REPLY_MORE);
         *msgp = make_ofp_stats_reply(reply->header.xid, reply->type, nbytes);
         ofconn_send_reply(ofconn, msg);

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 5f9c830833c35be7552833c97e2b1f5c9f5d9c5c..7511769421093711fa353d4eb9999e0cc9871728 100644 (file)
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -262,8 +262,8 @@ open_vconn(const char *name, struct vconn **vconnp)
 static void *
 alloc_stats_request(size_t body_len, uint16_t type, struct ofpbuf **bufferp)
 {
-    struct ofp_stats_request *rq;
-    rq = make_openflow((offsetof(struct ofp_stats_request, body)
+    struct ofp_stats_msg *rq;
+    rq = make_openflow((offsetof(struct ofp_stats_msg, body)
                         + body_len), OFPT_STATS_REQUEST, bufferp);
     rq->type = htons(type);
     rq->flags = htons(0);
@@ -314,12 +314,12 @@ dump_stats_transaction(const char *vconn_name, struct ofpbuf *request)
         run(vconn_recv_block(vconn, &reply), "OpenFlow packet receive failed");
         recv_xid = ((struct ofp_header *) reply->data)->xid;
         if (send_xid == recv_xid) {
-            struct ofp_stats_reply *osr;
+            struct ofp_stats_msg *osm;
 
             ofp_print(stdout, reply->data, reply->size, verbosity + 1);
 
-            osr = ofpbuf_at(reply, 0, sizeof *osr);
-            done = !osr || !(ntohs(osr->flags) & OFPSF_REPLY_MORE);
+            osm = ofpbuf_at(reply, 0, sizeof *osm);
+            done = !osm || !(ntohs(osm->flags) & OFPSF_REPLY_MORE);
         } else {
             VLOG_DBG("received reply with xid %08"PRIx32" "
                      "!= expected %08"PRIx32, recv_xid, send_xid);
@@ -1089,7 +1089,7 @@ read_flows_from_switch(struct vconn *vconn, enum nx_flow_format flow_format,
         recv_xid = ((struct ofp_header *) reply->data)->xid;
         if (send_xid == recv_xid) {
             const struct ofputil_msg_type *type;
-            const struct ofp_stats_reply *osr;
+            const struct ofp_stats_msg *osm;
             enum ofputil_msg_code code;
 
             ofputil_decode_msg_type(reply->data, &type);
@@ -1101,8 +1101,8 @@ read_flows_from_switch(struct vconn *vconn, enum nx_flow_format flow_format,
                                         verbosity + 1));
             }
 
-            osr = reply->data;
-            if (!(osr->flags & htons(OFPSF_REPLY_MORE))) {
+            osm = reply->data;
+            if (!(osm->flags & htons(OFPSF_REPLY_MORE))) {
                 done = true;
             }