ovs-ofctl: Fix write before beginning of string in "add-flow".
[sliver-openvswitch.git] / ofproto / ofproto.c
index 764d4a8..83dc99d 100644 (file)
@@ -108,6 +108,9 @@ struct rule {
     struct list list;
 
     /* OpenFlow actions.
+     *
+     * 'n_actions' is the number of elements in the 'actions' array.  A single
+     * action may take up more more than one element's worth of space.
      *
      * A subrule has no actions (it uses the super-rule's actions). */
     int n_actions;
@@ -187,10 +190,10 @@ struct ofproto {
     /* Settings. */
     uint64_t datapath_id;       /* Datapath ID. */
     uint64_t fallback_dpid;     /* Datapath ID if no better choice found. */
-    char *manufacturer;         /* Manufacturer. */
-    char *hardware;             /* Hardware. */
-    char *software;             /* Software version. */
-    char *serial;               /* Serial number. */
+    char *mfr_desc;             /* Manufacturer. */
+    char *hw_desc;              /* Hardware. */
+    char *sw_desc;              /* Software version. */
+    char *serial_desc;          /* Serial number. */
     char *dp_desc;              /* Datapath description. */
 
     /* Datapath. */
@@ -299,10 +302,10 @@ ofproto_create(const char *datapath, const char *datapath_type,
     p = xzalloc(sizeof *p);
     p->fallback_dpid = pick_fallback_dpid();
     p->datapath_id = p->fallback_dpid;
-    p->manufacturer = xstrdup(DEFAULT_MFR_DESC);
-    p->hardware = xstrdup(DEFAULT_HW_DESC);
-    p->software = xstrdup(DEFAULT_SW_DESC);
-    p->serial = xstrdup(DEFAULT_SERIAL_DESC);
+    p->mfr_desc = xstrdup(DEFAULT_MFR_DESC);
+    p->hw_desc = xstrdup(DEFAULT_HW_DESC);
+    p->sw_desc = xstrdup(DEFAULT_SW_DESC);
+    p->serial_desc = xstrdup(DEFAULT_SERIAL_DESC);
     p->dp_desc = xstrdup(DEFAULT_DP_DESC);
 
     /* Initialize datapath. */
@@ -390,27 +393,50 @@ ofproto_set_max_backoff(struct ofproto *p, int max_backoff)
 
 void
 ofproto_set_desc(struct ofproto *p,
-                 const char *manufacturer, const char *hardware,
-                 const char *software, const char *serial,
+                 const char *mfr_desc, const char *hw_desc,
+                 const char *sw_desc, const char *serial_desc,
                  const char *dp_desc)
 {
-    if (manufacturer) {
-        free(p->manufacturer);
-        p->manufacturer = xstrdup(manufacturer);
-    }
-    if (hardware) {
-        free(p->hardware);
-        p->hardware = xstrdup(hardware);
+    struct ofp_desc_stats *ods;
+
+    if (mfr_desc) {
+        if (strlen(mfr_desc) >= sizeof ods->mfr_desc) {
+            VLOG_WARN("truncating mfr_desc, must be less than %zu characters",
+                    sizeof ods->mfr_desc);
+        }
+        free(p->mfr_desc);
+        p->mfr_desc = xstrdup(mfr_desc);
     }
-    if (software) {
-        free(p->software);
-        p->software = xstrdup(software);
+    if (hw_desc) {
+        if (strlen(hw_desc) >= sizeof ods->hw_desc) {
+            VLOG_WARN("truncating hw_desc, must be less than %zu characters",
+                    sizeof ods->hw_desc);
+        }
+        free(p->hw_desc);
+        p->hw_desc = xstrdup(hw_desc);
     }
-    if (serial) {
-        free(p->serial);
-        p->serial = xstrdup(serial);
+    if (sw_desc) {
+        if (strlen(sw_desc) >= sizeof ods->sw_desc) {
+            VLOG_WARN("truncating sw_desc, must be less than %zu characters",
+                    sizeof ods->sw_desc);
+        }
+        free(p->sw_desc);
+        p->sw_desc = xstrdup(sw_desc);
+    }
+    if (serial_desc) {
+        if (strlen(serial_desc) >= sizeof ods->serial_num) {
+            VLOG_WARN("truncating serial_desc, must be less than %zu "
+                    "characters",
+                    sizeof ods->serial_num);
+        }
+        free(p->serial_desc);
+        p->serial_desc = xstrdup(serial_desc);
     }
     if (dp_desc) {
+        if (strlen(dp_desc) >= sizeof ods->dp_desc) {
+            VLOG_WARN("truncating dp_desc, must be less than %zu characters",
+                    sizeof ods->dp_desc);
+        }
         free(p->dp_desc);
         p->dp_desc = xstrdup(dp_desc);
     }
@@ -731,12 +757,14 @@ ofproto_destroy(struct ofproto *p)
 
     mac_learning_destroy(p->ml);
 
-    free(p->manufacturer);
-    free(p->hardware);
-    free(p->software);
-    free(p->serial);
+    free(p->mfr_desc);
+    free(p->hw_desc);
+    free(p->sw_desc);
+    free(p->serial_desc);
     free(p->dp_desc);
 
+    port_array_destroy(&p->ports);
+
     free(p);
 }
 
@@ -783,8 +811,8 @@ ofproto_run1(struct ofproto *p)
                 /* Someone destroyed the datapath behind our back.  The caller
                  * better destroy us and give up, because we're just going to
                  * spin from here on out. */
-                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-                VLOG_ERR_RL(&rl, "%s: datapath was destroyed externally",
+                static struct vlog_rate_limit rl2 = VLOG_RATE_LIMIT_INIT(1, 5);
+                VLOG_ERR_RL(&rl2, "%s: datapath was destroyed externally",
                             dpif_name(p->dpif));
                 return ENODEV;
             }
@@ -1971,7 +1999,7 @@ add_controller_action(struct odp_actions *actions,
 
 struct action_xlate_ctx {
     /* Input. */
-    const flow_t *flow;         /* Flow to which these actions correspond. */
+    flow_t flow;                /* Flow to which these actions correspond. */
     int recurse;                /* Recursion level, via xlate_table_action. */
     struct ofproto *ofproto;
     const struct ofpbuf *packet; /* The packet corresponding to 'flow', or a
@@ -2034,13 +2062,11 @@ static void
 xlate_table_action(struct action_xlate_ctx *ctx, uint16_t in_port)
 {
     if (!ctx->recurse) {
+        uint16_t old_in_port = ctx->flow.in_port;
         struct rule *rule;
-        flow_t flow;
-
-        flow = *ctx->flow;
-        flow.in_port = in_port;
 
-        rule = lookup_valid_rule(ctx->ofproto, &flow);
+        ctx->flow.in_port = in_port;
+        rule = lookup_valid_rule(ctx->ofproto, &ctx->flow);
         if (rule) {
             if (rule->super) {
                 rule = rule->super;
@@ -2050,6 +2076,7 @@ xlate_table_action(struct action_xlate_ctx *ctx, uint16_t in_port)
             do_xlate_actions(rule->actions, rule->n_actions, ctx);
             ctx->recurse--;
         }
+        ctx->flow.in_port = old_in_port;
     }
 }
 
@@ -2064,13 +2091,13 @@ xlate_output_action(struct action_xlate_ctx *ctx,
 
     switch (ntohs(oao->port)) {
     case OFPP_IN_PORT:
-        add_output_action(ctx, ctx->flow->in_port);
+        add_output_action(ctx, ctx->flow.in_port);
         break;
     case OFPP_TABLE:
-        xlate_table_action(ctx, ctx->flow->in_port);
+        xlate_table_action(ctx, ctx->flow.in_port);
         break;
     case OFPP_NORMAL:
-        if (!ctx->ofproto->ofhooks->normal_cb(ctx->flow, ctx->packet,
+        if (!ctx->ofproto->ofhooks->normal_cb(&ctx->flow, ctx->packet,
                                               ctx->out, ctx->tags,
                                               &ctx->nf_output_iface,
                                               ctx->ofproto->aux)) {
@@ -2093,7 +2120,7 @@ xlate_output_action(struct action_xlate_ctx *ctx,
         break;
     default:
         odp_port = ofp_port_to_odp_port(ntohs(oao->port));
-        if (odp_port != ctx->flow->in_port) {
+        if (odp_port != ctx->flow.in_port) {
             add_output_action(ctx, odp_port);
         }
         break;
@@ -2123,6 +2150,9 @@ xlate_nicira_action(struct action_xlate_ctx *ctx,
         xlate_table_action(ctx, ofp_port_to_odp_port(ntohs(nar->in_port)));
         break;
 
+    /* If you add a new action here that modifies flow data, don't forget to
+     * update the flow key in ctx->flow in the same key. */
+
     default:
         VLOG_DBG_RL(&rl, "unknown Nicira action type %"PRIu16, subtype);
         break;
@@ -2137,9 +2167,9 @@ do_xlate_actions(const union ofp_action *in, size_t n_in,
     const union ofp_action *ia;
     const struct ofport *port;
 
-    port = port_array_get(&ctx->ofproto->ports, ctx->flow->in_port);
+    port = port_array_get(&ctx->ofproto->ports, ctx->flow.in_port);
     if (port && port->opp.config & (OFPPC_NO_RECV | OFPPC_NO_RECV_STP) &&
-        port->opp.config & (eth_addr_equals(ctx->flow->dl_dst, stp_eth_addr)
+        port->opp.config & (eth_addr_equals(ctx->flow.dl_dst, stp_eth_addr)
                             ? OFPPC_NO_RECV_STP : OFPPC_NO_RECV)) {
         /* Drop this flow. */
         return;
@@ -2156,52 +2186,59 @@ do_xlate_actions(const union ofp_action *in, size_t n_in,
 
         case OFPAT_SET_VLAN_VID:
             oa = odp_actions_add(ctx->out, ODPAT_SET_VLAN_VID);
-            oa->vlan_vid.vlan_vid = ia->vlan_vid.vlan_vid;
+            ctx->flow.dl_vlan = oa->vlan_vid.vlan_vid = ia->vlan_vid.vlan_vid;
             break;
 
         case OFPAT_SET_VLAN_PCP:
             oa = odp_actions_add(ctx->out, ODPAT_SET_VLAN_PCP);
-            oa->vlan_pcp.vlan_pcp = ia->vlan_pcp.vlan_pcp;
+            ctx->flow.dl_vlan_pcp = oa->vlan_pcp.vlan_pcp = ia->vlan_pcp.vlan_pcp;
             break;
 
         case OFPAT_STRIP_VLAN:
             odp_actions_add(ctx->out, ODPAT_STRIP_VLAN);
+            ctx->flow.dl_vlan = OFP_VLAN_NONE;
+            ctx->flow.dl_vlan_pcp = 0;
             break;
 
         case OFPAT_SET_DL_SRC:
             oa = odp_actions_add(ctx->out, ODPAT_SET_DL_SRC);
             memcpy(oa->dl_addr.dl_addr,
                    ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN);
+            memcpy(ctx->flow.dl_src,
+                   ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN);
             break;
 
         case OFPAT_SET_DL_DST:
             oa = odp_actions_add(ctx->out, ODPAT_SET_DL_DST);
             memcpy(oa->dl_addr.dl_addr,
                    ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN);
+            memcpy(ctx->flow.dl_dst,
+                   ((struct ofp_action_dl_addr *) ia)->dl_addr, ETH_ADDR_LEN);
             break;
 
         case OFPAT_SET_NW_SRC:
             oa = odp_actions_add(ctx->out, ODPAT_SET_NW_SRC);
-            oa->nw_addr.nw_addr = ia->nw_addr.nw_addr;
+            ctx->flow.nw_src = oa->nw_addr.nw_addr = ia->nw_addr.nw_addr;
             break;
 
         case OFPAT_SET_NW_DST:
             oa = odp_actions_add(ctx->out, ODPAT_SET_NW_DST);
-            oa->nw_addr.nw_addr = ia->nw_addr.nw_addr;
+            ctx->flow.nw_dst = oa->nw_addr.nw_addr = ia->nw_addr.nw_addr;
+            break;
 
         case OFPAT_SET_NW_TOS:
             oa = odp_actions_add(ctx->out, ODPAT_SET_NW_TOS);
-            oa->nw_tos.nw_tos = ia->nw_tos.nw_tos;
+            ctx->flow.nw_tos = oa->nw_tos.nw_tos = ia->nw_tos.nw_tos;
             break;
 
         case OFPAT_SET_TP_SRC:
             oa = odp_actions_add(ctx->out, ODPAT_SET_TP_SRC);
-            oa->tp_port.tp_port = ia->tp_port.tp_port;
+            ctx->flow.tp_src = oa->tp_port.tp_port = ia->tp_port.tp_port;
             break;
 
         case OFPAT_SET_TP_DST:
             oa = odp_actions_add(ctx->out, ODPAT_SET_TP_DST);
-            oa->tp_port.tp_port = ia->tp_port.tp_port;
+            ctx->flow.tp_dst = oa->tp_port.tp_port = ia->tp_port.tp_port;
             break;
 
         case OFPAT_VENDOR:
@@ -2226,7 +2263,7 @@ xlate_actions(const union ofp_action *in, size_t n_in,
     struct action_xlate_ctx ctx;
     COVERAGE_INC(ofproto_ofp2odp);
     odp_actions_init(out);
-    ctx.flow = flow;
+    ctx.flow = *flow;
     ctx.recurse = 0;
     ctx.ofproto = ofproto;
     ctx.packet = packet;
@@ -2397,11 +2434,12 @@ handle_desc_stats_request(struct ofproto *p, struct ofconn *ofconn,
 
     msg = start_stats_reply(request, sizeof *ods);
     ods = append_stats_reply(sizeof *ods, ofconn, &msg);
-    strncpy(ods->mfr_desc, p->manufacturer, sizeof ods->mfr_desc);
-    strncpy(ods->hw_desc, p->hardware, sizeof ods->hw_desc);
-    strncpy(ods->sw_desc, p->software, sizeof ods->sw_desc);
-    strncpy(ods->serial_num, p->serial, sizeof ods->serial_num);
-    strncpy(ods->dp_desc, p->dp_desc, sizeof ods->dp_desc);
+    memset(ods, 0, sizeof *ods);
+    ovs_strlcpy(ods->mfr_desc, p->mfr_desc, sizeof ods->mfr_desc);
+    ovs_strlcpy(ods->hw_desc, p->hw_desc, sizeof ods->hw_desc);
+    ovs_strlcpy(ods->sw_desc, p->sw_desc, sizeof ods->sw_desc);
+    ovs_strlcpy(ods->serial_num, p->serial_desc, sizeof ods->serial_num);
+    ovs_strlcpy(ods->dp_desc, p->dp_desc, sizeof ods->dp_desc);
     queue_tx(msg, ofconn, ofconn->reply_counter);
 
     return 0;
@@ -2532,6 +2570,9 @@ struct flow_stats_cbdata {
     struct ofpbuf *msg;
 };
 
+/* Obtains statistic counters for 'rule' within 'p' and stores them into
+ * '*packet_countp' and '*byte_countp'.  If 'rule' is a wildcarded rule, the
+ * returned statistic include statistics for all of 'rule''s subrules. */
 static void
 query_stats(struct ofproto *p, struct rule *rule,
             uint64_t *packet_countp, uint64_t *byte_countp)
@@ -2541,9 +2582,19 @@ query_stats(struct ofproto *p, struct rule *rule,
     struct odp_flow *odp_flows;
     size_t n_odp_flows;
 
+    /* Start from historical data for 'rule' itself that are no longer tracked
+     * by the datapath.  This counts, for example, subrules that have
+     * expired. */
     packet_count = rule->packet_count;
     byte_count = rule->byte_count;
 
+    /* Prepare to ask the datapath for statistics on 'rule', or if it is
+     * wildcarded then on all of its subrules.
+     *
+     * Also, add any statistics that are not tracked by the datapath for each
+     * subrule.  This includes, for example, statistics for packets that were
+     * executed "by hand" by ofproto via dpif_execute() but must be accounted
+     * to a flow. */
     n_odp_flows = rule->cr.wc.wildcards ? list_size(&rule->list) : 1;
     odp_flows = xzalloc(n_odp_flows * sizeof *odp_flows);
     if (rule->cr.wc.wildcards) {
@@ -2557,8 +2608,7 @@ query_stats(struct ofproto *p, struct rule *rule,
         odp_flows[0].key = rule->cr.flow;
     }
 
-    packet_count = rule->packet_count;
-    byte_count = rule->byte_count;
+    /* Fetch up-to-date statistics from the datapath and add them in. */
     if (!dpif_flow_get_multiple(p->dpif, odp_flows, n_odp_flows)) {
         size_t i;
         for (i = 0; i < n_odp_flows; i++) {
@@ -2569,6 +2619,7 @@ query_stats(struct ofproto *p, struct rule *rule,
     }
     free(odp_flows);
 
+    /* Return the stats to the caller. */
     *packet_countp = packet_count;
     *byte_countp = byte_count;
 }
@@ -2834,9 +2885,18 @@ update_stats(struct ofproto *ofproto, struct rule *rule,
     }
 }
 
+/* Implements OFPFC_ADD and the cases for OFPFC_MODIFY and OFPFC_MODIFY_STRICT
+ * in which no matching flow already exists in the flow table.
+ *
+ * Adds the flow specified by 'ofm', which is followed by 'n_actions'
+ * ofp_actions, to 'p''s flow table.  Returns 0 on success or an OpenFlow error
+ * code as encoded by ofp_mkerr() on failure.
+ *
+ * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id,
+ * if any. */
 static int
 add_flow(struct ofproto *p, struct ofconn *ofconn,
-         struct ofp_flow_mod *ofm, size_t n_actions)
+         const struct ofp_flow_mod *ofm, size_t n_actions)
 {
     struct ofpbuf *packet;
     struct rule *rule;
@@ -2874,112 +2934,224 @@ add_flow(struct ofproto *p, struct ofconn *ofconn,
     return error;
 }
 
-static int
-modify_flow(struct ofproto *p, const struct ofp_flow_mod *ofm,
-            size_t n_actions, uint16_t command, struct rule *rule)
-{
-    if (rule_is_hidden(rule)) {
-        return 0;
-    }
-
-    if (command == OFPFC_DELETE) {
-        long long int now = time_msec();
-        send_flow_removed(p, rule, now, OFPRR_DELETE);
-        rule_remove(p, rule);
-    } else {
-        size_t actions_len = n_actions * sizeof *rule->actions;
-
-        if (n_actions == rule->n_actions
-            && !memcmp(ofm->actions, rule->actions, actions_len))
-        {
-            return 0;
-        }
-
-        free(rule->actions);
-        rule->actions = xmemdup(ofm->actions, actions_len);
-        rule->n_actions = n_actions;
-        rule->flow_cookie = ofm->cookie;
-
-        if (rule->cr.wc.wildcards) {
-            COVERAGE_INC(ofproto_mod_wc_flow);
-            p->need_revalidate = true;
-        } else {
-            rule_update_actions(p, rule);
-        }
-    }
-
-    return 0;
-}
-
-static int
-modify_flows_strict(struct ofproto *p, const struct ofp_flow_mod *ofm,
-                    size_t n_actions, uint16_t command)
+static struct rule *
+find_flow_strict(struct ofproto *p, const struct ofp_flow_mod *ofm)
 {
-    struct rule *rule;
     uint32_t wildcards;
     flow_t flow;
 
     flow_from_match(&flow, &wildcards, &ofm->match);
-    rule = rule_from_cls_rule(classifier_find_rule_exactly(
+    return rule_from_cls_rule(classifier_find_rule_exactly(
                                   &p->cls, &flow, wildcards,
                                   ntohs(ofm->priority)));
+}
 
-    if (rule) {
-        if (command == OFPFC_DELETE
-            && ofm->out_port != htons(OFPP_NONE)
-            && !rule_has_out_port(rule, ofm->out_port)) {
-            return 0;
-        }
+static int
+send_buffered_packet(struct ofproto *ofproto, struct ofconn *ofconn,
+                     struct rule *rule, const struct ofp_flow_mod *ofm)
+{
+    struct ofpbuf *packet;
+    uint16_t in_port;
+    flow_t flow;
+    int error;
 
-        modify_flow(p, ofm, n_actions, command, rule);
+    if (ofm->buffer_id == htonl(UINT32_MAX)) {
+        return 0;
+    }
+
+    error = pktbuf_retrieve(ofconn->pktbuf, ntohl(ofm->buffer_id),
+                            &packet, &in_port);
+    if (error) {
+        return error;
     }
+
+    flow_extract(packet, in_port, &flow);
+    rule_execute(ofproto, rule, packet, &flow);
+    ofpbuf_delete(packet);
+
     return 0;
 }
+\f
+/* OFPFC_MODIFY and OFPFC_MODIFY_STRICT. */
 
 struct modify_flows_cbdata {
     struct ofproto *ofproto;
     const struct ofp_flow_mod *ofm;
-    uint16_t out_port;
     size_t n_actions;
-    uint16_t command;
+    struct rule *match;
 };
 
+static int modify_flow(struct ofproto *, const struct ofp_flow_mod *,
+                       size_t n_actions, struct rule *);
+static void modify_flows_cb(struct cls_rule *, void *cbdata_);
+
+/* Implements OFPFC_MODIFY.  Returns 0 on success or an OpenFlow error code as
+ * encoded by ofp_mkerr() on failure.
+ *
+ * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id,
+ * if any. */
+static int
+modify_flows_loose(struct ofproto *p, struct ofconn *ofconn,
+                   const struct ofp_flow_mod *ofm, size_t n_actions)
+{
+    struct modify_flows_cbdata cbdata;
+    struct cls_rule target;
+
+    cbdata.ofproto = p;
+    cbdata.ofm = ofm;
+    cbdata.n_actions = n_actions;
+    cbdata.match = NULL;
+
+    cls_rule_from_match(&target, &ofm->match, 0);
+
+    classifier_for_each_match(&p->cls, &target, CLS_INC_ALL,
+                              modify_flows_cb, &cbdata);
+    if (cbdata.match) {
+        /* This credits the packet to whichever flow happened to happened to
+         * match last.  That's weird.  Maybe we should do a lookup for the
+         * flow that actually matches the packet?  Who knows. */
+        send_buffered_packet(p, ofconn, cbdata.match, ofm);
+        return 0;
+    } else {
+        return add_flow(p, ofconn, ofm, n_actions);
+    }
+}
+
+/* Implements OFPFC_MODIFY_STRICT.  Returns 0 on success or an OpenFlow error
+ * code as encoded by ofp_mkerr() on failure.
+ *
+ * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id,
+ * if any. */
+static int
+modify_flow_strict(struct ofproto *p, struct ofconn *ofconn,
+                   struct ofp_flow_mod *ofm, size_t n_actions)
+{
+    struct rule *rule = find_flow_strict(p, ofm);
+    if (rule && !rule_is_hidden(rule)) {
+        modify_flow(p, ofm, n_actions, rule);
+        return send_buffered_packet(p, ofconn, rule, ofm);
+    } else {
+        return add_flow(p, ofconn, ofm, n_actions);
+    }
+}
+
+/* Callback for modify_flows_loose(). */
 static void
 modify_flows_cb(struct cls_rule *rule_, void *cbdata_)
 {
     struct rule *rule = rule_from_cls_rule(rule_);
     struct modify_flows_cbdata *cbdata = cbdata_;
 
-    if (cbdata->out_port != htons(OFPP_NONE)
-        && !rule_has_out_port(rule, cbdata->out_port)) {
-        return;
+    if (!rule_is_hidden(rule)) {
+        cbdata->match = rule;
+        modify_flow(cbdata->ofproto, cbdata->ofm, cbdata->n_actions, rule);
     }
-
-    modify_flow(cbdata->ofproto, cbdata->ofm, cbdata->n_actions,
-                cbdata->command, rule);
 }
 
+/* Implements core of OFPFC_MODIFY and OFPFC_MODIFY_STRICT where 'rule' has
+ * been identified as a flow in 'p''s flow table to be modified, by changing
+ * the rule's actions to match those in 'ofm' (which is followed by 'n_actions'
+ * ofp_action[] structures). */
 static int
-modify_flows_loose(struct ofproto *p, const struct ofp_flow_mod *ofm,
-                   size_t n_actions, uint16_t command)
+modify_flow(struct ofproto *p, const struct ofp_flow_mod *ofm,
+            size_t n_actions, struct rule *rule)
 {
-    struct modify_flows_cbdata cbdata;
+    size_t actions_len = n_actions * sizeof *rule->actions;
+
+    rule->flow_cookie = ofm->cookie;
+
+    /* If the actions are the same, do nothing. */
+    if (n_actions == rule->n_actions
+        && !memcmp(ofm->actions, rule->actions, actions_len))
+    {
+        return 0;
+    }
+
+    /* Replace actions. */
+    free(rule->actions);
+    rule->actions = xmemdup(ofm->actions, actions_len);
+    rule->n_actions = n_actions;
+
+    /* Make sure that the datapath gets updated properly. */
+    if (rule->cr.wc.wildcards) {
+        COVERAGE_INC(ofproto_mod_wc_flow);
+        p->need_revalidate = true;
+    } else {
+        rule_update_actions(p, rule);
+    }
+
+    return 0;
+}
+\f
+/* OFPFC_DELETE implementation. */
+
+struct delete_flows_cbdata {
+    struct ofproto *ofproto;
+    uint16_t out_port;
+};
+
+static void delete_flows_cb(struct cls_rule *, void *cbdata_);
+static void delete_flow(struct ofproto *, struct rule *, uint16_t out_port);
+
+/* Implements OFPFC_DELETE. */
+static void
+delete_flows_loose(struct ofproto *p, const struct ofp_flow_mod *ofm)
+{
+    struct delete_flows_cbdata cbdata;
     struct cls_rule target;
 
     cbdata.ofproto = p;
-    cbdata.ofm = ofm;
-    cbdata.out_port = (command == OFPFC_DELETE ? ofm->out_port
-                       : htons(OFPP_NONE));
-    cbdata.n_actions = n_actions;
-    cbdata.command = command;
+    cbdata.out_port = ofm->out_port;
 
     cls_rule_from_match(&target, &ofm->match, 0);
 
     classifier_for_each_match(&p->cls, &target, CLS_INC_ALL,
-                              modify_flows_cb, &cbdata);
-    return 0;
+                              delete_flows_cb, &cbdata);
+}
+
+/* Implements OFPFC_DELETE_STRICT. */
+static void
+delete_flow_strict(struct ofproto *p, struct ofp_flow_mod *ofm)
+{
+    struct rule *rule = find_flow_strict(p, ofm);
+    if (rule) {
+        delete_flow(p, rule, ofm->out_port);
+    }
+}
+
+/* Callback for delete_flows_loose(). */
+static void
+delete_flows_cb(struct cls_rule *rule_, void *cbdata_)
+{
+    struct rule *rule = rule_from_cls_rule(rule_);
+    struct delete_flows_cbdata *cbdata = cbdata_;
+
+    delete_flow(cbdata->ofproto, rule, cbdata->out_port);
 }
 
+/* Implements core of OFPFC_DELETE and OFPFC_DELETE_STRICT where 'rule' has
+ * been identified as a flow to delete from 'p''s flow table, by deleting the
+ * flow and sending out a OFPT_FLOW_REMOVED message to any interested
+ * controller.
+ *
+ * Will not delete 'rule' if it is hidden.  Will delete 'rule' only if
+ * 'out_port' is htons(OFPP_NONE) or if 'rule' actually outputs to the
+ * specified 'out_port'. */
+static void
+delete_flow(struct ofproto *p, struct rule *rule, uint16_t out_port)
+{
+    if (rule_is_hidden(rule)) {
+        return;
+    }
+
+    if (out_port != htons(OFPP_NONE) && !rule_has_out_port(rule, out_port)) {
+        return;
+    }
+
+    send_flow_removed(p, rule, time_msec(), OFPRR_DELETE);
+    rule_remove(p, rule);
+}
+\f
 static int
 handle_flow_mod(struct ofproto *p, struct ofconn *ofconn,
                 struct ofp_flow_mod *ofm)
@@ -3017,16 +3189,18 @@ handle_flow_mod(struct ofproto *p, struct ofconn *ofconn,
         return add_flow(p, ofconn, ofm, n_actions);
 
     case OFPFC_MODIFY:
-        return modify_flows_loose(p, ofm, n_actions, OFPFC_MODIFY);
+        return modify_flows_loose(p, ofconn, ofm, n_actions);
 
     case OFPFC_MODIFY_STRICT:
-        return modify_flows_strict(p, ofm, n_actions, OFPFC_MODIFY);
+        return modify_flow_strict(p, ofconn, ofm, n_actions);
 
     case OFPFC_DELETE:
-        return modify_flows_loose(p, ofm, n_actions, OFPFC_DELETE);
+        delete_flows_loose(p, ofm);
+        return 0;
 
     case OFPFC_DELETE_STRICT:
-        return modify_flows_strict(p, ofm, n_actions, OFPFC_DELETE);
+        delete_flow_strict(p, ofm);
+        return 0;
 
     default:
         return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_BAD_COMMAND);
@@ -3295,7 +3469,7 @@ compose_flow_removed(const struct rule *rule, long long int now, uint8_t reason)
 {
     struct ofp_flow_removed *ofr;
     struct ofpbuf *buf;
-    long long int tdiff = time_msec() - rule->created;
+    long long int tdiff = now - rule->created;
     uint32_t sec = tdiff / 1000;
     uint32_t msec = tdiff - (sec * 1000);
 
@@ -3528,7 +3702,7 @@ send_packet_in_miss(struct ofpbuf *packet, void *p_)
                                   ? pktbuf_get_null()
                                   : pktbuf_save(pb, &payload, msg->port));
             int send_len = (buffer_id != UINT32_MAX ? ofconn->miss_send_len
-                            : UINT32_MAX);
+                            : INT_MAX);
             do_send_packet_in(ofconn, buffer_id, packet, send_len);
         }
     }