bridge: Avoid reading other_config columns with ovsdb_idl_get().
[sliver-openvswitch.git] / vswitchd / bridge.c
index 46d0618..3aac5ce 100644 (file)
@@ -152,6 +152,7 @@ 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_stp(struct bridge *);
 static void bridge_configure_remotes(struct bridge *,
                                      const struct sockaddr_in *managers,
                                      size_t n_managers);
@@ -161,6 +162,11 @@ static void bridge_pick_local_hw_addr(struct bridge *,
 static uint64_t bridge_pick_datapath_id(struct bridge *,
                                         const uint8_t bridge_ea[ETH_ADDR_LEN],
                                         struct iface *hw_addr_iface);
+static const char *bridge_get_other_config(const struct ovsrec_bridge *,
+                                            const char *key);
+static const char *get_port_other_config(const struct ovsrec_port *,
+                                         const char *key,
+                                         const char *default_value);
 static uint64_t dpid_from_hash(const void *, size_t nbytes);
 static bool bridge_has_bond_fake_iface(const struct bridge *,
                                        const char *name);
@@ -178,6 +184,7 @@ static struct lacp_settings *port_configure_lacp(struct port *,
                                                  struct lacp_settings *);
 static void port_configure_bond(struct port *, struct bond_settings *,
                                 uint32_t *bond_stable_ids);
+static bool port_is_synthetic(const struct port *);
 
 static void bridge_configure_mirrors(struct bridge *);
 static struct mirror *mirror_create(struct bridge *,
@@ -201,7 +208,6 @@ static void iface_configure_cfm(struct iface *);
 static void 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 *);
 
 static void shash_from_ovs_idl_map(char **keys, char **values, size_t n,
@@ -230,8 +236,11 @@ bridge_init(const char *remote)
     ovsdb_idl_omit(idl, &ovsrec_open_vswitch_col_system_version);
 
     ovsdb_idl_omit_alert(idl, &ovsrec_bridge_col_datapath_id);
+    ovsdb_idl_omit_alert(idl, &ovsrec_bridge_col_status);
     ovsdb_idl_omit(idl, &ovsrec_bridge_col_external_ids);
 
+    ovsdb_idl_omit_alert(idl, &ovsrec_port_col_status);
+    ovsdb_idl_omit_alert(idl, &ovsrec_port_col_statistics);
     ovsdb_idl_omit(idl, &ovsrec_port_col_external_ids);
     ovsdb_idl_omit(idl, &ovsrec_port_col_fake_bridge);
 
@@ -239,6 +248,7 @@ bridge_init(const char *remote)
     ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_duplex);
     ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_link_speed);
     ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_link_state);
+    ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_link_resets);
     ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_mtu);
     ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_ofport);
     ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_statistics);
@@ -272,11 +282,11 @@ bridge_init(const char *remote)
     ovsdb_idl_omit(idl, &ovsrec_ssl_col_external_ids);
 
     /* Register unixctl commands. */
-    unixctl_command_register("qos/show", qos_unixctl_show, NULL);
-    unixctl_command_register("bridge/dump-flows", bridge_unixctl_dump_flows,
-                             NULL);
-    unixctl_command_register("bridge/reconnect", bridge_unixctl_reconnect,
-                             NULL);
+    unixctl_command_register("qos/show", "interface", qos_unixctl_show, NULL);
+    unixctl_command_register("bridge/dump-flows", "bridge",
+                             bridge_unixctl_dump_flows, NULL);
+    unixctl_command_register("bridge/reconnect", "[bridge]",
+                             bridge_unixctl_reconnect, NULL);
     lacp_init();
     bond_init();
     cfm_init();
@@ -423,6 +433,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
         bridge_configure_remotes(br, managers, n_managers);
         bridge_configure_netflow(br);
         bridge_configure_sflow(br, &sflow_bridge_number);
+        bridge_configure_stp(br);
     }
     free(managers);
 
@@ -535,6 +546,8 @@ port_configure(struct port *port)
             s.vlan_mode = PORT_VLAN_TRUNK;
         }
     }
