From: Ben Pfaff Date: Tue, 12 Feb 2013 07:55:31 +0000 (-0800) Subject: ofp-util: Simplify struct ofputil_role_request. X-Git-Tag: sliver-openvswitch-1.10.90-1~11^2~48 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=f4f1ea7eaca92e2ca44c9624b3bb7d6426b2ddea;p=sliver-openvswitch.git ofp-util: Simplify struct ofputil_role_request. 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 --- diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 1a9ca0e17..f7872cb34 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 cd51ef7dd..ef8de6b4e 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 8a14f4152..955886de3 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 b94c33792..29321d50f 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 6ce413e8d..48b81193f 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 c0d94f72f..856d6faf6 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 9e55f650f..3a66d1bf1 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 fdd7c6406..ab0ecd6df 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 {