[ovs-dev] [PATCH v3 2/4] 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.

Ben Pfaff blp at nicira.com
Fri Dec 28 17:22:38 UTC 2012


On Fri, Dec 28, 2012 at 06:28:49PM +0200, Jarno Rajahalme wrote:
> 
> Signed-off-by: Jarno Rajahalme <jarno.rajahalme at nsn.com>
> ---
> 
> v3 makes ofputil_decode_role_request() handle also replies and
> check the validity of the role.  Now also used in ofp-print.c.

Thanks!  I'm going to apply this in a minute, with a few changes:

In ofp_print_role_message(), print the error if there is one.

Rename ofputil_decode_role_request() to ofputil_decode_role_message()
since it handles requests and replies now.

Add comment about EXT-272.

The incremental versus your patch is:

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 6594e34..8ad406d 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -1514,14 +1514,15 @@ static void
 ofp_print_role_message(struct ds *string, const struct ofp_header *oh)
 {
     struct ofputil_role_request rr;
+    enum ofperr error;
 
-    ds_put_cstr(string, " role=");
-
-    if (ofputil_decode_role_request(oh, &rr)) {
-        ds_put_format(string, "bad_role");
+    error = ofputil_decode_role_message(oh, &rr);
+    if (error) {
+        ofp_print_error(string, error);
         return;
     }
 
+    ds_put_cstr(string, " role=");
     if (rr.request_current_role_only) {
         ds_put_cstr(string, "nochange");
         return;
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 3a170cd..db94c4f 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -3361,7 +3361,7 @@ ofputil_encode_port_mod(const struct ofputil_port_mod *pm,
  * an abstract form in '*rr'.  Returns 0 if successful, otherwise an
  * OFPERR_* value. */
 enum ofperr
-ofputil_decode_role_request(const struct ofp_header *oh,
+ofputil_decode_role_message(const struct ofp_header *oh,
                             struct ofputil_role_request *rr)
 {
     const struct ofp12_role_request *orr = ofpmsg_body(oh);
@@ -3443,6 +3443,9 @@ ofputil_encode_role_reply(const struct ofp_header *request,
          * 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);
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index e256749a..f8c4260 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -515,7 +515,7 @@ struct ofputil_role_request {
     uint64_t generation_id;
 };
 
-enum ofperr ofputil_decode_role_request(const struct ofp_header *,
+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);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 75a5cef..98515c2 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3593,7 +3593,7 @@ handle_role_request(struct ofconn *ofconn, const struct ofp_header *oh)
     uint32_t role;
     enum ofperr error;
 
-    error = ofputil_decode_role_request(oh, &rr);
+    error = ofputil_decode_role_message(oh, &rr);
     if (error) {
         return error;
     }



More information about the dev mailing list