+    s.use_priority_tags = !strcmp("true", get_port_other_config(
+                                      cfg, "priority-tags", ""));
 
     /* Get LACP settings. */
     s.lacp = port_configure_lacp(port, &lacp_settings);
@@ -716,6 +729,196 @@ bridge_configure_sflow(struct bridge *br, int *sflow_bridge_number)
     sset_destroy(&oso.targets);
 }
 
+static void
+port_configure_stp(const struct ofproto *ofproto, struct port *port,
+                   struct ofproto_port_stp_settings *port_s,
+                   int *port_num_counter, unsigned long *port_num_bitmap)
+{
+    const char *config_str;
+    struct iface *iface;
+
+    config_str = get_port_other_config(port->cfg, "stp-enable", NULL);
+    if (config_str && !strcmp(config_str, "false")) {
+        port_s->enable = false;
+        return;
+    } else {
+        port_s->enable = true;
+    }
+
+    /* STP over bonds is not supported. */
+    if (!list_is_singleton(&port->ifaces)) {
+        VLOG_ERR("port %s: cannot enable STP on bonds, disabling",
+                 port->name);
+        port_s->enable = false;
+        return;
+    }
+
+    iface = CONTAINER_OF(list_front(&port->ifaces), struct iface, port_elem);
+
+    /* Internal ports shouldn't participate in spanning tree, so
+     * skip them. */
+    if (!strcmp(iface->type, "internal")) {
+        VLOG_DBG("port %s: disable STP on internal ports", port->name);
+        port_s->enable = false;
+        return;
+    }
+
+    /* STP on mirror output ports is not supported. */
+    if (ofproto_is_mirror_output_bundle(ofproto, port)) {
+        VLOG_DBG("port %s: disable STP on mirror ports", port->name);
+        port_s->enable = false;
+        return;
+    }
+
+    config_str = get_port_other_config(port->cfg, "stp-port-num", NULL);
+    if (config_str) {
+        unsigned long int port_num = strtoul(config_str, NULL, 0);
+        int port_idx = port_num - 1;
+
+        if (port_num < 1 || port_num > STP_MAX_PORTS) {
+            VLOG_ERR("port %s: invalid stp-port-num", port->name);
+            port_s->enable = false;
+            return;
+        }
+
+        if (bitmap_is_set(port_num_bitmap, port_idx)) {
+            VLOG_ERR("port %s: duplicate stp-port-num %lu, disabling",
+                    port->name, port_num);
+            port_s->enable = false;
+            return;
+        }
+        bitmap_set1(port_num_bitmap, port_idx);
+        port_s->port_num = port_idx;
+    } else {
+        if (*port_num_counter > STP_MAX_PORTS) {
+            VLOG_ERR("port %s: too many STP ports, disabling", port->name);
+            port_s->enable = false;
+            return;
+        }
+
+        port_s->port_num = (*port_num_counter)++;
+    }
+
+    config_str = get_port_other_config(port->cfg, "stp-path-cost", NULL);
+    if (config_str) {
+        port_s->path_cost = strtoul(config_str, NULL, 10);
+    } else {
+        uint32_t current;
+
+        if (netdev_get_features(iface->netdev, &current, NULL, NULL, NULL)) {
+            /* Couldn't get speed, so assume 100Mb/s. */
+            port_s->path_cost = 19;
+        } else {
+            unsigned int mbps;
+
+            mbps = netdev_features_to_bps(current) / 1000000;
+            port_s->path_cost = stp_convert_speed_to_cost(mbps);
+        }
+    }
+
+    config_str = get_port_other_config(port->cfg, "stp-port-priority", NULL);
+    if (config_str) {
+        port_s->priority = strtoul(config_str, NULL, 0);
+    } else {
+        port_s->priority = STP_DEFAULT_PORT_PRIORITY;
+    }
+}
+
+/* Set spanning tree configuration on 'br'. */
+static void
+bridge_configure_stp(struct bridge *br)
+{
+    if (!br->cfg->stp_enable) {
+        ofproto_set_stp(br->ofproto, NULL);
+    } else {
+        struct ofproto_stp_settings br_s;
+        const char *config_str;
+        struct port *port;
+        int port_num_counter;
+        unsigned long *port_num_bitmap;
+
+        config_str = bridge_get_other_config(br->cfg, "stp-system-id");
+        if (config_str) {
+            uint8_t ea[ETH_ADDR_LEN];
+
+            if (eth_addr_from_string(config_str, ea)) {
+                br_s.system_id = eth_addr_to_uint64(ea);
+            } else {
+                br_s.system_id = eth_addr_to_uint64(br->ea);
+                VLOG_ERR("bridge %s: invalid stp-system-id, defaulting "
+                         "to "ETH_ADDR_FMT, br->name, ETH_ADDR_ARGS(br->ea));
+            }
+        } else {
+            br_s.system_id = eth_addr_to_uint64(br->ea);
+        }
+
+        config_str = bridge_get_other_config(br->cfg, "stp-priority");
+        if (config_str) {
+            br_s.priority = strtoul(config_str, NULL, 0);
+        } else {
+            br_s.priority = STP_DEFAULT_BRIDGE_PRIORITY;
+        }
+
+        config_str = bridge_get_other_config(br->cfg, "stp-hello-time");
+        if (config_str) {
+            br_s.hello_time = strtoul(config_str, NULL, 10) * 1000;
+        } else {
+            br_s.hello_time = STP_DEFAULT_HELLO_TIME;
+        }
+
+        config_str = bridge_get_other_config(br->cfg, "stp-max-age");
+        if (config_str) {
+            br_s.max_age = strtoul(config_str, NULL, 10) * 1000;
+        } else {
+            br_s.max_age = STP_DEFAULT_MAX_AGE;
+        }
+
+        config_str = bridge_get_other_config(br->cfg, "stp-forward-delay");
+        if (config_str) {
+            br_s.fwd_delay = strtoul(config_str, NULL, 10) * 1000;
+        } else {
+            br_s.fwd_delay = STP_DEFAULT_FWD_DELAY;
+        }
+
+        /* Configure STP on the bridge. */
+        if (ofproto_set_stp(br->ofproto, &br_s)) {
+            VLOG_ERR("bridge %s: could not enable STP", br->name);
+            return;
+        }
+
+        /* Users must either set the port number with the "stp-port-num"
+         * configuration on all ports or none.  If manual configuration
+         * is not done, then we allocate them sequentially. */
+        port_num_counter = 0;
+        port_num_bitmap = bitmap_allocate(STP_MAX_PORTS);
+        HMAP_FOR_EACH (port, hmap_node, &br->ports) {
+            struct ofproto_port_stp_settings port_s;
+            struct iface *iface;
+
+            port_configure_stp(br->ofproto, port, &port_s,
+                               &port_num_counter, port_num_bitmap);
+
+            /* As bonds are not supported, just apply configuration to
+             * all interfaces. */
+            LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
+                if (ofproto_port_set_stp(br->ofproto, iface->ofp_port,
+                                         &port_s)) {
+                    VLOG_ERR("port %s: could not enable STP", port->name);
+                    continue;
+                }
+            }
+        }
+
+        if (bitmap_scan(port_num_bitmap, 0, STP_MAX_PORTS) != STP_MAX_PORTS
+                    && port_num_counter) {
+            VLOG_ERR("bridge %s: must manually configure all STP port "
+                     "IDs or none, disabling", br->name);
+            ofproto_set_stp(br->ofproto, NULL);
+        }
+        bitmap_free(port_num_bitmap);
+    }
+}
+
 static bool
 bridge_has_bond_fake_iface(const struct bridge *br, const char *name)
 {
@@ -739,9 +942,16 @@ add_del_bridges(const struct ovsrec_open_vswitch *cfg)
     /* Collect new bridges' names and types. */
     shash_init(&new_br);
     for (i = 0; i < cfg->n_bridges; i++) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
         const struct ovsrec_bridge *br_cfg = cfg->bridges[i];
-        if (!shash_add_once(&new_br, br_cfg->name, br_cfg)) {
-            VLOG_WARN("bridge %s specified twice", br_cfg->name);
+
+        if (strchr(br_cfg->name, '/')) {
+            /* Prevent remote ovsdb-server users from accessing arbitrary
+             * directories, e.g. consider a bridge named "../../../etc/". */
+            VLOG_WARN_RL(&rl, "ignoring bridge with invalid name \"%s\"",
+                         br_cfg->name);
+        } else if (!shash_add_once(&new_br, br_cfg->name, br_cfg)) {
+            VLOG_WARN_RL(&rl, "bridge %s specified twice", br_cfg->name);
         }
     }
 
@@ -974,25 +1184,24 @@ bridge_add_ofproto_ports(struct bridge *br)
 }
 
 static const char *
