[ovs-dev] [PATCH] ovs-ofctl: Handle any number of buckets in group statistics
Simon Horman
horms at verge.net.au
Mon Sep 2 02:28:10 UTC 2013
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
removing the 'bucket_stats' field from struct ofputil_group_stats
and instead allowing an instance of that structure to be
followed by an any number of struct bucket_counter.
Signed-off-by: Simon Horman <horms at verge.net.au>
---
Ben, feel free to squash this patch into
"Implement OpenFlow 1.1+ "groups" protocol" if it is
to your liking.
Compile tested only.
---
lib/ofp-print.c | 26 +++++++++++++++-----------
lib/ofp-util.c | 45 +++++++++++++++++++++++++++------------------
lib/ofp-util.h | 4 ++--
ofproto/ofproto.c | 26 +++++++++++++++-----------
4 files changed, 59 insertions(+), 42 deletions(-)
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index cc1bd73..9041384 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -2220,7 +2220,8 @@ ofp_print_group_stats(struct ds *s, const struct ofp_header *oh)
ofpbuf_use_const(&b, oh, ntohs(oh->length));
for (;;) {
- struct ofputil_group_stats gs;
+ struct ofputil_group_stats *gs;
+ struct bucket_counter *bc;
int retval;
retval = ofputil_decode_group_stats_reply(&b, &gs);
@@ -2234,24 +2235,27 @@ ofp_print_group_stats(struct ds *s, const struct ofp_header *oh)
ds_put_char(s, '\n');
ds_put_char(s, ' ');
- ds_put_format(s, "group_id=%"PRIu32",", gs.group_id);
+ ds_put_format(s, "group_id=%"PRIu32",", gs->group_id);
- if (gs.duration_sec != UINT32_MAX) {
+ if (gs->duration_sec != UINT32_MAX) {
ds_put_cstr(s, "duration=");
- ofp_print_duration(s, gs.duration_sec, gs.duration_nsec);
+ ofp_print_duration(s, gs->duration_sec, gs->duration_nsec);
ds_put_char(s, ',');
}
- ds_put_format(s, "ref_count=%"PRIu32",", gs.ref_count);
- ds_put_format(s, "packet_count=%"PRIu64",", gs.packet_count);
- ds_put_format(s, "byte_count=%"PRIu64"", gs.byte_count);
+ ds_put_format(s, "ref_count=%"PRIu32",", gs->ref_count);
+ ds_put_format(s, "packet_count=%"PRIu64",", gs->packet_count);
+ ds_put_format(s, "byte_count=%"PRIu64"", gs->byte_count);
- for (bucket_i = 0; bucket_i < gs.n_buckets; bucket_i++) {
- if (gs.bucket_stats[bucket_i].packet_count != UINT64_MAX) {
+ bc = (struct bucket_counter *)(gs + 1);
+ for (bucket_i = 0; bucket_i < gs->n_buckets; bucket_i++) {
+ if (bc[bucket_i].packet_count != UINT64_MAX) {
ds_put_format(s, ",bucket%"PRIu32":", bucket_i);
- ds_put_format(s, "packet_count=%"PRIu64",", gs.bucket_stats[bucket_i].packet_count);
- ds_put_format(s, "byte_count=%"PRIu64"", gs.bucket_stats[bucket_i].byte_count);
+ ds_put_format(s, "packet_count=%"PRIu64",", bc[bucket_i].packet_count);
+ ds_put_format(s, "byte_count=%"PRIu64"", bc[bucket_i].byte_count);
}
}
+
+ free(gs);
}
}
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 7407a3e..a3f132a 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -5321,6 +5321,7 @@ ofputil_group_stats_to_ofp11(const struct ofputil_group_stats *ogs,
size_t base_len, struct list *replies)
{
struct ofp11_bucket_counter *bc11;
+ const struct bucket_counter *obc = (struct bucket_counter *)(ogs + 1);
struct ofp11_group_stats *gs11;
size_t length;
int i;
@@ -5337,10 +5338,8 @@ ofputil_group_stats_to_ofp11(const struct ofputil_group_stats *ogs,
bc11 = (void *) (((uint8_t *) gs11) + base_len);
for (i = 0; i < ogs->n_buckets; i++) {
- const struct bucket_counter *obc = &ogs->bucket_stats[i];
-
- bc11[i].packet_count = htonll(obc->packet_count);
- bc11[i].byte_count = htonll(obc->byte_count);
+ bc11[i].packet_count = htonll(obc[i].packet_count);
+ bc11[i].byte_count = htonll(obc[i].byte_count);
}
return gs11;
@@ -5486,12 +5485,16 @@ ofputil_decode_group_stats_request(const struct ofp_header *request,
int
ofputil_decode_group_stats_reply(struct ofpbuf *msg,
- struct ofputil_group_stats *gs)
+ struct ofputil_group_stats **gs)
{
struct ofp11_bucket_counter *obc;
struct ofp11_group_stats *ogs11;
+ struct bucket_counter *bc;
enum ofpraw raw;
enum ofperr error;
+ uint32_t duration_sec;
+ uint32_t duration_nsec;
+ size_t n_buckets;
size_t base_len;
size_t length;
size_t i;
@@ -5510,7 +5513,7 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg,
if (raw == OFPRAW_OFPST11_GROUP_REPLY) {
base_len = sizeof *ogs11;
ogs11 = ofpbuf_try_pull(msg, sizeof *ogs11);
- gs->duration_sec = gs->duration_nsec = UINT32_MAX;
+ duration_sec = duration_nsec = UINT32_MAX;
} else if (raw == OFPRAW_OFPST13_GROUP_REPLY) {
struct ofp13_group_stats *ogs13;
@@ -5518,8 +5521,8 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg,
ogs13 = ofpbuf_try_pull(msg, sizeof *ogs13);
if (ogs13) {
ogs11 = &ogs13->gs;
- gs->duration_sec = ntohl(ogs13->duration_sec);
- gs->duration_nsec = ntohl(ogs13->duration_nsec);
+ duration_sec = ntohl(ogs13->duration_sec);
+ duration_nsec = ntohl(ogs13->duration_nsec);
} else {
ogs11 = NULL;
}
@@ -5539,22 +5542,28 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg,
return OFPERR_OFPBRC_BAD_LEN;
}
- gs->group_id = ntohl(ogs11->group_id);
- gs->ref_count = ntohl(ogs11->ref_count);
- gs->packet_count = ntohll(ogs11->packet_count);
- gs->byte_count = ntohll(ogs11->byte_count);
-
- gs->n_buckets = (length - base_len) / sizeof *obc;
- obc = ofpbuf_try_pull(msg, gs->n_buckets * sizeof *obc);
+ n_buckets = (length - base_len) / sizeof *obc;
+ obc = ofpbuf_try_pull(msg, n_buckets * sizeof *obc);
if (!obc) {
VLOG_WARN_RL(&bad_ofmsg_rl, "%s reply has %zu leftover bytes at end",
ofpraw_get_name(raw), msg->size);
return OFPERR_OFPBRC_BAD_LEN;
}
- 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);
+ *gs = xmalloc(sizeof **gs + sizeof *bc * n_buckets);
+
+ (*gs)->group_id = ntohl(ogs11->group_id);
+ (*gs)->ref_count = ntohl(ogs11->ref_count);
+ (*gs)->packet_count = ntohll(ogs11->packet_count);
+ (*gs)->byte_count = ntohll(ogs11->byte_count);
+ (*gs)->duration_sec = duration_sec;
+ (*gs)->duration_nsec = duration_nsec;
+ (*gs)->n_buckets = n_buckets;
+
+ bc = (struct bucket_counter *)((*gs) + 1);
+ for (i = 0; i < (*gs)->n_buckets; i++) {
+ bc[i].packet_count = ntohll(obc[i].packet_count);
+ bc[i].byte_count = ntohll(obc[i].byte_count);
}
return 0;
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index ad62c8a..2b30528 100644
--- 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[0]; */
};
/* Group features reply, independent of protocol. */
@@ -952,7 +952,7 @@ enum ofperr ofputil_decode_group_mod(const struct ofp_header *,
struct ofputil_group_mod *);
int ofputil_decode_group_stats_reply(struct ofpbuf *,
- struct ofputil_group_stats *);
+ struct ofputil_group_stats **);
int ofputil_decode_group_desc_reply(struct ofputil_group_desc *,
struct ofpbuf *);
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 9a4eda9..4948f68 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -4708,27 +4708,31 @@ ofproto_group_lookup(const struct ofproto *ofproto,
static void
append_group_stats(struct ofgroup *group, struct list *replies)
{
- struct ofputil_group_stats ogs;
+ struct ofputil_group_stats *ogs;
struct ofproto *ofproto = group->ofproto;
long long int now = time_msec();
int error;
+ ogs = xmalloc(sizeof *ogs + group->n_buckets *
+ sizeof(struct bucket_counter));
+
error = (ofproto->ofproto_class->group_get_stats
- ? ofproto->ofproto_class->group_get_stats(group, &ogs)
+ ? ofproto->ofproto_class->group_get_stats(group, ogs)
: EOPNOTSUPP);
if (error) {
- ogs.ref_count = UINT32_MAX;
- ogs.packet_count = UINT64_MAX;
- ogs.byte_count = UINT64_MAX;
- ogs.n_buckets = group->n_buckets;
- memset(ogs.bucket_stats, 0xff,
- ogs.n_buckets * sizeof *ogs.bucket_stats);
+ struct bucket_counter *bc = (struct bucket_counter *)(ogs + 1);
+
+ ogs->ref_count = UINT32_MAX;
+ ogs->packet_count = UINT64_MAX;
+ ogs->byte_count = UINT64_MAX;
+ ogs->n_buckets = group->n_buckets;
+ memset(bc, 0xff, ogs->n_buckets * sizeof *bc);
}
- ogs.group_id = group->group_id;
- calc_duration(group->created, now, &ogs.duration_sec, &ogs.duration_nsec);
+ ogs->group_id = group->group_id;
+ calc_duration(group->created, now, &ogs->duration_sec, &ogs->duration_nsec);
- ofputil_append_group_stats(replies, &ogs);
+ ofputil_append_group_stats(replies, ogs);
}
static enum ofperr
--
1.8.4
More information about the dev
mailing list