diff mbox

[ovs-dev,v3,3/4] ofproto: Meter translation.

Message ID 1448340875-2070-4-git-send-email-jarno@ovn.org
State Changes Requested
Headers show

Commit Message

Jarno Rajahalme Nov. 24, 2015, 4:54 a.m. UTC
Translate OpenFlow METER instructions to datapath meter actions.

Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
---
 lib/dpif.c                   | 38 +++++++++++++++++++++++++++++------
 lib/ofp-actions.c            |  1 +
 lib/ofp-actions.h            |  1 +
 ofproto/ofproto-dpif-xlate.c | 11 ++++++++++-
 ofproto/ofproto-provider.h   |  3 +--
 ofproto/ofproto.c            | 47 +++++++++++++++++++++++++-------------------
 6 files changed, 72 insertions(+), 29 deletions(-)
diff mbox

Patch

diff --git a/lib/dpif.c b/lib/dpif.c
index 7389c8f..65629cf 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1085,6 +1085,7 @@  dpif_flow_dump_next(struct dpif_flow_dump_thread *thread,
 struct dpif_execute_helper_aux {
     struct dpif *dpif;
     int error;
+    const struct nlattr *meter_action; /* Non-NULL, if have a meter action. */
 };
 
 /* This is called for actions that need the context of the datapath to be
@@ -1100,6 +1101,13 @@  dpif_execute_helper_cb(void *aux_, struct dp_packet **packets, int cnt,
     ovs_assert(cnt == 1);
 
     switch ((enum ovs_action_attr)type) {
+    case OVS_ACTION_ATTR_METER:
+        /* Maintain a pointer to the first meter action seen. */
+        if (!aux->meter_action) {
+            aux->meter_action = action;
+        }
+        return cnt;
+
     case OVS_ACTION_ATTR_CT:
     case OVS_ACTION_ATTR_OUTPUT:
     case OVS_ACTION_ATTR_TUNNEL_PUSH:
@@ -1111,12 +1119,28 @@  dpif_execute_helper_cb(void *aux_, struct dp_packet **packets, int cnt,
         uint64_t stub[256 / 8];
         struct pkt_metadata *md = &packet->md;
 
-        if (md->tunnel.ip_dst) {
+        if (md->tunnel.ip_dst || aux->meter_action) {
+            ofpbuf_use_stub(&execute_actions, stub, sizeof stub);
+
+            if (aux->meter_action) {
+                const struct nlattr *a = aux->meter_action;
+
+                do {
+                    ofpbuf_put(&execute_actions, a, NLA_ALIGN(a->nla_len));
+                    /* Find next meter action before 'action', if any. */
+                    do {
+                        a = nl_attr_next(a);
+                    } while (a != action &&
+                             nl_attr_type(a) != OVS_ACTION_ATTR_METER);
+                } while (a != action);
+            }
+
             /* The Linux kernel datapath throws away the tunnel information
              * that we supply as metadata.  We have to use a "set" action to
              * supply it. */
-            ofpbuf_use_stub(&execute_actions, stub, sizeof stub);
-            odp_put_tunnel_action(&md->tunnel, &execute_actions);
+            if (md->tunnel.ip_dst) {
+                odp_put_tunnel_action(&md->tunnel, &execute_actions);
+            }
             ofpbuf_put(&execute_actions, action, NLA_ALIGN(action->nla_len));
 
             execute.actions = execute_actions.data;
@@ -1133,15 +1157,17 @@  dpif_execute_helper_cb(void *aux_, struct dp_packet **packets, int cnt,
         aux->error = dpif_execute(aux->dpif, &execute);
         log_execute_message(aux->dpif, &execute, true, aux->error);
 
-        if (md->tunnel.ip_dst) {
+        if (md->tunnel.ip_dst || aux->meter_action) {
             ofpbuf_uninit(&execute_actions);
+
+            /* Do not re-use the same meters for later output actions. */
+            aux->meter_action = NULL;
         }
 
         return (aux->error == 0) ? cnt : 0;
     }
 
     case OVS_ACTION_ATTR_HASH:
-    case OVS_ACTION_ATTR_METER:
     case OVS_ACTION_ATTR_PUSH_VLAN:
     case OVS_ACTION_ATTR_POP_VLAN:
     case OVS_ACTION_ATTR_PUSH_MPLS:
@@ -1164,7 +1190,7 @@  dpif_execute_helper_cb(void *aux_, struct dp_packet **packets, int cnt,
 static int
 dpif_execute_with_help(struct dpif *dpif, struct dpif_execute *execute)
 {
-    struct dpif_execute_helper_aux aux = {dpif, 0};
+    struct dpif_execute_helper_aux aux = { dpif, 0, NULL };
     struct dp_packet *pp;
 
     COVERAGE_INC(dpif_execute_with_help);
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 43d4b72..e517213 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -5840,6 +5840,7 @@  ofpacts_pull_openflow_instructions(struct ofpbuf *openflow,
 
         om = ofpact_put_METER(ofpacts);
         om->meter_id = ntohl(oim->meter_id);
+        om->provider_meter_id = UINT32_MAX; /* No provider meter ID. */
     }
     if (insts[OVSINST_OFPIT11_APPLY_ACTIONS]) {
         const struct ofp_action_header *actions;
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 773b617..93e9442 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -458,6 +458,7 @@  struct ofpact_metadata {
 struct ofpact_meter {
     struct ofpact ofpact;
     uint32_t meter_id;
+    uint32_t provider_meter_id;
 };
 
 /* OFPACT_WRITE_ACTIONS.
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 1738394..b4241a2 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3979,6 +3979,15 @@  xlate_sample_action(struct xlate_ctx *ctx,
                           ODPP_NONE, false);
 }
 
+static void
+xlate_meter_action(struct xlate_ctx *ctx, const struct ofpact_meter *meter)
+{
+    if (meter->provider_meter_id != UINT32_MAX) {
+        nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER,
+                       meter->provider_meter_id);
+    }
+}
+
 static bool
 may_receive(const struct xport *xport, struct xlate_ctx *ctx)
 {
@@ -4598,7 +4607,7 @@  do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
             break;
 
         case OFPACT_METER:
-            /* Not implemented yet. */
+            xlate_meter_action(ctx, ofpact_get_METER(a));
             break;
 
         case OFPACT_GOTO_TABLE: {
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index dd45ee8..41413da 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1788,8 +1788,7 @@  void ofproto_delete_flow(struct ofproto *, const struct match *, int priority)
     OVS_EXCLUDED(ofproto_mutex);
 void ofproto_flush_flows(struct ofproto *);
 
-enum ofperr ofproto_check_ofpacts(struct ofproto *,
-                                  const struct ofpact ofpacts[],
+enum ofperr ofproto_check_ofpacts(struct ofproto *, struct ofpact ofpacts[],
                                   size_t ofpacts_len);
 
 static inline const struct rule_actions *
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 8e4495b..e5c2de1 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2889,8 +2889,8 @@  ofproto_group_unref(struct ofgroup *group)
     }
 }
 
-static uint32_t get_provider_meter_id(const struct ofproto *,
-                                      uint32_t of_meter_id);
+static bool ofproto_fix_meter_action(const struct ofproto *,
+                                     struct ofpact_meter *);
 
 /* Creates and returns a new 'struct rule_actions', whose actions are a copy
  * of from the 'ofpacts_len' bytes of 'ofpacts'. */
@@ -3329,23 +3329,23 @@  reject_slave_controller(struct ofconn *ofconn)
  * for 'ofproto':
  *
  *    - If they use a meter, then 'ofproto' has that meter configured.
+ *      Updates the meter action with ofproto's datapath's provider_meter_id.
  *
  *    - If they use any groups, then 'ofproto' has that group configured.
  *
  * Returns 0 if successful, otherwise an OpenFlow error. */
 enum ofperr
 ofproto_check_ofpacts(struct ofproto *ofproto,
-                      const struct ofpact ofpacts[], size_t ofpacts_len)
+                      struct ofpact ofpacts[], size_t ofpacts_len)
 {
-    const struct ofpact *a;
-    uint32_t mid;
-
-    mid = ofpacts_get_meter(ofpacts, ofpacts_len);
-    if (mid && get_provider_meter_id(ofproto, mid) == UINT32_MAX) {
-        return OFPERR_OFPMMFC_INVALID_METER;
-    }
+    struct ofpact *a;
 
     OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
+        if (a->type == OFPACT_METER &&
+            !ofproto_fix_meter_action(ofproto, ofpact_get_METER(a))) {
+            return OFPERR_OFPMMFC_INVALID_METER;
+        }
+
         if (a->type == OFPACT_GROUP
             && !ofproto_group_exists(ofproto, ofpact_get_GROUP(a)->group_id)) {
             return OFPERR_OFPBAC_BAD_OUT_GROUP;
@@ -5705,20 +5705,27 @@  struct meter {
 };
 
 /*
- * This is used in instruction validation at flow set-up time,
- * as flows may not use non-existing meters.
- * Return value of UINT32_MAX signifies an invalid meter.
+ * This is used in instruction validation at flow set-up time, to map
+ * the OpenFlow meter ID to the corresponding datapath provider meter
+ * ID.  If either does not exist, returns false.  Otherwise updates
+ * the meter action and returns true.
  */
-static uint32_t
-get_provider_meter_id(const struct ofproto *ofproto, uint32_t of_meter_id)
+static bool
+ofproto_fix_meter_action(const struct ofproto *ofproto,
+                         struct ofpact_meter *ma)
 {
-    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;
+    if (ma->meter_id && ma->meter_id <= ofproto->meter_features.max_meters) {
+        const struct meter *meter = ofproto->meters[ma->meter_id];
+
+        if (meter && meter->provider_meter_id.uint32 != UINT32_MAX) {
+            /* Update the action with the provider's meter ID, so that we
+             * do not need any synchronization between ofproto_dpif_xlate
+             * and ofproto for meter table access. */
+            ma->provider_meter_id = meter->provider_meter_id.uint32;
+            return true;
         }
     }
-    return UINT32_MAX;
+    return false;
 }
 
 /* Finds the meter invoked by 'rule''s actions and adds 'rule' to the meter's