-get_ovsrec_key_value(const struct ovsdb_idl_row *row,
-                     const struct ovsdb_idl_column *column,
-                     const char *key)
+get_ovsrec_key_value(char **keys, char **values, size_t n, const char *key)
 {
-    const struct ovsdb_datum *datum;
-    union ovsdb_atom atom;
-    unsigned int idx;
+    size_t i;
 
-    datum = ovsdb_idl_get(row, column, OVSDB_TYPE_STRING, OVSDB_TYPE_STRING);
-    atom.string = (char *) key;
-    idx = ovsdb_datum_find_key(datum, &atom, OVSDB_TYPE_STRING);
-    return idx == UINT_MAX ? NULL : datum->values[idx].string;
+    for (i = 0; i < n; i++) {
+        if (!strcmp(keys[i], key)) {
+            return values[i];
+        }
+    }
+    return NULL;
 }
 
 static const char *
 bridge_get_other_config(const struct ovsrec_bridge *br_cfg, const char *key)
 {
-    return get_ovsrec_key_value(&br_cfg->header_,
-                                &ovsrec_bridge_col_other_config, key);
+    return get_ovsrec_key_value(br_cfg->key_other_config,
+                                br_cfg->value_other_config,
+                                br_cfg->n_other_config, key);
 }
 
 /* Set Flow eviction threshold */
