Fix a bug in Openflow group implementation that neither group stats
nor per bucket stats are properly updated.
Reported-by: Marco Canini <marco.can...@acm.org>
Signed-off-by: Andy Zhou <az...@nicira.com>
---
AUTHORS | 1 +
ofproto/ofproto-dpif-xlate.c | 22 ++++++++++++++++------
ofproto/ofproto-dpif-xlate.h | 2 +-
ofproto/ofproto-dpif.c | 39 +++++++++++++++++++++++++++++++++++++++
ofproto/ofproto-dpif.h | 3 +++
5 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index 78640b8..e83af71 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -212,6 +212,7 @@ Len Gao l...@vmware.com
Logan Rosen logatron...@gmail.com
Luca Falavigna dktrkr...@debian.org
Luiz Henrique Ozaki luiz.oz...@gmail.com
+Marco Canini marco.can...@acm.org
Marco d'Itri m...@linux.it
Maxime Brun m.b...@alphalink.fr
Michael A. Collins mike.a.coll...@ark-net.org
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 7b6e9f7..5dd4f93 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -765,20 +765,21 @@ odp_port_is_alive(const struct xlate_ctx *ctx, ofp_port_t
ofp_port)
static const struct ofputil_bucket *
group_first_live_bucket(const struct xlate_ctx *, const struct group_dpif *,
- int depth);
+ int depth, int *index);
static bool
group_is_alive(const struct xlate_ctx *ctx, uint32_t group_id, int depth)
{
struct group_dpif *group;
bool hit;
+ int unused;
hit = group_dpif_lookup(ctx->xbridge->ofproto, group_id, &group);
if (!hit) {
return false;
}
- hit = group_first_live_bucket(ctx, group, depth) != NULL;
+ hit = group_first_live_bucket(ctx, group, depth, &unused) != NULL;
group_dpif_release(group);
return hit;
@@ -807,16 +808,19 @@ bucket_is_alive(const struct xlate_ctx *ctx,
static const struct ofputil_bucket *
group_first_live_bucket(const struct xlate_ctx *ctx,
- const struct group_dpif *group, int depth)
+ const struct group_dpif *group, int depth, int *index)
{
struct ofputil_bucket *bucket;
const struct list *buckets;
+ int i = 0;
group_dpif_get_buckets(group, &buckets);
LIST_FOR_EACH (bucket, list_node, buckets) {
if (bucket_is_alive(ctx, bucket, depth)) {
+ *index = i;
return bucket;
}
+ i++;
}
return NULL;
@@ -825,7 +829,7 @@ group_first_live_bucket(const struct xlate_ctx *ctx,
static const struct ofputil_bucket *
group_best_live_bucket(const struct xlate_ctx *ctx,
const struct group_dpif *group,
- uint32_t basis)
+ uint32_t basis, int *index)
{
const struct ofputil_bucket *best_bucket = NULL;
uint32_t best_score = 0;
@@ -841,6 +845,7 @@ group_best_live_bucket(const struct xlate_ctx *ctx,
if (score >= best_score) {
best_bucket = bucket;
best_score = score;
+ *index = i;
}
}
i++;
@@ -2022,16 +2027,19 @@ xlate_all_group(struct xlate_ctx *ctx, struct
group_dpif *group)
* just before applying the all or indirect group. */
ctx->xin->flow = old_flow;
}
+ group_dpif_credit_stats(group, -1, ctx->xin->resubmit_stats);
}
static void
xlate_ff_group(struct xlate_ctx *ctx, struct group_dpif *group)
{
const struct ofputil_bucket *bucket;
+ int index;
- bucket = group_first_live_bucket(ctx, group, 0);
+ bucket = group_first_live_bucket(ctx, group, 0, &index);
if (bucket) {
xlate_group_bucket(ctx, bucket);
+ group_dpif_credit_stats(group, index, ctx->xin->resubmit_stats);
}
}
@@ -2041,12 +2049,14 @@ xlate_select_group(struct xlate_ctx *ctx, struct
group_dpif *group)
struct flow_wildcards *wc = &ctx->xout->wc;
const struct ofputil_bucket *bucket;
uint32_t basis;
+ int index;
basis = hash_mac(ctx->xin->flow.dl_dst, 0, 0);
- bucket = group_best_live_bucket(ctx, group, basis);
+ bucket = group_best_live_bucket(ctx, group, basis, &index);
if (bucket) {
memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
xlate_group_bucket(ctx, bucket);
+ group_dpif_credit_stats(group, index, ctx->xin->resubmit_stats);
}
}
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index 8b53e10..bb3bad0 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -122,7 +122,7 @@ struct xlate_in {
void (*report_hook)(struct xlate_in *, const char *s, int recurse);
/* If nonnull, flow translation credits the specified statistics to each
- * rule reached through a resubmit or OFPP_TABLE action.
+ * rule reached through a resubmit, OFPP_TABLE or group action.
*
* This is normally null so the client has to set it manually after
* calling xlate_in_init(). */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index cc36a0f..eb510c6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3623,6 +3623,45 @@ group_dpif_get_type(const struct group_dpif *group)
{
return group->up.type;
}
+
+/* Credit flow stats to group. In addition, if 'bucket_index' is a negative
+ * value, credit the stats to all buckets. Otherwise credit the stats to
+ * a single bucket 'bucket_index' refers to. */
+void
+group_dpif_credit_stats(struct group_dpif *group, int bucket_index,
+ const struct dpif_flow_stats *stats)
+{
+ if (!stats) {
+ return;
+ }
+
+ ovs_mutex_lock(&group->stats_mutex);
+
+ group->packet_count += stats->n_packets;
+ group->byte_count += stats->n_bytes;
+
+ if (bucket_index >= 0) { /* Credit to a single bucket. */
+ struct bucket_counter *bucket = &group->bucket_stats[bucket_index];
+
+ if (bucket_index > group->up.n_buckets) {
+ OVS_NOT_REACHED();
+ }
+
+ bucket->packet_count += stats->n_packets;
+ bucket->byte_count += stats->n_bytes;
+ } else { /* Credit to all buckets. */
+ int i;
+
+ for (i = 0; i < group->up.n_buckets; i++) {
+ struct bucket_counter *bucket = &group->bucket_stats[i];
+
+ bucket->packet_count += stats->n_packets;
+ bucket->byte_count += stats->n_bytes;
+ }
+ }
+
+ ovs_mutex_unlock(&group->stats_mutex);
+}
/* Sends 'packet' out 'ofport'.
* May modify 'packet'.
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index ed0aa90..c10fe04 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -128,6 +128,9 @@ void group_dpif_get_buckets(const struct group_dpif *group,
const struct list **buckets);
enum ofp11_group_type group_dpif_get_type(const struct group_dpif *group);
+void group_dpif_credit_stats(struct group_dpif *group, int bucket_index,
+ const struct dpif_flow_stats *stats);
+
bool ofproto_has_vlan_splinters(const struct ofproto_dpif *);
ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *,
ofp_port_t realdev_ofp_port,
--
1.7.9.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
