[ovs-dev] [PATCH 2/4] Add Openflow 1.2 role request/reply processing, update OF 1.2 tests.
Jarno Rajahalme
jarno.rajahalme at nsn.com
Fri Dec 7 13:48:22 UTC 2012
Signed-off-by: Jarno Rajahalme <jarno.rajahalme at 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
More information about the dev
mailing list