From: Ben Pfaff Date: Thu, 26 May 2011 16:30:25 +0000 (-0700) Subject: openflow: Merge ofp_stats_request and ofp_stats_reply. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=28c8bad14f17fea722632e6483e61c80a1b9a8e0;p=openvswitch 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 f5395433..9213be64 100644 --- 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 0c3bea1d..90273edf 100644 --- 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 dfbbf1b6..2c6afbe0 100644 --- 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 357fc4ee..3f8bc47b 100644 --- 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 141aefbe..553680dc 100644 --- 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 5f9c8308..75117694 100644 --- 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; }