[ovs-dev] [meters 0/9] Revised meters patch 4/5 proposal

Ben Pfaff blp at nicira.com
Fri Jun 28 16:43:02 UTC 2013


On Fri, Jun 28, 2013 at 09:33:00AM -0700, Ben Pfaff wrote:
> On Fri, Jun 28, 2013 at 04:28:40PM +0000, Rajahalme, Jarno (NSN - FI/Espoo) wrote:
> > 
> > On Jun 28, 2013, at 19:09 , ext Ben Pfaff wrote:
> > 
> > > On Fri, Jun 28, 2013 at 07:17:09AM +0000, Rajahalme, Jarno (NSN - FI/Espoo) wrote:
> > >> 
> > >> On Jun 28, 2013, at 1:19 , ext Ben Pfaff wrote:
> > >>> ofproto: Use another method to find meter_id.
> > >> 
> > >> OK, with the rationale that actions lists are short on average, so
> > >> it does not matter scanning them twice.
> > > 
> > > I agree, but my actual rationale here was that, if it is present, the
> > > meter action will always be the first one, because of the ordering
> > > constraints.
> > 
> > I did not came to think about that, but for flows without a meter
> > instruction this does not matter...
> 
> Not sure what you mean by that.  To expand: if there's a meter
> instruction, it will be the first ofpact, so if we hit something that
> is not a meter instruction we can stop looking.  The actual code is a
> little more general in case we add another instruction that gets
> ordered before the meter instruction, but it should have the same
> effect.

Actually, this is cheap enough that we don't need to save it and pass
it through umpteen layers.  I'm folding in this incremental, and I'll
soon push the appended full patch to master.

Thanks,

Ben.

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 375f844..1fdfb5f 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -198,14 +198,13 @@ static bool rule_is_modifiable(const struct rule *);
 /* OpenFlow. */
 static enum ofperr add_flow(struct ofproto *, struct ofconn *,
                             const struct ofputil_flow_mod *,
-                            const struct ofp_header *, uint32_t meter_id);
+                            const struct ofp_header *);
 static void delete_flow__(struct rule *, struct ofopgroup *,
                           enum ofp_flow_removed_reason);
 static bool handle_openflow(struct ofconn *, const struct ofpbuf *);
 static enum ofperr handle_flow_mod__(struct ofproto *, struct ofconn *,
                                      const struct ofputil_flow_mod *,
-                                     const struct ofp_header *,
-                                     uint32_t meter_id);
+                                     const struct ofp_header *);
 static void calc_duration(long long int start, long long int now,
                           uint32_t *sec, uint32_t *nsec);
 
@@ -1591,7 +1590,7 @@ ofproto_port_del(struct ofproto *ofproto, ofp_port_t ofp_port)
 
 /* Adds a flow to OpenFlow flow table 0 in 'p' that matches 'cls_rule' and
  * performs the 'n_actions' actions in 'actions'.  The new flow will not
- * timeout and its actions may not include a meter action.
+ * timeout.
  *
  * If cls_rule->priority is in the range of priorities supported by OpenFlow
  * (0...65535, inclusive) then the flow will be visible to OpenFlow
@@ -1619,7 +1618,7 @@ ofproto_add_flow(struct ofproto *ofproto, const struct match *match,
         fm.buffer_id = UINT32_MAX;
         fm.ofpacts = xmemdup(ofpacts, ofpacts_len);
         fm.ofpacts_len = ofpacts_len;
-        add_flow(ofproto, NULL, &fm, NULL, 0);
+        add_flow(ofproto, NULL, &fm, NULL);
         free(fm.ofpacts);
     }
 }
@@ -1628,13 +1627,11 @@ ofproto_add_flow(struct ofproto *ofproto, const struct match *match,
  * OFPERR_* OpenFlow error code on failure, or OFPROTO_POSTPONE if the
  * operation cannot be initiated now but may be retried later.
  *
- * 'fm->ofpacts' may not contain a meter action.
- *
  * This is a helper function for in-band control and fail-open. */
 int
 ofproto_flow_mod(struct ofproto *ofproto, const struct ofputil_flow_mod *fm)
 {
-    return handle_flow_mod__(ofproto, NULL, fm, NULL, 0);
+    return handle_flow_mod__(ofproto, NULL, fm, NULL);
 }
 
 /* Searches for a rule with matching criteria exactly equal to 'target' in
@@ -2383,6 +2380,11 @@ reject_slave_controller(struct ofconn *ofconn)
     }
 }
 
+/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
+ * 'ofpacts'.  If found, returns its meter ID; if not, returns 0.
+ *
+ * This function relies on the order of 'ofpacts' being correct (as checked by
+ * ofpacts_verify()). */
 static uint32_t
 find_meter(const struct ofpact ofpacts[], size_t ofpacts_len)
 {
@@ -2405,13 +2407,11 @@ find_meter(const struct ofpact ofpacts[], size_t ofpacts_len)
 /* Checks that the 'ofpacts_len' bytes of actions in 'ofpacts' are
  * appropriate for a packet with the prerequisites satisfied by 'flow'.
  * 'flow' may be temporarily modified, but is restored at return.
- * On success, if 'meter_id' is nonnull, stores the meter ID from any meter
- * action in 'ofpacts' into '*meter_id', or 0 if no meter action is present.
  */
 static enum ofperr
 ofproto_check_ofpacts(struct ofproto *ofproto,
                       const struct ofpact ofpacts[], size_t ofpacts_len,
-                      struct flow *flow, uint32_t *meter_id)
+                      struct flow *flow)
 {
     enum ofperr error;
     uint32_t mid;
@@ -2425,9 +2425,6 @@ ofproto_check_ofpacts(struct ofproto *ofproto,
     if (mid && ofproto_get_provider_meter_id(ofproto, mid) == UINT32_MAX) {
         return OFPERR_OFPMMFC_INVALID_METER;
     }
-    if (meter_id) {
-        *meter_id = mid;
-    }
     return 0;
 }
 
@@ -2477,7 +2474,7 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
     /* Verify actions against packet, then send packet if successful. */
     in_port_.ofp_port = po.in_port;
     flow_extract(payload, 0, 0, NULL, &in_port_, &flow);
-    error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len, &flow, NULL);
+    error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len, &flow);
     if (!error) {
         error = p->ofproto_class->packet_out(p, payload, &flow,
                                              po.ofpacts, po.ofpacts_len);
@@ -3287,8 +3284,7 @@ is_flow_deletion_pending(const struct ofproto *ofproto,
  * if any. */
 static enum ofperr
 add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
-         const struct ofputil_flow_mod *fm, const struct ofp_header *request,
-         uint32_t meter_id)
+         const struct ofputil_flow_mod *fm, const struct ofp_header *request)
 {
     struct oftable *table;
     struct ofopgroup *group;
@@ -3365,7 +3361,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
        and OFPFF13_NO_BYT_COUNTS */
     rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
     rule->ofpacts_len = fm->ofpacts_len;
-    rule->meter_id = meter_id;
+    rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len);
     list_init(&rule->meter_list_node);
     rule->evictable = true;
     rule->eviction_group = NULL;
