[ovs-dev] [ext-272 4/4] Make OpenFlow 1.2+ role replies return the generation ID.

[Ben Pfaff]

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 at nsn.com>
Signed-off-by: Ben Pfaff <blp at 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 ef8de6b..dd6eed8 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 29321d5..1a9dc21 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 856d6fa..a9c7e76 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 4021291..2939125 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 45aa549..f41bfc38 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):"
 
-- 
1.7.10.4