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

Reply via email to