@@ -3439,7 +3435,7 @@ static enum ofperr
 modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
                const struct ofputil_flow_mod *fm,
                const struct ofp_header *request,
-               uint32_t meter_id, struct list *rules)
+               struct list *rules)
 {
     struct ofopgroup *group;
     struct rule *rule;
@@ -3474,7 +3470,7 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
             op->meter_id = rule->meter_id;
             rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
             rule->ofpacts_len = fm->ofpacts_len;
-            rule->meter_id = meter_id;
+            rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len);
             rule->ofproto->ofproto_class->rule_modify_actions(rule);
         } else {
             ofoperation_complete(op, 0);
@@ -3488,13 +3484,12 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
 static enum ofperr
 modify_flows_add(struct ofproto *ofproto, struct ofconn *ofconn,
                  const struct ofputil_flow_mod *fm,
-                 const struct ofp_header *request,
-                 uint32_t meter_id)
+                 const struct ofp_header *request)
 {
     if (fm->cookie_mask != htonll(0) || fm->new_cookie == htonll(UINT64_MAX)) {
         return 0;
     }
-    return add_flow(ofproto, ofconn, fm, request, meter_id);
+    return add_flow(ofproto, ofconn, fm, request);
 }
 
 /* Implements OFPFC_MODIFY.  Returns 0 on success or an OpenFlow error code on
@@ -3505,8 +3500,7 @@ modify_flows_add(struct ofproto *ofproto, struct ofconn *ofconn,
 static enum ofperr
 modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
                    const struct ofputil_flow_mod *fm,
-                   const struct ofp_header *request,
-                   uint32_t meter_id)
+                   const struct ofp_header *request)
 {
     struct list rules;
     int error;
@@ -3517,9 +3511,9 @@ modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
     if (error) {
         return error;
     } else if (list_is_empty(&rules)) {
-        return modify_flows_add(ofproto, ofconn, fm, request, meter_id);
+        return modify_flows_add(ofproto, ofconn, fm, request);
     } else {
-        return modify_flows__(ofproto, ofconn, fm, request, meter_id, &rules);
+        return modify_flows__(ofproto, ofconn, fm, request, &rules);
     }
 }
 
@@ -3531,7 +3525,7 @@ modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
 static enum ofperr
 modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
                    const struct ofputil_flow_mod *fm,
-                   const struct ofp_header *request, uint32_t meter_id)
+                   const struct ofp_header *request)
 {
     struct list rules;
     int error;
@@ -3543,11 +3537,10 @@ modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
     if (error) {
         return error;
     } else if (list_is_empty(&rules)) {
-        return modify_flows_add(ofproto, ofconn, fm, request, meter_id);
+        return modify_flows_add(ofproto, ofconn, fm, request);
     } else {
         return list_is_singleton(&rules) ? modify_flows__(ofproto, ofconn,
-                                                          fm, request,
-                                                          meter_id, &rules)
+                                                          fm, request, &rules)
                                          : 0;
     }
 }
@@ -3697,7 +3690,6 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
     struct ofpbuf ofpacts;
     enum ofperr error;
     long long int now;
-    uint32_t meter_id = 0;
 
     error = reject_slave_controller(ofconn);
     if (error) {
@@ -3709,11 +3701,11 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
                                     &ofpacts);
     if (!error) {
         error = ofproto_check_ofpacts(ofproto, fm.ofpacts, fm.ofpacts_len,
-                                      &fm.match.flow, &meter_id);
+                                      &fm.match.flow);
     }
 
     if (!error) {
-        error = handle_flow_mod__(ofproto, ofconn, &fm, oh, meter_id);
+        error = handle_flow_mod__(ofproto, ofconn, &fm, oh);
     }
     if (error) {
         goto exit_free_ofpacts;
@@ -3754,7 +3746,7 @@ exit:
 static enum ofperr
 handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn,
                   const struct ofputil_flow_mod *fm,
-                  const struct ofp_header *oh, uint32_t meter_id)
+                  const struct ofp_header *oh)
 {
     if (ofproto->n_pending >= 50) {
         ovs_assert(!list_is_empty(&ofproto->pending));
@@ -3763,13 +3755,13 @@ handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn,
 
     switch (fm->command) {
     case OFPFC_ADD:
-        return add_flow(ofproto, ofconn, fm, oh, meter_id);
+        return add_flow(ofproto, ofconn, fm, oh);
 
     case OFPFC_MODIFY:
-        return modify_flows_loose(ofproto, ofconn, fm, oh, meter_id);
+        return modify_flows_loose(ofproto, ofconn, fm, oh);
 
     case OFPFC_MODIFY_STRICT:
-        return modify_flow_strict(ofproto, ofconn, fm, oh, meter_id);
+        return modify_flow_strict(ofproto, ofconn, fm, oh);
 
     case OFPFC_DELETE:
         return delete_flows_loose(ofproto, ofconn, fm, oh);

--8<--------------------------cut here-------------------------->8--

>From 9cae45dc699f64e85ad68db5f3b2f50758ad6e34 Mon Sep 17 00:00:00 2001
From: Jarno Rajahalme <jarno.rajahalme at nsn.com>
Date: Thu, 27 Jun 2013 01:39:51 +0300
Subject: [PATCH] ofproto: Implement OpenFlow 1.3 meter table.

Signed-off-by: Jarno Rajahalme <jarno.rajahalme at nsn.com>
Co-authored-by: Ben Pfaff <blp at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/ofp-actions.c          |    8 +-
 lib/rconn.c                |   14 +-
 ofproto/ofproto-dpif.c     |    4 +
 ofproto/ofproto-provider.h |   57 ++++++-
 ofproto/ofproto.c          |  406 +++++++++++++++++++++++++++++++++++++++++++-
 ofproto/ofproto.h          |    3 +
 6 files changed, 473 insertions(+), 19 deletions(-)

diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 14b1961..e1b44d8 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1289,10 +1289,16 @@ ofpact_check__(const struct ofpact *a, struct flow *flow, ofp_port_t max_ports)
 
     case OFPACT_CLEAR_ACTIONS:
     case OFPACT_WRITE_METADATA:
-    case OFPACT_METER:
     case OFPACT_GOTO_TABLE:
         return 0;
 
+    case OFPACT_METER: {
+        uint32_t mid = ofpact_get_METER(a)->meter_id;
+        if (mid == 0 || mid > OFPM13_MAX) {
+            return OFPERR_OFPMMFC_INVALID_METER;
+        }
+        return 0;
+    }
     default:
         NOT_REACHED();
     }
diff --git a/lib/rconn.c b/lib/rconn.c
index 4922a5c..ffd2738 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -1137,19 +1137,12 @@ is_admitted_msg(const struct ofpbuf *b)
     case OFPTYPE_QUEUE_GET_CONFIG_REPLY:
     case OFPTYPE_GET_ASYNC_REQUEST:
     case OFPTYPE_GET_ASYNC_REPLY:
-    case OFPTYPE_METER_MOD:
     case OFPTYPE_GROUP_REQUEST:
     case OFPTYPE_GROUP_REPLY:
     case OFPTYPE_GROUP_DESC_REQUEST:
     case OFPTYPE_GROUP_DESC_REPLY:
     case OFPTYPE_GROUP_FEATURES_REQUEST:
     case OFPTYPE_GROUP_FEATURES_REPLY:
-    case OFPTYPE_METER_REQUEST:
-    case OFPTYPE_METER_REPLY:
-    case OFPTYPE_METER_CONFIG_REQUEST:
-    case OFPTYPE_METER_CONFIG_REPLY:
-    case OFPTYPE_METER_FEATURES_REQUEST:
-    case OFPTYPE_METER_FEATURES_REPLY:
     case OFPTYPE_TABLE_FEATURES_REQUEST:
     case OFPTYPE_TABLE_FEATURES_REPLY:
         return false;
@@ -1160,6 +1153,7 @@ is_admitted_msg(const struct ofpbuf *b)
     case OFPTYPE_PACKET_OUT:
     case OFPTYPE_FLOW_MOD:
     case OFPTYPE_PORT_MOD:
+    case OFPTYPE_METER_MOD:
     case OFPTYPE_BARRIER_REQUEST:
     case OFPTYPE_BARRIER_REPLY:
     case OFPTYPE_DESC_STATS_REQUEST:
@@ -1176,6 +1170,12 @@ is_admitted_msg(const struct ofpbuf *b)
     case OFPTYPE_QUEUE_STATS_REPLY:
     case OFPTYPE_PORT_DESC_STATS_REQUEST:
     case OFPTYPE_PORT_DESC_STATS_REPLY:
+    case OFPTYPE_METER_REQUEST:
+    case OFPTYPE_METER_REPLY:
+    case OFPTYPE_METER_CONFIG_REQUEST:
+    case OFPTYPE_METER_CONFIG_REPLY:
+    case OFPTYPE_METER_FEATURES_REQUEST:
+    case OFPTYPE_METER_FEATURES_REPLY:
     case OFPTYPE_ROLE_REQUEST:
     case OFPTYPE_ROLE_REPLY:
     case OFPTYPE_SET_FLOW_FORMAT:
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index e6f4ca3..4293824 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -6830,4 +6830,8 @@ const struct ofproto_class ofproto_dpif_class = {
     forward_bpdu_changed,
     set_mac_table_config,
     set_realdev,
+    NULL,                       /* meter_get_features */
+    NULL,                       /* meter_set */
+    NULL,                       /* meter_get */
+    NULL,                       /* meter_del */
 };
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index 1655c3a..7fc5e03 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -35,6 +35,7 @@ struct match;
 struct ofpact;
 struct ofputil_flow_mod;
 struct bfd_cfg;
