Set MTU in userspace rather than kernel.
[sliver-openvswitch.git] / ofproto / ofproto.c
index cb349ae..1309b49 100644 (file)
@@ -97,10 +97,10 @@ struct ofopgroup {
     int error;                  /* 0 if no error yet, otherwise error code. */
 };
 
-static struct ofopgroup *ofopgroup_create(struct ofproto *);
-static struct ofopgroup *ofopgroup_create_for_ofconn(struct ofconn *,
-                                                     const struct ofp_header *,
-                                                     uint32_t buffer_id);
+static struct ofopgroup *ofopgroup_create_unattached(struct ofproto *);
+static struct ofopgroup *ofopgroup_create(struct ofproto *, struct ofconn *,
+                                          const struct ofp_header *,
+                                          uint32_t buffer_id);
 static void ofopgroup_submit(struct ofopgroup *);
 static void ofopgroup_destroy(struct ofopgroup *);
 
@@ -136,22 +136,18 @@ static void ofproto_rule_send_removed(struct rule *, uint8_t reason);
 static void ofopgroup_destroy(struct ofopgroup *);
 
 static int add_flow(struct ofproto *, struct ofconn *,
-                    struct ofputil_flow_mod *, const struct ofp_header *);
-
-/* This return value tells handle_openflow() that processing of the current
- * OpenFlow message must be postponed until some ongoing operations have
- * completed.
- *
- * This particular value is a good choice because it is negative (so it won't
- * collide with any errno value or any value returned by ofp_mkerr()) and large
- * (so it won't accidentally collide with EOF or a negative errno value). */
-enum { OFPROTO_POSTPONE = -100000 };
+                    const struct ofputil_flow_mod *,
+                    const struct ofp_header *);
 
 static bool handle_openflow(struct ofconn *, struct ofpbuf *);
+static int handle_flow_mod__(struct ofproto *, struct ofconn *,
+                             const struct ofputil_flow_mod *,
+                             const struct ofp_header *);
 
 static void update_port(struct ofproto *, const char *devname);
 static int init_ports(struct ofproto *);
 static void reinit_ports(struct ofproto *);
+static void set_internal_devs_mtu(struct ofproto *);
 
 static void ofproto_unixctl_init(void);
 
@@ -338,6 +334,7 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     ofproto->connmgr = connmgr_create(ofproto, datapath_name, datapath_name);
     ofproto->state = S_OPENFLOW;
     list_init(&ofproto->pending);
+    ofproto->n_pending = 0;
     hmap_init(&ofproto->deletions);
 
     error = ofproto->ofproto_class->construct(ofproto, &n_tables);
@@ -430,7 +427,7 @@ ofproto_set_flow_eviction_threshold(struct ofproto *ofproto, unsigned threshold)
     }
 }
 
-/* If forward_bpdu is true, the NORMAL action will forward frames with 
+/* If forward_bpdu is true, the NORMAL action will forward frames with
  * reserved (e.g. STP) destination Ethernet addresses. if forward_bpdu is false,
  * the NORMAL action will drop these frames. */
 void
@@ -442,7 +439,7 @@ ofproto_set_forward_bpdu(struct ofproto *ofproto, bool forward_bpdu)
         if (ofproto->ofproto_class->forward_bpdu_changed) {
             ofproto->ofproto_class->forward_bpdu_changed(ofproto);
         }
-    }   
+    }
 }
 
 void
@@ -690,7 +687,7 @@ ofproto_flush__(struct ofproto *ofproto)
         ofproto->ofproto_class->flush(ofproto);
     }
 
-    group = ofopgroup_create(ofproto);
+    group = ofopgroup_create_unattached(ofproto);
     OFPROTO_FOR_EACH_TABLE (table, ofproto) {
         struct rule *rule, *next_rule;
         struct cls_cursor cursor;
@@ -713,6 +710,7 @@ ofproto_destroy__(struct ofproto *ofproto)
     struct classifier *table;
 
     assert(list_is_empty(&ofproto->pending));
+    assert(!ofproto->n_pending);
 
     connmgr_destroy(ofproto->connmgr);
 
@@ -1059,6 +1057,18 @@ ofproto_add_flow(struct ofproto *ofproto, const struct cls_rule *cls_rule,
     }
 }
 
