From 6ea4776bc043089fa307943705002a60ff4983b4 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Fri, 28 Dec 2012 18:28:49 +0200 Subject: [PATCH] Add Openflow 1.2 role request/reply processing, update OF 1.2 tests. Add struct ofputil_role_request and encode/decode functions for role request/reply. Signed-off-by: Jarno Rajahalme [blp@nicira.com made ofp-print print any error, renamed a function, added a comment] Signed-off-by: Ben Pfaff --- lib/ofp-msgs.h | 24 ++++++++---- lib/ofp-print.c | 47 +++++++++++++++------- lib/ofp-util.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++ lib/ofp-util.h | 13 ++++++ ofproto/connmgr.c | 25 ++++++++++++ ofproto/connmgr.h | 1 + ofproto/ofproto.c | 35 +++++++++++------ tests/ofp-print.at | 30 ++++++++++++++ tests/ofproto.at | 47 ++++++++++++++++++++-- 9 files changed, 282 insertions(+), 38 deletions(-) diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h index 808f2957c..2db4fc9d0 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 f45ffa4df..8ad406d2d 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -1511,20 +1511,39 @@ 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, const struct ofp_header *oh) { - unsigned int role = ntohl(nrr->role); + struct ofputil_role_request rr; + enum ofperr error; + + error = ofputil_decode_role_message(oh, &rr); + if (error) { + ofp_print_error(string, error); + return; + } ds_put_cstr(string, " role="); - if (role == NX_ROLE_OTHER) { - ds_put_cstr(string, "other"); - } else if (role == NX_ROLE_MASTER) { + if (rr.request_current_role_only) { + ds_put_cstr(string, "nochange"); + return; + } + + switch (rr.role) { + case NX_ROLE_OTHER: + ds_put_cstr(string, "equal"); /* OF 1.2 wording */ + break; + case NX_ROLE_MASTER: ds_put_cstr(string, "master"); - } else if (role == NX_ROLE_SLAVE) { + break; + case NX_ROLE_SLAVE: ds_put_cstr(string, "slave"); - } else { - ds_put_format(string, "%u", role); + break; + default: + NOT_REACHED(); + } + + if (rr.have_generation_id) { + ds_put_format(string, " generation_id=%"PRIu64, rr.generation_id); } } @@ -1895,6 +1914,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, oh); + break; + case OFPTYPE_DESC_STATS_REQUEST: case OFPTYPE_PORT_DESC_STATS_REQUEST: ofp_print_stats_request(string, oh); @@ -1955,11 +1979,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/lib/ofp-util.c b/lib/ofp-util.c index 075557e26..db94c4ff3 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -3355,6 +3355,104 @@ ofputil_encode_port_mod(const struct ofputil_port_mod *pm, return b; } +/* ofputil_role_request */ + +/* Decodes the OpenFlow "role request" or "role reply" message in '*oh' into + * an abstract form in '*rr'. Returns 0 if successful, otherwise an + * OFPERR_* value. */ +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) { + 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; + } + } + + /* 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; + } + + if (role != NX_ROLE_OTHER && role != NX_ROLE_MASTER + && role != NX_ROLE_SLAVE) { + return OFPERR_OFPRRFC_BAD_ROLE; + } + + rr->role = role; + return 0; +} + +/* Returns an encoded form of a role reply suitable for the "request" in a + * buffer owned by the caller. */ +struct ofpbuf * +ofputil_encode_role_reply(const struct ofp_header *request, + enum nx_role role) +{ + 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); + 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(); + } + + buf = ofpraw_alloc_reply(raw, request, 0); + reply = ofpbuf_put_zeros(buf, reply_size); + + 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 + * 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. + * + * A request for clarification has been filed with the Open Networking + * Foundation as EXT-272. + */ + } + reply->role = htonl(role); + + return buf; +} + /* Table stats. */ static void diff --git a/lib/ofp-util.h b/lib/ofp-util.h index c8bf11e17..f8c4260fe 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -507,6 +507,19 @@ enum ofperr ofputil_decode_port_mod(const struct ofp_header *, struct ofpbuf *ofputil_encode_port_mod(const struct ofputil_port_mod *, enum ofputil_protocol); +/* Abstract ofp_role_request and reply. */ +struct ofputil_role_request { + bool request_current_role_only; /* no role change */ + 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); + /* Abstract table stats. * * For now we use ofp12_table_stats as a superset of the other protocol diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c index 77b7b3916..2dc524906 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 4122bc1a3..a2f7d5a64 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 d8f696d9e..98515c298 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -3588,27 +3588,38 @@ 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; + struct ofputil_role_request rr; struct ofpbuf *buf; uint32_t role; + enum ofperr error; + + error = ofputil_decode_role_message(oh, &rr); + if (error) { + return error; + } - role = ntohl(nrr->role); - if (role != NX_ROLE_OTHER && role != NX_ROLE_MASTER - && role != NX_ROLE_SLAVE) { - return OFPERR_OFPRRFC_BAD_ROLE; + 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 (rr.have_generation_id) { + if (!ofconn_set_master_election_id(ofconn, rr.generation_id)) { + return OFPERR_OFPRRFC_STALE; + } + } + ofconn_set_role(ofconn, role); - buf = ofpraw_alloc_reply(OFPRAW_NXT_ROLE_REPLY, oh, 0); - reply = ofpbuf_put_zeros(buf, sizeof *reply); - reply->role = htonl(role); +reply: + buf = ofputil_encode_role_reply(oh, role); ofconn_send_reply(ofconn, buf); return 0; @@ -4019,14 +4030,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 8491dd05b..a8adda2bc 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 9c0c6df7c..8ee701abf 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 -- 2.47.0