Option to forward BPDU (Ethernet control class) frames
[sliver-openvswitch.git] / vswitchd / bridge.c
index 0aa395e..d99d6a1 100644 (file)
 
 #include <config.h>
 #include "bridge.h"
-#include "byte-order.h"
 #include <assert.h>
 #include <errno.h>
-#include <arpa/inet.h>
-#include <ctype.h>
 #include <inttypes.h>
-#include <sys/socket.h>
-#include <net/if.h>
-#include <openflow/openflow.h>
-#include <signal.h>
 #include <stdlib.h>
-#include <strings.h>
-#include <sys/stat.h>
-#include <sys/socket.h>
-#include <sys/types.h>
-#include <unistd.h>
 #include "bitmap.h"
 #include "bond.h"
 #include "cfm.h"
-#include "classifier.h"
 #include "coverage.h"
 #include "daemon.h"
 #include "dirs.h"
-#include "dpif.h"
 #include "dynamic-string.h"
-#include "flow.h"
 #include "hash.h"
 #include "hmap.h"
 #include "jsonrpc.h"
 #include "lacp.h"
 #include "list.h"
-#include "mac-learning.h"
 #include "netdev.h"
-#include "netlink.h"
-#include "odp-util.h"
 #include "ofp-print.h"
 #include "ofpbuf.h"
-#include "ofproto/netflow.h"
 #include "ofproto/ofproto.h"
-#include "ovsdb-data.h"
-#include "packets.h"
 #include "poll-loop.h"
-#include "process.h"
 #include "sha1.h"
 #include "shash.h"
 #include "socket-util.h"
 #include "stream-ssl.h"
 #include "sset.h"
-#include "svec.h"
 #include "system-stats.h"
 #include "timeval.h"
 #include "util.h"
 #include "unixctl.h"
-#include "vconn.h"
 #include "vswitchd/vswitch-idl.h"
 #include "xenserver.h"
 #include "vlog.h"