@@ -1268,9 +1477,6 @@ iface_refresh_status(struct iface *iface)
         ovsrec_interface_set_link_speed(iface->cfg, NULL, 0);
     }
 
-    ovsrec_interface_set_link_state(iface->cfg,
-                                    iface_get_carrier(iface) ? "up" : "down");
-
     error = netdev_get_mtu(iface->netdev, &mtu);
     if (!error) {
         mtu_64 = mtu;
@@ -1281,8 +1487,7 @@ iface_refresh_status(struct iface *iface)
     }
 }
 
-/* Writes 'iface''s CFM statistics to the database.  Returns true if anything
- * changed, false otherwise. */
+/* Writes 'iface''s CFM statistics to the database. */
 static void
 iface_refresh_cfm_stats(struct iface *iface)
 {
@@ -1314,20 +1519,6 @@ iface_refresh_cfm_stats(struct iface *iface)
     }
 }
 
-static bool
-iface_refresh_lacp_stats(struct iface *iface)
-{
-    struct ofproto *ofproto = iface->port->bridge->ofproto;
-    int old = iface->cfg->lacp_current ? *iface->cfg->lacp_current : -1;
-    int new = ofproto_port_is_lacp_current(ofproto, iface->ofp_port);
-
-    if (old != new) {
-        bool current = new;
-        ovsrec_interface_set_lacp_current(iface->cfg, &current, new >= 0);
-    }
-    return old != new;
-}
-
 static void
 iface_refresh_stats(struct iface *iface)
 {
@@ -1372,14 +1563,108 @@ iface_refresh_stats(struct iface *iface)
 #undef IFACE_STATS
 }
 
