[ovs-dev] [PATCH v5 3/4] ofproto: Meter translation.
Jarno Rajahalme
jarno at ovn.org
Thu Feb 23 19:27:56 UTC 2017
Translate OpenFlow METER instructions to datapath meter actions.
Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
Signed-off-by: Andy Zhou <azhou at ovn.org>
---
include/openvswitch/ofp-actions.h | 1 +
lib/dpif.c | 47 +++++++++++++++++++++++++++++++-------
lib/ofp-actions.c | 1 +
ofproto/ofproto-dpif-xlate.c | 11 ++++++++-
ofproto/ofproto.c | 48 +++++++++++++++++++++++----------------
5 files changed, 79 insertions(+), 29 deletions(-)
diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
index 88f573d..b487e6de 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -534,6 +534,7 @@ struct ofpact_metadata {
struct ofpact_meter {
struct ofpact ofpact;
uint32_t meter_id;
+ uint32_t provider_meter_id;
};
/* OFPACT_WRITE_ACTIONS, OFPACT_CLONE.
diff --git a/lib/dpif.c b/lib/dpif.c
index 3f36ccd..aa6f37a 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1104,6 +1104,7 @@ struct dpif_execute_helper_aux {
struct dpif *dpif;
const struct flow *flow;
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
@@ -1119,6 +1120,13 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
ovs_assert(packets_->count == 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;
+ }
+ break;
+
case OVS_ACTION_ATTR_CT:
case OVS_ACTION_ATTR_OUTPUT:
case OVS_ACTION_ATTR_TUNNEL_PUSH:
@@ -1129,15 +1137,36 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
struct ofpbuf execute_actions;
uint64_t stub[256 / 8];
struct pkt_metadata *md = &packet->md;
- bool dst_set;
- dst_set = flow_tnl_dst_is_set(&md->tunnel);
- if (dst_set) {
+ if (flow_tnl_dst_is_set(&md->tunnel) || aux->meter_action) {
+ ofpbuf_use_stub(&execute_actions, stub, sizeof stub);
+
+ if (aux->meter_action) {
+ const struct nlattr *a = aux->meter_action;
+
+ /* XXX: This code collects meter actions since the last action
+ * execution via the datapath to be executed right before the
+ * current action that needs to be executed by the datapath.
+ * This is only an approximation, but better than nothing.
+ * Fundamentally, we should have a mechanism by which the
+ * datapath could return the result of the meter action so that
+ * we could execute them at the right order. */
+ 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;
@@ -1170,14 +1199,16 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
dp_packet_delete(clone);
- if (dst_set) {
+ if (flow_tnl_dst_is_set(&md->tunnel) || aux->meter_action) {
ofpbuf_uninit(&execute_actions);
+
+ /* Do not re-use the same meters for later output actions. */
+ aux->meter_action = NULL;
}
break;
}
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:
@@ -1201,7 +1232,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch *packets_,
static int
dpif_execute_with_help(struct dpif *dpif, struct dpif_execute *execute)
{
- struct dpif_execute_helper_aux aux = {dpif, execute->flow, 0};
+ struct dpif_execute_helper_aux aux = {dpif, execute->flow, 0, NULL};
struct dp_packet_batch pb;
COVERAGE_INC(dpif_execute_with_help);
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index ce80f57..ab49eff 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -6938,6 +6938,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/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 503a347..577a1a4 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4579,6 +4579,15 @@ xlate_clone(struct xlate_ctx *ctx, const struct ofpact_nest *oc)
ctx->was_mpls = old_was_mpls;
}
+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)
{
@@ -5388,7 +5397,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.c b/ofproto/ofproto.c
index 61dc657..c0ee5b9 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3004,8 +3004,8 @@ remove_groups_rcu(struct ofgroup **groups)
free(groups);
}
-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'. */
@@ -3398,6 +3398,7 @@ 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.
*
@@ -3409,16 +3410,16 @@ ofproto_check_ofpacts(struct ofproto *ofproto,
const struct ofpact ofpacts[], size_t ofpacts_len)
OVS_REQUIRES(ofproto_mutex)
{
- uint32_t mid;
+ const struct ofpact *a;
- mid = ofpacts_get_meter(ofpacts, ofpacts_len);
- if (mid && get_provider_meter_id(ofproto, mid) == UINT32_MAX) {
- return OFPERR_OFPMMFC_INVALID_METER;
- }
+ OFPACT_FOR_EACH_FLATTENED (a, ofpacts, ofpacts_len) {
+ if (a->type == OFPACT_METER &&
+ !ofproto_fix_meter_action(ofproto, ofpact_get_METER(a))) {
+ return OFPERR_OFPMMFC_INVALID_METER;
+ }
- const struct ofpact_group *a;
- OFPACT_FOR_EACH_TYPE_FLATTENED (a, GROUP, ofpacts, ofpacts_len) {
- if (!ofproto_group_exists(ofproto, a->group_id)) {
+ if (a->type == OFPACT_GROUP
+ && !ofproto_group_exists(ofproto, ofpact_get_GROUP(a)->group_id)) {
return OFPERR_OFPBAC_BAD_OUT_GROUP;
}
}
@@ -6149,20 +6150,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
--
2.1.4
More information about the dev
mailing list