@@ -89,8 +65,8 @@ struct iface {
 
     /* These members are valid only after bridge_reconfigure() causes them to
      * be initialized. */
-    struct hmap_node dp_ifidx_node; /* In struct bridge's "ifaces" hmap. */
-    int dp_ifidx;               /* Index within kernel datapath. */
+    struct hmap_node ofp_port_node; /* In struct bridge's "ifaces" hmap. */
+    int ofp_port;               /* OpenFlow port number, -1 if unknown. */
     struct netdev *netdev;      /* Network device. */
     const char *type;           /* Usually same as cfg->type. */
     const struct ovsrec_interface *cfg;
@@ -128,7 +104,7 @@ struct bridge {
 
     /* Bridge ports. */
     struct hmap ports;          /* "struct port"s indexed by name. */
-    struct hmap ifaces;         /* "struct iface"s indexed by dp_ifidx. */
+    struct hmap ifaces;         /* "struct iface"s indexed by ofp_port. */
     struct hmap iface_by_name;  /* "struct iface"s indexed by name. */
 
     /* Port mirroring. */
@@ -158,8 +134,8 @@ static long long int stats_timer = LLONG_MIN;
 static long long int db_limiter = LLONG_MIN;
 
 static void add_del_bridges(const struct ovsrec_open_vswitch *);
-static void bridge_del_dps(void);
-static bool bridge_add_dp(struct bridge *);
+static void bridge_del_ofprotos(void);
+static bool bridge_add_ofprotos(struct bridge *);
 static void bridge_create(const struct ovsrec_bridge *);
 static void bridge_destroy(struct bridge *);
 static struct bridge *bridge_lookup(const char *name);
@@ -170,9 +146,11 @@ static size_t bridge_get_controllers(const struct bridge *br,
 static void bridge_add_del_ports(struct bridge *);
 static void bridge_add_ofproto_ports(struct bridge *);
 static void bridge_del_ofproto_ports(struct bridge *);
-static void bridge_refresh_dp_ifidx(struct bridge *);
+static void bridge_refresh_ofp_port(struct bridge *);
 static void bridge_configure_datapath_id(struct bridge *);
+static void bridge_configure_flow_eviction_threshold(struct bridge *);
 static void bridge_configure_netflow(struct bridge *);
+static void bridge_configure_forward_bpdu(struct bridge *);
 static void bridge_configure_sflow(struct bridge *, int *sflow_bridge_number);
 static void bridge_configure_remotes(struct bridge *,
                                      const struct sockaddr_in *managers,
@@ -188,7 +166,6 @@ static bool bridge_has_bond_fake_iface(const struct bridge *,
                                        const char *name);
 static bool port_is_bond_fake_iface(const struct port *);
 
-static unixctl_cb_func cfm_unixctl_show;
 static unixctl_cb_func qos_unixctl_show;
 
 static struct port *port_create(struct bridge *, const struct ovsrec_port *);
@@ -199,7 +176,8 @@ static struct port *port_lookup(const struct bridge *, const char *name);
 static void port_configure(struct port *);
 static struct lacp_settings *port_configure_lacp(struct port *,
                                                  struct lacp_settings *);
-static void port_configure_bond(struct port *, struct bond_settings *);
+static void port_configure_bond(struct port *, struct bond_settings *,
+                                uint32_t *bond_stable_ids);
 
 static void bridge_configure_mirrors(struct bridge *);
 static struct mirror *mirror_create(struct bridge *,
@@ -213,13 +191,15 @@ static struct iface *iface_create(struct port *port,
 static void iface_destroy(struct iface *);
 static struct iface *iface_lookup(const struct bridge *, const char *name);
 static struct iface *iface_find(const char *name);
-static struct iface *iface_from_dp_ifidx(const struct bridge *,
-                                         uint16_t dp_ifidx);
+static struct iface *iface_from_ofp_port(const struct bridge *,
+                                         uint16_t ofp_port);
 static void iface_set_mac(struct iface *);
 static void iface_set_ofport(const struct ovsrec_interface *, int64_t ofport);
 static void iface_configure_qos(struct iface *, const struct ovsrec_qos *);
 static void iface_configure_cfm(struct iface *);
-static bool iface_refresh_cfm_stats(struct iface *iface);
+static bool iface_refresh_cfm_stats(struct iface *);
+static void iface_refresh_stats(struct iface *);
+static void iface_refresh_status(struct iface *);
 static bool iface_get_carrier(const struct iface *);
 static bool iface_is_synthetic(const struct iface *);
 
@@ -238,6 +218,7 @@ bridge_init(const char *remote)
 {
     /* Create connection to database. */
     idl = ovsdb_idl_create(remote, &ovsrec_idl_class, true);
+    ovsdb_idl_set_lock(idl, "ovs_vswitchd");
 
     ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_cur_cfg);
     ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_statistics);
@@ -268,10 +249,6 @@ bridge_init(const char *remote)
     ovsdb_idl_omit_alert(idl, &ovsrec_controller_col_status);
     ovsdb_idl_omit(idl, &ovsrec_controller_col_external_ids);
 
-    ovsdb_idl_omit_alert(idl, &ovsrec_maintenance_point_col_fault);
-
-    ovsdb_idl_omit_alert(idl, &ovsrec_monitor_col_fault);
-
     ovsdb_idl_omit(idl, &ovsrec_qos_col_external_ids);
 
     ovsdb_idl_omit(idl, &ovsrec_queue_col_external_ids);
@@ -291,7 +268,6 @@ bridge_init(const char *remote)
     ovsdb_idl_omit(idl, &ovsrec_ssl_col_external_ids);
 
     /* Register unixctl commands. */
-    unixctl_command_register("cfm/show", cfm_unixctl_show, NULL);
     unixctl_command_register("qos/show", qos_unixctl_show, NULL);
     unixctl_command_register("bridge/dump-flows", bridge_unixctl_dump_flows,
                              NULL);
@@ -299,6 +275,7 @@ bridge_init(const char *remote)
                              NULL);
     lacp_init();
     bond_init();
+    cfm_init();
 }
 
 void
@@ -395,7 +372,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
      * that port already belongs to a different datapath, so we must do all
      * port deletions before any port additions.  A datapath always has a
      * "local port" so we must delete not-configured datapaths too. */
-    bridge_del_dps();
+    bridge_del_ofprotos();
     HMAP_FOR_EACH (br, node, &all_bridges) {
         if (br->ofproto) {
             bridge_del_ofproto_ports(br);
@@ -406,15 +383,15 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
      *
      * After this is done, we have our final set of bridges, ports, and
      * interfaces.  Every "struct bridge" has an ofproto, every "struct port"
-     * has at least one iface, every "struct iface" has a valid dp_ifidx and
+     * has at least one iface, every "struct iface" has a valid ofp_port and
      * netdev. */
     HMAP_FOR_EACH_SAFE (br, next, node, &all_bridges) {
-        if (!br->ofproto && !bridge_add_dp(br)) {
+        if (!br->ofproto && !bridge_add_ofprotos(br)) {
             bridge_destroy(br);
         }
     }
     HMAP_FOR_EACH (br, node, &all_bridges) {
-        bridge_refresh_dp_ifidx(br);
+        bridge_refresh_ofp_port(br);
         bridge_add_ofproto_ports(br);
     }
 
@@ -429,7 +406,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
 
             port_configure(port);
 
-            HMAP_FOR_EACH (iface, dp_ifidx_node, &br->ifaces) {
+            LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
                 iface_configure_cfm(iface);
                 iface_configure_qos(iface, port->cfg->qos);
                 iface_set_mac(iface);
@@ -437,6 +414,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
         }
         bridge_configure_mirrors(br);
         bridge_configure_datapath_id(br);
+        bridge_configure_flow_eviction_threshold(br);
+        bridge_configure_forward_bpdu(br);
         bridge_configure_remotes(br, managers, n_managers);
         bridge_configure_netflow(br);
         bridge_configure_sflow(br, &sflow_bridge_number);
@@ -448,40 +427,35 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
     daemonize_complete();
 }
 
-/* Iterate over all system dpifs and delete any of them that do not have a
+/* Iterate over all ofprotos and delete any of them that do not have a
  * configured bridge or that are the wrong type. */
 static void
-bridge_del_dps(void)
+bridge_del_ofprotos(void)
 {
-    struct sset dpif_names;
-    struct sset dpif_types;
+    struct sset names;
+    struct sset types;
     const char *type;
 
-    sset_init(&dpif_names);
-    sset_init(&dpif_types);
-    dp_enumerate_types(&dpif_types);
-    SSET_FOR_EACH (type, &dpif_types) {
+    sset_init(&names);
+    sset_init(&types);
+    ofproto_enumerate_types(&types);
+    SSET_FOR_EACH (type, &types) {
         const char *name;
 
-        dp_enumerate_names(type, &dpif_names);
-        SSET_FOR_EACH (name, &dpif_names) {
+        ofproto_enumerate_names(type, &names);
+        SSET_FOR_EACH (name, &names) {
             struct bridge *br = bridge_lookup(name);
             if (!br || strcmp(type, br->type)) {
-                struct dpif *dpif;
-
-                if (!dpif_open(name, type, &dpif)) {
-                    dpif_delete(dpif);
-                    dpif_close(dpif);
-                }
+                ofproto_delete(name, type);
             }
         }
     }
-    sset_destroy(&dpif_names);
-    sset_destroy(&dpif_types);
+    sset_destroy(&names);
+    sset_destroy(&types);
 }
 
 static bool
-bridge_add_dp(struct bridge *br)
+bridge_add_ofprotos(struct bridge *br)
 {
     int error = ofproto_create(br->name, br->type, &br->ofproto);
     if (error) {
@@ -507,7 +481,7 @@ port_configure(struct port *port)
     s.n_slaves = 0;
     s.slaves = xmalloc(list_size(&port->ifaces) * sizeof *s.slaves);
     LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
-        s.slaves[s.n_slaves++] = iface->dp_ifidx;
+        s.slaves[s.n_slaves++] = iface->ofp_port;
     }
 
     /* Get VLAN tag. */
@@ -550,18 +524,26 @@ port_configure(struct port *port)
 
     /* Get bond settings. */
     if (s.n_slaves > 1) {
-        port_configure_bond(port, &bond_settings);
         s.bond = &bond_settings;
+        s.bond_stable_ids = xmalloc(s.n_slaves * sizeof *s.bond_stable_ids);
+        port_configure_bond(port, &bond_settings, s.bond_stable_ids);
     } else {
         s.bond = NULL;
+        s.bond_stable_ids = NULL;
+
+        LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
+            netdev_set_miimon_interval(iface->netdev, 0);
+        }
     }
 
     /* Register. */
     ofproto_bundle_register(port->bridge->ofproto, port, &s);
 
     /* Clean up. */
+    free(s.slaves);
     free(s.trunks);
     free(s.lacp_slaves);
+    free(s.bond_stable_ids);
 }
 
 /* Pick local port hardware address and datapath ID for 'br'. */
@@ -575,7 +557,7 @@ bridge_configure_datapath_id(struct bridge *br)
     char *dpid_string;
 
     bridge_pick_local_hw_addr(br, ea, &hw_addr_iface);
-    local_iface = iface_from_dp_ifidx(br, ODPP_LOCAL);
+    local_iface = iface_from_ofp_port(br, OFPP_LOCAL);
     if (local_iface) {
         int error = netdev_set_etheraddr(local_iface->netdev, ea);
         if (error) {
@@ -740,8 +722,8 @@ add_del_bridges(const struct ovsrec_open_vswitch *cfg)
      * Update 'cfg' of bridges that still exist. */
     HMAP_FOR_EACH_SAFE (br, next, node, &all_bridges) {
         br->cfg = shash_find_data(&new_br, br->name);
-        if (!br->cfg || strcmp(br->type,
-                               dpif_normalize_type(br->cfg->datapath_type))) {
+        if (!br->cfg || strcmp(br->type, ofproto_normalize_type(
+                                   br->cfg->datapath_type))) {
             bridge_destroy(br);
         }
     }
@@ -802,7 +784,6 @@ bridge_del_ofproto_ports(struct bridge *br)
                       br->name, name, strerror(error));
         }
         if (iface) {
-            ofproto_port_unregister(br->ofproto, ofproto_port.ofp_port);
             netdev_close(iface->netdev);
             iface->netdev = NULL;
         }
@@ -810,108 +791,120 @@ bridge_del_ofproto_ports(struct bridge *br)
 }
 
 static void
-iface_set_dp_ifidx(struct iface *iface, int dp_ifidx)
+iface_set_ofp_port(struct iface *iface, int ofp_port)
 {
     struct bridge *br = iface->port->bridge;
 
-    assert(iface->dp_ifidx < 0 && dp_ifidx >= 0);
-    iface->dp_ifidx = dp_ifidx;
-    hmap_insert(&br->ifaces, &iface->dp_ifidx_node, hash_int(dp_ifidx, 0));
-
+    assert(iface->ofp_port < 0 && ofp_port >= 0);
+    iface->ofp_port = ofp_port;
+    hmap_insert(&br->ifaces, &iface->ofp_port_node, hash_int(ofp_port, 0));
+    iface_set_ofport(iface->cfg, ofp_port);
 }
 
 static void
-bridge_refresh_dp_ifidx(struct bridge *br)
+bridge_refresh_ofp_port(struct bridge *br)
 {
     struct ofproto_port_dump dump;
     struct ofproto_port ofproto_port;
     struct port *port;
 
-    /* Clear all the "dp_ifidx"es. */
+    /* Clear all the "ofp_port"es. */
     hmap_clear(&br->ifaces);
     HMAP_FOR_EACH (port, hmap_node, &br->ports) {
         struct iface *iface;
 
         LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
-            iface->dp_ifidx = -1;
+            iface->ofp_port = -1;
         }
     }
 
-    /* Obtain the correct "dp_ifidx"es from ofproto. */
+    /* Obtain the correct "ofp_port"s from ofproto. */
     OFPROTO_PORT_FOR_EACH (&ofproto_port, &dump, br->ofproto) {
-        uint32_t odp_port = ofp_port_to_odp_port(ofproto_port.ofp_port);
         struct iface *iface = iface_lookup(br, ofproto_port.name);
         if (iface) {
-            if (iface->dp_ifidx >= 0) {
+            if (iface->ofp_port >= 0) {
                 VLOG_WARN("bridge %s: interface %s reported twice",
                           br->name, ofproto_port.name);
-            } else if (iface_from_dp_ifidx(br, odp_port)) {
+            } else if (iface_from_ofp_port(br, ofproto_port.ofp_port)) {
                 VLOG_WARN("bridge %s: interface %"PRIu16" reported twice",
-                          br->name, odp_port);
+                          br->name, ofproto_port.ofp_port);
             } else {
-                iface_set_dp_ifidx(iface, odp_port);
+                iface_set_ofp_port(iface, ofproto_port.ofp_port);
             }
         }
     }
 }
 
-/* Add a dpif port for any "struct iface" that doesn't have one.
+/* Add an ofproto port for any "struct iface" that doesn't have one.
  * Delete any "struct iface" for which this fails.
  * Delete any "struct port" that thereby ends up with no ifaces. */
 static void
 bridge_add_ofproto_ports(struct bridge *br)
 {
     struct port *port, *next_port;
-    struct ofproto_port ofproto_port;
 
     HMAP_FOR_EACH_SAFE (port, next_port, hmap_node, &br->ports) {
         struct iface *iface, *next_iface;
+        struct ofproto_port ofproto_port;
 
         LIST_FOR_EACH_SAFE (iface, next_iface, port_elem, &port->ifaces) {
-            struct shash args;
             int error;
 
-            /* Open the netdev or reconfigure it. */
-            shash_init(&args);
-            shash_from_ovs_idl_map(iface->cfg->key_options,
-                                   iface->cfg->value_options,
-                                   iface->cfg->n_options, &args);
+            /* Open the netdev. */
             if (!iface->netdev) {
-                struct netdev_options options;
-                options.name = iface->name;
-                options.type = iface->type;
-                options.args = &args;
-                options.ethertype = NETDEV_ETH_TYPE_NONE;
-                error = netdev_open(&options, &iface->netdev);
+                error = netdev_open(iface->name, iface->type, &iface->netdev);
+                if (error) {
+                    VLOG_WARN("could not open network device %s (%s)",
+                              iface->name, strerror(error));
+                }
             } else {
-                error = netdev_set_config(iface->netdev, &args);
+                error = 0;
             }
-            shash_destroy(&args);
-            if (error) {
-                VLOG_WARN("could not %s network device %s (%s)",
-                          iface->netdev ? "reconfigure" : "open",
-                          iface->name, strerror(error));
+
+            /* Configure the netdev. */
+            if (iface->netdev) {
+                struct shash args;
+
+                shash_init(&args);
+                shash_from_ovs_idl_map(iface->cfg->key_options,
+                                       iface->cfg->value_options,
+                                       iface->cfg->n_options, &args);
+                error = netdev_set_config(iface->netdev, &args);
+                shash_destroy(&args);
+
+                if (error) {
+                    VLOG_WARN("could not configure network device %s (%s)",
+                              iface->name, strerror(error));
+                    netdev_close(iface->netdev);
+                    iface->netdev = NULL;
+                }
             }
 
             /* Add the port, if necessary. */
-            if (iface->netdev && iface->dp_ifidx < 0) {
+            if (iface->netdev && iface->ofp_port < 0) {
                 uint16_t ofp_port;
                 int error;
 
                 error = ofproto_port_add(br->ofproto, iface->netdev,
                                          &ofp_port);
                 if (!error) {
-                    iface_set_dp_ifidx(iface, ofp_port_to_odp_port(ofp_port));
+                    iface_set_ofp_port(iface, ofp_port);
                 } else {
                     netdev_close(iface->netdev);
                     iface->netdev = NULL;
                 }
             }
 
-            /* Delete the iface if  */
-            if (iface->netdev && iface->dp_ifidx >= 0) {
+            /* Populate stats columns in new Interface rows. */
+            if (iface->netdev && !iface->cfg->mtu) {
+                iface_refresh_stats(iface);
+                iface_refresh_status(iface);
+            }
+
+            /* Delete the iface if we failed. */
+            if (iface->netdev && iface->ofp_port >= 0) {
                 VLOG_DBG("bridge %s: interface %s is on port %d",
-                         br->name, iface->name, iface->dp_ifidx);
+                         br->name, iface->name, iface->ofp_port);
             } else {
                 if (iface->netdev) {
                     VLOG_ERR("bridge %s: missing %s interface, dropping",
@@ -934,15 +927,10 @@ bridge_add_ofproto_ports(struct bridge *br)
         if (port_is_bond_fake_iface(port)) {
             if (ofproto_port_query_by_name(br->ofproto, port->name,
                                            &ofproto_port)) {
-                struct netdev_options options;
                 struct netdev *netdev;
                 int error;
 
-                options.name = port->name;
-                options.type = "internal";
-                options.args = NULL;
-                options.ethertype = NETDEV_ETH_TYPE_NONE;
-                error = netdev_open(&options, &netdev);
+                error = netdev_open(port->name, "internal", &netdev);
                 if (!error) {
                     ofproto_port_add(br->ofproto, netdev, NULL);
                     netdev_close(netdev);
@@ -980,6 +968,36 @@ bridge_get_other_config(const struct ovsrec_bridge *br_cfg, const char *key)
                                 &ovsrec_bridge_col_other_config, key);
 }
 
+/* Set Flow eviction threshold */
+static void
+bridge_configure_flow_eviction_threshold(struct bridge *br)
+{
+    const char *threshold_str;
+    unsigned threshold;
+
+    threshold_str = bridge_get_other_config(br->cfg, "flow-eviction-threshold");
+    if (threshold_str) {
+        threshold = strtoul(threshold_str, NULL, 10);
+    } else {
+        threshold = OFPROTO_FLOW_EVICTON_THRESHOLD_DEFAULT;
+    }
+    ofproto_set_flow_eviction_threshold(br->ofproto, threshold);
+}
+
+/* Set forward BPDU option. */
+static void
+bridge_configure_forward_bpdu(struct bridge *br)
+{
+    const char *forward_bpdu_str;
+    bool forward_bpdu = false;
+
+    forward_bpdu_str = bridge_get_other_config(br->cfg, "forward-bpdu");
+    if (forward_bpdu_str && !strcmp(forward_bpdu_str, "true")) {
+        forward_bpdu = true;
+    }
+    ofproto_set_forward_bpdu(br->ofproto, forward_bpdu);
+}
+
 static void
 bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
                           struct iface **hw_addr_iface)
@@ -1043,16 +1061,13 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
 
             /* The local port doesn't count (since we're trying to choose its
              * MAC address anyway). */
-            if (iface->dp_ifidx == ODPP_LOCAL) {
+            if (iface->ofp_port == OFPP_LOCAL) {
                 continue;
             }
 
             /* Grab MAC. */
             error = netdev_get_etheraddr(iface->netdev, iface_ea);
             if (error) {
-                static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
-                VLOG_ERR_RL(&rl, "failed to obtain Ethernet address of %s: %s",
-                            iface->name, strerror(error));
                 continue;
             }
         }
@@ -1243,33 +1258,20 @@ iface_refresh_status(struct iface *iface)
 static bool
 iface_refresh_cfm_stats(struct iface *iface)
 {
-    const struct ovsrec_monitor *mon;
-    const struct cfm *cfm;
+    const struct ovsrec_interface *cfg = iface->cfg;
     bool changed = false;
-    size_t i;
+    int fault;
 
-    mon = iface->cfg->monitor;
-    cfm = ofproto_port_get_cfm(iface->port->bridge->ofproto, iface->dp_ifidx);
+    fault = ofproto_port_get_cfm_fault(iface->port->bridge->ofproto,
+                                       iface->ofp_port);
 
-    if (!cfm || !mon) {
+    if (fault < 0) {
         return false;
     }
 
-    for (i = 0; i < mon->n_remote_mps; i++) {
-        const struct ovsrec_maintenance_point *mp;
-        const struct remote_mp *rmp;
-
-        mp = mon->remote_mps[i];
-        rmp = cfm_get_remote_mp(cfm, mp->mpid);
-
-        if (mp->n_fault != 1 || mp->fault[0] != rmp->fault) {
-            ovsrec_maintenance_point_set_fault(mp, &rmp->fault, 1);
-            changed = true;
-        }
-    }
-
-    if (mon->n_fault != 1 || mon->fault[0] != cfm->fault) {
-        ovsrec_monitor_set_fault(mon, &cfm->fault, 1);
+    if (cfg->n_cfm_fault != 1 || cfg->cfm_fault[0] != fault) {
+        bool fault_bool = fault;
+        ovsrec_interface_set_cfm_fault(cfg, &fault_bool, 1);
         changed = true;
     }
 
@@ -1280,9 +1282,8 @@ static bool
 iface_refresh_lacp_stats(struct iface *iface)
 {
     struct ofproto *ofproto = iface->port->bridge->ofproto;
-    uint16_t ofp_port = odp_port_to_ofp_port(iface->dp_ifidx);
     int old = iface->cfg->lacp_current ? *iface->cfg->lacp_current : -1;
-    int new = ofproto_port_is_lacp_current(ofproto, ofp_port);
+    int new = ofproto_port_is_lacp_current(ofproto, iface->ofp_port);
 
     if (old != new) {
         bool current = new;
@@ -1294,30 +1295,25 @@ iface_refresh_lacp_stats(struct iface *iface)
 static void
 iface_refresh_stats(struct iface *iface)
 {
-    struct iface_stat {
-        char *name;
-        int offset;
-    };
-    static const struct iface_stat iface_stats[] = {
-        { "rx_packets", offsetof(struct netdev_stats, rx_packets) },
-        { "tx_packets", offsetof(struct netdev_stats, tx_packets) },
-        { "rx_bytes", offsetof(struct netdev_stats, rx_bytes) },
-        { "tx_bytes", offsetof(struct netdev_stats, tx_bytes) },
-        { "rx_dropped", offsetof(struct netdev_stats, rx_dropped) },
-        { "tx_dropped", offsetof(struct netdev_stats, tx_dropped) },
-        { "rx_errors", offsetof(struct netdev_stats, rx_errors) },
-        { "tx_errors", offsetof(struct netdev_stats, tx_errors) },
-        { "rx_frame_err", offsetof(struct netdev_stats, rx_frame_errors) },
-        { "rx_over_err", offsetof(struct netdev_stats, rx_over_errors) },
-        { "rx_crc_err", offsetof(struct netdev_stats, rx_crc_errors) },
-        { "collisions", offsetof(struct netdev_stats, collisions) },
-    };
-    enum { N_STATS = ARRAY_SIZE(iface_stats) };
-    const struct iface_stat *s;
-
-    char *keys[N_STATS];
-    int64_t values[N_STATS];
-    int n;
+#define IFACE_STATS                             \
+    IFACE_STAT(rx_packets,      "rx_packets")   \
+    IFACE_STAT(tx_packets,      "tx_packets")   \
+    IFACE_STAT(rx_bytes,        "rx_bytes")     \
+    IFACE_STAT(tx_bytes,        "tx_bytes")     \
+    IFACE_STAT(rx_dropped,      "rx_dropped")   \
+    IFACE_STAT(tx_dropped,      "tx_dropped")   \
+    IFACE_STAT(rx_errors,       "rx_errors")    \
+    IFACE_STAT(tx_errors,       "tx_errors")    \
+    IFACE_STAT(rx_frame_errors, "rx_frame_err") \
+    IFACE_STAT(rx_over_errors,  "rx_over_err")  \
+    IFACE_STAT(rx_crc_errors,   "rx_crc_err")   \
+    IFACE_STAT(collisions,      "collisions")
+
+#define IFACE_STAT(MEMBER, NAME) NAME,
+    static char *keys[] = { IFACE_STATS };
+#undef IFACE_STAT
+    int64_t values[ARRAY_SIZE(keys)];
+    int i;
 
     struct netdev_stats stats;
 
@@ -1329,17 +1325,32 @@ iface_refresh_stats(struct iface *iface)
      * all-1s, and we will deal with that correctly below. */
     netdev_get_stats(iface->netdev, &stats);
 
-    n = 0;
-    for (s = iface_stats; s < &iface_stats[N_STATS]; s++) {
-        uint64_t value = *(uint64_t *) (((char *) &stats) + s->offset);
-        if (value != UINT64_MAX) {
-            keys[n] = s->name;
-            values[n] = value;
-            n++;
-        }
+    /* Copy statistics into values[] array. */
+    i = 0;
+#define IFACE_STAT(MEMBER, NAME) values[i++] = stats.MEMBER;
+    IFACE_STATS;
+#undef IFACE_STAT
+    assert(i == ARRAY_SIZE(keys));
+
+    ovsrec_interface_set_statistics(iface->cfg, keys, values, ARRAY_SIZE(keys));
+#undef IFACE_STATS
+}
+
+static bool
+enable_system_stats(const struct ovsrec_open_vswitch *cfg)
+{
+    const char *enable;
+
+    /* Use other-config:enable-system-stats by preference. */
+    enable = get_ovsrec_key_value(&cfg->header_,
+                                  &ovsrec_open_vswitch_col_other_config,
+                                  "enable-statistics");
+    if (enable) {
+        return !strcmp(enable, "true");
     }
 
-    ovsrec_interface_set_statistics(iface->cfg, keys, values, n);
+    /* Disable by default. */
+    return false;
 }
 
 static void
@@ -1349,7 +1360,9 @@ refresh_system_stats(const struct ovsrec_open_vswitch *cfg)
     struct shash stats;
 
     shash_init(&stats);
-    get_system_stats(&stats);
+    if (enable_system_stats(cfg)) {
+        get_system_stats(&stats);
+    }
 
     ovsdb_datum_from_shash(&datum, &stats);
     ovsdb_idl_txn_write(&cfg->header_, &ovsrec_open_vswitch_col_statistics,
@@ -1372,13 +1385,20 @@ nx_role_to_str(enum nx_role role)
 }
 
 static void
-bridge_refresh_controller_status(const struct bridge *br)
+refresh_controller_status(void)
 {
+    struct bridge *br;
     struct shash info;
     const struct ovsrec_controller *cfg;
 
-    ofproto_get_ofproto_controller_info(br->ofproto, &info);
+    shash_init(&info);
 
+    /* Accumulate status for controllers on all bridges. */
+    HMAP_FOR_EACH (br, node, &all_bridges) {
+        ofproto_get_ofproto_controller_info(br->ofproto, &info);
+    }
+
+    /* Update each controller in the database with current status. */
     OVSREC_CONTROLLER_FOR_EACH(cfg, idl) {
         struct ofproto_controller_info *cinfo =
             shash_find_data(&info, cfg->target);
@@ -1408,6 +1428,24 @@ bridge_run(void)
     bool database_changed;
     struct bridge *br;
 
+    /* (Re)configure if necessary. */
+    database_changed = ovsdb_idl_run(idl);
+    if (ovsdb_idl_is_lock_contended(idl)) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+        struct bridge *br, *next_br;
+
+        VLOG_ERR_RL(&rl, "another ovs-vswitchd process is running, "
+                    "disabling this process until it goes away");
+
+        HMAP_FOR_EACH_SAFE (br, next_br, node, &all_bridges) {
+            bridge_destroy(br);
+        }
+        return;
+    } else if (!ovsdb_idl_has_lock(idl)) {
+        return;
+    }
+    cfg = ovsrec_open_vswitch_first(idl);
+
     /* Let each bridge do the work that it needs to do. */
     datapath_destroyed = false;
     HMAP_FOR_EACH (br, node, &all_bridges) {
@@ -1420,10 +1458,6 @@ bridge_run(void)
         }
     }
 
-    /* (Re)configure if necessary. */
-    database_changed = ovsdb_idl_run(idl);
-    cfg = ovsrec_open_vswitch_first(idl);
-#ifdef HAVE_OPENSSL
     /* Re-configure SSL.  We do this on every trip through the main loop,
      * instead of just when the database changes, because the contents of the
      * key and certificate files can change without the database changing.
@@ -1436,7 +1470,7 @@ bridge_run(void)
         stream_ssl_set_key_and_cert(ssl->private_key, ssl->certificate);
         stream_ssl_set_ca_cert_file(ssl->ca_cert, ssl->bootstrap_ca_cert);
     }
-#endif
+
     if (database_changed || datapath_destroyed) {
         if (cfg) {
             struct ovsdb_idl_txn *txn = ovsdb_idl_txn_create(idl);
@@ -1472,9 +1506,9 @@ bridge_run(void)
                         iface_refresh_status(iface);
                     }
                 }
-                bridge_refresh_controller_status(br);
             }
             refresh_system_stats(cfg);
+            refresh_controller_status();
             ovsdb_idl_txn_commit(txn);
             ovsdb_idl_txn_destroy(txn); /* XXX */
         }
@@ -1512,44 +1546,19 @@ bridge_run(void)
 void
 bridge_wait(void)
 {
-    struct bridge *br;
-
-    HMAP_FOR_EACH (br, node, &all_bridges) {
-        ofproto_wait(br->ofproto);
-    }
     ovsdb_idl_wait(idl);
-    poll_timer_wait_until(stats_timer);
+    if (!hmap_is_empty(&all_bridges)) {
+        struct bridge *br;
 
-    if (db_limiter > time_msec()) {
-        poll_timer_wait_until(db_limiter);
-    }
-}
-\f
-/* CFM unixctl user interface functions. */
-static void
-cfm_unixctl_show(struct unixctl_conn *conn,
-                 const char *args, void *aux OVS_UNUSED)
-{
-    struct ds ds = DS_EMPTY_INITIALIZER;
-    struct iface *iface;
-    const struct cfm *cfm;
-
-    iface = iface_find(args);
-    if (!iface) {
-        unixctl_command_reply(conn, 501, "no such interface");
-        return;
-    }
-
-    cfm = ofproto_port_get_cfm(iface->port->bridge->ofproto, iface->dp_ifidx);
+        HMAP_FOR_EACH (br, node, &all_bridges) {
+            ofproto_wait(br->ofproto);
+        }
+        poll_timer_wait_until(stats_timer);
 
-    if (!cfm) {
-        unixctl_command_reply(conn, 501, "CFM not enabled");
-        return;
+        if (db_limiter > time_msec()) {
+            poll_timer_wait_until(db_limiter);
+        }
     }
-
-    cfm_dump_ds(cfm, &ds);
-    unixctl_command_reply(conn, 200, ds_cstr(&ds));
-    ds_destroy(&ds);
 }
 \f
 /* QoS unixctl user interface functions. */
@@ -1655,9 +1664,13 @@ bridge_create(const struct ovsrec_bridge *br_cfg)
     br = xzalloc(sizeof *br);
 
     br->name = xstrdup(br_cfg->name);
-    br->type = xstrdup(dpif_normalize_type(br_cfg->datapath_type));
+    br->type = xstrdup(ofproto_normalize_type(br_cfg->datapath_type));
     br->cfg = br_cfg;
-    eth_addr_nicira_random(br->default_ea);
+
+    /* Derive the default Ethernet address from the bridge's UUID.  This should
+     * be unique and it will be stable between ovs-vswitchd runs.  */
+    memcpy(br->default_ea, &br_cfg->header_.uuid, ETH_ADDR_LEN);
+    eth_addr_mark_random(br->default_ea);
 
     hmap_init(&br->ports);
     hmap_init(&br->ifaces);
@@ -1881,7 +1894,7 @@ bridge_configure_local_iface_netdev(struct bridge *br,
     struct in_addr ip;
 
     /* If there's no local interface or no IP address, give up. */
-    local_iface = iface_from_dp_ifidx(br, ODPP_LOCAL);
+    local_iface = iface_from_ofp_port(br, OFPP_LOCAL);
     if (!local_iface || !c->local_ip || !inet_aton(c->local_ip, &ip)) {
         return;
     }
@@ -2052,7 +2065,10 @@ port_del_ifaces(struct port *port)
     sset_init(&new_ifaces);
     for (i = 0; i < port->cfg->n_interfaces; i++) {
         const char *name = port->cfg->interfaces[i]->name;
-        sset_add(&new_ifaces, name);
+        const char *type = port->cfg->interfaces[i]->name;
+        if (strcmp(type, "null")) {
+            sset_add(&new_ifaces, name);
+        }
     }
 
     /* Get rid of deleted interfaces. */
@@ -2078,7 +2094,8 @@ port_add_ifaces(struct port *port)
     shash_init(&new_ifaces);
     for (i = 0; i < port->cfg->n_interfaces; i++) {
         const struct ovsrec_interface *cfg = port->cfg->interfaces[i];
-        if (!shash_add_once(&new_ifaces, cfg->name, cfg)) {
+        if (strcmp(cfg->type, "null")
+            && !shash_add_once(&new_ifaces, cfg->name, cfg)) {
             VLOG_WARN("port %s: %s specified twice as port interface",
                       port->name, cfg->name);
             iface_set_ofport(cfg, -1);
@@ -2190,9 +2207,10 @@ port_configure_lacp(struct port *port, struct lacp_settings *s)
                    ? priority
                    : UINT16_MAX - !list_is_short(&port->ifaces));
 
-    s->strict = !strcmp(get_port_other_config(port->cfg, "lacp-strict",
-                                              "false"),
-                        "true");
+    s->heartbeat = !strcmp(get_port_other_config(port->cfg,
+                                                 "lacp-heartbeat",
+                                                 "false"), "true");
+
 
     lacp_time = get_port_other_config(port->cfg, "lacp-time", "slow");
     custom_time = atoi(lacp_time);
@@ -2213,30 +2231,40 @@ port_configure_lacp(struct port *port, struct lacp_settings *s)
 static void
 iface_configure_lacp(struct iface *iface, struct lacp_slave_settings *s)
 {
-    int priority, portid;
+    int priority, portid, key;
 
     portid = atoi(get_interface_other_config(iface->cfg, "lacp-port-id", "0"));
     priority = atoi(get_interface_other_config(iface->cfg,
                                                "lacp-port-priority", "0"));
+    key = atoi(get_interface_other_config(iface->cfg, "lacp-aggregation-key",
+                                          "0"));
 
     if (portid <= 0 || portid > UINT16_MAX) {
-        portid = iface->dp_ifidx;
+        portid = iface->ofp_port;
     }
 
     if (priority <= 0 || priority > UINT16_MAX) {
         priority = UINT16_MAX;
     }
 
+    if (key < 0 || key > UINT16_MAX) {
+        key = 0;
+    }
+
     s->name = iface->name;
-    s->id = iface->dp_ifidx;
     s->id = portid;
     s->priority = priority;
+    s->key = key;
 }
 
 static void
-port_configure_bond(struct port *port, struct bond_settings *s)
+port_configure_bond(struct port *port, struct bond_settings *s,
+                    uint32_t *bond_stable_ids)
 {
     const char *detect_s;
+    struct iface *iface;
+    int miimon_interval;
+    size_t i;
 
     s->name = port->name;
     s->balance = BM_SLB;
@@ -2246,23 +2274,30 @@ port_configure_bond(struct port *port, struct bond_settings *s)
                   port->name, port->cfg->bond_mode,
                   bond_mode_to_string(s->balance));
     }
+    if (s->balance == BM_SLB && port->bridge->cfg->n_flood_vlans) {
+        VLOG_WARN("port %s: SLB bonds are incompatible with flood_vlans, "
+                  "please use another bond type or disable flood_vlans",
+                  port->name);
+    }
 
-    s->detect = BLSM_CARRIER;
-    detect_s = get_port_other_config(port->cfg, "bond-detect-mode", NULL);
-    if (detect_s && !bond_detect_mode_from_string(&s->detect, detect_s)) {
-        VLOG_WARN("port %s: unsupported bond-detect-mode %s, "
-                  "defaulting to %s",
-                  port->name, detect_s, bond_detect_mode_to_string(s->detect));
+    miimon_interval = atoi(get_port_other_config(port->cfg,
+                                                 "bond-miimon-interval", "0"));
+    if (miimon_interval <= 0) {
+        miimon_interval = 200;
     }
 
-    s->miimon_interval = atoi(
-        get_port_other_config(port->cfg, "bond-miimon-interval", "200"));
-    if (s->miimon_interval < 100) {
-        s->miimon_interval = 100;
+    detect_s = get_port_other_config(port->cfg, "bond-detect-mode", "carrier");
+    if (!strcmp(detect_s, "carrier")) {
+        miimon_interval = 0;
+    } else if (strcmp(detect_s, "miimon")) {
+        VLOG_WARN("port %s: unsupported bond-detect-mode %s, "
+                  "defaulting to carrier", port->name, detect_s);
+        miimon_interval = 0;
     }
 
     s->up_delay = MAX(0, port->cfg->bond_updelay);
     s->down_delay = MAX(0, port->cfg->bond_downdelay);
+    s->basis = atoi(get_port_other_config(port->cfg, "bond-hash-basis", "0"));
     s->rebalance_interval = atoi(
         get_port_other_config(port->cfg, "bond-rebalance-interval", "10000"));
     if (s->rebalance_interval < 1000) {
@@ -2270,6 +2305,20 @@ port_configure_bond(struct port *port, struct bond_settings *s)
     }
 
     s->fake_iface = port->cfg->bond_fake_iface;
+
+    i = 0;
+    LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
+        long long stable_id;
+
+        stable_id = atoll(get_interface_other_config(iface->cfg,
+                                                     "bond-stable-id", "0"));
+        if (stable_id <= 0 || stable_id >= UINT32_MAX) {
+            stable_id = iface->ofp_port;
+        }
+        bond_stable_ids[i++] = stable_id;
+
+        netdev_set_miimon_interval(iface->netdev, miimon_interval);
+    }
 }
 \f
 /* Interface functions. */
@@ -2284,7 +2333,7 @@ iface_create(struct port *port, const struct ovsrec_interface *if_cfg)
     iface = xzalloc(sizeof *iface);
     iface->port = port;
     iface->name = xstrdup(name);
-    iface->dp_ifidx = -1;
+    iface->ofp_port = -1;
     iface->tag = tag_create_random();
     iface->netdev = NULL;
     iface->cfg = if_cfg;
@@ -2305,12 +2354,12 @@ iface_destroy(struct iface *iface)
         struct port *port = iface->port;
         struct bridge *br = port->bridge;
 
-        if (br->ofproto && iface->dp_ifidx >= 0) {
-            ofproto_port_unregister(br->ofproto, iface->dp_ifidx);
+        if (br->ofproto && iface->ofp_port >= 0) {
+            ofproto_port_unregister(br->ofproto, iface->ofp_port);
         }
 
-        if (iface->dp_ifidx >= 0) {
-            hmap_remove(&br->ifaces, &iface->dp_ifidx_node);
+        if (iface->ofp_port >= 0) {
+            hmap_remove(&br->ifaces, &iface->ofp_port_node);
         }
 
         list_remove(&iface->port_elem);
@@ -2354,13 +2403,13 @@ iface_find(const char *name)
 }
 
 static struct iface *
-iface_from_dp_ifidx(const struct bridge *br, uint16_t dp_ifidx)
+iface_from_ofp_port(const struct bridge *br, uint16_t ofp_port)
 {
     struct iface *iface;
 
-    HMAP_FOR_EACH_IN_BUCKET (iface, dp_ifidx_node,
-                             hash_int(dp_ifidx, 0), &br->ifaces) {
-        if (iface->dp_ifidx == dp_ifidx) {
+    HMAP_FOR_EACH_IN_BUCKET (iface, ofp_port_node,
+                             hash_int(ofp_port, 0), &br->ifaces) {
+        if (iface->ofp_port == ofp_port) {
             return iface;
         }
     }
@@ -2376,7 +2425,7 @@ iface_set_mac(struct iface *iface)
 
     if (!strcmp(iface->type, "internal")
         && iface->cfg->mac && eth_addr_from_string(iface->cfg->mac, ea)) {
-        if (iface->dp_ifidx == ODPP_LOCAL) {
+        if (iface->ofp_port == OFPP_LOCAL) {
             VLOG_ERR("interface %s: ignoring mac in Interface record "
                      "(use Bridge record to set local port's mac)",
                      iface->name);
@@ -2478,11 +2527,12 @@ iface_delete_queues(unsigned int queue_id,
 static void
 iface_configure_qos(struct iface *iface, const struct ovsrec_qos *qos)
 {
-    if (!qos || qos->type[0] == '\0') {
+    if (!qos || qos->type[0] == '\0' || qos->n_queues < 1) {
         netdev_set_qos(iface->netdev, NULL, NULL);
     } else {
         struct iface_delete_queues_cbdata cbdata;
         struct shash details;
+        bool queue_zero;
         size_t i;
 
         /* Configure top-level Qos for 'iface'. */
@@ -2498,16 +2548,28 @@ iface_configure_qos(struct iface *iface, const struct ovsrec_qos *qos)
         netdev_dump_queues(iface->netdev, iface_delete_queues, &cbdata);
 
         /* Configure queues for 'iface'. */
+        queue_zero = false;
         for (i = 0; i < qos->n_queues; i++) {
             const struct ovsrec_queue *queue = qos->value_queues[i];
             unsigned int queue_id = qos->key_queues[i];
 
+            if (queue_id == 0) {
+                queue_zero = true;
+            }
+
             shash_from_ovs_idl_map(queue->key_other_config,
                                    queue->value_other_config,
                                    queue->n_other_config, &details);
             netdev_set_queue(iface->netdev, queue_id, &details);
             shash_destroy(&details);
         }
+        if (!queue_zero) {
+            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+            VLOG_WARN_RL(&rl, "interface %s: QoS configured without a default "
+                         "queue (queue 0).  Packets not directed to a "
+                         "correctly configured queue may be dropped.",
+                         iface->name);
+        }
     }
 
     netdev_set_policing(iface->netdev,
@@ -2518,37 +2580,27 @@ iface_configure_qos(struct iface *iface, const struct ovsrec_qos *qos)
 static void
 iface_configure_cfm(struct iface *iface)
 {
-    size_t i;
-    struct cfm cfm;
-    uint16_t *remote_mps;
-    struct ovsrec_monitor *mon;
-    uint8_t maid[CCM_MAID_LEN];
-
-    mon = iface->cfg->monitor;
+    const struct ovsrec_interface *cfg = iface->cfg;
+    struct cfm_settings s;
+    uint16_t remote_mpid;
 
-    if (!mon) {
-        ofproto_port_clear_cfm(iface->port->bridge->ofproto, iface->dp_ifidx);
+    if (!cfg->n_cfm_mpid || !cfg->n_cfm_remote_mpid) {
+        ofproto_port_clear_cfm(iface->port->bridge->ofproto, iface->ofp_port);
         return;
     }
 
-    if (!cfm_generate_maid(mon->md_name, mon->ma_name, maid)) {
-        VLOG_WARN("interface %s: Failed to generate MAID.", iface->name);
-        return;
-    }
-
-    cfm.mpid     = mon->mpid;
-    cfm.interval = mon->interval ? *mon->interval : 1000;
-
-    memcpy(cfm.maid, maid, sizeof cfm.maid);
+    s.mpid = *cfg->cfm_mpid;
+    remote_mpid = *cfg->cfm_remote_mpid;
+    s.remote_mpids = &remote_mpid;
+    s.n_remote_mpids = 1;
 
-    remote_mps = xzalloc(mon->n_remote_mps * sizeof *remote_mps);
-    for(i = 0; i < mon->n_remote_mps; i++) {
-        remote_mps[i] = mon->remote_mps[i]->mpid;
+    s.interval = atoi(get_interface_other_config(iface->cfg, "cfm_interval",
+                                                 "0"));
+    if (s.interval <= 0) {
+        s.interval = 1000;
     }
 
-    ofproto_port_set_cfm(iface->port->bridge->ofproto, iface->dp_ifidx,
-                         &cfm, remote_mps, mon->n_remote_mps);
-    free(remote_mps);
+    ofproto_port_set_cfm(iface->port->bridge->ofproto, iface->ofp_port, &s);
 }
 
 /* Read carrier or miimon status directly from 'iface''s netdev, according to
@@ -2680,8 +2732,6 @@ static bool
 mirror_configure(struct mirror *m, const struct ovsrec_mirror *cfg)
 {
     struct ofproto_mirror_settings s;
-    struct port *out_port;
-    struct port *port;
 
     /* Set name. */
     if (strcmp(cfg->name, m->name)) {
@@ -2693,7 +2743,7 @@ mirror_configure(struct mirror *m, const struct ovsrec_mirror *cfg)
     /* Get output port or VLAN. */
     if (cfg->output_port) {
         s.out_bundle = port_lookup(m->bridge, cfg->output_port->name);
-        if (!out_port) {
+        if (!s.out_bundle) {
             VLOG_ERR("bridge %s: mirror %s outputs to port not on bridge",
                      m->bridge->name, m->name);
             return false;
@@ -2719,6 +2769,7 @@ mirror_configure(struct mirror *m, const struct ovsrec_mirror *cfg)
     if (cfg->select_all) {
         size_t n_ports = hmap_count(&m->bridge->ports);
         void **ports = xmalloc(n_ports * sizeof *ports);
+        struct port *port;
         size_t i;
 
         i = 0;
@@ -2738,7 +2789,6 @@ mirror_configure(struct mirror *m, const struct ovsrec_mirror *cfg)
                              &s.srcs, &s.n_srcs);
         mirror_collect_ports(m, cfg->select_dst_port, cfg->n_select_dst_port,
                              &s.dsts, &s.n_dsts);
-
     }
 
     /* Get VLAN selection. */