ovs-ofctl: Handle any number of buckets in group statistics

struct ofputil_group_stats has an arbitrary limit
of 16 buckets for which it can record statistics.
However the code does not appear to enforce this
limit and it seems to me that the code could overflow.

This patch aims to remove the arbitrary limit by
changing the 'bucket_stats' field of struct ofputil_group_stats
from a fixed length array to a pointer whose storage is allocated and freed
as necessary.

Signed-off-by: Simon Horman <horms@verge.net.au>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 702d9c1..518f9af 100644 (file)
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2252,6 +2252,8 @@ ofp_print_group_stats(struct ds *s, const struct ofp_header *oh)
                 ds_put_format(s, "byte_count=%"PRIu64"", gs.bucket_stats[bucket_i].byte_count);
             }
         }
+
+        free(gs.bucket_stats);
      }
 }
 

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index b181a0d..8ac9186 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -5480,7 +5480,8 @@ ofputil_decode_group_stats_request(const struct ofp_header *request,
 }
 
 /* Converts a group stats reply in 'msg' into an abstract ofputil_group_stats
- * in 'gs'.
+ * in 'gs'.  Assigns freshly allocated memory to gs->bucket_stats for the
+ * caller to eventually free.
  *
  * Multiple group stats replies can be packed into a single OpenFlow message.
  * Calling this function multiple times for a single 'msg' iterates through the
@@ -5501,6 +5502,7 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg,
     size_t length;
     size_t i;
 
+    gs->bucket_stats = NULL;
     error = (msg->l2
              ? ofpraw_decode(&raw, msg->l2)
              : ofpraw_pull(&raw, msg));
@@ -5557,6 +5559,7 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg,
         return OFPERR_OFPBRC_BAD_LEN;
     }
 
+    gs->bucket_stats = xmalloc(gs->n_buckets * sizeof *gs->bucket_stats);
     for (i = 0; i < gs->n_buckets; i++) {
         gs->bucket_stats[i].packet_count = ntohll(obc[i].packet_count);
         gs->bucket_stats[i].byte_count = ntohll(obc[i].byte_count);

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 6de2a05..eae8ed5 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -913,7 +913,7 @@ struct ofputil_group_stats {
     uint32_t duration_sec;      /* UINT32_MAX if unknown. */
     uint32_t duration_nsec;
     uint32_t n_buckets;
-    struct bucket_counter bucket_stats[16];
+    struct bucket_counter *bucket_stats;
 };
 
 /* Group features reply, independent of protocol. */

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index dacbb71..b94fd8e 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -4786,6 +4786,8 @@ append_group_stats(struct ofgroup *group, struct list *replies)
     long long int now = time_msec();
     int error;
 
+    ogs.bucket_stats = xmalloc(group->n_buckets * sizeof *ogs.bucket_stats);
+
     error = (ofproto->ofproto_class->group_get_stats
              ? ofproto->ofproto_class->group_get_stats(group, &ogs)
              : EOPNOTSUPP);
@@ -4802,6 +4804,8 @@ append_group_stats(struct ofgroup *group, struct list *replies)
     calc_duration(group->created, now, &ogs.duration_sec, &ogs.duration_nsec);
 
     ofputil_append_group_stats(replies, &ogs);
+
+    free(ogs.bucket_stats);
 }
 
 static enum ofperr