[ovs-dev] [ACL Meters 6/7] ofproto: Add support for specifying a meter in controller actions.
Justin Pettit
jpettit at ovn.org
Mon Jul 30 06:46:37 UTC 2018
Signed-off-by: Justin Pettit <jpettit at ovn.org>
---
include/openvswitch/ofp-actions.h | 8 ++++++
lib/ofp-actions.c | 27 +++++++++++++++++--
ofproto/ofproto-dpif-xlate.c | 24 +++++++++++++----
ofproto/ofproto-dpif.c | 1 +
ofproto/ofproto.c | 44 +++++++++++++++++++++++++++++++
utilities/ovs-ofctl.8.in | 4 +++
6 files changed, 101 insertions(+), 7 deletions(-)
diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h
index b3dd0959d53e..3b0350496294 100644
--- a/include/openvswitch/ofp-actions.h
+++ b/include/openvswitch/ofp-actions.h
@@ -287,6 +287,8 @@ struct ofpact_output {
uint16_t max_len; /* Max send len, for port OFPP_CONTROLLER. */
};
+#define NX_CTLR_NO_METER 0
+
/* OFPACT_CONTROLLER.
*
* Used for NXAST_CONTROLLER. */
@@ -305,6 +307,12 @@ struct ofpact_controller {
/* Arbitrary data to include in the packet-in message (currently,
* only in NXT_PACKET_IN2). */
uint16_t userdata_len;
+
+ /* Meter to which this controller action should be associated.
+ * If requested, this will override a "controller" virtual meter.
+ * A value of NX_CTLR_NO_METER means no meter is requested. */
+ uint32_t meter_id;
+ uint32_t provider_meter_id;
);
uint8_t userdata[0];
};
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index fa94cbbd9436..5aacff62a34a 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -754,6 +754,7 @@ enum nx_action_controller2_prop_type {
NXAC2PT_REASON, /* uint8_t reason (OFPR_*), default 0. */
NXAC2PT_USERDATA, /* Data to copy into NXPINT_USERDATA. */
NXAC2PT_PAUSE, /* Flag to pause pipeline to resume later. */
+ NXAC2PT_METER_ID, /* ovs_b32 meter (default NX_CTLR_NO_METER). */
};
/* The action structure for NXAST_CONTROLLER2 is "struct ext_action_header",
@@ -771,6 +772,7 @@ decode_NXAST_RAW_CONTROLLER(const struct nx_action_controller *nac,
oc->max_len = ntohs(nac->max_len);
oc->controller_id = ntohs(nac->controller_id);
oc->reason = nac->reason;
+ oc->meter_id = NX_CTLR_NO_METER;
ofpact_finish_CONTROLLER(out, &oc);
return 0;
@@ -790,6 +792,7 @@ decode_NXAST_RAW_CONTROLLER2(const struct ext_action_header *eah,
oc->ofpact.raw = NXAST_RAW_CONTROLLER2;
oc->max_len = UINT16_MAX;
oc->reason = OFPR_ACTION;
+ oc->meter_id = NX_CTLR_NO_METER;
struct ofpbuf properties;
ofpbuf_use_const(&properties, eah, ntohs(eah->len));
@@ -831,6 +834,10 @@ decode_NXAST_RAW_CONTROLLER2(const struct ext_action_header *eah,
oc->pause = true;
break;
+ case NXAC2PT_METER_ID:
+ error = ofpprop_parse_u32(&payload, &oc->meter_id);
+ break;
+
default:
error = OFPPROP_UNKNOWN(false, "NXAST_RAW_CONTROLLER2", type);
break;
@@ -852,6 +859,7 @@ encode_CONTROLLER(const struct ofpact_controller *controller,
{
if (controller->userdata_len
|| controller->pause
+ || controller->meter_id != NX_CTLR_NO_METER
|| controller->ofpact.raw == NXAST_RAW_CONTROLLER2) {
size_t start_ofs = out->size;
put_NXAST_CONTROLLER2(out);
@@ -872,6 +880,9 @@ encode_CONTROLLER(const struct ofpact_controller *controller,
if (controller->pause) {
ofpprop_put_flag(out, NXAC2PT_PAUSE);
}
+ if (controller->meter_id != NX_CTLR_NO_METER) {
+ ofpprop_put_u32(out, NXAC2PT_METER_ID, controller->meter_id);
+ }
pad_ofpat(out, start_ofs);
} else {
struct nx_action_controller *nac;
@@ -889,6 +900,7 @@ parse_CONTROLLER(char *arg, const struct ofpact_parse_params *pp)
enum ofp_packet_in_reason reason = OFPR_ACTION;
uint16_t controller_id = 0;
uint16_t max_len = UINT16_MAX;
+ uint32_t meter_id = NX_CTLR_NO_METER;
const char *userdata = NULL;
bool pause = false;
@@ -921,6 +933,11 @@ parse_CONTROLLER(char *arg, const struct ofpact_parse_params *pp)
userdata = value;
} else if (!strcmp(name, "pause")) {
pause = true;
+ } else if (!strcmp(name, "meter_id")) {
+ char *error = str_to_u32(value, &meter_id);
+ if (error) {
+ return error;
+ }
} else {
return xasprintf("unknown key \"%s\" parsing controller "
"action", name);
@@ -928,7 +945,8 @@ parse_CONTROLLER(char *arg, const struct ofpact_parse_params *pp)
}
}
- if (reason == OFPR_ACTION && controller_id == 0 && !userdata && !pause) {
+ if (reason == OFPR_ACTION && controller_id == 0 && !userdata && !pause
+ && meter_id == NX_CTLR_NO_METER) {
struct ofpact_output *output;
output = ofpact_put_OUTPUT(pp->ofpacts);
@@ -942,6 +960,7 @@ parse_CONTROLLER(char *arg, const struct ofpact_parse_params *pp)
controller->reason = reason;
controller->controller_id = controller_id;
controller->pause = pause;
+ controller->meter_id = meter_id;
if (userdata) {
size_t start_ofs = pp->ofpacts->size;
@@ -976,7 +995,7 @@ format_CONTROLLER(const struct ofpact_controller *a,
const struct ofpact_format_params *fp)
{
if (a->reason == OFPR_ACTION && !a->controller_id && !a->userdata_len
- && !a->pause) {
+ && !a->pause && a->meter_id == NX_CTLR_NO_METER) {
ds_put_format(fp->s, "%sCONTROLLER:%s%"PRIu16,
colors.special, colors.end, a->max_len);
} else {
@@ -1006,6 +1025,10 @@ format_CONTROLLER(const struct ofpact_controller *a,
if (a->pause) {
ds_put_format(fp->s, "%spause%s,", colors.value, colors.end);
}
+ if (a->meter_id != NX_CTLR_NO_METER) {
+ ds_put_format(fp->s, "%smeter_id=%s%"PRIu32",",
+ colors.param, colors.end, a->meter_id);
+ }
ds_chomp(fp->s, ',');
ds_put_format(fp->s, "%s)%s", colors.paren, colors.end);
}
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index dc63afa35a6b..01f1fafeb356 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4619,6 +4619,7 @@ static void
xlate_controller_action(struct xlate_ctx *ctx, int len,
enum ofp_packet_in_reason reason,
uint16_t controller_id,
+ uint32_t provider_meter_id,
const uint8_t *userdata, size_t userdata_len)
{
xlate_commit_actions(ctx);
@@ -4655,9 +4656,21 @@ xlate_controller_action(struct xlate_ctx *ctx, int len,
}
recirc_refs_add(&ctx->xout->recircs, recirc_id);
+ /* If the controller action didn't request a meter (indicated by a
+ * 'meter_id' argument other than NX_CTLR_NO_METER), see if one was
+ * configured through the "controller" virtual meter.
+ *
+ * Internally, ovs-vswitchd uses UINT32_MAX to indicate no meter is
+ * configured. */
+ uint32_t meter_id;
+ if (provider_meter_id == UINT32_MAX) {
+ meter_id = ctx->xbridge->ofproto->up.controller_meter_id;
+ } else {
+ meter_id = provider_meter_id;
+ }
+
size_t offset;
size_t ac_offset;
- uint32_t meter_id = ctx->xbridge->ofproto->up.controller_meter_id;
if (meter_id != UINT32_MAX) {
/* If controller meter is configured, generate clone(meter, userspace)
* action. */
@@ -4852,7 +4865,7 @@ compose_dec_ttl(struct xlate_ctx *ctx, struct ofpact_cnt_ids *ids)
for (i = 0; i < ids->n_controllers; i++) {
xlate_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL,
- ids->cnt_ids[i], NULL, 0);
+ ids->cnt_ids[i], UINT32_MAX, NULL, 0);
}
/* Stop processing for current table. */
@@ -4893,7 +4906,7 @@ compose_dec_nsh_ttl_action(struct xlate_ctx *ctx)
return false;
} else {
xlate_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL,
- 0, NULL, 0);
+ 0, UINT32_MAX, NULL, 0);
}
}
@@ -4926,7 +4939,7 @@ compose_dec_mpls_ttl_action(struct xlate_ctx *ctx)
return false;
} else {
xlate_controller_action(ctx, UINT16_MAX, OFPR_INVALID_TTL, 0,
- NULL, 0);
+ UINT32_MAX, NULL, 0);
}
}
@@ -4985,7 +4998,7 @@ xlate_output_action(struct xlate_ctx *ctx, ofp_port_t port,
: group_bucket_action ? OFPR_GROUP
: ctx->in_action_set ? OFPR_ACTION_SET
: OFPR_ACTION),
- 0, NULL, 0);
+ 0, UINT32_MAX, NULL, 0);
break;
case OFPP_NONE:
break;
@@ -6392,6 +6405,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
xlate_controller_action(ctx, controller->max_len,
controller->reason,
controller->controller_id,
+ controller->provider_meter_id,
controller->userdata,
controller->userdata_len);
}
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ad1e8af43f6e..afe333e35166 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1520,6 +1520,7 @@ add_internal_flows(struct ofproto_dpif *ofproto)
controller->max_len = UINT16_MAX;
controller->controller_id = 0;
controller->reason = OFPR_IMPLICIT_MISS;
+ controller->meter_id = NX_CTLR_NO_METER;
ofpact_finish_CONTROLLER(&ofpacts, &controller);
error = add_internal_miss_flow(ofproto, id++, &ofpacts,
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index f946e27b77a9..e71431129af9 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -3017,6 +3017,9 @@ remove_groups_rcu(struct ofgroup **groups)
static bool ofproto_fix_meter_action(const struct ofproto *,
struct ofpact_meter *);
+static bool ofproto_fix_controller_action(const struct ofproto *,
+ struct ofpact_controller *);
+
/* Creates and returns a new 'struct rule_actions', whose actions are a copy
* of from the 'ofpacts_len' bytes of 'ofpacts'. */
const struct rule_actions *
@@ -3429,6 +3432,17 @@ ofproto_check_ofpacts(struct ofproto *ofproto,
return OFPERR_OFPMMFC_INVALID_METER;
}
+ if (a->type == OFPACT_CONTROLLER) {
+ struct ofpact_controller *ca = ofpact_get_CONTROLLER(a);
+
+ if (!ofproto_fix_controller_action(ofproto, ca)) {
+ static struct vlog_rate_limit rl2 = VLOG_RATE_LIMIT_INIT(1, 5);
+ VLOG_INFO_RL(&rl2, "%s: controller action specified an "
+ "unknown meter id: %d",
+ ofproto->name, ca->meter_id);
+ }
+ }
+
if (a->type == OFPACT_GROUP
&& !ofproto_group_exists(ofproto, ofpact_get_GROUP(a)->group_id)) {
return OFPERR_OFPBAC_BAD_OUT_GROUP;
@@ -6292,6 +6306,36 @@ ofproto_fix_meter_action(const struct ofproto *ofproto,
return false;
}
+/* This is used in instruction validation at flow set-up time, to map
+ * the OpenFlow meter ID in a controller action to the corresponding
+ * datapath provider meter ID. If either does not exist, sets the
+ * provider meter id to a value to prevent the provider from using it
+ * and returns false. Otherwise, updates the meter action and returns
+ * true. */
+static bool
+ofproto_fix_controller_action(const struct ofproto *ofproto,
+ struct ofpact_controller *ca)
+{
+ if (ca->meter_id == NX_CTLR_NO_METER) {
+ ca->provider_meter_id = UINT32_MAX;
+ return true;
+ }
+
+ const struct meter *meter = ofproto_get_meter(ofproto, ca->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. */
+ ca->provider_meter_id = meter->provider_meter_id.uint32;
+ return true;
+ }
+
+ /* Prevent the meter from being set by the ofproto provider. */
+ ca->provider_meter_id = UINT32_MAX;
+ return false;
+}
+
/* Finds the meter invoked by 'rule''s actions and adds 'rule' to the meter's
* list of rules. */
static void
diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
index 5f26c4cca34a..f7dbd250ef37 100644
--- a/utilities/ovs-ofctl.8.in
+++ b/utilities/ovs-ofctl.8.in
@@ -859,6 +859,10 @@ switch in an \fBNXT_RESUME\fR message, which will restart the packet's
traversal from the point where it was interrupted. This permits an
OpenFlow controller to interpose on a packet midway through processing
in Open vSwitch.
+.IP "\fBmeter_id=\fIid\fR"
+Associate packets sent to the controller with meter \fIid\fR. By
+default, packets sent to the controller are associated with the
+"controller" virtual meter, if one was configured.
.
.RE
.IP
--
2.17.1
More information about the dev
mailing list