[ovs-dev] [ext-272 3/4] ofp-util: Simplify struct ofputil_role_request.
Ben Pfaff
blp at nicira.com
Tue Feb 12 08:04:00 UTC 2013
It makes more sense to use enum ofp12_controller_role here than
to use enum nx_role, because the former is a superset of the latter and
we can then get rid of a bool member too.
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
lib/ofp-print.c | 15 ++++----
lib/ofp-util.c | 101 ++++++++++++++++++++++++++++-------------------------
lib/ofp-util.h | 7 ++--
ofproto/connmgr.c | 29 +++++++--------
ofproto/connmgr.h | 7 ++--
ofproto/ofproto.c | 42 ++++++++++------------
ofproto/ofproto.h | 4 +--
vswitchd/bridge.c | 12 ++++---
8 files changed, 111 insertions(+), 106 deletions(-)
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 1a9ca0e..f7872cb 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -1550,19 +1550,18 @@ ofp_print_role_message(struct ds *string, const struct ofp_header *oh)
}
ds_put_cstr(string, " role=");
- if (rr.request_current_role_only) {
- ds_put_cstr(string, "nochange");
- return;
- }
switch (rr.role) {
- case NX_ROLE_OTHER:
+ case OFPCR12_ROLE_NOCHANGE:
+ ds_put_cstr(string, "nochange");
+ break;
+ case OFPCR12_ROLE_EQUAL:
ds_put_cstr(string, "equal"); /* OF 1.2 wording */
break;
- case NX_ROLE_MASTER:
+ case OFPCR12_ROLE_MASTER:
ds_put_cstr(string, "master");
break;
- case NX_ROLE_SLAVE:
+ case OFPCR12_ROLE_SLAVE:
ds_put_cstr(string, "slave");
break;
default:
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index cd51ef7..ef8de6b 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3382,43 +3382,53 @@ enum ofperr
ofputil_decode_role_message(const struct ofp_header *oh,
struct ofputil_role_request *rr)
{
- const struct ofp12_role_request *orr = ofpmsg_body(oh);
- uint32_t role = ntohl(orr->role);
struct ofpbuf b;
enum ofpraw raw;
- memset(rr, 0, sizeof *rr);
-
ofpbuf_use_const(&b, oh, ntohs(oh->length));
raw = ofpraw_pull_assert(&b);
- if (raw == OFPRAW_OFPT12_ROLE_REQUEST
- || raw == OFPRAW_OFPT12_ROLE_REPLY) {
+ if (raw == OFPRAW_OFPT12_ROLE_REQUEST ||
+ raw == OFPRAW_OFPT12_ROLE_REPLY) {
+ const struct ofp12_role_request *orr = b.l3;
- if (raw == OFPRAW_OFPT12_ROLE_REQUEST) {
- if (role == OFPCR12_ROLE_NOCHANGE) {
- rr->request_current_role_only = true;
- return 0;
- }
- if (role == OFPCR12_ROLE_MASTER || role == OFPCR12_ROLE_SLAVE) {
- rr->generation_id = ntohll(orr->generation_id);
- rr->have_generation_id = true;
- }
+ if (orr->role != htonl(OFPCR12_ROLE_NOCHANGE) &&
+ orr->role != htonl(OFPCR12_ROLE_EQUAL) &&
+ orr->role != htonl(OFPCR12_ROLE_MASTER) &&
+ orr->role != htonl(OFPCR12_ROLE_SLAVE)) {
+ return OFPERR_OFPRRFC_BAD_ROLE;
}
- /* Map to enum nx_role */
- role -= 1; /* OFPCR12_ROLE_MASTER -> NX_ROLE_MASTER etc. */
- } else if (raw != OFPRAW_NXT_ROLE_REQUEST
- && raw != OFPRAW_NXT_ROLE_REPLY) {
- return OFPERR_OFPBRC_BAD_TYPE;
- }
+ rr->role = ntohl(orr->role);
+ if (raw == OFPRAW_OFPT12_ROLE_REPLY
+ || orr->role == htonl(OFPCR12_ROLE_NOCHANGE)) {
+ rr->have_generation_id = false;
+ rr->generation_id = 0;
+ } else {
+ rr->have_generation_id = true;
+ rr->generation_id = ntohll(orr->generation_id);
+ }
+ } else if (raw == OFPRAW_NXT_ROLE_REQUEST ||
+ raw == OFPRAW_NXT_ROLE_REPLY) {
+ const struct nx_role_request *nrr = b.l3;
+
+ BUILD_ASSERT(NX_ROLE_OTHER + 1 == OFPCR12_ROLE_EQUAL);
+ BUILD_ASSERT(NX_ROLE_MASTER + 1 == OFPCR12_ROLE_MASTER);
+ BUILD_ASSERT(NX_ROLE_SLAVE + 1 == OFPCR12_ROLE_SLAVE);
+
+ if (nrr->role != htonl(NX_ROLE_OTHER) &&
+ nrr->role != htonl(NX_ROLE_MASTER) &&
+ nrr->role != htonl(NX_ROLE_SLAVE)) {
+ return OFPERR_OFPRRFC_BAD_ROLE;
+ }
- if (role != NX_ROLE_OTHER && role != NX_ROLE_MASTER
- && role != NX_ROLE_SLAVE) {
- return OFPERR_OFPRRFC_BAD_ROLE;
+ rr->role = ntohl(nrr->role) + 1;
+ rr->have_generation_id = false;
+ rr->generation_id = 0;
+ } else {
+ NOT_REACHED();
}
- rr->role = role;
return 0;
}
@@ -3426,34 +3436,19 @@ ofputil_decode_role_message(const struct ofp_header *oh,
* buffer owned by the caller. */
struct ofpbuf *
ofputil_encode_role_reply(const struct ofp_header *request,
- enum nx_role role)
+ const struct ofputil_role_request *rr)
{
- struct ofp12_role_request *reply;
struct ofpbuf *buf;
- size_t reply_size;
-
- struct ofpbuf b;
enum ofpraw raw;
- ofpbuf_use_const(&b, request, ntohs(request->length));
- raw = ofpraw_pull_assert(&b);
+ raw = ofpraw_decode_assert(request);
if (raw == OFPRAW_OFPT12_ROLE_REQUEST) {
- reply_size = sizeof (struct ofp12_role_request);
- raw = OFPRAW_OFPT12_ROLE_REPLY;
- }
- else if (raw == OFPRAW_NXT_ROLE_REQUEST) {
- reply_size = sizeof (struct nx_role_request);
- raw = OFPRAW_NXT_ROLE_REPLY;
- } else {
- NOT_REACHED();
- }
+ struct ofp12_role_request *orr;
- buf = ofpraw_alloc_reply(raw, request, 0);
- reply = ofpbuf_put_zeros(buf, reply_size);
+ buf = ofpraw_alloc_reply(OFPRAW_OFPT12_ROLE_REPLY, request, 0);
+ orr = ofpbuf_put_zeros(buf, sizeof *orr);
+ orr->role = htonl(rr->role);
- if (raw == OFPRAW_OFPT12_ROLE_REPLY) {
- /* Map to OpenFlow enum ofp12_controller_role */
- role += 1; /* NX_ROLE_MASTER -> OFPCR12_ROLE_MASTER etc. */
/*
* OpenFlow specification does not specify use of generation_id field
* on reply messages. Intuitively, it would seem a good idea to return
@@ -3465,8 +3460,20 @@ ofputil_encode_role_reply(const struct ofp_header *request,
* A request for clarification has been filed with the Open Networking
* Foundation as EXT-272.
*/
+ orr->generation_id = htonll(0);
+ } else if (raw == OFPRAW_NXT_ROLE_REQUEST) {
+ struct nx_role_request *nrr;
+
+ BUILD_ASSERT(NX_ROLE_OTHER == OFPCR12_ROLE_EQUAL - 1);
+ BUILD_ASSERT(NX_ROLE_MASTER == OFPCR12_ROLE_MASTER - 1);
+ BUILD_ASSERT(NX_ROLE_SLAVE == OFPCR12_ROLE_SLAVE - 1);
+
+ buf = ofpraw_alloc_reply(OFPRAW_NXT_ROLE_REPLY, request, 0);
+ nrr = ofpbuf_put_zeros(buf, sizeof *nrr);
+ nrr->role = htonl(rr->role - 1);
+ } else {
+ NOT_REACHED();
}
- reply->role = htonl(role);
return buf;
}
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 8a14f41..955886d 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -508,16 +508,15 @@ struct ofpbuf *ofputil_encode_port_mod(const struct ofputil_port_mod *,
/* Abstract ofp_role_request and reply. */
struct ofputil_role_request {
- bool request_current_role_only; /* no role change */
+ enum ofp12_controller_role role;
bool have_generation_id;
- enum nx_role role;
uint64_t generation_id;
};
enum ofperr ofputil_decode_role_message(const struct ofp_header *,
struct ofputil_role_request *);
struct ofpbuf *ofputil_encode_role_reply(const struct ofp_header *,
- enum nx_role current_role);
+ const struct ofputil_role_request *);
/* Abstract table stats.
*
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index b94c337..29321d5 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -60,7 +60,7 @@ struct ofconn {
/* State that should be cleared from one connection to the next. */
/* OpenFlow state. */
- enum nx_role role; /* Role. */
+ enum ofp12_controller_role role; /* Role. */
enum ofputil_protocol protocol; /* Current protocol variant. */
enum nx_packet_in_format packet_in_format; /* OFPT_PACKET_IN format. */
@@ -777,12 +777,13 @@ static int
snoop_preference(const struct ofconn *ofconn)
{
switch (ofconn->role) {
- case NX_ROLE_MASTER:
+ case OFPCR12_ROLE_MASTER:
return 3;
- case NX_ROLE_OTHER:
+ case OFPCR12_ROLE_EQUAL:
return 2;
- case NX_ROLE_SLAVE:
+ case OFPCR12_ROLE_SLAVE:
return 1;
+ case OFPCR12_ROLE_NOCHANGE:
default:
/* Shouldn't happen. */
return 0;
@@ -844,24 +845,24 @@ ofconn_set_master_election_id(struct ofconn *ofconn, uint64_t id)
/* Returns the role configured for 'ofconn'.
*
- * The default role, if no other role has been set, is NX_ROLE_OTHER. */
-enum nx_role
+ * The default role, if no other role has been set, is OFPCR12_ROLE_EQUAL. */
+enum ofp12_controller_role
ofconn_get_role(const struct ofconn *ofconn)
{
return ofconn->role;
}
-/* Changes 'ofconn''s role to 'role'. If 'role' is NX_ROLE_MASTER then any
- * existing master is demoted to a slave. */
+/* Changes 'ofconn''s role to 'role'. If 'role' is OFPCR12_ROLE_MASTER then
+ * any existing master is demoted to a slave. */
void
-ofconn_set_role(struct ofconn *ofconn, enum nx_role role)
+ofconn_set_role(struct ofconn *ofconn, enum ofp12_controller_role role)
{
- if (role == NX_ROLE_MASTER) {
+ if (role == OFPCR12_ROLE_MASTER) {
struct ofconn *other;
HMAP_FOR_EACH (other, hmap_node, &ofconn->connmgr->controllers) {
- if (other->role == NX_ROLE_MASTER) {
- other->role = NX_ROLE_SLAVE;
+ if (other->role == OFPCR12_ROLE_MASTER) {
+ other->role = OFPCR12_ROLE_SLAVE;
}
}
}
@@ -1092,7 +1093,7 @@ ofconn_flush(struct ofconn *ofconn)
struct ofmonitor *monitor, *next_monitor;
int i;
- ofconn->role = NX_ROLE_OTHER;
+ ofconn->role = OFPCR12_ROLE_EQUAL;
ofconn_set_protocol(ofconn, OFPUTIL_P_NONE);
ofconn->packet_in_format = NXPIF_OPENFLOW10;
@@ -1307,7 +1308,7 @@ ofconn_receives_async_msg(const struct ofconn *ofconn,
return false;
}
- async_config = (ofconn->role == NX_ROLE_SLAVE
+ async_config = (ofconn->role == OFPCR12_ROLE_SLAVE
? ofconn->slave_async_config
: ofconn->master_async_config);
if (!(async_config[type] & (1u << reason))) {
diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index 6ce413e..48b8119 100644
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -96,9 +96,10 @@ void connmgr_get_snoops(const struct connmgr *, struct sset *snoops);
/* Individual connections to OpenFlow controllers. */
enum ofconn_type ofconn_get_type(const struct ofconn *);
+bool ofconn_get_master_election_id(const struct ofconn *, uint64_t *idp);
bool ofconn_set_master_election_id(struct ofconn *, uint64_t);
-enum nx_role ofconn_get_role(const struct ofconn *);
-void ofconn_set_role(struct ofconn *, enum nx_role);
+enum ofp12_controller_role ofconn_get_role(const struct ofconn *);
+void ofconn_set_role(struct ofconn *, enum ofp12_controller_role);
enum ofputil_protocol ofconn_get_protocol(const struct ofconn *);
void ofconn_set_protocol(struct ofconn *, enum ofputil_protocol);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index c0d94f7..856d6fa 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2238,7 +2238,7 @@ handle_set_config(struct ofconn *ofconn, const struct ofp_header *oh)
uint16_t flags = ntohs(osc->flags);
if (ofconn_get_type(ofconn) != OFCONN_PRIMARY
- || ofconn_get_role(ofconn) != NX_ROLE_SLAVE) {
+ || ofconn_get_role(ofconn) != OFPCR12_ROLE_SLAVE) {
enum ofp_config_flags cur = ofproto->frag_handling;
enum ofp_config_flags next = flags & OFPC_FRAG_MASK;
@@ -2272,7 +2272,7 @@ static enum ofperr
reject_slave_controller(struct ofconn *ofconn)
{
if (ofconn_get_type(ofconn) == OFCONN_PRIMARY
- && ofconn_get_role(ofconn) == NX_ROLE_SLAVE) {
+ && ofconn_get_role(ofconn) == OFPCR12_ROLE_SLAVE) {
return OFPERR_OFPBRC_EPERM;
} else {
return 0;
@@ -3576,38 +3576,34 @@ handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn,
static enum ofperr
handle_role_request(struct ofconn *ofconn, const struct ofp_header *oh)
{
- struct ofputil_role_request rr;
+ struct ofputil_role_request request;
+ struct ofputil_role_request reply;
struct ofpbuf *buf;
- uint32_t role;
enum ofperr error;
- error = ofputil_decode_role_message(oh, &rr);
+ error = ofputil_decode_role_message(oh, &request);
if (error) {
return error;
}
- if (rr.request_current_role_only) {
- role = ofconn_get_role(ofconn); /* NX_ROLE_* */
- goto reply;
- }
-
- role = rr.role;
-
- if (ofconn_get_role(ofconn) != role
- && ofconn_has_pending_opgroups(ofconn)) {
- return OFPROTO_POSTPONE;
- }
+ if (request.role != OFPCR12_ROLE_NOCHANGE) {
+ if (ofconn_get_role(ofconn) != request.role
+ && ofconn_has_pending_opgroups(ofconn)) {
+ return OFPROTO_POSTPONE;
+ }
- if (rr.have_generation_id) {
- if (!ofconn_set_master_election_id(ofconn, rr.generation_id)) {
- return OFPERR_OFPRRFC_STALE;
+ if (request.have_generation_id
+ && !ofconn_set_master_election_id(ofconn, request.generation_id)) {
+ return OFPERR_OFPRRFC_STALE;
}
- }
- ofconn_set_role(ofconn, role);
+ ofconn_set_role(ofconn, request.role);
+ }
-reply:
- buf = ofputil_encode_role_reply(oh, role);
+ reply.role = ofconn_get_role(ofconn);
+ reply.have_generation_id = false;
+ reply.generation_id = 0;
+ buf = ofputil_encode_role_reply(oh, &reply);
ofconn_send_reply(ofconn, buf);
return 0;
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 9e55f65..3a66d1b 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -44,7 +44,7 @@ struct netdev_stats;
struct ofproto_controller_info {
bool is_connected;
- enum nx_role role;
+ enum ofp12_controller_role role;
struct {
const char *keys[4];
const char *values[4];
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index fdd7c64..ab0ecd6 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1961,15 +1961,16 @@ run_system_stats(void)
}
static inline const char *
-nx_role_to_str(enum nx_role role)
+ofp12_controller_role_to_str(enum ofp12_controller_role role)
{
switch (role) {
- case NX_ROLE_OTHER:
+ case OFPCR12_ROLE_EQUAL:
return "other";
- case NX_ROLE_MASTER:
+ case OFPCR12_ROLE_MASTER:
return "master";
- case NX_ROLE_SLAVE:
+ case OFPCR12_ROLE_SLAVE:
return "slave";
+ case OFPCR12_ROLE_NOCHANGE:
default:
return "*** INVALID ROLE ***";
}
@@ -2005,7 +2006,8 @@ refresh_controller_status(void)
}
ovsrec_controller_set_is_connected(cfg, cinfo->is_connected);
- ovsrec_controller_set_role(cfg, nx_role_to_str(cinfo->role));
+ ovsrec_controller_set_role(cfg, ofp12_controller_role_to_str(
+ cinfo->role));
ovsrec_controller_set_status(cfg, &smap);
smap_destroy(&smap);
} else {
--
1.7.10.4
More information about the dev
mailing list