From 147cc9d35839b0894135c1a8ba62a0867675c329 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@nicira.com>
Date: Tue, 12 Feb 2013 00:00:42 -0800
Subject: [PATCH] Make OpenFlow 1.2+ role replies return the generation ID.
OpenFlow extensibility working group issue EXT-272 clarifies the use of
the generation_id in role reply messages as used for the current generation
ID or all-1-bits if there is no current generation ID. This commit
implements EXT-272 in Open vSwitch.
Unfortunately the full text of EXT-272 is not available freely online.
(The "open" part of the Open Networking Foundation is the network, not
the foundation)
EXT-272.
CC: Jarno Rajahalme <jarno.rajahalme@nsn.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
---
lib/ofp-util.c | 22 +++++++---------------
ofproto/connmgr.c | 11 +++++++++++
ofproto/ofproto.c | 4 ++--
tests/ofp-print.at | 4 ++--
tests/ofproto.at | 4 ++--
5 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index ef8de6b4e..dd6eed87d 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3400,8 +3400,9 @@ ofputil_decode_role_message(const struct ofp_header *oh,
}
rr->role = ntohl(orr->role);
- if (raw == OFPRAW_OFPT12_ROLE_REPLY
- || orr->role == htonl(OFPCR12_ROLE_NOCHANGE)) {
+ if (raw == OFPRAW_OFPT12_ROLE_REQUEST
+ ? orr->role == htonl(OFPCR12_ROLE_NOCHANGE)
+ : orr->generation_id == htonll(UINT64_MAX)) {
rr->have_generation_id = false;
rr->generation_id = 0;
} else {
@@ -3447,20 +3448,11 @@ ofputil_encode_role_reply(const struct ofp_header *request,
buf = ofpraw_alloc_reply(OFPRAW_OFPT12_ROLE_REPLY, request, 0);
orr = ofpbuf_put_zeros(buf, sizeof *orr);
- orr->role = htonl(rr->role);
- /*
- * 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.
- */
- orr->generation_id = htonll(0);
+ orr->role = htonl(rr->role);
+ orr->generation_id = htonll(rr->have_generation_id
+ ? rr->generation_id
+ : UINT64_MAX);
} else if (raw == OFPRAW_NXT_ROLE_REQUEST) {
struct nx_role_request *nrr;
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 29321d50f..1a9dc21b1 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -823,6 +823,17 @@ ofconn_get_type(const struct ofconn *ofconn)
return ofconn->type;
}
+/* If a master election id is defined, stores it into '*idp' and returns
+ * true. Otherwise, stores UINT64_MAX into '*idp' and returns false. */
+bool
+ofconn_get_master_election_id(const struct ofconn *ofconn, uint64_t *idp)
+{
+ *idp = (ofconn->connmgr->master_election_id_defined
+ ? ofconn->connmgr->master_election_id
+ : UINT64_MAX);
+ return ofconn->connmgr->master_election_id_defined;
+}
+
/* Sets the master election id.
*
* Returns true if successful, false if the id is stale
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 856d6faf6..a9c7e7672 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3601,8 +3601,8 @@ handle_role_request(struct ofconn *ofconn, const struct ofp_header *oh)
}
reply.role = ofconn_get_role(ofconn);
- reply.have_generation_id = false;
- reply.generation_id = 0;
+ reply.have_generation_id = ofconn_get_master_election_id(
+ ofconn, &reply.generation_id);
buf = ofputil_encode_role_reply(oh, &reply);
ofconn_send_reply(ofconn, buf);
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 40212918b..29391259a 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -1549,9 +1549,9 @@ 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 \
+12 34 56 78 ab cd ef 90 \
"], [0], [dnl
-OFPT_ROLE_REPLY (OF1.2) (xid=0x2): role=slave
+OFPT_ROLE_REPLY (OF1.2) (xid=0x2): role=slave generation_id=1311768467750121360
])
AT_CLEANUP
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 45aa54917..f41bfc385 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1332,7 +1332,7 @@ 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"
+echo >>expout "OFPT_ROLE_REPLY (OF1.2) (xid=0x3): role=slave generation_id=9223372036854775810"
# Try to become the master using a stale generation ID
ovs-appctl -t ovs-ofctl ofctl/send 031800180000000400000002000000000000000000000002
@@ -1343,7 +1343,7 @@ 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"
+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):"
--
2.47.0