[ovs-dev] [of1.5 8/9] Implement OpenFlow 1.5 group desc stats request.

Ben Pfaff blp at nicira.com
Thu May 8 06:56:47 UTC 2014


OpenFlow 1.4 and earlier always send the description of every group in
response to an OFPMP_GROUP_DESC request.  OpenFlow 1.5 proposes allowing
the controller to request a description of a single group.  This commit
implements a prototype.

EXT-69.
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/ofp-msgs.h           |  9 ++++---
 lib/ofp-print.c          | 10 ++++++++
 lib/ofp-util.c           | 38 +++++++++++++++++++++++++---
 lib/ofp-util.h           |  5 +++-
 ofproto/ofproto.c        | 66 +++++++++++++++++++++++++-----------------------
 tests/ofp-print.at       | 13 ++++++++--
 tests/ofproto.at         |  5 ++++
 utilities/ovs-ofctl.8.in | 14 +++++++---
 utilities/ovs-ofctl.c    | 12 ++++++---
 9 files changed, 123 insertions(+), 49 deletions(-)

diff --git a/lib/ofp-msgs.h b/lib/ofp-msgs.h
index b548f6b..5610793 100644
--- a/lib/ofp-msgs.h
+++ b/lib/ofp-msgs.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012, 2013 Nicira, Inc.
+ * Copyright (c) 2012, 2013, 2014 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -321,8 +321,10 @@ enum ofpraw {
     /* OFPST 1.3+ (6): uint8_t[8][]. */
     OFPRAW_OFPST13_GROUP_REPLY,
 
-    /* OFPST 1.1+ (7): void. */
+    /* OFPST 1.1-1.4 (7): void. */
     OFPRAW_OFPST11_GROUP_DESC_REQUEST,
+    /* OFPST 1.5+ (7): ovs_be32. */
+    OFPRAW_OFPST15_GROUP_DESC_REQUEST,
 
     /* OFPST 1.1+ (7): uint8_t[8][]. */
     OFPRAW_OFPST11_GROUP_DESC_REPLY,
@@ -559,7 +561,8 @@ enum ofptype {
     OFPTYPE_GROUP_STATS_REPLY,       /* OFPRAW_OFPST11_GROUP_REPLY.
                                       * OFPRAW_OFPST13_GROUP_REPLY. */
 
-    OFPTYPE_GROUP_DESC_STATS_REQUEST, /* OFPRAW_OFPST11_GROUP_DESC_REQUEST. */
+    OFPTYPE_GROUP_DESC_STATS_REQUEST, /* OFPRAW_OFPST11_GROUP_DESC_REQUEST.
+                                       * OFPRAW_OFPST15_GROUP_DESC_REQUEST. */
 
     OFPTYPE_GROUP_DESC_STATS_REPLY,  /* OFPRAW_OFPST11_GROUP_DESC_REPLY. */
 
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 3d4b8f81..df9bcc3 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2356,6 +2356,15 @@ ofp_print_group(struct ds *s, uint32_t group_id, uint8_t type,
 }
 
 static void
+ofp_print_ofpst_group_desc_request(struct ds *string,
+                                   const struct ofp_header *oh)
+{
+    uint32_t group_id = ofputil_decode_group_desc_request(oh);
+    ds_put_cstr(string, " group_id=");
+    ofputil_format_group(group_id, string);
+}
+
+static void
 ofp_print_group_desc(struct ds *s, const struct ofp_header *oh)
 {
     struct ofpbuf b;
@@ -2782,6 +2791,7 @@ ofp_to_string__(const struct ofp_header *oh, enum ofpraw raw,
 
     case OFPTYPE_GROUP_DESC_STATS_REQUEST:
         ofp_print_stats_request(string, oh);
+        ofp_print_ofpst_group_desc_request(string, oh);
         break;
 
     case OFPTYPE_GROUP_DESC_STATS_REPLY:
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 4ec323d..5fe3c05 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -6388,16 +6388,40 @@ ofputil_encode_group_stats_request(enum ofp_version ofp_version,
     return request;
 }
 
+/* Decodes the OpenFlow group description request in 'oh', returning the group
+ * whose description is requested, or OFPG_ALL if stats for all groups was
+ * requested. */
+uint32_t
+ofputil_decode_group_desc_request(const struct ofp_header *oh)
+{
+    struct ofpbuf request;
+    enum ofpraw raw;
+
+    ofpbuf_use_const(&request, oh, ntohs(oh->length));
+    raw = ofpraw_pull_assert(&request);
+    if (raw == OFPRAW_OFPST11_GROUP_DESC_REQUEST) {
+        return OFPG_ALL;
+    } else if (raw == OFPRAW_OFPST15_GROUP_DESC_REQUEST) {
+        ovs_be32 *group_id = ofpbuf_pull(&request, sizeof *group_id);
+        return ntohl(*group_id);
+    } else {
+        OVS_NOT_REACHED();
+    }
+}
+
 /* Returns an OpenFlow group description request for OpenFlow version
- * 'ofp_version', that requests stats for group 'group_id'.  (Use OFPG_ALL to
- * request stats for all groups.)
+ * 'ofp_version', that requests stats for group 'group_id'.  Use OFPG_ALL to
+ * request stats for all groups (OpenFlow 1.4 and earlier always request all
+ * groups).
  *
  * Group descriptions include the bucket and action configuration for each
  * group. */
 struct ofpbuf *
-ofputil_encode_group_desc_request(enum ofp_version ofp_version)
+ofputil_encode_group_desc_request(enum ofp_version ofp_version,
+                                  uint32_t group_id)
 {
     struct ofpbuf *request;
+    ovs_be32 gid;
 
     switch (ofp_version) {
     case OFP10_VERSION:
@@ -6407,8 +6431,14 @@ ofputil_encode_group_desc_request(enum ofp_version ofp_version)
     case OFP12_VERSION:
     case OFP13_VERSION:
     case OFP14_VERSION:
+        request = ofpraw_alloc(OFPRAW_OFPST11_GROUP_DESC_REQUEST,
+                               ofp_version, 0);
+        break;
     case OFP15_VERSION:
-        request = ofpraw_alloc(OFPRAW_OFPST11_GROUP_DESC_REQUEST, ofp_version, 0);
+        request = ofpraw_alloc(OFPRAW_OFPST15_GROUP_DESC_REQUEST,
+                               ofp_version, 0);
+        gid = htonl(group_id);
+        ofpbuf_put(request, &gid, sizeof gid);
         break;
     default:
         OVS_NOT_REACHED();
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index ce10bfc..5308668 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -1130,13 +1130,16 @@ enum ofperr ofputil_decode_group_mod(const struct ofp_header *,
 int ofputil_decode_group_stats_reply(struct ofpbuf *,
                                      struct ofputil_group_stats *);
 
+uint32_t ofputil_decode_group_desc_request(const struct ofp_header *);
+struct ofpbuf *ofputil_encode_group_desc_request(enum ofp_version,
+                                                 uint32_t group_id);
+
 int ofputil_decode_group_desc_reply(struct ofputil_group_desc *,
                                     struct ofpbuf *, enum ofp_version);
 
 void ofputil_append_group_desc_reply(const struct ofputil_group_desc *,
                                      struct list *buckets,
                                      struct list *replies);
-struct ofpbuf *ofputil_encode_group_desc_request(enum ofp_version);
 
 struct ofputil_bundle_ctrl_msg {
     uint32_t    bundle_id;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 557f124..2978b5b 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -5448,64 +5448,66 @@ append_group_stats(struct ofgroup *group, struct list *replies)
     free(ogs.bucket_stats);
 }
 
-static enum ofperr
-handle_group_stats_request(struct ofconn *ofconn,
-                           const struct ofp_header *request)
+static void
+handle_group_request(struct ofconn *ofconn,
+                     const struct ofp_header *request, uint32_t group_id,
+                     void (*cb)(struct ofgroup *, struct list *replies))
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-    struct list replies;
-    enum ofperr error;
     struct ofgroup *group;
-    uint32_t group_id;
-
-    error = ofputil_decode_group_stats_request(request, &group_id);
-    if (error) {
-        return error;
-    }
+    struct list replies;
 
     ofpmp_init(&replies, request);
-
     if (group_id == OFPG_ALL) {
         ovs_rwlock_rdlock(&ofproto->groups_rwlock);
         HMAP_FOR_EACH (group, hmap_node, &ofproto->groups) {
             ovs_rwlock_rdlock(&group->rwlock);
-            append_group_stats(group, &replies);
+            cb(group, &replies);
             ovs_rwlock_unlock(&group->rwlock);
         }
         ovs_rwlock_unlock(&ofproto->groups_rwlock);
     } else {
         if (ofproto_group_lookup(ofproto, group_id, &group)) {
-            append_group_stats(group, &replies);
+            cb(group, &replies);
             ofproto_group_release(group);
         }
     }
-
     ofconn_send_replies(ofconn, &replies);
-
-    return 0;
 }
 
 static enum ofperr
-handle_group_desc_stats_request(struct ofconn *ofconn,
-                                const struct ofp_header *request)
+handle_group_stats_request(struct ofconn *ofconn,
+                           const struct ofp_header *request)
 {
-    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-    struct list replies;
-    struct ofputil_group_desc gds;
-    struct ofgroup *group;
-
-    ofpmp_init(&replies, request);
+    uint32_t group_id;
+    enum ofperr error;
 
-    ovs_rwlock_rdlock(&ofproto->groups_rwlock);
-    HMAP_FOR_EACH (group, hmap_node, &ofproto->groups) {
-        gds.group_id = group->group_id;
-        gds.type = group->type;
-        ofputil_append_group_desc_reply(&gds, &group->buckets, &replies);
+    error = ofputil_decode_group_stats_request(request, &group_id);
+    if (error) {
+        return error;
     }
-    ovs_rwlock_unlock(&ofproto->groups_rwlock);
 
-    ofconn_send_replies(ofconn, &replies);
+    handle_group_request(ofconn, request, group_id, append_group_stats);
+    return 0;
+}
+
+static void
+append_group_desc(struct ofgroup *group, struct list *replies)
+{
+    struct ofputil_group_desc gds;
 
+    gds.group_id = group->group_id;
+    gds.type = group->type;
+    ofputil_append_group_desc_reply(&gds, &group->buckets, replies);
+}
+
+static enum ofperr
+handle_group_desc_stats_request(struct ofconn *ofconn,
+                                const struct ofp_header *request)
+{
+    handle_group_request(ofconn, request,
+                         ofputil_decode_group_desc_request(request),
+                         append_group_desc);
     return 0;
 }
 
diff --git a/tests/ofp-print.at b/tests/ofp-print.at
index 640d114..81f5fde 100644
--- a/tests/ofp-print.at
+++ b/tests/ofp-print.at
@@ -1798,11 +1798,20 @@ OFPST_GROUP reply (OF1.3) (xid=0x2):
 ])
 AT_CLEANUP
 
-AT_SETUP([OFPST_GROUP_DESC request])
+AT_SETUP([OFPST_GROUP_DESC request - OF1.1])
 AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
 AT_CHECK([ovs-ofctl ofp-print "\
 02 12 00 10 00 00 00 02 00 07 00 00 00 00 00 00 \
-"], [0], [OFPST_GROUP_DESC request (OF1.1) (xid=0x2):
+"], [0], [OFPST_GROUP_DESC request (OF1.1) (xid=0x2): group_id=ALL
+])
+AT_CLEANUP
+
+AT_SETUP([OFPST_GROUP_DESC request - OF1.5])
+AT_KEYWORDS([ofp-print OFPT_STATS_REQUEST])
+AT_CHECK([ovs-ofctl ofp-print "\
+06 12 00 14 00 00 00 02 00 07 00 00 00 00 00 00 \
+00 00 00 01
+"], [0], [OFPST_GROUP_DESC request (OF1.5) (xid=0x2): group_id=1
 ])
 AT_CLEANUP
 
diff --git a/tests/ofproto.at b/tests/ofproto.at
index 2ffe653..c1a6637 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -206,6 +206,11 @@ group_id=1234,type=all,bucket=output:10
 group_id=1235,type=all,bucket=output:10
 ])
 AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-groups br0 groups.txt])
+AT_CHECK([ovs-ofctl -F OXM-OpenFlow15 -O OpenFlow15 -vwarn dump-groups br0 1234], [0], [stdout])
+AT_CHECK([STRIP_XIDS stdout], [0], [dnl
+OFPST_GROUP_DESC reply (OF1.5):
+ group_id=1234,type=all,bucket=actions=output:10
+])
 AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn del-groups br0 group_id=1234])
 AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn dump-groups br0], [0], [stdout])
 AT_CHECK([STRIP_XIDS stdout], [0], [dnl
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 9923715..2ccb21a 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -204,9 +204,15 @@ the switch itself (with the \fBprotocols\fR column in the \fBBridge\fR
 table).  For more information, see ``Q: What versions of OpenFlow does
 Open vSwitch support?'' in the Open vSwitch FAQ.
 .
-.IP "\fBdump\-groups \fIswitch"
-Prints to the console all group entries in \fIswitch\fR's tables. Each line
-of output is a group entry as described in \fBGroup Syntax\fR below.
+.IP "\fBdump\-groups \fIswitch\fR [\fIgroup\fR]"
+Prints group entries in \fIswitch\fR's tables to console.  To dump
+only a specific group, specify its number as \fIgroup\fR.  Otherwise,
+if \fIgroup\fR is omitted, or if it is specified as \fBALL\fR, then
+all groups are printed.  Each line of output is a group entry as
+described in \fBGroup Syntax\fR below.
+.IP
+Only OpenFlow 1.5 and later support dumping a specific group.  Earlier
+versions of OpenFlow always dump all groups.
 .
 .IP "\fBdump\-group\-features \fIswitch"
 Prints to the console the group features of the \fIswitch\fR.
@@ -1830,7 +1836,7 @@ of each field is honoured.
 .PP
 .IP \fBgroup_id=\fIid\fR
 The integer group id of group.
-When this field is specified in \fBdel-groups\fR or \fBdump-groups\fR,
+When this field is specified in \fBdel\-groups\fR or \fBdump\-groups\fR,
 the keyword "all" may be used to designate all groups.
 .
 This field is required.
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 2776613..c2baed3 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -327,7 +327,7 @@ usage(void)
            "  mod-group SWITCH GROUP      modify specific group\n"
            "  del-groups SWITCH [GROUP]   delete matching GROUPs\n"
            "  dump-group-features SWITCH  print group features\n"
-           "  dump-groups SWITCH          print group description\n"
+           "  dump-groups SWITCH [GROUP]  print group description\n"
            "  dump-group-stats SWITCH [GROUP]  print group statistics\n"
            "  queue-get-config SWITCH PORT  print queue information for port\n"
            "  add-meter SWITCH METER      add meter described by METER\n"
@@ -2162,10 +2162,16 @@ ofctl_dump_group_desc(int argc OVS_UNUSED, char *argv[])
 {
     struct ofpbuf *request;
     struct vconn *vconn;
+    uint32_t group_id;
 
     open_vconn(argv[1], &vconn);
 
-    request = ofputil_encode_group_desc_request(vconn_get_version(vconn));
+    if (argc < 3 || !ofputil_group_from_string(argv[2], &group_id)) {
+        group_id = OFPG11_ALL;
+    }
+
+    request = ofputil_encode_group_desc_request(vconn_get_version(vconn),
+                                                group_id);
     if (request) {
         dump_stats_transaction(vconn, request);
     }
@@ -3503,7 +3509,7 @@ static const struct command all_commands[] = {
     { "add-groups", 1, 2, ofctl_add_groups },
     { "mod-group", 1, 2, ofctl_mod_group },
     { "del-groups", 1, 2, ofctl_del_groups },
-    { "dump-groups", 1, 1, ofctl_dump_group_desc },
+    { "dump-groups", 1, 2, ofctl_dump_group_desc },
     { "dump-group-stats", 1, 2, ofctl_dump_group_stats },
     { "dump-group-features", 1, 1, ofctl_dump_group_features },
     { "help", 0, INT_MAX, ofctl_help },
-- 
1.9.1




More information about the dev mailing list