Signed-off-by: Jarno Rajahalme <jarno.rajaha...@nsn.com> --- lib/ofp-msgs.h | 24 +++++++++++++------- lib/ofp-print.c | 43 ++++++++++++++++++++++++++++-------- ofproto/connmgr.c | 25 +++++++++++++++++++++ ofproto/connmgr.h | 1 + ofproto/ofproto.c | 62 ++++++++++++++++++++++++++++++++++++++++++++-------- tests/ofp-print.at | 30 +++++++++++++++++++++++++ tests/ofproto.at | 47 +++++++++++++++++++++++++++++++++++---- 7 files changed, 202 insertions(+), 30 deletions(-)
diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h index 808f295..2db4fc9 100644 --- a/lib/ofp-msgs.h +++ b/lib/ofp-msgs.h @@ -198,6 +198,16 @@ enum ofpraw { /* OFPT 1.1+ (23): struct ofp11_queue_get_config_reply, struct ofp_packet_queue[]. */ OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY, + /* OFPT 1.2+ (24): struct ofp12_role_request. */ + OFPRAW_OFPT12_ROLE_REQUEST, + /* NXT 1.0+ (10): struct nx_role_request. */ + OFPRAW_NXT_ROLE_REQUEST, + + /* OFPT 1.2+ (25): struct ofp12_role_request. */ + OFPRAW_OFPT12_ROLE_REPLY, + /* NXT 1.0+ (11): struct nx_role_request. */ + OFPRAW_NXT_ROLE_REPLY, + /* OFPT 1.3+ (26): void. */ OFPRAW_OFPT13_GET_ASYNC_REQUEST, /* OFPT 1.3+ (27): struct ofp13_async_config. */ @@ -339,12 +349,6 @@ enum ofpraw { * Nicira extensions that correspond to standard OpenFlow messages are listed * alongside the standard versions above. */ - /* NXT 1.0+ (10): struct nx_role_request. */ - OFPRAW_NXT_ROLE_REQUEST, - - /* NXT 1.0+ (11): struct nx_role_request. */ - OFPRAW_NXT_ROLE_REPLY, - /* NXT 1.0 (12): struct nx_set_flow_format. */ OFPRAW_NXT_SET_FLOW_FORMAT, @@ -468,6 +472,12 @@ enum ofptype { OFPTYPE_QUEUE_GET_CONFIG_REQUEST, /* OFPRAW_OFPT11_QUEUE_GET_CONFIG_REQUEST. */ OFPTYPE_QUEUE_GET_CONFIG_REPLY, /* OFPRAW_OFPT11_QUEUE_GET_CONFIG_REPLY. */ + /* Controller role change request messages. */ + OFPTYPE_ROLE_REQUEST, /* OFPRAW_OFPT12_ROLE_REQUEST. + * OFPRAW_NXT_ROLE_REQUEST. */ + OFPTYPE_ROLE_REPLY, /* OFPRAW_OFPT12_ROLE_REPLY. + * OFPRAW_NXT_ROLE_REPLY. */ + /* Asynchronous message configuration. */ OFPTYPE_GET_ASYNC_REQUEST, /* OFPRAW_OFPT13_GET_ASYNC_REQUEST. */ OFPTYPE_GET_ASYNC_REPLY, /* OFPRAW_OFPT13_GET_ASYNC_REPLY. */ @@ -543,8 +553,6 @@ enum ofptype { * OFPRAW_OFPST11_PORT_DESC_REPLY. */ /* Nicira extensions. */ - OFPTYPE_ROLE_REQUEST, /* OFPRAW_NXT_ROLE_REQUEST. */ - OFPTYPE_ROLE_REPLY, /* OFPRAW_NXT_ROLE_REPLY. */ OFPTYPE_SET_FLOW_FORMAT, /* OFPRAW_NXT_SET_FLOW_FORMAT. */ OFPTYPE_FLOW_MOD_TABLE_ID, /* OFPRAW_NXT_FLOW_MOD_TABLE_ID. */ OFPTYPE_SET_PACKET_IN_FORMAT, /* OFPRAW_NXT_SET_PACKET_IN_FORMAT. */ diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 84c37cf..b56d9d1 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -1511,14 +1511,34 @@ ofp_print_echo(struct ds *string, const struct ofp_header *oh, int verbosity) } static void -ofp_print_nxt_role_message(struct ds *string, - const struct nx_role_request *nrr) +ofp_print_role_message(struct ds *string, enum ofpraw raw, + const struct ofp12_role_request *ocr) { - unsigned int role = ntohl(nrr->role); + unsigned int role = ntohl(ocr->role); + bool have_gen_id = false; ds_put_cstr(string, " role="); + + if (raw == OFPRAW_OFPT12_ROLE_REQUEST + || raw == OFPRAW_OFPT12_ROLE_REPLY ) { + + if (raw == OFPRAW_OFPT12_ROLE_REQUEST) { + /* OFPCR12_ROLE_NOCHANGE is a pseudo-role, valid for role requests. + * Response must include the current role instead. */ + if (role == OFPCR12_ROLE_NOCHANGE) { + ds_put_cstr(string, "nochange"); + return; + } + /* generation_id is meaningful for master/slave election requests + * only */ + have_gen_id = (role == OFPCR12_ROLE_MASTER || role == OFPCR12_ROLE_SLAVE); + } + /* Map to Nicira role */ + role -= 1; /* OFPCR12_ROLE_MASTER -> NX_ROLE_MASTER etc. */ + } + if (role == NX_ROLE_OTHER) { - ds_put_cstr(string, "other"); + ds_put_cstr(string, "equal"); /* OF 1.2 wording */ } else if (role == NX_ROLE_MASTER) { ds_put_cstr(string, "master"); } else if (role == NX_ROLE_SLAVE) { @@ -1526,6 +1546,11 @@ ofp_print_nxt_role_message(struct ds *string, } else { ds_put_format(string, "%u", role); } + + if (have_gen_id) { + ds_put_format(string, " generation_id=%"PRIu64, + ntohll(ocr->generation_id)); + } } static void @@ -1895,6 +1920,11 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw, case OFPTYPE_BARRIER_REPLY: break; + case OFPTYPE_ROLE_REQUEST: + case OFPTYPE_ROLE_REPLY: + ofp_print_role_message(string, raw, ofpmsg_body(oh)); + break; + case OFPTYPE_DESC_STATS_REQUEST: case OFPTYPE_PORT_DESC_STATS_REQUEST: ofp_print_stats_request(string, oh); @@ -1955,11 +1985,6 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw, ofp_print_ofpst_port_desc_reply(string, oh); break; - case OFPTYPE_ROLE_REQUEST: - case OFPTYPE_ROLE_REPLY: - ofp_print_nxt_role_message(string, ofpmsg_body(oh)); - break; - case OFPTYPE_FLOW_MOD_TABLE_ID: ofp_print_nxt_flow_mod_table_id(string, ofpmsg_body(oh)); break; diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index 77b7b39..2dc5249 100644 --- a/ofproto/connmgr.c +++ b/ofproto/connmgr.c @@ -154,6 +154,9 @@ struct connmgr { /* OpenFlow connections. */ struct hmap controllers; /* Controller "struct ofconn"s. */ struct list all_conns; /* Contains "struct ofconn"s. */ + uint64_t master_election_id; /* monotonically increasing sequence number + * for master election */ + bool master_election_id_defined; /* OpenFlow listeners. */ struct hmap services; /* Contains "struct ofservice"s. */ @@ -193,6 +196,8 @@ connmgr_create(struct ofproto *ofproto, hmap_init(&mgr->controllers); list_init(&mgr->all_conns); + mgr->master_election_id = 0; + mgr->master_election_id_defined = false; hmap_init(&mgr->services); mgr->snoops = NULL; @@ -817,6 +822,26 @@ ofconn_get_type(const struct ofconn *ofconn) return ofconn->type; } +/* Sets the master election id. + * + * Returns true if successful, false if the id is stale + */ +bool +ofconn_set_master_election_id(struct ofconn *ofconn, uint64_t id) +{ + if (ofconn->connmgr->master_election_id_defined + && + /* Unsigned difference interpreted as a two's complement signed + * value */ + (int64_t)(id - ofconn->connmgr->master_election_id) < 0) { + return false; + } + ofconn->connmgr->master_election_id = id; + ofconn->connmgr->master_election_id_defined = true; + + return true; +} + /* Returns the role configured for 'ofconn'. * * The default role, if no other role has been set, is NX_ROLE_OTHER. */ diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h index 4122bc1..a2f7d5a 100644 --- a/ofproto/connmgr.h +++ b/ofproto/connmgr.h @@ -96,6 +96,7 @@ 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_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); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 181d70b..581b915 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -3615,12 +3615,42 @@ handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn, static enum ofperr handle_role_request(struct ofconn *ofconn, const struct ofp_header *oh) { - const struct nx_role_request *nrr = ofpmsg_body(oh); - struct nx_role_request *reply; + const struct ofp12_role_request *ocr = ofpmsg_body(oh); + struct ofp12_role_request *reply; struct ofpbuf *buf; - uint32_t role; + uint32_t role = ntohl(ocr->role); + size_t reply_size; + + struct ofpbuf b; + enum ofpraw raw; + + ofpbuf_use_const(&b, oh, ntohs(oh->length)); + raw = ofpraw_pull_assert(&b); + if (raw == OFPRAW_OFPT12_ROLE_REQUEST) { + + if (role == OFPCR12_ROLE_MASTER || role == OFPCR12_ROLE_SLAVE) { + if (!ofconn_set_master_election_id(ofconn, + ntohll(ocr->generation_id))) { + return OFPERR_OFPRRFC_STALE; + } + } + + reply_size = sizeof (struct ofp12_role_request); + raw = OFPRAW_OFPT12_ROLE_REPLY; + + if (role == OFPCR12_ROLE_NOCHANGE) { + role = ofconn_get_role(ofconn); /* NX_ROLE_* */ + goto reply; + } + + /* Map to Nicira enum nx_role */ + role -= 1; /* OFPCR12_ROLE_MASTER -> NX_ROLE_MASTER etc. */ + } + else { + reply_size = sizeof (struct nx_role_request); + raw = OFPRAW_NXT_ROLE_REPLY; + } - role = ntohl(nrr->role); if (role != NX_ROLE_OTHER && role != NX_ROLE_MASTER && role != NX_ROLE_SLAVE) { return OFPERR_OFPRRFC_BAD_ROLE; @@ -3633,8 +3663,22 @@ handle_role_request(struct ofconn *ofconn, const struct ofp_header *oh) ofconn_set_role(ofconn, role); - buf = ofpraw_alloc_reply(OFPRAW_NXT_ROLE_REPLY, oh, 0); - reply = ofpbuf_put_zeros(buf, sizeof *reply); +reply: + buf = ofpraw_alloc_reply(raw, oh, 0); + reply = ofpbuf_put_zeros(buf, reply_size); + + /* Map to OpenFlow enum ofp12_controller_role */ + if (raw == OFPRAW_OFPT12_ROLE_REPLY) { + 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 + * the current value. However, the current value is undefined + * initially, and there is no way to insert an undefined value in the + * message. Therefore we leave the generation_id zeroed on reply + * messages. + */ + } reply->role = htonl(role); ofconn_send_reply(ofconn, buf); @@ -4046,14 +4090,14 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg) case OFPTYPE_BARRIER_REQUEST: return handle_barrier_request(ofconn, oh); + case OFPTYPE_ROLE_REQUEST: + return handle_role_request(ofconn, oh); + /* OpenFlow replies. */ case OFPTYPE_ECHO_REPLY: return 0; /* Nicira extension requests. */ - case OFPTYPE_ROLE_REQUEST: - return handle_role_request(ofconn, oh); - case OFPTYPE_FLOW_MOD_TABLE_ID: return handle_nxt_flow_mod_table_id(ofconn, oh); diff --git a/tests/ofp-print.at b/tests/ofp-print.at index 8491dd0..a8adda2 100644 --- a/tests/ofp-print.at +++ b/tests/ofp-print.at @@ -1515,6 +1515,26 @@ OFPT_SET_ASYNC (OF1.3) (xid=0x0): ]) AT_CLEANUP +AT_SETUP([OFPT_ROLE_REQUEST - OF1.2]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +03 18 00 18 00 00 00 02 00 00 00 02 00 00 00 00 \ +00 00 00 00 00 00 00 03 \ +"], [0], [dnl +OFPT_ROLE_REQUEST (OF1.2) (xid=0x2): role=master generation_id=3 +]) +AT_CLEANUP + +AT_SETUP([OFPT_ROLE_REQUEST - nochange - OF1.2]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +03 18 00 18 00 00 00 02 00 00 00 00 00 00 00 00 \ +00 00 00 00 00 00 00 00 \ +"], [0], [dnl +OFPT_ROLE_REQUEST (OF1.2) (xid=0x2): role=nochange +]) +AT_CLEANUP + AT_SETUP([NXT_ROLE_REQUEST]) AT_KEYWORDS([ofp-print]) AT_CHECK([ovs-ofctl ofp-print "\ @@ -1525,6 +1545,16 @@ NXT_ROLE_REQUEST (xid=0x2): role=master ]) AT_CLEANUP +AT_SETUP([OFPT_ROLE_REPLY - OF1.2]) +AT_KEYWORDS([ofp-print]) +AT_CHECK([ovs-ofctl ofp-print "\ +03 19 00 18 00 00 00 02 00 00 00 03 00 00 00 00 \ +00 00 00 00 00 00 00 00 \ +"], [0], [dnl +OFPT_ROLE_REPLY (OF1.2) (xid=0x2): role=slave +]) +AT_CLEANUP + AT_SETUP([NXT_ROLE_REPLY]) AT_KEYWORDS([ofp-print]) AT_CHECK([ovs-ofctl ofp-print "\ diff --git a/tests/ofproto.at b/tests/ofproto.at index 107e366..d3bf786 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -1191,8 +1191,8 @@ check_async 2 OFPR_ACTION OFPPR_ADD OFPPR_DELETE OFPRR_DELETE ovs-appctl -t ovs-ofctl ofctl/send 0309000c0123456700040080 check_async 3 OFPR_ACTION OFPR_INVALID_TTL OFPPR_ADD OFPPR_DELETE OFPRR_DELETE -# Become slave, which should disable everything except port status. -ovs-appctl -t ovs-ofctl ofctl/send 0304001400000002000023200000000a00000002 +# Become slave (OF 1.2), which should disable everything except port status. +ovs-appctl -t ovs-ofctl ofctl/send 031800180000000200000003000000000000000000000001 check_async 4 OFPPR_ADD OFPPR_DELETE # Use NXT_SET_ASYNC_CONFIG to enable a patchwork of asynchronous messages. @@ -1206,14 +1206,53 @@ check_async 6 OFPR_NO_MATCH OFPPR_DELETE OFPRR_DELETE # Restore controller ID 0. ovs-appctl -t ovs-ofctl ofctl/send 030400180000000300002320000000140000000000000000 -# Become master. -ovs-appctl -t ovs-ofctl ofctl/send 0304001400000002000023200000000a00000001 +# Become master (OF 1.2). +ovs-appctl -t ovs-ofctl ofctl/send 031800180000000400000002000000000000000000000002 check_async 7 OFPR_ACTION OFPPR_ADD ovs-appctl -t ovs-ofctl exit OVS_VSWITCHD_STOP AT_CLEANUP +dnl This test checks that the role request/response messaging works +dnl and that generation_id is handled properly. +AT_SETUP([ofproto - controller role (OpenFlow 1.2)]) +OVS_VSWITCHD_START +AT_CHECK([ovs-ofctl -O OpenFlow12 monitor br0 --detach --no-chdir --pidfile]) + +ovs-appctl -t ovs-ofctl ofctl/barrier +ovs-appctl -t ovs-ofctl ofctl/set-output-file monitor.log +: > expout + +# find out current role +ovs-appctl -t ovs-ofctl ofctl/send 031800180000000200000000000000000000000000000000 +echo >>expout "send: OFPT_ROLE_REQUEST (OF1.2) (xid=0x2): role=nochange" +echo >>expout "OFPT_ROLE_REPLY (OF1.2) (xid=0x2): role=equal" + +# Become slave (generation_id is initially undefined, so 2^63+2 should not be stale) +ovs-appctl -t ovs-ofctl ofctl/send 031800180000000300000003000000008000000000000002 +echo >>expout "send: OFPT_ROLE_REQUEST (OF1.2) (xid=0x3): role=slave generation_id=9223372036854775810" +echo >>expout "OFPT_ROLE_REPLY (OF1.2) (xid=0x3): role=slave" + +# Try to become the master using a stale generation ID +ovs-appctl -t ovs-ofctl ofctl/send 031800180000000400000002000000000000000000000002 +echo >>expout "send: OFPT_ROLE_REQUEST (OF1.2) (xid=0x4): role=master generation_id=2" +echo >>expout "OFPT_ERROR (OF1.2) (xid=0x4): OFPRRFC_STALE" +echo >>expout "OFPT_ROLE_REQUEST (OF1.2) (xid=0x4): role=master generation_id=2" + +# Become master using a valid generation ID +ovs-appctl -t ovs-ofctl ofctl/send 031800180000000500000002000000000000000000000001 +echo >>expout "send: OFPT_ROLE_REQUEST (OF1.2) (xid=0x5): role=master generation_id=1" +echo >>expout "OFPT_ROLE_REPLY (OF1.2) (xid=0x5): role=master" +ovs-appctl -t ovs-ofctl ofctl/barrier +echo >>expout "OFPT_BARRIER_REPLY (OF1.2) (xid=0x3):" + +AT_CHECK([cat monitor.log], [0], [expout]) + +ovs-appctl -t ovs-ofctl exit +OVS_VSWITCHD_STOP +AT_CLEANUP + dnl This test checks that OFPT_PACKET_OUT accepts both OFPP_NONE (as dnl specified by OpenFlow 1.0) and OFPP_CONTROLLER (used by some dnl controllers despite the spec) as meaning a packet that was generated -- 1.7.10.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev