[ovs-dev] [pre-async 2/8] openflow: Merge ofp_stats_request and ofp_stats_reply.
Ben Pfaff
blp at nicira.com
Tue May 24 23:25:14 UTC 2011
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.
---
include/openflow/openflow.h | 30 +++++++-----------
lib/ofp-print.c | 5 +--
lib/ofp-util.c | 72 +++++++++++++++++++-----------------------
lib/vconn.h | 2 +-
ofproto/ofproto.c | 9 ++---
utilities/ovs-ofctl.c | 8 ++--
6 files changed, 56 insertions(+), 70 deletions(-)
diff --git a/include/openflow/openflow.h b/include/openflow/openflow.h
index f539543..9213be6 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 89656cc..5a430cf 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1291,8 +1291,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"***",
@@ -1303,7 +1302,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 e723fd2..16b462b 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -431,19 +431,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);
}
@@ -523,35 +521,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 = {
@@ -560,10 +558,9 @@ 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 *osr = (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(oh->length), typep);
@@ -577,35 +574,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 = {
@@ -614,7 +611,7 @@ 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 *osr = (const struct ofp_stats_msg *) oh;
int error;
error = ofputil_lookup_openflow_message(&ofpst_reply_category,
@@ -703,11 +700,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",
@@ -1191,7 +1188,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 {
@@ -1565,19 +1562,18 @@ 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);
+ struct ofp_stats_msg *osr;
+ osr = make_openflow(sizeof *osr + body_len, OFPT_STATS_REQUEST, bufferp);
osr->type = htons(type);
osr->flags = htons(0);
- return osr->body;
+ return osr + 1;
}
/* Creates a stats request message with Nicira as vendor and the given
@@ -1596,22 +1592,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 357fc4e..3f8bc47 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 263c318..4ffa241 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1576,7 +1576,7 @@ 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 *osr;
struct ofpbuf *msg;
msg = ofpbuf_new(MIN(sizeof *osr + body_len, UINT16_MAX));
@@ -1589,8 +1589,7 @@ make_ofp_stats_reply(ovs_be32 xid, ovs_be16 type, size_t body_len)
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;
+ const struct ofp_stats_msg *osr = (const struct ofp_stats_msg *) request;
return make_ofp_stats_reply(osr->header.xid, osr->type, body_len);
}
@@ -1599,9 +1598,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 5f9c830..52e89d0 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,7 +314,7 @@ 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 *osr;
ofp_print(stdout, reply->data, reply->size, verbosity + 1);
@@ -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 *osr;
enum ofputil_msg_code code;
ofputil_decode_msg_type(reply->data, &type);
--
1.7.4.4
More information about the dev
mailing list