+static void
+br_refresh_stp_status(struct bridge *br)
+{
+    struct ofproto *ofproto = br->ofproto;
+    struct ofproto_stp_status status;
+    char *keys[3], *values[3];
+    size_t i;
+
+    if (ofproto_get_stp_status(ofproto, &status)) {
+        return;
+    }
+
+    if (!status.enabled) {
+        ovsrec_bridge_set_status(br->cfg, NULL, NULL, 0);
+        return;
+    }
+
+    keys[0] = "stp_bridge_id",
+    values[0] = xasprintf(STP_ID_FMT, STP_ID_ARGS(status.bridge_id));
+    keys[1] = "stp_designated_root",
+    values[1] = xasprintf(STP_ID_FMT, STP_ID_ARGS(status.designated_root));
+    keys[2] = "stp_root_path_cost",
+    values[2] = xasprintf("%d", status.root_path_cost);
+
+    ovsrec_bridge_set_status(br->cfg, keys, values, ARRAY_SIZE(values));
+
+    for (i = 0; i < ARRAY_SIZE(values); i++) {
+        free(values[i]);
+    }
+}
+
+static void
+port_refresh_stp_status(struct port *port)
+{
+    struct ofproto *ofproto = port->bridge->ofproto;
+    struct iface *iface;
+    struct ofproto_port_stp_status status;
+    char *keys[4];
+    char *str_values[4];
+    int64_t int_values[3];
+    size_t i;
+
+    if (port_is_synthetic(port)) {
+        return;
+    }
+
+    /* STP doesn't currently support bonds. */
+    if (!list_is_singleton(&port->ifaces)) {
+        ovsrec_port_set_status(port->cfg, NULL, NULL, 0);
+        return;
+    }
+
+    iface = CONTAINER_OF(list_front(&port->ifaces), struct iface, port_elem);
+
+    if (ofproto_port_get_stp_status(ofproto, iface->ofp_port, &status)) {
+        return;
+    }
+
+    if (!status.enabled) {
+        ovsrec_port_set_status(port->cfg, NULL, NULL, 0);
+        ovsrec_port_set_statistics(port->cfg, NULL, NULL, 0);
+        return;
+    }
+
+    /* Set Status column. */
+    keys[0] = "stp_port_id";
+    str_values[0] = xasprintf(STP_PORT_ID_FMT, status.port_id);
+    keys[1] = "stp_state";
+    str_values[1] = xstrdup(stp_state_name(status.state));
+    keys[2] = "stp_sec_in_state";
+    str_values[2] = xasprintf("%u", status.sec_in_state);
+    keys[3] = "stp_role";
+    str_values[3] = xstrdup(stp_role_name(status.role));
+
+    ovsrec_port_set_status(port->cfg, keys, str_values,
+                           ARRAY_SIZE(str_values));
+
+    for (i = 0; i < ARRAY_SIZE(str_values); i++) {
+        free(str_values[i]);
+    }
+
+    /* Set Statistics column. */
+    keys[0] = "stp_tx_count";
+    int_values[0] = status.tx_count;
+    keys[1] = "stp_rx_count";
+    int_values[1] = status.rx_count;
+    keys[2] = "stp_error_count";
+    int_values[2] = status.error_count;
+
+    ovsrec_port_set_statistics(port->cfg, keys, int_values,
+                               ARRAY_SIZE(int_values));
+}
+
 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 = get_ovsrec_key_value(cfg->key_other_config,
+                                  cfg->value_other_config,
+                                  cfg->n_other_config,
                                   "enable-statistics");
     if (enable) {
         return !strcmp(enable, "true");
@@ -1579,29 +1864,47 @@ bridge_run(void)
 
     if (time_msec() >= db_limiter) {
         struct ovsdb_idl_txn *txn;
-        bool changed = false;
 
         txn = ovsdb_idl_txn_create(idl);
         HMAP_FOR_EACH (br, node, &all_bridges) {
+            struct iface *iface;
             struct port *port;
 
+            br_refresh_stp_status(br);
+
             HMAP_FOR_EACH (port, hmap_node, &br->ports) {
-                struct iface *iface;
+                port_refresh_stp_status(port);
+            }
 
-                LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
-                    /* XXX: Eventually we need to remove the lacp_current flag
-                     * from the database so that we can completely get rid of
-                     * this rate limiter code. */
-                    changed = iface_refresh_lacp_stats(iface) || changed;
+            HMAP_FOR_EACH (iface, name_node, &br->iface_by_name) {
+                const char *link_state;
+                int64_t link_resets;
+                int current;
+
+                if (iface_is_synthetic(iface)) {
+                    continue;
                 }
+
+                current = ofproto_port_is_lacp_current(br->ofproto,
+                                                       iface->ofp_port);
+                if (current >= 0) {
+                    bool bl = current;
+                    ovsrec_interface_set_lacp_current(iface->cfg, &bl, 1);
+                } else {
+                    ovsrec_interface_set_lacp_current(iface->cfg, NULL, 0);
+                }
+
+                link_state = netdev_get_carrier(iface->netdev) ? "up" : "down";
+                ovsrec_interface_set_link_state(iface->cfg, link_state);
+
+                link_resets = netdev_get_carrier_resets(iface->netdev);
+                ovsrec_interface_set_link_resets(iface->cfg, &link_resets, 1);
             }
         }
 
-        if (changed) {
+        if (ovsdb_idl_txn_commit(txn) != TXN_UNCHANGED) {
             db_limiter = time_msec() + DB_LIMIT_INTERVAL;
         }
-
-        ovsdb_idl_txn_commit(txn);
         ovsdb_idl_txn_destroy(txn);
     }
 
@@ -1990,6 +2293,26 @@ bridge_configure_local_iface_netdev(struct bridge *br,
     }
 }
 
+/* Returns true if 'a' and 'b' are the same except that any number of slashes
+ * in either string are treated as equal to any number of slashes in the other,
+ * e.g. "x///y" is equal to "x/y". */
+static bool
+equal_pathnames(const char *a, const char *b)
+{
+    while (*a == *b) {
+        if (*a == '/') {
+            a += strspn(a, "/");
+            b += strspn(b, "/");
+        } else if (*a == '\0') {
+            return true;
+        } else {
+            a++;
+            b++;
+        }
+    }
+    return false;
+}
+
 static void
 bridge_configure_remotes(struct bridge *br,
                          const struct sockaddr_in *managers, size_t n_managers)
@@ -2036,13 +2359,26 @@ bridge_configure_remotes(struct bridge *br,
         if (!strncmp(c->target, "punix:", 6)
             || !strncmp(c->target, "unix:", 5)) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+            char *whitelist;
+
+            whitelist = xasprintf("unix:%s/%s.controller",
+                                  ovs_rundir(), br->name);
+            if (!equal_pathnames(c->target, whitelist)) {
+                /* Prevent remote ovsdb-server users from accessing arbitrary
+                 * Unix domain sockets and overwriting arbitrary local
+                 * files. */
+                VLOG_ERR_RL(&rl, "bridge %s: Not adding Unix domain socket "
+                            "controller \"%s\" due to possibility for remote "
+                            "exploit.  Instead, specify whitelisted \"%s\" or "
+                            "connect to \"unix:%s/%s.mgmt\" (which is always "
+                            "available without special configuration).",
+                            br->name, c->target, whitelist,
+                            ovs_rundir(), br->name);
+                free(whitelist);
+                continue;
+            }
 
-            /* Prevent remote ovsdb-server users from accessing arbitrary Unix
-             * domain sockets and overwriting arbitrary local files. */
-            VLOG_ERR_RL(&rl, "bridge %s: not adding Unix domain socket "
-                        "controller \"%s\" due to possibility for remote "
-                        "exploit", br->name, c->target);
-            continue;
+            free(whitelist);
         }
 
         bridge_configure_local_iface_netdev(br, c);
@@ -2102,8 +2438,9 @@ get_port_other_config(const struct ovsrec_port *port, const char *key,
 {
     const char *value;
 
-    value = get_ovsrec_key_value(&port->header_, &ovsrec_port_col_other_config,
-                                 key);
+    value = get_ovsrec_key_value(port->key_other_config,
+                                 port->value_other_config,
+                                 port->n_other_config, key);
     return value ? value : default_value;
 }
 
@@ -2113,8 +2450,9 @@ get_interface_other_config(const struct ovsrec_interface *iface,
 {
     const char *value;
 
-    value = get_ovsrec_key_value(&iface->header_,
-                                 &ovsrec_interface_col_other_config, key);
+    value = get_ovsrec_key_value(iface->key_other_config,
+                                 iface->value_other_config,
+                                 iface->n_other_config, key);
     return value ? value : default_value;
 }
 
@@ -2385,6 +2723,14 @@ port_configure_bond(struct port *port, struct bond_settings *s,
         netdev_set_miimon_interval(iface->netdev, miimon_interval);
     }
 }
+
+/* Returns true if 'port' is synthetic, that is, if we constructed it locally
+ * instead of obtaining it from the database. */
+static bool
+port_is_synthetic(const struct port *port)
+{
+    return ovsdb_idl_row_is_synthetic(&port->cfg->header_);
+}
 \f
 /* Interface functions. */
 
@@ -2615,6 +2961,10 @@ iface_delete_queues(unsigned int queue_id,
 static void
 iface_configure_qos(struct iface *iface, const struct ovsrec_qos *qos)
 {
+    struct ofpbuf queues_buf;
+
+    ofpbuf_init(&queues_buf, 0);
+
     if (!qos || qos->type[0] == '\0' || qos->n_queues < 1) {
         netdev_set_qos(iface->netdev, NULL, NULL);
     } else {
@@ -2645,6 +2995,15 @@ iface_configure_qos(struct iface *iface, const struct ovsrec_qos *qos)
                 queue_zero = true;
             }
 
+            if (queue->n_dscp == 1) {
+                struct ofproto_port_queue *port_queue;
+
+                port_queue = ofpbuf_put_uninit(&queues_buf,
+                                               sizeof *port_queue);
+                port_queue->queue = queue_id;
+                port_queue->dscp = queue->dscp[0];
+            }
+
             shash_from_ovs_idl_map(queue->key_other_config,
                                    queue->value_other_config,
                                    queue->n_other_config, &details);
@@ -2660,16 +3019,26 @@ iface_configure_qos(struct iface *iface, const struct ovsrec_qos *qos)
         }
     }
 
+    if (iface->ofp_port >= 0) {
+        const struct ofproto_port_queue *port_queues = queues_buf.data;
+        size_t n_queues = queues_buf.size / sizeof *port_queues;
+
+        ofproto_port_set_queues(iface->port->bridge->ofproto, iface->ofp_port,
+                                port_queues, n_queues);
+    }
+
     netdev_set_policing(iface->netdev,
                         iface->cfg->ingress_policing_rate,
                         iface->cfg->ingress_policing_burst);
+
+    ofpbuf_uninit(&queues_buf);
 }
 
 static void
 iface_configure_cfm(struct iface *iface)
 {
     const struct ovsrec_interface *cfg = iface->cfg;
-    const char *extended_str;
+    const char *extended_str, *opstate_str;
     struct cfm_settings s;
 
     if (!cfg->n_cfm_mpid) {
@@ -2680,6 +3049,8 @@ iface_configure_cfm(struct iface *iface)
     s.mpid = *cfg->cfm_mpid;
     s.interval = atoi(get_interface_other_config(iface->cfg, "cfm_interval",
                                                  "0"));
+    s.ccm_vlan = atoi(get_interface_other_config(iface->cfg, "cfm_ccm_vlan",
+                                                 "0"));
     if (s.interval <= 0) {
         s.interval = 1000;
     }
@@ -2688,18 +3059,10 @@ iface_configure_cfm(struct iface *iface)
                                               "false");
     s.extended = !strcasecmp("true", extended_str);
 
-    ofproto_port_set_cfm(iface->port->bridge->ofproto, iface->ofp_port, &s);
-}
+    opstate_str = get_interface_other_config(iface->cfg, "cfm_opstate", "up");
+    s.opup = !strcasecmp("up", opstate_str);
 
-/* Read carrier or miimon status directly from 'iface''s netdev, according to
- * how 'iface''s port is configured.
- *
- * Returns true if 'iface' is up, false otherwise. */
-static bool
-iface_get_carrier(const struct iface *iface)
-{
-    /* XXX */
-    return netdev_get_carrier(iface->netdev);
+    ofproto_port_set_cfm(iface->port->bridge->ofproto, iface->ofp_port, &s);
 }
 
 /* Returns true if 'iface' is synthetic, that is, if we constructed it locally