[ovs-dev] [meters 6/9] ofproto: Allocate meter bands separately from the meters themselves.

[Ben Pfaff]

This simplifies updates since nothing has to move except the meter bands.

Signed-off-by: Ben Pfaff <b...@nicira.com>
---
 ofproto/ofproto.c |   96 ++++++++++++++++------------------------------------
 1 files changed, 30 insertions(+), 66 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 341cf10..f5be583 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -4207,7 +4207,7 @@ struct meter {
     ofproto_meter_id provider_meter_id;
     uint16_t flags;             /* Meter flags. */
     uint16_t n_bands;           /* Number of meter bands. */
-    struct ofputil_meter_band bands[0];
+    struct ofputil_meter_band *bands;
 };
 
 /*
@@ -4230,51 +4230,31 @@ ofproto_get_provider_meter_id(const struct ofproto * 
ofproto,
     return UINT32_MAX;
 }
 
-static struct meter *
-meter_create(const struct ofputil_meter_config *config)
+static void
+meter_update(struct meter *meter, const struct ofputil_meter_config *config)
 {
-    struct meter *meter
-        = xmalloc(sizeof *meter + config->n_bands * sizeof *meter->bands);
-    int i;
+    free(meter->bands);
 
-    if (meter) {
-        meter->provider_meter_id.uint32 = UINT32_MAX;
-        meter->flags = config->flags;
-        meter->n_bands = config->n_bands;
-        meter->created = time_msec();
-        list_init(&meter->rules);
-        for (i = 0; i < config->n_bands; ++i) {
-            meter->bands[i] = config->bands[i];
-        }
-    }
-    return meter;
+    meter->flags = config->flags;
+    meter->n_bands = config->n_bands;
+    meter->bands = xmemdup(config->bands,
+                           config->n_bands * sizeof *meter->bands);
 }
 
-static void
-meter_update(struct ofproto *ofproto, struct meter *meter,
-             struct meter *new_meter,
-             const struct ofputil_meter_config *config)
-{
-    if (new_meter) {
-        new_meter->provider_meter_id = meter->provider_meter_id;
-        /* Move rules list over. */
-        if (list_is_empty(&meter->rules)) {
-            list_init(&new_meter->rules);
-        } else {
-            new_meter->rules = meter->rules;
-            list_moved(&new_meter->rules);
-        }
-        ofproto->meters[config->meter_id] = new_meter;
-        free(meter);
-    } else {
-        int i;
-        /* Update existing meter. */
-        meter->flags = config->flags;
-        meter->n_bands = config->n_bands;
-        for (i = 0; i < config->n_bands; ++i) {
-            meter->bands[i] = config->bands[i];
-        }
-    }
+static struct meter *
+meter_create(const struct ofputil_meter_config *config,
+             ofproto_meter_id provider_meter_id)
+{
+    struct meter *meter;
+
+    meter = xzalloc(sizeof *meter);
+    meter->provider_meter_id = provider_meter_id;
+    meter->created = time_msec();
+    list_init(&meter->rules);
+
+    meter_update(meter, config);
+
+    return meter;
 }
 
 static enum ofperr
@@ -4315,56 +4295,39 @@ handle_meter_mod(struct ofconn *ofconn, const struct 
ofp_header *oh)
 
     switch (mm.command) {
     case OFPMC13_ADD: {
-        struct meter *meter;
+        ofproto_meter_id provider_meter_id = { UINT32_MAX };
 
         if (ofproto->meters[meter_id]) {
             error = OFPERR_OFPMMFC_METER_EXISTS;
             break;
         }
 
-        meter = meter_create(&mm.meter);
-        if (!meter) {
-            error = OFPERR_OFPMMFC_OUT_OF_METERS;
-            break;
-        }
-
-        error = ofproto->ofproto_class->meter_set(ofproto,
-                                                  &meter->provider_meter_id,
+        error = ofproto->ofproto_class->meter_set(ofproto, &provider_meter_id,
                                                   &mm.meter);
         if (!error) {
-            ofproto->meters[meter_id] = meter;
+            ovs_assert(provider_meter_id.uint32 != UINT32_MAX);
+            ofproto->meters[meter_id] = meter_create(&mm.meter,
+                                                     provider_meter_id);
             if (ofproto->last_meter < meter_id) {
                 ofproto->last_meter = meter_id;
             }
-        } else {
-            free(meter);
         }
         break;
     }
     case OFPMC13_MODIFY: {
         struct meter *meter = ofproto->meters[meter_id];
-        struct meter *new_meter = NULL;
 
         if (!meter) {
             error = OFPERR_OFPMMFC_UNKNOWN_METER;
             break;
         }
-        /* Realloc meter if number of bands increases. */
-        if (mm.meter.n_bands > meter->n_bands) {
-            new_meter = meter_create(&mm.meter);
-            if (!new_meter) {
-                error = OFPERR_OFPMMFC_OUT_OF_METERS;
-                break;
-            }
-        }
 
         error = ofproto->ofproto_class->meter_set(ofproto,
                                                   &meter->provider_meter_id,
                                                   &mm.meter);
+        ovs_assert(meter->provider_meter_id.uint32 != UINT32_MAX);
         if (!error) {
-            meter_update(ofproto, meter, new_meter, &mm.meter);
-        } else if (new_meter) {
-            free(new_meter);
+            meter_update(meter, &mm.meter);
         }
         break;
     }
@@ -4421,6 +4384,7 @@ handle_meter_mod(struct ofconn *ofconn, const struct 
ofp_header *oh)
             ofproto->meters[meter_id] = NULL;
             ofproto->ofproto_class->meter_del(ofproto,
                                               meter->provider_meter_id);
+            free(meter->bands);
             free(meter);
         }
 
-- 
1.7.2.5

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev