[ovs-dev] [PATCH 1/2] ofproto-dpif: implement group and group bucket stats

Andy Zhou azhou at nicira.com
Wed Apr 9 23:43:32 UTC 2014


Fix a bug in Openflow group implementation that neither group stats
nor per bucket stats are properly updated.

Reported-by: Marco Canini <marco.canini at acm.org>
Signed-off-by: Andy Zhou <azhou at 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                 leng at vmware.com
 Logan Rosen             logatronico at gmail.com
 Luca Falavigna          dktrkranz at debian.org
 Luiz Henrique Ozaki     luiz.ozaki at gmail.com
+Marco Canini            marco.canini at acm.org
 Marco d'Itri            md at Linux.IT
 Maxime Brun             m.brun at alphalink.fr
 Michael A. Collins      mike.a.collins at 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




More information about the dev mailing list