Translate OpenFlow METER instructions to datapath meter actions.
Signed-off-by: Jarno Rajahalme <ja...@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 --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
--
2.1.4
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