+struct meter;
 
 /* An OpenFlow switch.
  *
@@ -76,6 +77,13 @@ struct ofproto {
      * These flows should all be present in tables. */
     struct list expirable;      /* Expirable 'struct rule"s in all tables. */
 
+    /* Meter table.
+     * OpenFlow meters start at 1.  To avoid confusion we leave the first
+     * pointer in the array un-used, and index directly with the OpenFlow
+     * meter_id. */
+    struct ofputil_meter_features meter_features;
+    struct meter **meters; /* 'meter_features.max_meter' + 1 pointers. */
+
     /* OpenFlow connections. */
     struct connmgr *connmgr;
 
@@ -224,6 +232,9 @@ struct rule {
     struct ofpact *ofpacts;      /* Sequence of "struct ofpacts". */
     unsigned int ofpacts_len;    /* Size of 'ofpacts', in bytes. */
 
+    uint32_t meter_id;           /* Non-zero OF meter_id, or zero. */
+    struct list meter_list_node; /* In owning meter's 'rules' list. */
+
     /* Flow monitors. */
     enum nx_flow_monitor_flags monitor_flags;
     uint64_t add_seqno;         /* Sequence number when added. */
@@ -1321,10 +1332,52 @@ struct ofproto_class {
      * If 'realdev_ofp_port' is zero, then this function deconfigures 'ofport'
      * as a VLAN splinter port.
      *
-     * This function should be NULL if a an implementation does not support
-     * it. */
+     * This function should be NULL if an implementation does not support it.
+     */
     int (*set_realdev)(struct ofport *ofport,
                        ofp_port_t realdev_ofp_port, int vid);
+
+/* ## ------------------------ ## */
+/* ## OpenFlow meter functions ## */
+/* ## ------------------------ ## */
+
+    /* These functions should be NULL if an implementation does not support
+     * them.  They must be all null or all non-null.. */
+
+    /* Initializes 'features' to describe the metering features supported by
+     * 'ofproto'. */
+    void (*meter_get_features)(const struct ofproto *ofproto,
+                               struct ofputil_meter_features *features);
+
+    /* If '*id' is UINT32_MAX, adds a new meter with the given 'config'.  On
+     * success the function must store a provider meter ID other than
+     * UINT32_MAX in '*id'.  All further references to the meter will be made
+     * with the returned provider meter id rather than the OpenFlow meter id.
+     * The caller does not try to interpret the provider meter id, giving the
+     * implementation the freedom to either use the OpenFlow meter_id value
+     * provided in the meter configuration, or any other value suitable for the
+     * implementation.
+     *
+     * If '*id' is a value other than UINT32_MAX, modifies the existing meter
+     * with that meter provider ID to have configuration 'config'.  On failure,
+     * the existing meter configuration is left intact.  Regardless of success,
+     * any change to '*id' updates the provider meter id used for this
+     * meter. */
+    enum ofperr (*meter_set)(struct ofproto *ofproto, ofproto_meter_id *id,
+                             const struct ofputil_meter_config *config);
+
+    /* Gets the meter and meter band packet and byte counts for maximum of
+     * 'stats->n_bands' bands for the meter with provider ID 'id' within
+     * 'ofproto'.  The caller fills in the other stats values.  The band stats
+     * are copied to memory at 'stats->bands' provided by the caller.  The
+     * number of returned band stats is returned in 'stats->n_bands'. */
+    enum ofperr (*meter_get)(const struct ofproto *ofproto,
+                             ofproto_meter_id id,
+                             struct ofputil_meter_stats *stats);
+
+    /* Deletes a meter, making the 'ofproto_meter_id' invalid for any
+     * further calls. */
+    void (*meter_del)(struct ofproto *, ofproto_meter_id);
 };
 
 extern const struct ofproto_class ofproto_dpif_class;
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 7a844ba..1fdfb5f 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -126,6 +126,7 @@ struct ofoperation {
     /* OFOPERATION_MODIFY: The old actions, if the actions are changing. */
     struct ofpact *ofpacts;
     size_t ofpacts_len;
+    uint32_t meter_id;
 
     /* OFOPERATION_DELETE. */
     enum ofp_flow_removed_reason reason; /* Reason flow was removed. */
@@ -466,6 +467,16 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ofproto->datapath_id = pick_datapath_id(ofproto);
     init_ports(ofproto);
 
+    /* Initialize meters table. */
+    if (ofproto->ofproto_class->meter_get_features) {
+        ofproto->ofproto_class->meter_get_features(ofproto,
+                                                   &ofproto->meter_features);
+    } else {
+        memset(&ofproto->meter_features, 0, sizeof ofproto->meter_features);
+    }
+    ofproto->meters = xzalloc((ofproto->meter_features.max_meters + 1)
+                              * sizeof(struct meter *));
+
     *ofprotop = ofproto;
     return 0;
 }
@@ -2369,6 +2380,54 @@ reject_slave_controller(struct ofconn *ofconn)
     }
 }
 
