@@ -531,10 +531,12 @@ OFP_ASSERT(sizeof(struct ofp11_group_stats_request) == 8);
/* Used in group stats replies. */
struct ofp11_bucket_counter {
+ ovs_be32 bucket_id;
+ uint8_t pad2[4];
ovs_be64 packet_count; /* Number of packets processed by bucket. */
ovs_be64 byte_count; /* Number of bytes processed by bucket. */
};
-OFP_ASSERT(sizeof(struct ofp11_bucket_counter) == 16);
+OFP_ASSERT(sizeof(struct ofp11_bucket_counter) == 24);
/* Body of reply to OFPST11_GROUP request */
struct ofp11_group_stats {
@@ -132,6 +132,12 @@ char *parse_ofp_group_mod_str(struct ofputil_group_mod *, int command,
enum ofputil_protocol *usable_protocols)
OVS_WARN_UNUSED_RESULT;
+struct ofputil_bucket_counter {
+ uint32_t bucket_id;
+ uint64_t packet_count; /* Number of packets processed by bucket. */
+ uint64_t byte_count; /* Number of bytes processed by bucket. */
+};
+
/* Group stats reply, independent of protocol. */
struct ofputil_group_stats {
uint32_t group_id; /* Group identifier. */
@@ -141,7 +147,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;
+ struct ofputil_bucket_counter *bucket_stats;
};
struct ofpbuf *ofputil_encode_group_stats_request(enum ofp_version,
@@ -311,6 +311,7 @@ ofputil_group_bucket_counters_to_ofp11(const struct ofputil_group_stats *gs,
int i;
for (i = 0; i < gs->n_buckets; i++) {
+ bucket_cnts[i].bucket_id = htonl(gs->bucket_stats[i].bucket_id);
bucket_cnts[i].packet_count = htonll(gs->bucket_stats[i].packet_count);
bucket_cnts[i].byte_count = htonll(gs->bucket_stats[i].byte_count);
}
@@ -567,6 +568,7 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg,
gs->bucket_stats = xmalloc(gs->n_buckets * sizeof *gs->bucket_stats);
for (i = 0; i < gs->n_buckets; i++) {
+ gs->bucket_stats[i].bucket_id = ntohl(obc[i].bucket_id);
gs->bucket_stats[i].packet_count = ntohll(obc[i].packet_count);
gs->bucket_stats[i].byte_count = ntohll(obc[i].byte_count);
}
@@ -625,9 +627,12 @@ ofputil_group_stats_format(struct ds *s, const struct ofp_header *oh)
for (uint32_t bucket_i = 0; bucket_i < gs.n_buckets; bucket_i++) {
if (gs.bucket_stats[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, ",bucket%"PRIu32":",
+ gs.bucket_stats[bucket_i].bucket_id);
+ 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);
}
}
@@ -4955,9 +4955,10 @@ group_get_stats(const struct ofgroup *group_, struct ofputil_group_stats *ogs)
ogs->packet_count = group->packet_count;
ogs->byte_count = group->byte_count;
- struct bucket_counter *bucket_stats = ogs->bucket_stats;
+ struct ofputil_bucket_counter *bucket_stats = ogs->bucket_stats;
struct ofputil_bucket *bucket;
LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
+ bucket_stats->bucket_id = bucket->bucket_id;
bucket_stats->packet_count = bucket->stats.packet_count;
bucket_stats->byte_count = bucket->stats.byte_count;
bucket_stats++;
@@ -1273,6 +1273,23 @@ OFPST_GROUP reply (OF1.5):
OVS_VSWITCHD_STOP
AT_CLEANUP
+dnl This is really bare-bones.
+dnl It at least checks request and reply serialization and deserialization.
+AT_SETUP([ofproto - group stats to check bucket id (OpenFlow 1.5)])
+OVS_VSWITCHD_START
+AT_DATA([groups.txt], [dnl
+group_id=1234,type=select,selection_method=hash bucket=bucket_id=1,weight:100,actions=output:10
+])
+AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn add-groups br0 groups.txt])
+AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn insert-buckets br0 'group_id=1234, command_bucket_id=last, bucket=bucket_id=8,weight:100,actions=output:10'])
+AT_CHECK([ovs-ofctl -O OpenFlow15 -vwarn dump-group-stats br0 group_id=1234], [0], [stdout])
+AT_CHECK([strip_xids < stdout | sed 's/duration=[[0-9.]]*s/duration=?s/' | sort], [0], [dnl
+ group_id=1234,duration=?s,ref_count=0,packet_count=0,byte_count=0,bucket1:packet_count=0,byte_count=0,bucket8:packet_count=0,byte_count=0
+OFPST_GROUP reply (OF1.5):
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
dnl This found a use-after-free error in bridge destruction in the
dnl presence of groups.
AT_SETUP([ofproto - group add then bridge delete (OpenFlow 1.3)])
The number of the buckets that are dumped start from 0 and is incremented. If the bucket in the group is adjusted, such as delete or added, the order will be disrupted. Then we can't know which bucket the output value corresponds to. So, also dump the bucket id when doing dump-group-stats command. Signed-off-by: solomon <liwei.solomon@gmail.com> --- include/openflow/openflow-1.1.h | 4 +++- include/openvswitch/ofp-group.h | 8 +++++++- lib/ofp-group.c | 11 ++++++++--- ofproto/ofproto-dpif.c | 3 ++- tests/ofproto.at | 17 +++++++++++++++++ 5 files changed, 37 insertions(+), 6 deletions(-)