Only OpenFlow 1.4 and later support role status messages, but this code
tried to send them to all controllers, which caused an assertion failure.
Also, add tests to check that role status messages work, and that they
don't cause trouble with OF1.2.
Reported-by: Anup Khadka <khadka...@gmail.com>
Signed-off-by: Ben Pfaff <b...@nicira.com>
---
AUTHORS | 1 +
lib/ofp-util.c | 27 ++++++----
ofproto/connmgr.c | 5 +-
tests/ofproto.at | 147 ++++++++++++++++++++++++++++++++++++++++++-----------
4 files changed, 139 insertions(+), 41 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index 3e97bbe..b505a07 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -161,6 +161,7 @@ Andreas Beckmann deb...@abeckmann.de
Andrei Andone andrei.and...@softvision.ro
Anshuman Manral anshuman.man...@outlook.com
Anton Matsiuk anton.mats...@gmail.com
+Anup Khadka khadka...@gmail.com
Anuprem Chalvadi achalv...@vmware.com
Ariel Tubaltsev atubalt...@vmware.com
Atzm Watanabe a...@stratosphere.co.jp
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 912def9..a91031c 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -4977,22 +4977,31 @@ ofputil_encode_role_reply(const struct ofp_header
*request,
return buf;
}
+/* Encodes "role status" message 'status' for sending in the given
+ * 'protocol'. Returns the role status message, if 'protocol' supports them,
+ * otherwise a null pointer. */
struct ofpbuf *
ofputil_encode_role_status(const struct ofputil_role_status *status,
enum ofputil_protocol protocol)
{
- struct ofpbuf *buf;
enum ofp_version version;
- struct ofp14_role_status *rstatus;
version = ofputil_protocol_to_ofp_version(protocol);
- buf = ofpraw_alloc_xid(OFPRAW_OFPT14_ROLE_STATUS, version, htonl(0), 0);
- rstatus = ofpbuf_put_zeros(buf, sizeof *rstatus);
- rstatus->role = htonl(status->role);
- rstatus->reason = status->reason;
- rstatus->generation_id = htonll(status->generation_id);
-
- return buf;
+ if (version >= OFP14_VERSION) {
+ struct ofp14_role_status *rstatus;
+ struct ofpbuf *buf;
+
+ buf = ofpraw_alloc_xid(OFPRAW_OFPT14_ROLE_STATUS, version, htonl(0),
+ 0);
+ rstatus = ofpbuf_put_zeros(buf, sizeof *rstatus);
+ rstatus->role = htonl(status->role);
+ rstatus->reason = status->reason;
+ rstatus->generation_id = htonll(status->generation_id);
+
+ return buf;
+ } else {
+ return NULL;
+ }
}
enum ofperr
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index be99529..05fe0c4 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -920,8 +920,9 @@ ofconn_send_role_status(struct ofconn *ofconn, uint32_t
role, uint8_t reason)
ofconn_get_master_election_id(ofconn, &status.generation_id);
buf = ofputil_encode_role_status(&status, ofconn_get_protocol(ofconn));
-
- ofconn_send(ofconn, buf, NULL);
+ if (buf) {
+ ofconn_send(ofconn, buf, NULL);
+ }
}
/* Changes 'ofconn''s role to 'role'. If 'role' is OFPCR12_ROLE_MASTER then
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 729ef7b..3a55ce3 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1875,41 +1875,128 @@ 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
-: > experr
+ON_EXIT([kill `cat c1.pid c2.pid`])
+
+# Start two ovs-ofctl controller processes.
+AT_CAPTURE_FILE([monitor1.log])
+AT_CAPTURE_FILE([expout1])
+AT_CAPTURE_FILE([experr1])
+AT_CAPTURE_FILE([monitor2.log])
+AT_CAPTURE_FILE([expout2])
+AT_CAPTURE_FILE([experr2])
+for i in 1 2; do
+ AT_CHECK([ovs-ofctl -O OpenFlow12 monitor br0 --detach --no-chdir
--pidfile=`pwd`/c$i.pid --unixctl=`pwd`/c$i])
+ ovs-appctl -t `pwd`/c$i ofctl/barrier
+ ovs-appctl -t `pwd`/c$i ofctl/set-output-file monitor$i.log
+ : > expout$i
+ : > experr$i
+
+ # find out current role
+ ovs-appctl -t `pwd`/c$i ofctl/send
031800180000000200000000000000000000000000000000
+ echo >>experr$i "send: OFPT_ROLE_REQUEST (OF1.2): role=nochange"
+ echo >>expout$i "OFPT_ROLE_REPLY (OF1.2): role=equal"
+done
-# find out current role
-ovs-appctl -t ovs-ofctl ofctl/send
031800180000000200000000000000000000000000000000
-echo >>experr "send: OFPT_ROLE_REQUEST (OF1.2) (xid=0x2): role=nochange"
-echo >>expout "OFPT_ROLE_REPLY (OF1.2) (xid=0x2): role=equal"
+# controller 1: Become slave (generation_id is initially undefined, so
+# 2^63+2 should not be stale)
+ovs-appctl -t `pwd`/c1 ofctl/send
031800180000000300000003000000008000000000000002
+echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.2): role=slave
generation_id=9223372036854775810"
+echo >>expout1 "OFPT_ROLE_REPLY (OF1.2): role=slave
generation_id=9223372036854775810"
+
+# controller 2: Become master.
+ovs-appctl -t `pwd`/c2 ofctl/send
031800180000000300000002000000008000000000000003
+echo >>experr2 "send: OFPT_ROLE_REQUEST (OF1.2): role=master
generation_id=9223372036854775811"
+echo >>expout2 "OFPT_ROLE_REPLY (OF1.2): role=master
generation_id=9223372036854775811"
+
+# controller 1: Try to become the master using a stale generation ID
+ovs-appctl -t `pwd`/c1 ofctl/send
031800180000000400000002000000000000000000000003
+echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.2): role=master generation_id=3"
+echo >>expout1 "OFPT_ERROR (OF1.2): OFPRRFC_STALE"
+echo >>expout1 "OFPT_ROLE_REQUEST (OF1.2): role=master generation_id=3"
+
+# controller 1: Become master using a valid generation ID
+ovs-appctl -t `pwd`/c1 ofctl/send
031800180000000500000002000000000000000000000001
+echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.2): role=master generation_id=1"
+echo >>expout1 "OFPT_ROLE_REPLY (OF1.2): role=master generation_id=1"
+
+for i in 1 2; do
+ ovs-appctl -t `pwd`/c$i ofctl/barrier
+ echo >>expout$i "OFPT_BARRIER_REPLY (OF1.2):"
+done
-# Become slave (generation_id is initially undefined, so 2^63+2 should not be
stale)
-ovs-appctl -t ovs-ofctl ofctl/send
031800180000000300000003000000008000000000000002
-echo >>experr "send: OFPT_ROLE_REQUEST (OF1.2) (xid=0x3): role=slave
generation_id=9223372036854775810"
-echo >>expout "OFPT_ROLE_REPLY (OF1.2) (xid=0x3): role=slave
generation_id=9223372036854775810"
+# Check output.
+for i in 1 2; do
+ cp expout$i expout
+ AT_CHECK([grep -v '^send:' monitor$i.log | STRIP_XIDS], [0], [expout])
+ cp experr$i expout
+ AT_CHECK([grep '^send:' monitor$i.log | STRIP_XIDS], [0], [expout])
+done
+OVS_VSWITCHD_STOP
+AT_CLEANUP
-# Try to become the master using a stale generation ID
-ovs-appctl -t ovs-ofctl ofctl/send
031800180000000400000002000000000000000000000002
-echo >>experr "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 >>experr "send: OFPT_ROLE_REQUEST (OF1.2) (xid=0x5): role=master
generation_id=1"
-echo >>expout "OFPT_ROLE_REPLY (OF1.2) (xid=0x5): role=master generation_id=1"
-ovs-appctl -t ovs-ofctl ofctl/barrier
-echo >>expout "OFPT_BARRIER_REPLY (OF1.2) (xid=0x3):"
+dnl This test checks that the role request/response messaging works,
+dnl that generation_id is handled properly, and that role status update
+dnl messages are sent when a controller's role gets changed from master
+dnl to slave.
+AT_SETUP([ofproto - controller role (OpenFlow 1.4)])
+OVS_VSWITCHD_START
+ON_EXIT([kill `cat c1.pid c2.pid`])
+
+# Start two ovs-ofctl controller processes.
+AT_CAPTURE_FILE([monitor1.log])
+AT_CAPTURE_FILE([expout1])
+AT_CAPTURE_FILE([experr1])
+AT_CAPTURE_FILE([monitor2.log])
+AT_CAPTURE_FILE([expout2])
+AT_CAPTURE_FILE([experr2])
+for i in 1 2; do
+ AT_CHECK([ovs-ofctl -O OpenFlow14 monitor br0 --detach --no-chdir
--pidfile=`pwd`/c$i.pid --unixctl=`pwd`/c$i])
+ ovs-appctl -t `pwd`/c$i ofctl/barrier
+ ovs-appctl -t `pwd`/c$i ofctl/set-output-file monitor$i.log
+ : > expout$i
+ : > experr$i
+
+ # find out current role
+ ovs-appctl -t `pwd`/c$i ofctl/send
051800180000000200000000000000000000000000000000
+ echo >>experr$i "send: OFPT_ROLE_REQUEST (OF1.4): role=nochange"
+ echo >>expout$i "OFPT_ROLE_REPLY (OF1.4): role=equal"
+done
-AT_CHECK([grep -v '^send:' monitor.log], [0], [expout])
-mv experr expout
-AT_CHECK([grep '^send:' monitor.log], [0], [expout])
+# controller 1: Become slave (generation_id is initially undefined, so
+# 2^63+2 should not be stale)
+ovs-appctl -t `pwd`/c1 ofctl/send
051800180000000300000003000000008000000000000002
+echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.4): role=slave
generation_id=9223372036854775810"
+echo >>expout1 "OFPT_ROLE_REPLY (OF1.4): role=slave
generation_id=9223372036854775810"
+
+# controller 2: Become master.
+ovs-appctl -t `pwd`/c2 ofctl/send
051800180000000300000002000000008000000000000003
+echo >>experr2 "send: OFPT_ROLE_REQUEST (OF1.4): role=master
generation_id=9223372036854775811"
+echo >>expout2 "OFPT_ROLE_REPLY (OF1.4): role=master
generation_id=9223372036854775811"
+
+# controller 1: Try to become the master using a stale generation ID
+ovs-appctl -t `pwd`/c1 ofctl/send
051800180000000400000002000000000000000000000003
+echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.4): role=master generation_id=3"
+echo >>expout1 "OFPT_ERROR (OF1.4): OFPRRFC_STALE"
+echo >>expout1 "OFPT_ROLE_REQUEST (OF1.4): role=master generation_id=3"
+
+# controller 1: Become master using a valid generation ID
+ovs-appctl -t `pwd`/c1 ofctl/send
051800180000000500000002000000000000000000000001
+echo >>experr1 "send: OFPT_ROLE_REQUEST (OF1.4): role=master generation_id=1"
+echo >>expout1 "OFPT_ROLE_REPLY (OF1.4): role=master generation_id=1"
+echo >>expout2 "OFPT_ROLE_STATUS (OF1.4): role=slave generation_id=1
reason=master_request"
+
+for i in 1 2; do
+ ovs-appctl -t `pwd`/c$i ofctl/barrier
+ echo >>expout$i "OFPT_BARRIER_REPLY (OF1.4):"
+done
-ovs-appctl -t ovs-ofctl exit
+# Check output.
+for i in 1 2; do
+ cp expout$i expout
+ AT_CHECK([grep -v '^send:' monitor$i.log | STRIP_XIDS], [0], [expout])
+ cp experr$i expout
+ AT_CHECK([grep '^send:' monitor$i.log | STRIP_XIDS], [0], [expout])
+done
OVS_VSWITCHD_STOP
AT_CLEANUP
--
1.7.10.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