+/* Executes the flow modification specified in 'fm'.  Returns 0 on success, an
+ * OpenFlow error code as encoded by ofp_mkerr() on failure, or
+ * OFPROTO_POSTPONE if the operation cannot be initiated now but may be retried
+ * later.
+ *
+ * This is a helper function for in-band control and fail-open. */
+int
+ofproto_flow_mod(struct ofproto *ofproto, const struct ofputil_flow_mod *fm)
+{
+    return handle_flow_mod__(ofproto, NULL, fm, NULL);
+}
+
 /* Searches for a rule with matching criteria exactly equal to 'target' in
  * ofproto's table 0 and, if it finds one, deletes it.
  *
@@ -1079,7 +1089,7 @@ ofproto_delete_flow(struct ofproto *ofproto, const struct cls_rule *target)
         return false;
     } else {
         /* Initiate deletion -> success. */
-        struct ofopgroup *group = ofopgroup_create(ofproto);
+        struct ofopgroup *group = ofopgroup_create_unattached(ofproto);
         ofoperation_create(group, rule, OFOPERATION_DELETE);
         classifier_remove(&ofproto->tables[rule->table_id], &rule->cr);
         rule->ofproto->ofproto_class->rule_destruct(rule);
@@ -1184,6 +1194,7 @@ ofport_install(struct ofproto *p,
 {
     const char *netdev_name = netdev_get_name(netdev);
     struct ofport *ofport;
+    int dev_mtu;
     int error;
 
     /* Create ofport. */
@@ -1202,6 +1213,13 @@ ofport_install(struct ofproto *p,
     hmap_insert(&p->ports, &ofport->hmap_node, hash_int(ofport->ofp_port, 0));
     shash_add(&p->port_by_name, netdev_name, ofport);
 
+    if (!netdev_get_mtu(netdev, &dev_mtu)) {
+        set_internal_devs_mtu(p);
+        ofport->mtu = dev_mtu;
+    } else {
+        ofport->mtu = 0;
+    }
+
     /* Let the ofproto_class initialize its private data. */
     error = p->ofproto_class->port_construct(ofport);
     if (error) {
@@ -1328,12 +1346,22 @@ update_port(struct ofproto *ofproto, const char *name)
         port = ofproto_get_port(ofproto, ofproto_port.ofp_port);
         if (port && !strcmp(netdev_get_name(port->netdev), name)) {
             struct netdev *old_netdev = port->netdev;
+            int dev_mtu;
 
             /* 'name' hasn't changed location.  Any properties changed? */
             if (!ofport_equal(&port->opp, &opp)) {
                 ofport_modified(port, &opp);
             }
 
+            /* If this is a non-internal port and the MTU changed, check
+             * if the datapath's MTU needs to be updated. */
+            if (strcmp(netdev_get_type(netdev), "internal")
+                    && !netdev_get_mtu(netdev, &dev_mtu)
+                    && port->mtu != dev_mtu) {
+                set_internal_devs_mtu(ofproto);
+                port->mtu = dev_mtu;
+            }
+
             /* Install the newly opened netdev in case it has changed.
              * Don't close the old netdev yet in case port_modified has to
              * remove a retained reference to it.*/
@@ -1389,6 +1417,52 @@ init_ports(struct ofproto *p)
 
     return 0;
 }
+
+/* Find the minimum MTU of all non-datapath devices attached to 'p'.
+ * Returns ETH_PAYLOAD_MAX or the minimum of the ports. */
+static int
+find_min_mtu(struct ofproto *p)
+{
+    struct ofport *ofport;
+    int mtu = 0;
+
+    HMAP_FOR_EACH (ofport, hmap_node, &p->ports) {
+        struct netdev *netdev = ofport->netdev;
+        int dev_mtu;
+
+        /* Skip any internal ports, since that's what we're trying to
+         * set. */
+        if (!strcmp(netdev_get_type(netdev), "internal")) {
+            continue;
+        }
+
+        if (netdev_get_mtu(netdev, &dev_mtu)) {
+            continue;
+        }
+        if (!mtu || dev_mtu < mtu) {
+            mtu = dev_mtu;
+        }
+    }
+
+    return mtu ? mtu: ETH_PAYLOAD_MAX;
+}
+
+/* Set the MTU of all datapath devices on 'p' to the minimum of the
+ * non-datapath ports. */
+static void
+set_internal_devs_mtu(struct ofproto *p)
+{
+    struct ofport *ofport;
+    int mtu = find_min_mtu(p);
+
+    HMAP_FOR_EACH (ofport, hmap_node, &p->ports) {
+        struct netdev *netdev = ofport->netdev;
+
+        if (!strcmp(netdev_get_type(netdev), "internal")) {
+            netdev_set_mtu(netdev, mtu);
+        }
+    }
+}
 \f
 static void
 ofproto_rule_destroy__(struct rule *rule)
@@ -1581,7 +1655,6 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
     struct ofpbuf request;
     struct flow flow;
     size_t n_ofp_actions;
-    uint16_t in_port;
     int error;
 
     COVERAGE_INC(ofproto_packet_out);
@@ -1605,7 +1678,7 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
     /* Get payload. */
     if (opo->buffer_id != htonl(UINT32_MAX)) {
         error = ofconn_pktbuf_retrieve(ofconn, ntohl(opo->buffer_id),
-                                       &buffer, &in_port);
+                                       &buffer, NULL);
         if (error || !buffer) {
             return error;
         }
@@ -1899,7 +1972,7 @@ handle_flow_stats_request(struct ofconn *ofconn,
                           const struct ofp_stats_msg *osm)
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-    struct flow_stats_request fsr;
+    struct ofputil_flow_stats_request fsr;
     struct list replies;
     struct list rules;
     struct rule *rule;
@@ -2003,12 +2076,31 @@ ofproto_port_get_cfm_fault(const struct ofproto *ofproto, uint16_t ofp_port)
             : -1);
 }
 
+/* Gets the MPIDs of the remote maintenance points broadcasting to 'ofp_port'
+ * within 'ofproto'.  Populates 'rmps' with an array of MPIDs owned by
+ * 'ofproto', and 'n_rmps' with the number of MPIDs in 'rmps'.  Returns a
+ * number less than 0 if CFM is not enabled on 'ofp_port'. */
+int
+ofproto_port_get_cfm_remote_mpids(const struct ofproto *ofproto,
+                                  uint16_t ofp_port, const uint64_t **rmps,
+                                  size_t *n_rmps)
+{
+    struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
+
+    *rmps = NULL;
+    *n_rmps = 0;
+    return (ofport && ofproto->ofproto_class->get_cfm_remote_mpids
+            ? ofproto->ofproto_class->get_cfm_remote_mpids(ofport, rmps,
+                                                           n_rmps)
+            : -1);
+}
+
 static int
 handle_aggregate_stats_request(struct ofconn *ofconn,
                                const struct ofp_stats_msg *osm)
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-    struct flow_stats_request request;
+    struct ofputil_flow_stats_request request;
     struct ofputil_aggregate_stats stats;
     bool unknown_packets, unknown_bytes;
     struct ofpbuf *reply;
@@ -2168,14 +2260,15 @@ is_flow_deletion_pending(const struct ofproto *ofproto,
  * 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 the ofproto's flow table.  Returns 0 on success or an
- * OpenFlow error code as encoded by ofp_mkerr() on failure.
+ * ofp_actions, to the ofproto's flow table.  Returns 0 on success, an OpenFlow
+ * error code as encoded by ofp_mkerr() on failure, or OFPROTO_POSTPONE if the
+ * operation cannot be initiated now but may be retried later.
  *
  * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id,
  * if any. */
 static int
 add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
-         struct ofputil_flow_mod *fm, const struct ofp_header *request)
+         const struct ofputil_flow_mod *fm, const struct ofp_header *request)
 {
     struct classifier *table;
     struct ofopgroup *group;
@@ -2230,7 +2323,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     rule->cr = fm->cr;
     rule->pending = NULL;
     rule->flow_cookie = fm->cookie;
-    rule->created = time_msec();
+    rule->created = rule->modified = time_msec();
     rule->idle_timeout = fm->idle_timeout;
     rule->hard_timeout = fm->hard_timeout;
     rule->table_id = table - ofproto->tables;
@@ -2243,9 +2336,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
     if (victim && victim->pending) {
         error = OFPROTO_POSTPONE;
     } else {
-        group = (ofconn
-                 ? ofopgroup_create_for_ofconn(ofconn, request, fm->buffer_id)
-                 : ofopgroup_create(ofproto));
+        group = ofopgroup_create(ofproto, ofconn, request, fm->buffer_id);
         ofoperation_create(group, rule, OFOPERATION_ADD);
         rule->pending->victim = victim;
 
@@ -2278,13 +2369,14 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
  *
  * Returns 0 on success, otherwise an OpenFlow error code. */
 static int
-modify_flows__(struct ofconn *ofconn, const struct ofputil_flow_mod *fm,
+modify_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
+               const struct ofputil_flow_mod *fm,
                const struct ofp_header *request, struct list *rules)
 {
     struct ofopgroup *group;
     struct rule *rule;
 
-    group = ofopgroup_create_for_ofconn(ofconn, request, fm->buffer_id);
+    group = ofopgroup_create(ofproto, ofconn, request, fm->buffer_id);
     LIST_FOR_EACH (rule, ofproto_node, rules) {
         if (!ofputil_actions_equal(fm->actions, fm->n_actions,
                                    rule->actions, rule->n_actions)) {
@@ -2294,6 +2386,8 @@ modify_flows__(struct ofconn *ofconn, const struct ofputil_flow_mod *fm,
             rule->actions = ofputil_actions_clone(fm->actions, fm->n_actions);
             rule->n_actions = fm->n_actions;
             rule->ofproto->ofproto_class->rule_modify_actions(rule);
+        } else {
+            rule->modified = time_msec();
         }
         rule->flow_cookie = fm->cookie;
     }
@@ -2308,17 +2402,18 @@ modify_flows__(struct ofconn *ofconn, const struct ofputil_flow_mod *fm,
  * 'ofconn' is used to retrieve the packet buffer specified in fm->buffer_id,
  * if any. */
 static int
-modify_flows_loose(struct ofconn *ofconn, struct ofputil_flow_mod *fm,
+modify_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
+                   const struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
 {
-    struct ofproto *p = ofconn_get_ofproto(ofconn);
     struct list rules;
     int error;
 
-    error = collect_rules_loose(p, fm->table_id, &fm->cr, OFPP_NONE, &rules);
+    error = collect_rules_loose(ofproto, fm->table_id, &fm->cr, OFPP_NONE,
+                                &rules);
     return (error ? error
-            : list_is_empty(&rules) ? add_flow(p, ofconn, fm, request)
-            : modify_flows__(ofconn, fm, request, &rules));
+            : list_is_empty(&rules) ? add_flow(ofproto, ofconn, fm, request)
+            : modify_flows__(ofproto, ofconn, fm, request, &rules));
 }
 
 /* Implements OFPFC_MODIFY_STRICT.  Returns 0 on success or an OpenFlow error
@@ -2327,18 +2422,19 @@ modify_flows_loose(struct ofconn *ofconn, struct ofputil_flow_mod *fm,
  * 'ofconn' is used to retrieve the packet buffer specified in fm->buffer_id,
  * if any. */
 static int
-modify_flow_strict(struct ofconn *ofconn, struct ofputil_flow_mod *fm,
+modify_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
+                   const struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
 {
-    struct ofproto *p = ofconn_get_ofproto(ofconn);
     struct list rules;
     int error;
 
-    error = collect_rules_strict(p, fm->table_id, &fm->cr, OFPP_NONE, &rules);
+    error = collect_rules_strict(ofproto, fm->table_id, &fm->cr, OFPP_NONE,
+                                 &rules);
     return (error ? error
-            : list_is_empty(&rules) ? add_flow(p, ofconn, fm, request)
-            : list_is_singleton(&rules) ? modify_flows__(ofconn, fm, request,
-                                                         &rules)
+            : list_is_empty(&rules) ? add_flow(ofproto, ofconn, fm, request)
+            : list_is_singleton(&rules) ? modify_flows__(ofproto, ofconn,
+                                                         fm, request, &rules)
             : 0);
 }
 \f
@@ -2348,14 +2444,13 @@ modify_flow_strict(struct ofconn *ofconn, struct ofputil_flow_mod *fm,
  *
  * Returns 0 on success, otherwise an OpenFlow error code. */
 static int
-delete_flows__(struct ofconn *ofconn, const struct ofp_header *request,
-               struct list *rules)
+delete_flows__(struct ofproto *ofproto, struct ofconn *ofconn,
+               const struct ofp_header *request, struct list *rules)
 {
-    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     struct rule *rule, *next;
     struct ofopgroup *group;
 
-    group = ofopgroup_create_for_ofconn(ofconn, request, UINT32_MAX);
+    group = ofopgroup_create(ofproto, ofconn, request, UINT32_MAX);
     LIST_FOR_EACH_SAFE (rule, next, ofproto_node, rules) {
         ofproto_rule_send_removed(rule, OFPRR_DELETE);
 
@@ -2370,34 +2465,35 @@ delete_flows__(struct ofconn *ofconn, const struct ofp_header *request,
 
 /* Implements OFPFC_DELETE. */
 static int
-delete_flows_loose(struct ofconn *ofconn, const struct ofputil_flow_mod *fm,
+delete_flows_loose(struct ofproto *ofproto, struct ofconn *ofconn,
+                   const struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
 {
-    struct ofproto *p = ofconn_get_ofproto(ofconn);
     struct list rules;
     int error;
 
-    error = collect_rules_loose(p, fm->table_id, &fm->cr, fm->out_port,
+    error = collect_rules_loose(ofproto, fm->table_id, &fm->cr, fm->out_port,
                                 &rules);
     return (error ? error
-            : !list_is_empty(&rules) ? delete_flows__(ofconn, request, &rules)
+            : !list_is_empty(&rules) ? delete_flows__(ofproto, ofconn, request,
+                                                      &rules)
             : 0);
 }
 
 /* Implements OFPFC_DELETE_STRICT. */
 static int
-delete_flow_strict(struct ofconn *ofconn, struct ofputil_flow_mod *fm,
+delete_flow_strict(struct ofproto *ofproto, struct ofconn *ofconn,
+                   const struct ofputil_flow_mod *fm,
                    const struct ofp_header *request)
 {
-    struct ofproto *p = ofconn_get_ofproto(ofconn);
     struct list rules;
     int error;
 
-    error = collect_rules_strict(p, fm->table_id, &fm->cr, fm->out_port,
+    error = collect_rules_strict(ofproto, fm->table_id, &fm->cr, fm->out_port,
                                  &rules);
     return (error ? error
-            : list_is_singleton(&rules) ? delete_flows__(ofconn, request,
-                                                         &rules)
+            : list_is_singleton(&rules) ? delete_flows__(ofproto, ofconn,
+                                                         request, &rules)
             : 0);
 }
 
@@ -2437,7 +2533,7 @@ ofproto_rule_expire(struct rule *rule, uint8_t reason)
 
     ofproto_rule_send_removed(rule, reason);
 
-    group = ofopgroup_create(ofproto);
+    group = ofopgroup_create_unattached(ofproto);
     ofoperation_create(group, rule, OFOPERATION_DELETE);
     classifier_remove(&ofproto->tables[rule->table_id], &rule->cr);
     rule->ofproto->ofproto_class->rule_destruct(rule);
@@ -2447,7 +2543,6 @@ ofproto_rule_expire(struct rule *rule, uint8_t reason)
 static int
 handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
 {
-    struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     struct ofputil_flow_mod fm;
     int error;
 
@@ -2456,10 +2551,6 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
         return error;
     }
 
-    if (list_size(&ofproto->pending) >= 50) {
-        return OFPROTO_POSTPONE;
-    }
-
     error = ofputil_decode_flow_mod(&fm, oh,
                                     ofconn_get_flow_mod_table_id(ofconn));
     if (error) {
@@ -2474,24 +2565,37 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
         return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_ALL_TABLES_FULL);
     }
 
-    switch (fm.command) {
+    return handle_flow_mod__(ofconn_get_ofproto(ofconn), ofconn, &fm, oh);
+}
+
+static int
+handle_flow_mod__(struct ofproto *ofproto, struct ofconn *ofconn,
+                  const struct ofputil_flow_mod *fm,
+                  const struct ofp_header *oh)
+{
+    if (ofproto->n_pending >= 50) {
+        assert(!list_is_empty(&ofproto->pending));
+        return OFPROTO_POSTPONE;
+    }
+
+    switch (fm->command) {
     case OFPFC_ADD:
-        return add_flow(ofproto, ofconn, &fm, oh);
+        return add_flow(ofproto, ofconn, fm, oh);
 
     case OFPFC_MODIFY:
-        return modify_flows_loose(ofconn, &fm, oh);
+        return modify_flows_loose(ofproto, ofconn, fm, oh);
 
     case OFPFC_MODIFY_STRICT:
-        return modify_flow_strict(ofconn, &fm, oh);
+        return modify_flow_strict(ofproto, ofconn, fm, oh);
 
     case OFPFC_DELETE:
-        return delete_flows_loose(ofconn, &fm, oh);
+        return delete_flows_loose(ofproto, ofconn, fm, oh);
 
     case OFPFC_DELETE_STRICT:
-        return delete_flow_strict(ofconn, &fm, oh);
+        return delete_flow_strict(ofproto, ofconn, fm, oh);
 
     default:
-        if (fm.command > 0xff) {
+        if (fm->command > 0xff) {
             VLOG_WARN_RL(&rl, "flow_mod has explicit table_id but "
                          "flow_mod_table_id extension is not enabled");
         }
@@ -2713,7 +2817,7 @@ handle_openflow(struct ofconn *ofconn, struct ofpbuf *ofp_msg)
  * The caller should add operations to the returned group with
  * ofoperation_create() and then submit it with ofopgroup_submit(). */
 static struct ofopgroup *
-ofopgroup_create(struct ofproto *ofproto)
+ofopgroup_create_unattached(struct ofproto *ofproto)
 {
     struct ofopgroup *group = xzalloc(sizeof *group);
     group->ofproto = ofproto;
@@ -2723,26 +2827,33 @@ ofopgroup_create(struct ofproto *ofproto)
     return group;
 }
 
-/* Creates and returns a new ofopgroup that is associated with 'ofconn'.  If
- * the ofopgroup eventually fails, then the error reply will include 'request'.
- * If the ofopgroup eventually succeeds, then the packet with buffer id
- * 'buffer_id' on 'ofconn' will be sent by 'ofconn''s ofproto.
+/* Creates and returns a new ofopgroup for 'ofproto'.
+ *
+ * If 'ofconn' is NULL, the new ofopgroup is not associated with any OpenFlow
+ * connection.  The 'request' and 'buffer_id' arguments are ignored.
+ *
+ * If 'ofconn' is nonnull, then the new ofopgroup is associated with 'ofconn'.
+ * If the ofopgroup eventually fails, then the error reply will include
+ * 'request'.  If the ofopgroup eventually succeeds, then the packet with
+ * buffer id 'buffer_id' on 'ofconn' will be sent by 'ofconn''s ofproto.
  *
  * The caller should add operations to the returned group with
  * ofoperation_create() and then submit it with ofopgroup_submit(). */
 static struct ofopgroup *
-ofopgroup_create_for_ofconn(struct ofconn *ofconn,
-                            const struct ofp_header *request,
-                            uint32_t buffer_id)
+ofopgroup_create(struct ofproto *ofproto, struct ofconn *ofconn,
+                 const struct ofp_header *request, uint32_t buffer_id)
 {
-    struct ofopgroup *group = ofopgroup_create(ofconn_get_ofproto(ofconn));
-    size_t request_len = ntohs(request->length);
+    struct ofopgroup *group = ofopgroup_create_unattached(ofproto);
+    if (ofconn) {
+        size_t request_len = ntohs(request->length);
 
-    ofconn_add_opgroup(ofconn, &group->ofconn_node);
-    group->ofconn = ofconn;
-    group->request = xmemdup(request, MIN(request_len, 64));
-    group->buffer_id = buffer_id;
+        assert(ofconn_get_ofproto(ofconn) == ofproto);
 
+        ofconn_add_opgroup(ofconn, &group->ofconn_node);
+        group->ofconn = ofconn;
+        group->request = xmemdup(request, MIN(request_len, 64));
+        group->buffer_id = buffer_id;
+    }
     return group;
 }
 
@@ -2759,6 +2870,7 @@ ofopgroup_submit(struct ofopgroup *group)
         ofopgroup_destroy(group);
     } else {
         list_push_back(&group->ofproto->pending, &group->ofproto_node);
+        group->ofproto->n_pending++;
     }
 }
 
@@ -2767,6 +2879,8 @@ ofopgroup_destroy(struct ofopgroup *group)
 {
     assert(list_is_empty(&group->ops));
     if (!list_is_empty(&group->ofproto_node)) {
+        assert(group->ofproto->n_pending > 0);
+        group->ofproto->n_pending--;
         list_remove(&group->ofproto_node);
     }
     if (!list_is_empty(&group->ofconn_node)) {
@@ -2828,8 +2942,29 @@ ofoperation_destroy(struct ofoperation *op)
  * indicate success or an OpenFlow error code (constructed with
  * e.g. ofp_mkerr()).
  *
- * If 'op' is a "delete flow" operation, 'error' must be 0.  That is, flow
- * deletions are not allowed to fail.
+ * If 'error' is 0, indicating success, the operation will be committed
+ * permanently to the flow table.  There is one interesting subcase:
+ *
+ *   - If 'op' is an "add flow" operation that is replacing an existing rule in
+ *     the flow table (the "victim" rule) by a new one, then the caller must
+ *     have uninitialized any derived state in the victim rule, as in step 5 in
+ *     the "Life Cycle" in ofproto/ofproto-provider.h.  ofoperation_complete()
+ *     performs steps 6 and 7 for the victim rule, most notably by calling its
+ *     ->rule_dealloc() function.
+ *
+ * If 'error' is nonzero, then generally the operation will be rolled back:
+ *
+ *   - If 'op' is an "add flow" operation, ofproto removes the new rule or
+ *     restores the original rule.  The caller must have uninitialized any
+ *     derived state in the new rule, as in step 5 of in the "Life Cycle" in
+ *     ofproto/ofproto-provider.h.  ofoperation_complete() performs steps 6 and
+ *     and 7 for the new rule, calling its ->rule_dealloc() function.
+ *
+ *   - If 'op' is a "modify flow" operation, ofproto restores the original
+ *     actions.
+ *
+ *   - 'op' must not be a "delete flow" operation.  Removing a rule is not
+ *     allowed to fail.  It must always succeed.
  *
  * Please see the large comment in ofproto/ofproto-provider.h titled
  * "Asynchronous Operation Support" for more information. */
@@ -2889,7 +3024,9 @@ ofoperation_complete(struct ofoperation *op, int error)
         break;
 
     case OFOPERATION_MODIFY:
-        if (error) {
+        if (!error) {
+            rule->modified = time_msec();
+        } else {
             free(rule->actions);
             rule->actions = op->actions;
             rule->n_actions = op->n_actions;