Signed-off-by: Justin Pettit <jpet...@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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev