X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=blobdiff_plain;f=ofproto%2Fofproto.c;h=b7b31b0228bffd0757d382609df7638a8097b8ee;hb=859ad376132e1eb13d0a1a28cae2c477300fd45f;hp=ff5cb0149ee43db7b3a45858bb01590868bf3438;hpb=7ee20df120d4d56df894abc00a60fe1bc43a058d;p=openvswitch

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index ff5cb014..b7b31b02 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -33,13 +33,13 @@
 #include "ofp-print.h"
 #include "ofp-util.h"
 #include "ofpbuf.h"
+#include "ofproto-provider.h"
 #include "openflow/nicira-ext.h"
 #include "openflow/openflow.h"
 #include "packets.h"
 #include "pinsched.h"
 #include "pktbuf.h"
 #include "poll-loop.h"
-#include "private.h"
 #include "shash.h"
 #include "sset.h"
 #include "timeval.h"
@@ -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 *);
 
@@ -135,23 +135,19 @@ 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 flow_mod *,
+static int add_flow(struct ofproto *, struct ofconn *,
+                    const 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 };
-
 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);
 
@@ -289,7 +285,9 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
                struct ofproto **ofprotop)
 {
     const struct ofproto_class *class;
+    struct classifier *table;
     struct ofproto *ofproto;
+    int n_tables;
     int error;
 
     *ofprotop = NULL;
@@ -320,12 +318,16 @@ ofproto_create(const char *datapath_name, const char *datapath_type,
     hmap_insert(&all_ofprotos, &ofproto->hmap_node,
                 hash_string(ofproto->name, 0));
     ofproto->datapath_id = 0;
+    ofproto_set_flow_eviction_threshold(ofproto,
+                                        OFPROTO_FLOW_EVICTON_THRESHOLD_DEFAULT);
+    ofproto->forward_bpdu = false;
     ofproto->fallback_dpid = pick_fallback_dpid();
     ofproto->mfr_desc = xstrdup(DEFAULT_MFR_DESC);
     ofproto->hw_desc = xstrdup(DEFAULT_HW_DESC);
     ofproto->sw_desc = xstrdup(DEFAULT_SW_DESC);
     ofproto->serial_desc = xstrdup(DEFAULT_SERIAL_DESC);
     ofproto->dp_desc = xstrdup(DEFAULT_DP_DESC);
+    ofproto->frag_handling = OFPC_FRAG_NORMAL;
     hmap_init(&ofproto->ports);
     shash_init(&ofproto->port_by_name);
     ofproto->tables = NULL;
@@ -333,16 +335,23 @@ 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);
+    error = ofproto->ofproto_class->construct(ofproto, &n_tables);
     if (error) {
         VLOG_ERR("failed to open datapath %s: %s",
                  datapath_name, strerror(error));
         ofproto_destroy__(ofproto);
         return error;
     }
-    assert(ofproto->n_tables > 0);
+
+    assert(n_tables >= 1 && n_tables <= 255);
+    ofproto->n_tables = n_tables;
+    ofproto->tables = xmalloc(n_tables * sizeof *ofproto->tables);
+    OFPROTO_FOR_EACH_TABLE (table, ofproto) {
+        classifier_init(table);
+    }
 
     ofproto->datapath_id = pick_datapath_id(ofproto);
     VLOG_INFO("using datapath ID %016"PRIx64, ofproto->datapath_id);
@@ -407,6 +416,33 @@ ofproto_set_in_band_queue(struct ofproto *ofproto, int queue_id)
     connmgr_set_in_band_queue(ofproto->connmgr, queue_id);
 }
 
+/* Sets the number of flows at which eviction from the kernel flow table
+ * will occur. */
+void
+ofproto_set_flow_eviction_threshold(struct ofproto *ofproto, unsigned threshold)
+{
+    if (threshold < OFPROTO_FLOW_EVICTION_THRESHOLD_MIN) {
+        ofproto->flow_eviction_threshold = OFPROTO_FLOW_EVICTION_THRESHOLD_MIN;
+    } else {
+        ofproto->flow_eviction_threshold = threshold;
+    }
+}
+
+/* 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
+ofproto_set_forward_bpdu(struct ofproto *ofproto, bool forward_bpdu)
+{
+    bool old_val = ofproto->forward_bpdu;
+    ofproto->forward_bpdu = forward_bpdu;
+    if (old_val != ofproto->forward_bpdu) {
+        if (ofproto->ofproto_class->forward_bpdu_changed) {
+            ofproto->ofproto_class->forward_bpdu_changed(ofproto);
+        }
+    }
+}
+
 void
 ofproto_set_desc(struct ofproto *p,
                  const char *mfr_desc, const char *hw_desc,
@@ -494,6 +530,79 @@ ofproto_set_sflow(struct ofproto *ofproto,
     }
 }
 
+/* Spanning Tree Protocol (STP) configuration. */
+
+/* Configures STP on 'ofproto' using the settings defined in 's'.  If
+ * 's' is NULL, disables STP.
+ *
+ * Returns 0 if successful, otherwise a positive errno value. */
+int
+ofproto_set_stp(struct ofproto *ofproto,
+                const struct ofproto_stp_settings *s)
+{
+    return (ofproto->ofproto_class->set_stp
+            ? ofproto->ofproto_class->set_stp(ofproto, s)
+            : EOPNOTSUPP);
+}
+
+/* Retrieves STP status of 'ofproto' and stores it in 's'.  If the
+ * 'enabled' member of 's' is false, then the other members are not
+ * meaningful.
+ *
+ * Returns 0 if successful, otherwise a positive errno value. */
+int
+ofproto_get_stp_status(struct ofproto *ofproto,
+                       struct ofproto_stp_status *s)
+{
+    return (ofproto->ofproto_class->get_stp_status
+            ? ofproto->ofproto_class->get_stp_status(ofproto, s)
+            : EOPNOTSUPP);
+}
+
+/* Configures STP on 'ofp_port' of 'ofproto' using the settings defined
+ * in 's'.  The caller is responsible for assigning STP port numbers
+ * (using the 'port_num' member in the range of 1 through 255, inclusive)
+ * and ensuring there are no duplicates.  If the 's' is NULL, then STP
+ * is disabled on the port.
+ *
+ * Returns 0 if successful, otherwise a positive errno value.*/
+int
+ofproto_port_set_stp(struct ofproto *ofproto, uint16_t ofp_port,
+                     const struct ofproto_port_stp_settings *s)
+{
+    struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
+    if (!ofport) {
+        VLOG_WARN("%s: cannot configure STP on nonexistent port %"PRIu16,
+                  ofproto->name, ofp_port);
+        return ENODEV;
+    }
+
+    return (ofproto->ofproto_class->set_stp_port
+            ? ofproto->ofproto_class->set_stp_port(ofport, s)
+            : EOPNOTSUPP);
+}
+
+/* Retrieves STP port status of 'ofp_port' on 'ofproto' and stores it in
+ * 's'.  If the 'enabled' member in 's' is false, then the other members
+ * are not meaningful.
+ *
+ * Returns 0 if successful, otherwise a positive errno value.*/
+int
+ofproto_port_get_stp_status(struct ofproto *ofproto, uint16_t ofp_port,
+                            struct ofproto_port_stp_status *s)
+{
+    struct ofport *ofport = ofproto_get_port(ofproto, ofp_port);
+    if (!ofport) {
+        VLOG_WARN("%s: cannot get STP status on nonexistent port %"PRIu16,
+                  ofproto->name, ofp_port);
+        return ENODEV;
+    }
+
+    return (ofproto->ofproto_class->get_stp_port_status
+            ? ofproto->ofproto_class->get_stp_port_status(ofport, s)
+            : EOPNOTSUPP);
+}
+
 /* Connectivity Fault Management configuration. */
 
 /* Clears the CFM configuration from 'ofp_port' on 'ofproto'. */
@@ -623,7 +732,7 @@ ofproto_set_flood_vlans(struct ofproto *ofproto, unsigned long *flood_vlans)
 /* Returns true if 'aux' is a registered bundle that is currently in use as the
  * output for a mirror. */
 bool
-ofproto_is_mirror_output_bundle(struct ofproto *ofproto, void *aux)
+ofproto_is_mirror_output_bundle(const struct ofproto *ofproto, void *aux)
 {
     return (ofproto->ofproto_class->is_mirror_output_bundle
             ? ofproto->ofproto_class->is_mirror_output_bundle(ofproto, aux)
@@ -652,9 +761,8 @@ ofproto_flush__(struct ofproto *ofproto)
         ofproto->ofproto_class->flush(ofproto);
     }
 
-    group = ofopgroup_create(ofproto);
-    for (table = ofproto->tables; table < &ofproto->tables[ofproto->n_tables];
-         table++) {
+    group = ofopgroup_create_unattached(ofproto);
+    OFPROTO_FOR_EACH_TABLE (table, ofproto) {
         struct rule *rule, *next_rule;
         struct cls_cursor cursor;
 
@@ -673,9 +781,10 @@ ofproto_flush__(struct ofproto *ofproto)
 static void
 ofproto_destroy__(struct ofproto *ofproto)
 {
-    size_t i;
+    struct classifier *table;
 
     assert(list_is_empty(&ofproto->pending));
+    assert(!ofproto->n_pending);
 
     connmgr_destroy(ofproto->connmgr);
 
@@ -690,9 +799,9 @@ ofproto_destroy__(struct ofproto *ofproto)
     hmap_destroy(&ofproto->ports);
     shash_destroy(&ofproto->port_by_name);
 
-    for (i = 0; i < ofproto->n_tables; i++) {
-        assert(classifier_is_empty(&ofproto->tables[i]));
-        classifier_destroy(&ofproto->tables[i]);
+    OFPROTO_FOR_EACH_TABLE (table, ofproto) {
+        assert(classifier_is_empty(table));
+        classifier_destroy(table);
     }
     free(ofproto->tables);
 
@@ -845,16 +954,7 @@ ofproto_get_ofproto_controller_info(const struct ofproto *ofproto,
 void
 ofproto_free_ofproto_controller_info(struct shash *info)
 {
-    struct shash_node *node;
-
-    SHASH_FOR_EACH (node, info) {
-        struct ofproto_controller_info *cinfo = node->data;
-        while (cinfo->pairs.n) {
-            free((char *) cinfo->pairs.values[--cinfo->pairs.n]);
-        }
-        free(cinfo);
-    }
-    shash_destroy(info);
+    connmgr_free_controller_info(info);
 }
 
 /* Makes a deep copy of 'old' into 'port'. */
@@ -1020,7 +1120,7 @@ ofproto_add_flow(struct ofproto *ofproto, const struct cls_rule *cls_rule,
                                     &ofproto->tables[0], cls_rule));
     if (!rule || !ofputil_actions_equal(rule->actions, rule->n_actions,
                                         actions, n_actions)) {
-        struct flow_mod fm;
+        struct ofputil_flow_mod fm;
 
         memset(&fm, 0, sizeof fm);
         fm.cr = *cls_rule;
@@ -1031,6 +1131,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.
  *
@@ -1051,7 +1163,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);
@@ -1102,17 +1214,11 @@ static struct netdev *
 ofport_open(const struct ofproto_port *ofproto_port, struct ofp_phy_port *opp)
 {
     uint32_t curr, advertised, supported, peer;
-    struct netdev_options netdev_options;
     enum netdev_flags flags;
     struct netdev *netdev;
     int error;
 
-    memset(&netdev_options, 0, sizeof netdev_options);
-    netdev_options.name = ofproto_port->name;
-    netdev_options.type = ofproto_port->type;
-    netdev_options.ethertype = NETDEV_ETH_TYPE_NONE;
-
-    error = netdev_open(&netdev_options, &netdev);
+    error = netdev_open(ofproto_port->name, ofproto_port->type, &netdev);
     if (error) {
         VLOG_WARN_RL(&rl, "ignoring port %s (%"PRIu16") because netdev %s "
                      "cannot be opened (%s)",
@@ -1162,6 +1268,7 @@ ofport_install(struct ofproto *p,
 {
     const char *netdev_name = netdev_get_name(netdev);
     struct ofport *ofport;
+    int dev_mtu;
     int error;
 
     /* Create ofport. */
@@ -1180,6 +1287,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) {
@@ -1218,7 +1332,7 @@ ofport_remove_with_name(struct ofproto *ofproto, const char *name)
     }
 }
 
-/* Updates 'port' within 'ofproto' with the new 'netdev' and 'opp'.
+/* Updates 'port' with new 'opp' description.
  *
  * Does not handle a name or port number change.  The caller must implement
  * such a change as a delete followed by an add.  */
@@ -1237,6 +1351,17 @@ ofport_modified(struct ofport *port, struct ofp_phy_port *opp)
     connmgr_send_port_status(port->ofproto->connmgr, &port->opp, OFPPR_MODIFY);
 }
 
+/* Update OpenFlow 'state' in 'port' and notify controller. */
+void
+ofproto_port_set_state(struct ofport *port, ovs_be32 state)
+{
+    if (port->opp.state != state) {
+        port->opp.state = state;
+        connmgr_send_port_status(port->ofproto->connmgr, &port->opp,
+                                 OFPPR_MODIFY);
+    }
+}
+
 void
 ofproto_port_unregister(struct ofproto *ofproto, uint16_t ofp_port)
 {
@@ -1306,12 +1431,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.*/
@@ -1367,6 +1502,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);
+        }
+    }
+}
 
 static void
 ofproto_rule_destroy__(struct rule *rule)
@@ -1378,8 +1559,8 @@ ofproto_rule_destroy__(struct rule *rule)
 /* This function allows an ofproto implementation to destroy any rules that
  * remain when its ->destruct() function is called.  The caller must have
  * already uninitialized any derived members of 'rule' (step 5 described in the
- * large comment in ofproto/private.h titled "Life Cycle").  This function
- * implements steps 6 and 7.
+ * large comment in ofproto/ofproto-provider.h titled "Life Cycle").
+ * This function implements steps 6 and 7.
  *
  * This function should only be called from an ofproto implementation's
  * ->destruct() function.  It is not suitable elsewhere. */
@@ -1398,13 +1579,12 @@ static bool
 rule_has_out_port(const struct rule *rule, uint16_t out_port)
 {
     const union ofp_action *oa;
-    struct actions_iterator i;
+    size_t left;
 
     if (out_port == OFPP_NONE) {
         return true;
     }
-    for (oa = actions_first(&i, rule->actions, rule->n_actions); oa;
-         oa = actions_next(&i)) {
+    OFPUTIL_ACTION_FOR_EACH_UNSAFE (oa, left, rule->actions, rule->n_actions) {
         if (action_outputs_to_port(oa, htons(out_port))) {
             return true;
         }
@@ -1427,7 +1607,7 @@ rule_execute(struct rule *rule, uint16_t in_port, struct ofpbuf *packet)
 
     assert(ofpbuf_headroom(packet) >= sizeof(struct ofp_packet_in));
 
-    flow_extract(packet, 0, in_port, &flow);
+    flow_extract(packet, 0, 0, in_port, &flow);
     return rule->ofproto->ofproto_class->rule_execute(rule, &flow, packet);
 }
 
@@ -1467,7 +1647,7 @@ handle_features_request(struct ofconn *ofconn, const struct ofp_header *oh)
     osf->n_buffers = htonl(pktbuf_capacity());
     osf->n_tables = ofproto->n_tables;
     osf->capabilities = htonl(OFPC_FLOW_STATS | OFPC_TABLE_STATS |
-                              OFPC_PORT_STATS);
+                              OFPC_PORT_STATS | OFPC_QUEUE_STATS);
     if (arp_match_ip) {
         osf->capabilities |= htonl(OFPC_ARP_MATCH_IP);
     }
@@ -1485,18 +1665,12 @@ static int
 handle_get_config_request(struct ofconn *ofconn, const struct ofp_header *oh)
 {
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
-    struct ofpbuf *buf;
     struct ofp_switch_config *osc;
-    uint16_t flags;
-    bool drop_frags;
-
-    /* Figure out flags. */
-    drop_frags = ofproto->ofproto_class->get_drop_frags(ofproto);
-    flags = drop_frags ? OFPC_FRAG_DROP : OFPC_FRAG_NORMAL;
+    struct ofpbuf *buf;
 
     /* Send reply. */
     osc = make_openflow_xid(sizeof *osc, OFPT_GET_CONFIG_REPLY, oh->xid, &buf);
-    osc->flags = htons(flags);
+    osc->flags = htons(ofproto->frag_handling);
     osc->miss_send_len = htons(ofconn_get_miss_send_len(ofconn));
     ofconn_send_reply(ofconn, buf);
 
@@ -1509,19 +1683,20 @@ handle_set_config(struct ofconn *ofconn, const struct ofp_switch_config *osc)
     struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
     uint16_t flags = ntohs(osc->flags);
 
-    if (ofconn_get_type(ofconn) == OFCONN_PRIMARY
-        && ofconn_get_role(ofconn) != NX_ROLE_SLAVE) {
-        switch (flags & OFPC_FRAG_MASK) {
-        case OFPC_FRAG_NORMAL:
-            ofproto->ofproto_class->set_drop_frags(ofproto, false);
-            break;
-        case OFPC_FRAG_DROP:
-            ofproto->ofproto_class->set_drop_frags(ofproto, true);
-            break;
-        default:
-            VLOG_WARN_RL(&rl, "requested bad fragment mode (flags=%"PRIx16")",
-                         osc->flags);
-            break;
+    if (ofconn_get_type(ofconn) != OFCONN_PRIMARY
+        || ofconn_get_role(ofconn) != NX_ROLE_SLAVE) {
+        enum ofp_config_flags cur = ofproto->frag_handling;
+        enum ofp_config_flags next = flags & OFPC_FRAG_MASK;
+
+        assert((cur & OFPC_FRAG_MASK) == cur);
+        if (cur != next) {
+            if (ofproto->ofproto_class->set_frag_handling(ofproto, next)) {
+                ofproto->frag_handling = next;
+            } else {
+                VLOG_WARN_RL(&rl, "%s: unsupported fragment handling mode %s",
+                             ofproto->name,
+                             ofputil_frag_handling_to_string(next));
+            }
         }
     }
 
@@ -1532,18 +1707,12 @@ handle_set_config(struct ofconn *ofconn, const struct ofp_switch_config *osc)
 
 /* Checks whether 'ofconn' is a slave controller.  If so, returns an OpenFlow
  * error message code (composed with ofp_mkerr()) for the caller to propagate
- * upward.  Otherwise, returns 0.
- *
- * The log message mentions 'msg_type'. */
+ * upward.  Otherwise, returns 0. */
 static int
-reject_slave_controller(struct ofconn *ofconn, const char *msg_type)
+reject_slave_controller(const struct ofconn *ofconn)
 {
     if (ofconn_get_type(ofconn) == OFCONN_PRIMARY
         && ofconn_get_role(ofconn) == NX_ROLE_SLAVE) {
-        static struct vlog_rate_limit perm_rl = VLOG_RATE_LIMIT_INIT(1, 5);
-        VLOG_WARN_RL(&perm_rl, "rejecting %s message from slave controller",
-                     msg_type);
-
         return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_EPERM);
     } else {
         return 0;
@@ -1565,7 +1734,7 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
 
     COVERAGE_INC(ofproto_packet_out);
 
-    error = reject_slave_controller(ofconn, "OFPT_PACKET_OUT");
+    error = reject_slave_controller(ofconn);
     if (error) {
         return error;
     }
@@ -1584,7 +1753,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;
         }
@@ -1594,8 +1763,18 @@ handle_packet_out(struct ofconn *ofconn, const struct ofp_header *oh)
         buffer = NULL;
     }
 
+    /* Get in_port and partially validate it.
+     *
+     * We don't know what range of ports the ofproto actually implements, but
+     * we do know that only certain reserved ports (numbered OFPP_MAX and
+     * above) are valid. */
+    in_port = ntohs(opo->in_port);
+    if (in_port >= OFPP_MAX && in_port != OFPP_LOCAL && in_port != OFPP_NONE) {
+        return ofp_mkerr_nicira(OFPET_BAD_REQUEST, NXBRC_BAD_IN_PORT);
+    }
+
     /* Send out packet. */
-    flow_extract(&payload, 0, ntohs(opo->in_port), &flow);
+    flow_extract(&payload, 0, 0, in_port, &flow);
     error = p->ofproto_class->packet_out(p, &payload, &flow,
                                          ofp_actions, n_ofp_actions);
     ofpbuf_delete(buffer);
@@ -1633,7 +1812,7 @@ handle_port_mod(struct ofconn *ofconn, const struct ofp_header *oh)
     struct ofport *port;
     int error;
 
-    error = reject_slave_controller(ofconn, "OFPT_PORT_MOD");
+    error = reject_slave_controller(ofconn);
     if (error) {
         return error;
     }
@@ -1757,6 +1936,17 @@ calc_flow_duration__(long long int start, uint32_t *sec, uint32_t *nsec)
     *nsec = (msecs % 1000) * (1000 * 1000);
 }
 
+/* Checks whether 'table_id' is 0xff or a valid table ID in 'ofproto'.  Returns
+ * 0 if 'table_id' is OK, otherwise an OpenFlow error code.  */
+static int
+check_table_id(const struct ofproto *ofproto, uint8_t table_id)
+{
+    return (table_id == 0xff || table_id < ofproto->n_tables
+            ? 0
+            : ofp_mkerr_nicira(OFPET_BAD_REQUEST, NXBRC_BAD_TABLE_ID));
+
+}
+
 static struct classifier *
 first_matching_table(struct ofproto *ofproto, uint8_t table_id)
 {
@@ -1765,11 +1955,6 @@ first_matching_table(struct ofproto *ofproto, uint8_t table_id)
     } else if (table_id < ofproto->n_tables) {
         return &ofproto->tables[table_id];
     } else {
-        /* It would probably be better to reply with an error but there doesn't
-         * seem to be any appropriate value, so that might just be
-         * confusing. */
-        VLOG_WARN_RL(&rl, "controller asked for invalid table %"PRIu8,
-                     table_id);
         return NULL;
     }
 }
@@ -1792,8 +1977,9 @@ next_matching_table(struct ofproto *ofproto,
  *   - If TABLE_ID is the number of a table in OFPROTO, then the loop iterates
  *     only once, for that table.
  *
- *   - Otherwise, TABLE_ID isn't valid for OFPROTO, so ofproto logs a warning
- *     and does not enter the loop at all.
+ *   - Otherwise, TABLE_ID isn't valid for OFPROTO, so the loop won't be
+ *     entered at all.  (Perhaps you should have validated TABLE_ID with
+ *     check_table_id().)
  *
  * All parameters are evaluated multiple times.
  */
@@ -1819,6 +2005,12 @@ collect_rules_loose(struct ofproto *ofproto, uint8_t table_id,
                     struct list *rules)
 {
     struct classifier *cls;
+    int error;
+
+    error = check_table_id(ofproto, table_id);
+    if (error) {
+        return error;
+    }
 
     list_init(rules);
     FOR_EACH_MATCHING_TABLE (cls, table_id, ofproto) {
@@ -1855,6 +2047,12 @@ collect_rules_strict(struct ofproto *ofproto, uint8_t table_id,
                      struct list *rules)
 {
     struct classifier *cls;
+    int error;
+
+    error = check_table_id(ofproto, table_id);
+    if (error) {
+        return error;
+    }
 
     list_init(rules);
     FOR_EACH_MATCHING_TABLE (cls, table_id, ofproto) {
@@ -1878,7 +2076,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;
@@ -1921,7 +2119,6 @@ static void
 flow_stats_ds(struct rule *rule, struct ds *results)
 {
     uint64_t packet_count, byte_count;
-    size_t act_len = sizeof *rule->actions * rule->n_actions;
 
     rule->ofproto->ofproto_class->rule_get_stats(rule,
                                                  &packet_count, &byte_count);
@@ -1936,8 +2133,8 @@ flow_stats_ds(struct rule *rule, struct ds *results)
     ds_put_format(results, "n_bytes=%"PRIu64", ", byte_count);
     cls_rule_format(&rule->cr, results);
     ds_put_char(results, ',');
-    if (act_len > 0) {
-        ofp_print_actions(results, &rule->actions->header, act_len);
+    if (rule->n_actions > 0) {
+        ofp_print_actions(results, rule->actions, rule->n_actions);
     } else {
         ds_put_cstr(results, "drop");
     }
@@ -1951,7 +2148,7 @@ ofproto_get_all_flows(struct ofproto *p, struct ds *results)
 {
     struct classifier *cls;
 
-    for (cls = &p->tables[0]; cls < &p->tables[p->n_tables]; cls++) {
+    OFPROTO_FOR_EACH_TABLE (cls, p) {
         struct cls_cursor cursor;
         struct rule *rule;
 
@@ -1983,13 +2180,33 @@ 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;
     struct list rules;
     struct rule *rule;
@@ -2007,6 +2224,7 @@ handle_aggregate_stats_request(struct ofconn *ofconn,
     }
 
     memset(&stats, 0, sizeof stats);
+    unknown_packets = unknown_bytes = false;
     LIST_FOR_EACH (rule, ofproto_node, &rules) {
         uint64_t packet_count;
         uint64_t byte_count;
@@ -2014,10 +2232,26 @@ handle_aggregate_stats_request(struct ofconn *ofconn,
         ofproto->ofproto_class->rule_get_stats(rule, &packet_count,
                                                &byte_count);
 
-        stats.packet_count += packet_count;
-        stats.byte_count += byte_count;
+        if (packet_count == UINT64_MAX) {
+            unknown_packets = true;
+        } else {
+            stats.packet_count += packet_count;
+        }
+
+        if (byte_count == UINT64_MAX) {
+            unknown_bytes = true;
+        } else {
+            stats.byte_count += byte_count;
+        }
+
         stats.flow_count++;
     }
+    if (unknown_packets) {
+        stats.packet_count = UINT64_MAX;
+    }
+    if (unknown_bytes) {
+        stats.byte_count = UINT64_MAX;
+    }
 
     reply = ofputil_encode_aggregate_stats_reply(&stats, osm);
     ofconn_send_reply(ofconn, reply);
@@ -2130,14 +2364,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 flow_mod *fm,
-         const struct ofp_header *request)
+add_flow(struct ofproto *ofproto, struct ofconn *ofconn,
+         const struct ofputil_flow_mod *fm, const struct ofp_header *request)
 {
     struct classifier *table;
     struct ofopgroup *group;
@@ -2145,21 +2380,15 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, struct flow_mod *fm,
     struct rule *rule;
     int error;
 
-    /* Check for overlap, if requested. */
-    if (fm->flags & OFPFF_CHECK_OVERLAP) {
-        struct classifier *cls;
-
-        FOR_EACH_MATCHING_TABLE (cls, fm->table_id, ofproto) {
-            if (classifier_rule_overlaps(cls, &fm->cr)) {
-                return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_OVERLAP);
-            }
-        }
+    error = check_table_id(ofproto, fm->table_id);
+    if (error) {
+        return error;
     }
 
     /* Pick table. */
     if (fm->table_id == 0xff) {
         uint8_t table_id;
-        if (ofproto->n_tables > 1) {
+        if (ofproto->ofproto_class->rule_choose_table) {
             error = ofproto->ofproto_class->rule_choose_table(ofproto, &fm->cr,
                                                               &table_id);
             if (error) {
@@ -2176,6 +2405,12 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, struct flow_mod *fm,
         return ofp_mkerr_nicira(OFPET_FLOW_MOD_FAILED, NXFMFC_BAD_TABLE_ID);
     }
 
+    /* Check for overlap, if requested. */
+    if (fm->flags & OFPFF_CHECK_OVERLAP
+        && classifier_rule_overlaps(table, &fm->cr)) {
+        return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_OVERLAP);
+    }
+
     /* Serialize against pending deletion. */
     if (is_flow_deletion_pending(ofproto, &fm->cr, table - ofproto->tables)) {
         return OFPROTO_POSTPONE;
@@ -2192,7 +2427,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, struct flow_mod *fm,
     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;
@@ -2205,9 +2440,7 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, struct flow_mod *fm,
     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;
 
@@ -2240,13 +2473,14 @@ add_flow(struct ofproto *ofproto, struct ofconn *ofconn, struct flow_mod *fm,
  *
  * Returns 0 on success, otherwise an OpenFlow error code. */
 static int
-modify_flows__(struct ofconn *ofconn, const struct 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)) {
@@ -2256,6 +2490,8 @@ modify_flows__(struct ofconn *ofconn, const struct 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;
     }
@@ -2270,17 +2506,18 @@ modify_flows__(struct ofconn *ofconn, const struct 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 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
@@ -2289,18 +2526,19 @@ modify_flows_loose(struct ofconn *ofconn, struct 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 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);
 }
 
@@ -2310,14 +2548,13 @@ modify_flow_strict(struct ofconn *ofconn, struct 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);
 
@@ -2332,34 +2569,35 @@ delete_flows__(struct ofconn *ofconn, const struct ofp_header *request,
 
 /* Implements OFPFC_DELETE. */
 static int
-delete_flows_loose(struct ofconn *ofconn, const struct 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 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);
 }
 
@@ -2399,7 +2637,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);
@@ -2409,19 +2647,14 @@ 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 flow_mod fm;
+    struct ofputil_flow_mod fm;
     int error;
 
-    error = reject_slave_controller(ofconn, "flow_mod");
+    error = reject_slave_controller(ofconn);
     if (error) {
         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) {
@@ -2436,24 +2669,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");
         }
@@ -2470,17 +2716,13 @@ handle_role_request(struct ofconn *ofconn, const struct ofp_header *oh)
     uint32_t role;
 
     if (ofconn_get_type(ofconn) != OFCONN_PRIMARY) {
-        VLOG_WARN_RL(&rl, "ignoring role request on service connection");
         return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_EPERM);
     }
 
     role = ntohl(nrr->role);
     if (role != NX_ROLE_OTHER && role != NX_ROLE_MASTER
         && role != NX_ROLE_SLAVE) {
-        VLOG_WARN_RL(&rl, "received request for unknown role %"PRIu32, role);
-
-        /* There's no good error code for this. */
-        return ofp_mkerr(OFPET_BAD_REQUEST, -1);
+        return ofp_mkerr_nicira(OFPET_BAD_REQUEST, NXBRC_BAD_ROLE);
     }
 
     if (ofconn_get_role(ofconn) != role
@@ -2621,7 +2863,7 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
     case OFPUTIL_OFPST_QUEUE_REQUEST:
         return handle_queue_stats_request(ofconn, msg->data);
 
-    case OFPUTIL_INVALID:
+    case OFPUTIL_MSG_INVALID:
     case OFPUTIL_OFPT_HELLO:
     case OFPUTIL_OFPT_ERROR:
     case OFPUTIL_OFPT_FEATURES_REPLY:
@@ -2643,11 +2885,6 @@ handle_openflow__(struct ofconn *ofconn, const struct ofpbuf *msg)
     case OFPUTIL_NXST_FLOW_REPLY:
     case OFPUTIL_NXST_AGGREGATE_REPLY:
     default:
-        if (VLOG_IS_WARN_ENABLED()) {
-            char *s = ofp_to_string(oh, ntohs(oh->length), 2);
-            VLOG_DBG_RL(&rl, "OpenFlow message ignored: %s", s);
-            free(s);
-        }
         if (oh->type == OFPT_STATS_REQUEST || oh->type == OFPT_STATS_REPLY) {
             return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_STAT);
         } else {
@@ -2675,7 +2912,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;
@@ -2685,26 +2922,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;
 }
 
@@ -2721,6 +2965,7 @@ ofopgroup_submit(struct ofopgroup *group)
         ofopgroup_destroy(group);
     } else {
         list_push_back(&group->ofproto->pending, &group->ofproto_node);
+        group->ofproto->n_pending++;
     }
 }
 
@@ -2729,6 +2974,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)) {
@@ -2790,11 +3037,32 @@ 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.
  *
- * Please see the large comment in ofproto/private.h titled "Asynchronous
- * Operation Support" for more information. */
+ *   - 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. */
 void
 ofoperation_complete(struct ofoperation *op, int error)
 {
@@ -2851,7 +3119,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;
@@ -2940,5 +3210,5 @@ ofproto_unixctl_init(void)
     }
     registered = true;
 
-    unixctl_command_register("ofproto/list", ofproto_unixctl_list, NULL);
+    unixctl_command_register("ofproto/list", "", ofproto_unixctl_list, NULL);
 }