+/* Finds the OFPACT_METER action, if any, in the 'ofpacts_len' bytes of
+ * 'ofpacts'.  If found, returns its meter ID; if not, returns 0.
+ *
+ * This function relies on the order of 'ofpacts' being correct (as checked by
+ * ofpacts_verify()). */
+static uint32_t
+find_meter(const struct ofpact ofpacts[], size_t ofpacts_len)
+{
+    const struct ofpact *a;
+
+    OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+        enum ovs_instruction_type inst;
+
+        inst = ovs_instruction_type_from_ofpact_type(a->type);
+        if (a->type == OFPACT_METER) {
+            return ofpact_get_METER(a)->meter_id;
+        } else if (inst > OVSINST_OFPIT13_METER) {
+            break;
+        }
+    }
+
+    return 0;
+}
+
+/* Checks that the 'ofpacts_len' bytes of actions in 'ofpacts' are
+ * appropriate for a packet with the prerequisites satisfied by 'flow'.
+ * 'flow' may be temporarily modified, but is restored at return.
+ */
+static enum ofperr
+ofproto_check_ofpacts(struct ofproto *ofproto,
+                      const struct ofpact ofpacts[], size_t ofpacts_len,
+                      struct flow *flow)
+{
+    enum ofperr error;
+    uint32_t mid;
+
+    error = ofpacts_check(ofpacts, ofpacts_len, flow, ofproto->max_ports);
+    if (error) {
+        return error;
+    }
+
+    mid = find_meter(ofpacts, ofpacts_len);
+    if (mid && ofproto_get_provider_meter_id(ofproto, mid) == UINT32_MAX) {
+        return OFPERR_OFPMMFC_INVALID_METER;
+    }
+    return 0;
+}
+
 static enum ofperr
 handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
 {
@@ -2415,7 +2474,7 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
     /* Verify actions against packet, then send packet if successful. */
     in_port_.ofp_port = po.in_port;
     flow_extract(payload, 0, 0, NULL, &in_port_, &flow);
-    error = ofpacts_check(po.ofpacts, po.ofpacts_len, &flow, p->max_ports);
+    error = ofproto_check_ofpacts(p, po.ofpacts, po.ofpacts_len, &flow);
     if (!error) {
         error = p->ofproto_class->packet_out(p, payload, &flow,
                                              po.ofpacts, po.ofpacts_len);
@@ -3302,6 +3361,8 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
        and OFPFF13_NO_BYT_COUNTS */
     rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
     rule->ofpacts_len = fm->ofpacts_len;
+    rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len);
+    list_init(&rule->meter_list_node);
     rule->evictable = true;
     rule->eviction_group = NULL;
     list_init(&rule->expirable);
@@ -3373,7 +3434,8 @@ exit:
 static enum ofperr
 modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
                const struct ofputil_flow_mod *fm,
-               const struct ofp_header *request, struct list *rules)
+               const struct ofp_header *request,
+               struct list *rules)
 {
     struct ofopgroup *group;
     struct rule *rule;
@@ -3405,8 +3467,10 @@ modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
         if (actions_changed) {
             op->ofpacts = rule->ofpacts;
             op->ofpacts_len = rule->ofpacts_len;
+            op->meter_id = rule->meter_id;
             rule->ofpacts = xmemdup(fm->ofpacts, fm->ofpacts_len);
             rule->ofpacts_len = fm->ofpacts_len;
+            rule->meter_id = find_meter(rule->ofpacts, rule->ofpacts_len);
             rule->ofproto->ofproto_class->rule_modify_actions(rule);
         } else {
             ofoperation_complete(op, 0);
@@ -3636,9 +3700,10 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
     error = ofputil_decode_flow_mod(&fm, oh, ofconn_get_protocol(ofconn),
                                     &ofpacts);
     if (!error) {
-        error = ofpacts_check(fm.ofpacts, fm.ofpacts_len,
-                              &fm.match.flow, ofproto->max_ports);
+        error = ofproto_check_ofpacts(ofproto, fm.ofpacts, fm.ofpacts_len,
+                                      &fm.match.flow);
     }
+
     if (!error) {
         error = handle_flow_mod__(ofproto, ofconn, &fm, oh);
     }
@@ -4117,6 +4182,311 @@ handle_flow_monitor_cancel(struct ofconn *ofconn, const struct ofp_header *oh)
     return 0;
 }
 
+/* Meters implementation.
+ *
+ * Meter table entry, indexed by the OpenFlow meter_id.
+ * These are always dynamically allocated to allocate enough space for
+ * the bands.
+ * 'created' is used to compute the duration for meter stats.
+ * 'list rules' is needed so that we can delete the dependent rules when the
+ * meter table entry is deleted.
+ * 'provider_meter_id' is for the provider's private use.
+ */
+struct meter {
+    long long int created;      /* Time created. */
+    struct list rules;          /* List of "struct rule_dpif"s. */
+    ofproto_meter_id provider_meter_id;
+    uint16_t flags;             /* Meter flags. */
+    uint16_t n_bands;           /* Number of meter bands. */
+    struct ofputil_meter_band *bands;
+};
+
+/*
+ * This is used in instruction validation at flow set-up time,
+ * as flows may not use non-existing meters.
+ * This is also used by ofproto-providers to translate OpenFlow meter_ids
+ * in METER instructions to the corresponding provider meter IDs.
+ * Return value of UINT32_MAX signifies an invalid meter.
+ */
+uint32_t
+ofproto_get_provider_meter_id(const struct ofproto * ofproto,
+                              uint32_t of_meter_id)
+{
+    if (of_meter_id && of_meter_id <= ofproto->meter_features.max_meters) {
+        const struct meter *meter = ofproto->meters[of_meter_id];
+        if (meter) {
+            return meter->provider_meter_id.uint32;
+        }
+    }
+    return UINT32_MAX;
+}
+
+static void
+meter_update(struct meter *meter, const struct ofputil_meter_config *config)
+{
+    free(meter->bands);
+
+    meter->flags = config->flags;
+    meter->n_bands = config->n_bands;
+    meter->bands = xmemdup(config->bands,
+                           config->n_bands * sizeof *meter->bands);
+}
+
+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
+handle_add_meter(struct ofproto *ofproto, struct ofputil_meter_mod *mm)
+{
+    ofproto_meter_id provider_meter_id = { UINT32_MAX };
+    struct meter **meterp = &ofproto->meters[mm->meter.meter_id];
+    enum ofperr error;
+
+    if (*meterp) {
+        return OFPERR_OFPMMFC_METER_EXISTS;
+    }
+
+    error = ofproto->ofproto_class->meter_set(ofproto, &provider_meter_id,
+                                              &mm->meter);
+    if (!error) {
+        ovs_assert(provider_meter_id.uint32 != UINT32_MAX);
+        *meterp = meter_create(&mm->meter, provider_meter_id);
+    }
+    return 0;
+}
+
+static enum ofperr
+handle_modify_meter(struct ofproto *ofproto, struct ofputil_meter_mod *mm)
+{
+    struct meter *meter = ofproto->meters[mm->meter.meter_id];
+    enum ofperr error;
+
+    if (!meter) {
+        return OFPERR_OFPMMFC_UNKNOWN_METER;
+    }
+
+    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(meter, &mm->meter);
+    }
+    return error;
+}
+
+static enum ofperr
+handle_delete_meter(struct ofconn *ofconn, const struct ofp_header *oh,
+                    struct ofputil_meter_mod *mm)
+{
+    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
+    uint32_t meter_id = mm->meter.meter_id;
+    uint32_t first, last;
+    struct list rules;
+
+    if (meter_id == OFPM13_ALL) {
+        first = 1;
+        last = ofproto->meter_features.max_meters;
+    } else {
+        if (!meter_id || meter_id > ofproto->meter_features.max_meters) {
+            return 0;
+        }
+        first = last = meter_id;
+    }
+
+    /* First delete the rules that use this meter.  If any of those rules are
+     * currently being modified, postpone the whole operation until later. */
+    list_init(&rules);
+    for (meter_id = first; meter_id <= last; ++meter_id) {
+        struct meter *meter = ofproto->meters[meter_id];
+        if (meter && !list_is_empty(&meter->rules)) {
+            struct rule *rule;
+
+            LIST_FOR_EACH (rule, meter_list_node, &meter->rules) {
+                if (rule->pending) {
+                    return OFPROTO_POSTPONE;
+                }
+                list_push_back(&rules, &rule->ofproto_node);
+            }
+        }
+    }
+    if (!list_is_empty(&rules)) {
+        delete_flows__(ofproto, ofconn, oh, &rules, OFPRR_METER_DELETE);
+    }
+
+    /* Delete the meters. */
+    for (meter_id = first; meter_id <= last; ++meter_id) {
+        struct meter *meter = ofproto->meters[meter_id];
+        if (meter) {
+            ofproto->meters[meter_id] = NULL;
+            ofproto->ofproto_class->meter_del(ofproto,
+                                              meter->provider_meter_id);
+            free(meter->bands);
+            free(meter);
+        }
+    }
+
+    return 0;
+}
+
+static enum ofperr
+handle_meter_mod(struct ofconn *ofconn, const struct ofp_header *oh)
+{
+    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
+    struct ofputil_meter_mod mm;
+    uint64_t bands_stub[256 / 8];
+    struct ofpbuf bands;
+    uint32_t meter_id;
+    enum ofperr error;
+
+    error = reject_slave_controller(ofconn);
+    if (error) {
+        return error;
+    }
+
+    ofpbuf_use_stub(&bands, bands_stub, sizeof bands_stub);
+
+    error = ofputil_decode_meter_mod(oh, &mm, &bands);
+    if (error) {
+        goto exit_free_bands;
+    }
+
+    meter_id = mm.meter.meter_id;
+
+    if (mm.command != OFPMC13_DELETE) {
+        /* Fails also when meters are not implemented by the provider. */
+        if (!meter_id || meter_id > ofproto->meter_features.max_meters) {
+            error = OFPERR_OFPMMFC_INVALID_METER;
+            goto exit_free_bands;
+        }
+        if (mm.meter.n_bands > ofproto->meter_features.max_bands) {
+            error = OFPERR_OFPMMFC_OUT_OF_BANDS;
+            goto exit_free_bands;
+        }
+    }
+
+    switch (mm.command) {
+    case OFPMC13_ADD:
+        error = handle_add_meter(ofproto, &mm);
+        break;
+
+    case OFPMC13_MODIFY:
+        error = handle_modify_meter(ofproto, &mm);
+        break;
+
+    case OFPMC13_DELETE:
+        error = handle_delete_meter(ofconn, oh, &mm);
+        break;
+
+    default:
+        error = OFPERR_OFPMMFC_BAD_COMMAND;
+        break;
+    }
+
+exit_free_bands:
+    ofpbuf_uninit(&bands);
+    return error;
+}
+
+static enum ofperr
+handle_meter_features_request(struct ofconn *ofconn,
+                              const struct ofp_header *request)
+{
+    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
+    struct ofputil_meter_features features;
+    struct ofpbuf *b;
+
+    if (ofproto->ofproto_class->meter_get_features) {
+        ofproto->ofproto_class->meter_get_features(ofproto, &features);
+    } else {
+        memset(&features, 0, sizeof features);
+    }
+    b = ofputil_encode_meter_features_reply(&features, request);
+
+    ofconn_send_reply(ofconn, b);
+    return 0;
+}
+
+static enum ofperr
+handle_meter_request(struct ofconn *ofconn, const struct ofp_header *request,
+                     enum ofptype type)
+{
+    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
+    struct list replies;
+    uint64_t bands_stub[256 / 8];
+    struct ofpbuf bands;
+    uint32_t meter_id, first, last;
+
+    ofputil_decode_meter_request(request, &meter_id);
+
+    if (meter_id == OFPM13_ALL) {
+        first = 1;
+        last = ofproto->meter_features.max_meters;
+    } else {
+        if (!meter_id || meter_id > ofproto->meter_features.max_meters ||
+            !ofproto->meters[meter_id]) {
+            return OFPERR_OFPMMFC_UNKNOWN_METER;
+        }
+        first = last = meter_id;
+    }
+
+    ofpbuf_use_stub(&bands, bands_stub, sizeof bands_stub);
+    ofpmp_init(&replies, request);
+
+    for (meter_id = first; meter_id <= last; ++meter_id) {
+        struct meter *meter = ofproto->meters[meter_id];
+        if (!meter) {
+            continue; /* Skip non-existing meters. */
+        }
+        if (type == OFPTYPE_METER_REQUEST) {
+            struct ofputil_meter_stats stats;
+
+            stats.meter_id = meter_id;
+
+            /* Provider sets the packet and byte counts, we do the rest. */
+            stats.flow_count = list_size(&meter->rules);
+            calc_duration(meter->created, time_msec(),
+                          &stats.duration_sec, &stats.duration_nsec);
+            stats.n_bands = meter->n_bands;
+            ofpbuf_clear(&bands);
+            stats.bands
+                = ofpbuf_put_uninit(&bands,
+                                    meter->n_bands * sizeof *stats.bands);
+
+            if (!ofproto->ofproto_class->meter_get(ofproto,
+                                                   meter->provider_meter_id,
+                                                   &stats)) {
+                ofputil_append_meter_stats(&replies, &stats);
+            }
+        } else { /* type == OFPTYPE_METER_CONFIG_REQUEST */
+            struct ofputil_meter_config config;
+
+            config.meter_id = meter_id;
+            config.flags = meter->flags;
+            config.n_bands = meter->n_bands;
+            config.bands = meter->bands;
+            ofputil_append_meter_config(&replies, &config);
+        }
+    }
+
+    ofconn_send_replies(ofconn, &replies);
+    ofpbuf_uninit(&bands);
+    return 0;
+}
+
 static enum ofperr
 handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
 {
@@ -4152,6 +4522,9 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
     case OFPTYPE_FLOW_MOD:
         return handle_flow_mod(ofconn, oh);
 
+    case OFPTYPE_METER_MOD:
+        return handle_meter_mod(ofconn, oh);
+
     case OFPTYPE_BARRIER_REQUEST:
         return handle_barrier_request(ofconn, oh);
 
@@ -4210,16 +4583,19 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
     case OFPTYPE_FLOW_MONITOR_STATS_REQUEST:
         return handle_flow_monitor_request(ofconn, oh);
 
+    case OFPTYPE_METER_REQUEST:
+    case OFPTYPE_METER_CONFIG_REQUEST:
+        return handle_meter_request(ofconn, oh, type);
+
+    case OFPTYPE_METER_FEATURES_REQUEST:
+        return handle_meter_features_request(ofconn, oh);
+
         /* FIXME: Change the following once they are implemented: */
     case OFPTYPE_QUEUE_GET_CONFIG_REQUEST:
     case OFPTYPE_GET_ASYNC_REQUEST:
-    case OFPTYPE_METER_MOD:
     case OFPTYPE_GROUP_REQUEST:
     case OFPTYPE_GROUP_DESC_REQUEST:
     case OFPTYPE_GROUP_FEATURES_REQUEST:
-    case OFPTYPE_METER_REQUEST:
-    case OFPTYPE_METER_CONFIG_REQUEST:
-    case OFPTYPE_METER_FEATURES_REQUEST:
     case OFPTYPE_TABLE_FEATURES_REQUEST:
         return OFPERR_OFPBRC_BAD_TYPE;
 
@@ -4972,11 +5348,17 @@ oftable_remove_rule(struct rule *rule)
     struct oftable *table = &ofproto->tables[rule->table_id];
 
     classifier_remove(&table->cls, &rule->cr);
+    if (rule->meter_id) {
+        list_remove(&rule->meter_list_node);
+    }
     cookies_remove(ofproto, rule);
     eviction_group_remove_rule(rule);
     if (!list_is_empty(&rule->expirable)) {
         list_remove(&rule->expirable);
     }
+    if (!list_is_empty(&rule->meter_list_node)) {
+        list_remove(&rule->meter_list_node);
+    }
 }
 
 /* Inserts 'rule' into its oftable.  Removes any existing rule from 'rule''s
@@ -4994,9 +5376,15 @@ oftable_replace_rule(struct rule *rule)
         list_insert(&ofproto->expirable, &rule->expirable);
     }
     cookies_insert(ofproto, rule);
-
+    if (rule->meter_id) {
+        struct meter *meter = ofproto->meters[rule->meter_id];
+        list_insert(&meter->rules, &rule->meter_list_node);
+    }
     victim = rule_from_cls_rule(classifier_replace(&table->cls, &rule->cr));
     if (victim) {
+        if (victim->meter_id) {
+            list_remove(&victim->meter_list_node);
+        }
         cookies_remove(ofproto, victim);
 
         if (!list_is_empty(&victim->expirable)) {
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 8c90b86..792df89 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -430,6 +430,9 @@ bool ofproto_has_vlan_usage_changed(const struct ofproto *);
 int ofproto_port_set_realdev(struct ofproto *, ofp_port_t vlandev_ofp_port,
                              ofp_port_t realdev_ofp_port, int vid);
 
+uint32_t ofproto_get_provider_meter_id(const struct ofproto *,
+                                       uint32_t of_meter_id);
+
 #ifdef  __cplusplus
 }
 #endif
-- 
1.7.2.5




More information about the dev mailing list