The existing bridge_reconfigure() implementation is sub-optimal.
When adding lots of new ports, on every pass through the run loop
it allocates a bunch of "struct iface"s and "struct port"s, only to
destroy them when out of time. Additionally, when there are errors
adding or deleting ports, it can fail to converge. Instead it will
attempt and fail to add the same set of ports forever.
This patch rewrites bridge_reconfigure() using a new strategy.
Whenever the database changes, some initial book keeping is done,
and a list of future work is compiled. The bridge begins whittling
down this list, and stops processing database changes until
finished.
Bug #10902.
Signed-off-by: Ethan Jackson <et...@nicira.com>
---
vswitchd/bridge.c | 772 ++++++++++++++++++++++++++++++-----------------------
1 file changed, 439 insertions(+), 333 deletions(-)
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 15f6cb7..d7a9e88 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -60,6 +60,19 @@ VLOG_DEFINE_THIS_MODULE(bridge);
COVERAGE_DEFINE(bridge_reconfigure);
+/* Configuration of an uninstantiated iface. */
+struct if_cfg {
+ struct list list_node; /* Node in bridge's if_cfg_queue. */
+ const struct ovsrec_interface *cfg; /* Interface record. */
+ const struct ovsrec_port *parent; /* Parent port record. */
+};
+
+/* OpenFlow port slated for removal from ofproto. */
+struct ofpp_inmate {
+ struct list list_node; /* Node in bridge's ofpp_death_row. */
+ uint16_t ofp_port; /* Port to be deleted. */
+};
+
struct iface {
/* These members are always valid. */
struct list port_elem; /* Element in struct port's "ifaces" list. */
@@ -113,6 +126,9 @@ struct bridge {
struct hmap ifaces; /* "struct iface"s indexed by ofp_port. */
struct hmap iface_by_name; /* "struct iface"s indexed by name. */
+ struct list ofpp_death_row; /* "struct ofpp_inmate"s slated for removal. */
+ struct list if_cfg_queue; /* "struct if_cfg"s slated for creation. */
+
/* Port mirroring. */
struct hmap mirrors; /* "struct mirror" indexed by UUID. */
@@ -151,11 +167,11 @@ static long long int db_limiter = LLONG_MIN;
* This allows the rest of the code to catch up on important things like
* forwarding packets. */
#define OFP_PORT_ACTION_WINDOW 10
-static bool need_reconfigure = false;
+static bool reconfiguring = false;
static void add_del_bridges(const struct ovsrec_open_vswitch *);
-static void bridge_del_ofprotos(void);
-static bool bridge_add_ofprotos(struct bridge *);
+static void bridge_update_ofprotos(void);
+static void bridge_populate_ofpp_death_row(struct bridge *);
static void bridge_create(const struct ovsrec_bridge *);
static void bridge_destroy(struct bridge *);
static struct bridge *bridge_lookup(const char *name);
@@ -165,10 +181,6 @@ static size_t bridge_get_controllers(const struct bridge
*br,
struct ovsrec_controller ***controllersp);
static void bridge_add_del_ports(struct bridge *,
const unsigned long int *splinter_vlans);
-static void bridge_add_ofproto_ports(struct bridge *,
- long long int *port_action_timer);
-static void bridge_del_ofproto_ports(struct bridge *,
- long long int *port_action_timer);
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 *);
@@ -187,6 +199,9 @@ 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 void bridge_queue_if_cfg(struct bridge *,
+ const struct ovsrec_interface *,
+ const struct ovsrec_port *);
static uint64_t dpid_from_hash(const void *, size_t nbytes);
static bool bridge_has_bond_fake_iface(const struct bridge *,
const char *name);
@@ -195,7 +210,6 @@ static bool port_is_bond_fake_iface(const struct port *);
static unixctl_cb_func qos_unixctl_show;
static struct port *port_create(struct bridge *, const struct ovsrec_port *);
-static void port_add_ifaces(struct port *);
static void port_del_ifaces(struct port *);
static void port_destroy(struct port *);
static struct port *port_lookup(const struct bridge *, const char *name);
@@ -216,6 +230,9 @@ static void mirror_refresh_stats(struct mirror *);
static void iface_configure_lacp(struct iface *, struct lacp_slave_settings *);
static struct iface *iface_create(struct port *port,
const struct ovsrec_interface *if_cfg);
+static struct iface *iface_from_if_cfg(struct bridge *, struct if_cfg *);
+static void iface_refresh_type(struct iface *);
+static void iface_init(struct iface *);
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);
@@ -407,23 +424,18 @@ static void
bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
{
unsigned long int *splinter_vlans;
- long long int port_action_timer;
- struct sockaddr_in *managers;
- struct bridge *br, *next;
- int sflow_bridge_number;
- size_t n_managers;
+ struct bridge *br;
COVERAGE_INC(bridge_reconfigure);
- port_action_timer = LLONG_MAX;
- need_reconfigure = false;
+ reconfiguring = true;
- /* Create and destroy "struct bridge"s, "struct port"s, and "struct
- * iface"s according to 'ovs_cfg', with only very minimal configuration
- * otherwise.
+ /* Destroy "struct bridge"s, "struct port"s, and "struct iface"s according
+ * to 'ovs_cfg' while update the "if_cfg_queue", with only very minimal
+ * configuration otherwise.
*
- * This is mostly an update to bridge data structures. Very little is
- * pushed down to ofproto or lower layers. */
+ * This is mostly an update to bridge data structures. Nothing is pushed
+ * down to ofproto or lower layers. */
add_del_bridges(ovs_cfg);
splinter_vlans = collect_splinter_vlans(ovs_cfg);
HMAP_FOR_EACH (br, node, &all_bridges) {
@@ -431,39 +443,93 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
*ovs_cfg)
}
free(splinter_vlans);
- /* Delete all datapaths and datapath ports that are no longer configured.
- *
- * The kernel will reject any attempt to add a given port to a datapath if
- * 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_ofprotos();
+ /* Delete datapaths that are no longer configured, and create ones which
+ * don't exist but should. */
+ bridge_update_ofprotos();
+
+ /* Make sure each "struct iface" has a correct ofp_port in its ofproto. */
HMAP_FOR_EACH (br, node, &all_bridges) {
- if (br->ofproto) {
- bridge_del_ofproto_ports(br, &port_action_timer);
+ bridge_refresh_ofp_port(br);
+ }
+
+ /* Schedule ofp_ports which don't belong for deletion. */
+ HMAP_FOR_EACH (br, node, &all_bridges) {
+ bridge_populate_ofpp_death_row(br);
+ }
+
+ /* Clear database records for "if_cfg"s which haven't been instantiated. */
+ HMAP_FOR_EACH (br, node, &all_bridges) {
+ struct if_cfg *if_cfg;
+
+ LIST_FOR_EACH (if_cfg, list_node, &br->if_cfg_queue) {
+ iface_clear_db_record(if_cfg->cfg);
}
}
+}
- /* Create datapaths and datapath ports that are missing.
- *
- * 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 ofp_port and
- * netdev. */
- HMAP_FOR_EACH_SAFE (br, next, node, &all_bridges) {
- if (!br->ofproto) {
- if (bridge_add_ofprotos(br)) {
- bridge_del_ofproto_ports(br, &port_action_timer);
- } else {
- bridge_destroy(br);
+static bool
+bridge_reconfigure_ofp(void)
+{
+ long long int deadline;
+ struct bridge *br;
+ bool done = true;
+
+ /* Do any work needed to complete configuration. Don't consider ourselves
+ * done unless no work was needed in this iteration. This guarantees that
+ * ofproto_run() has a chance to respond to newly added ports. */
+
+ time_refresh();
+ deadline = time_msec() + OFP_PORT_ACTION_WINDOW;
+
+ /* The kernel will reject any attempt to add a given port to a datapath if
+ * that port already belongs to a different datapath, so we must do all
+ * port deletions before any port additions. */
+ HMAP_FOR_EACH (br, node, &all_bridges) {
+ struct ofpp_inmate *inmate, *next;
+
+ LIST_FOR_EACH_SAFE (inmate, next, list_node, &br->ofpp_death_row) {
+ ofproto_port_del(br->ofproto, inmate->ofp_port);
+ list_remove(&inmate->list_node);
+ free(inmate);
+ done = false;
+
+ time_refresh();
+ if (time_msec() >= deadline) {
+ return done;
}
}
}
+
HMAP_FOR_EACH (br, node, &all_bridges) {
- bridge_refresh_ofp_port(br);
- bridge_add_ofproto_ports(br, &port_action_timer);
+ struct if_cfg *if_cfg, *next;
+
+ LIST_FOR_EACH_SAFE (if_cfg, next, list_node, &br->if_cfg_queue) {
+ iface_init(iface_from_if_cfg(br, if_cfg));
+ list_remove(&if_cfg->list_node);
+ free(if_cfg);
+ done = false;
+
+ time_refresh();
+ if (time_msec() >= deadline) {
+ return done;
+ }
+ }
}
+ return done;
+}
+
+static bool
+bridge_reconfigure_continue(const struct ovsrec_open_vswitch *ovs_cfg)
+{
+ struct sockaddr_in *managers;
+ int sflow_bridge_number;
+ size_t n_managers;
+ struct bridge *br;
+ bool done;
+
+ done = bridge_reconfigure_ofp();
+
/* Complete the configuration. */
sflow_bridge_number = 0;
collect_in_band_managers(ovs_cfg, &managers, &n_managers);
@@ -497,22 +563,27 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
*ovs_cfg)
}
free(managers);
- if (!need_reconfigure) {
+ if (done) {
/* ovs-vswitchd has completed initialization, so allow the process that
* forked us to exit successfully. */
daemonize_complete();
+ reconfiguring = false;
}
+
+ return done;
}
-/* Iterate over all ofprotos and delete any of them that do not have a
- * configured bridge or that are the wrong type. */
+/* Delete ofprotos which aren't configured or have the wrong type. Create
+ * ofprotos which don't exist but need to. */
static void
-bridge_del_ofprotos(void)
+bridge_update_ofprotos(void)
{
+ struct bridge *br, *next;
struct sset names;
struct sset types;
const char *type;
+ /* Delete ofprotos with no bridge or with the wrong type. */
sset_init(&names);
sset_init(&types);
ofproto_enumerate_types(&types);
@@ -521,7 +592,7 @@ bridge_del_ofprotos(void)
ofproto_enumerate_names(type, &names);
SSET_FOR_EACH (name, &names) {
- struct bridge *br = bridge_lookup(name);
+ br = bridge_lookup(name);
if (!br || strcmp(type, br->type)) {
ofproto_delete(name, type);
}
@@ -529,17 +600,39 @@ bridge_del_ofprotos(void)
}
sset_destroy(&names);
sset_destroy(&types);
-}
-static bool
-bridge_add_ofprotos(struct bridge *br)
-{
- int error = ofproto_create(br->name, br->type, &br->ofproto);
- if (error) {
- VLOG_ERR("failed to create bridge %s: %s", br->name, strerror(error));
- return false;
+ /* Suppose we have an ofproto named "a" with a port "b" and we need to
+ * create an ofproto named "b". Since port "b" exists on "a", creation of
+ * ofproto "b" will fail because it won't be able to make an internal port.
+ * To avoid this problem, we remove all ports from datapaths which have the
+ * same name as a different ofproto. This situation is unlikely to occur
+ * frequently, so we don't rate limit it for simplicity. */
+ HMAP_FOR_EACH (br, node, &all_bridges) {
+ if (br->ofproto) {
+ struct ofproto_port ofproto_port;
+ struct ofproto_port_dump dump;
+
+ OFPROTO_PORT_FOR_EACH (&ofproto_port, &dump, br->ofproto) {
+ struct bridge *ofp_br = bridge_lookup(ofproto_port.name);
+
+ if (ofp_br && ofp_br != br) {
+ ofproto_port_del(br->ofproto, ofproto_port.ofp_port);
+ }
+ }
+ }
+ }
+
+ /* Add ofprotos for bridges which don't have one yet. */
+ HMAP_FOR_EACH_SAFE (br, next, node, &all_bridges) {
+ if (!br->ofproto) {
+ int error = ofproto_create(br->name, br->type, &br->ofproto);
+ if (error) {
+ VLOG_ERR("failed to create bridge %s: %s", br->name,
+ strerror(error));
+ bridge_destroy(br);
+ }
+ }
}
- return true;
}
static void
@@ -1050,80 +1143,6 @@ add_del_bridges(const struct ovsrec_open_vswitch *cfg)
shash_destroy(&new_br);
}
-static bool
-may_port_action(long long int *port_action_timer)
-{
- if (need_reconfigure) {
- return false;
- }
-
- time_refresh();
- if (*port_action_timer == LLONG_MAX) {
- *port_action_timer = time_msec() + OFP_PORT_ACTION_WINDOW;
- } else if (time_msec() > *port_action_timer) {
- need_reconfigure = true;
- return false;
- }
- return true;
-}
-
-/* Delete each ofproto port on 'br' that doesn't have a corresponding "struct
- * iface".
- *
- * The kernel will reject any attempt to add a given port to a datapath if that
- * port already belongs to a different datapath, so we must do all port
- * deletions before any port additions. */
-static void
-bridge_del_ofproto_ports(struct bridge *br,
- long long int *port_action_timer)
-{
- struct ofproto_port_dump dump;
- struct ofproto_port ofproto_port;
-
- OFPROTO_PORT_FOR_EACH (&ofproto_port, &dump, br->ofproto) {
- const char *name = ofproto_port.name;
- struct iface *iface;
- const char *type;
- int error;
-
- /* Ignore the local port. We can't change it anyhow. */
- if (!strcmp(name, br->name)) {
- continue;
- }
-
- /* Get the type that 'ofproto_port' should have (ordinarily the
- * type of its corresponding iface) or NULL if it should be
- * deleted. */
- iface = iface_lookup(br, name);
- type = (iface ? iface->type
- : bridge_has_bond_fake_iface(br, name) ? "internal"
- : NULL);
-
- /* If it's the wrong type then delete the ofproto port. */
- if (type
- && !strcmp(ofproto_port.type, type)
- && (!iface || !iface->netdev
- || !strcmp(netdev_get_type(iface->netdev), type))) {
- continue;
- }
-
- if (may_port_action(port_action_timer)) {
- error = ofproto_port_del(br->ofproto, ofproto_port.ofp_port);
- if (error) {
- VLOG_WARN("bridge %s: failed to remove %s interface (%s)",
- br->name, name, strerror(error));
- } else {
- VLOG_INFO("bridge %s: removed interface %s (%d)", br->name,
- name, ofproto_port.ofp_port);
- }
- }
- if (iface) {
- netdev_close(iface->netdev);
- iface->netdev = NULL;
- }
- }
-}
-
static void
iface_set_ofp_port(struct iface *iface, int ofp_port)
{
@@ -1136,13 +1155,45 @@ iface_set_ofp_port(struct iface *iface, int ofp_port)
}
static void
+bridge_ofproto_port_del(struct bridge *br, struct ofproto_port ofproto_port)
+{
+ int error = ofproto_port_del(br->ofproto, ofproto_port.ofp_port);
+ if (error) {
+ VLOG_WARN("bridge %s: failed to remove %s interface (%s)",
+ br->name, ofproto_port.name, strerror(error));
+ } else {
+ VLOG_INFO("bridge %s: removed interface %s (%d)", br->name,
+ ofproto_port.name, ofproto_port.ofp_port);
+ }
+}
+
+/* Update bridges "if_cfg"s, "struct port"s, and "struct iface"s to be
+ * consistent with the ofp_ports in "br->ofproto". */
+static void
bridge_refresh_ofp_port(struct bridge *br)
{
struct ofproto_port_dump dump;
struct ofproto_port ofproto_port;
- struct port *port;
+ struct port *port, *port_next;
+ struct if_cfg *if_cfg, *if_cfg_next;
+
+ /* Find any "if_cfg"s which already exist in the datapath and promote them
+ * to full fledged "struct iface"s. */
+ LIST_FOR_EACH_SAFE (if_cfg, if_cfg_next, list_node, &br->if_cfg_queue) {
+ if (!ofproto_port_query_by_name(br->ofproto, if_cfg->cfg->name,
+ &ofproto_port)) {
+ struct iface *iface = iface_from_if_cfg(br, if_cfg);
+
+ iface_set_ofp_port(iface, ofproto_port.ofp_port);
+ iface_init(iface);
- /* Clear all the "ofp_port"es. */
+ ofproto_port_destroy(&ofproto_port);
+ list_remove(&if_cfg->list_node);
+ free(if_cfg);
+ }
+ }
+
+ /* Clear each "struct iface"s ofp_port so we can get it's correct value. */
hmap_clear(&br->ifaces);
HMAP_FOR_EACH (port, hmap_node, &br->ports) {
struct iface *iface;
@@ -1162,149 +1213,163 @@ bridge_refresh_ofp_port(struct bridge *br)
} else if (iface_from_ofp_port(br, ofproto_port.ofp_port)) {
VLOG_WARN("bridge %s: interface %"PRIu16" reported twice",
br->name, ofproto_port.ofp_port);
- } else {
+ } else if (!strcmp(ofproto_port.type, iface->type)) {
iface_set_ofp_port(iface, ofproto_port.ofp_port);
+ } else {
+ /* Port has incorrect type so delete it later. */
+ }
+ } else if (bridge_has_bond_fake_iface(br, ofproto_port.name)
+ && strcmp(ofproto_port.type, "internal")) {
+ /* Bond fake iface with the wrong type. */
+ bridge_ofproto_port_del(br, ofproto_port);
+ }
+ }
+
+ /* Some ifaces may not have "ofp_port"s in ofproto and therefore don't
+ * deserve to have "struct iface"s. Demote these to "if_cfg"s so that
+ * later they can be added to ofproto. */
+ HMAP_FOR_EACH_SAFE (port, port_next, hmap_node, &br->ports) {
+ struct iface *iface, *iface_next;
+
+ LIST_FOR_EACH_SAFE (iface, iface_next, port_elem, &port->ifaces) {
+ if (iface->ofp_port < 0) {
+ bridge_queue_if_cfg(br, iface->cfg, port->cfg);
+ iface_destroy(iface);
}
}
+
+ if (list_is_empty(&port->ifaces)) {
+ port_destroy(port);
+ }
}
}
-/* 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,
- long long int *port_action_timer)
+bridge_populate_ofpp_death_row(struct bridge *br)
{
- struct port *port, *next_port;
+ struct ofproto_port_dump dump;
+ 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;
+ OFPROTO_PORT_FOR_EACH (&ofproto_port, &dump, br->ofproto) {
+ if (!iface_lookup(br, ofproto_port.name)) {
+ struct ofpp_inmate *inmate = xmalloc(sizeof *inmate);
+ inmate->ofp_port = ofproto_port.ofp_port;
+ list_push_front(&br->ofpp_death_row, &inmate->list_node);
+ }
+ }
+}
- LIST_FOR_EACH_SAFE (iface, next_iface, port_elem, &port->ifaces) {
- int error;
+/* Initialize 'iface' with a netdev and ofp_port if necessary. */
+static void
+iface_init(struct iface *iface)
+{
+ struct port *port = iface->port;
+ struct bridge *br = port->bridge;
+ struct ofproto_port ofproto_port;
+ int error;
- if (iface->ofp_port < 0 && !may_port_action(port_action_timer)) {
- iface_clear_db_record(iface->cfg);
- iface_destroy(iface);
- continue;
- }
+ assert(!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));
+ }
- /* Open the netdev. */
- if (!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));
- }
+ if (iface->netdev
+ && port->cfg->vlan_mode
+ && !strcmp(port->cfg->vlan_mode, "splinter")) {
+ netdev_turn_flags_on(iface->netdev, NETDEV_UP, true);
+ }
- if (iface->netdev
- && port->cfg->vlan_mode
- && !strcmp(port->cfg->vlan_mode, "splinter")) {
- netdev_turn_flags_on(iface->netdev, NETDEV_UP, true);
- }
- } else {
- error = 0;
- }
+ /* Configure the netdev. */
+ if (iface->netdev) {
+ struct shash args;
- /* 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;
- }
- }
+ 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);
- /* Add the port, if necessary. */
- 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) {
- VLOG_INFO("bridge %s: added interface %s (%d)", br->name,
- iface->name, ofp_port);
- iface_set_ofp_port(iface, ofp_port);
- } else {
- netdev_close(iface->netdev);
- iface->netdev = NULL;
- }
- }
+ if (error) {
+ VLOG_WARN("could not configure network device %s (%s)",
+ iface->name, strerror(error));
+ netdev_close(iface->netdev);
+ iface->netdev = NULL;
+ }
+ }
- /* Populate stats columns in new Interface rows. */
- if (iface->netdev && iface->need_refresh) {
- iface_refresh_stats(iface);
- iface_refresh_status(iface);
- iface->need_refresh = false;
- }
+ /* Add the port, if necessary. */
+ if (iface->netdev && iface->ofp_port < 0) {
+ uint16_t ofp_port;
+ int error;
- /* 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->ofp_port);
- } else {
- if (iface->netdev) {
- VLOG_ERR("bridge %s: missing %s interface, dropping",
- br->name, iface->name);
- } else {
- /* We already reported a related error, don't bother
- * duplicating it. */
- }
- if (!ofproto_port_query_by_name(br->ofproto, port->name,
- &ofproto_port)) {
- VLOG_INFO("bridge %s: removed interface %s (%d)",
- br->name, port->name, ofproto_port.ofp_port);
- ofproto_port_del(br->ofproto, ofproto_port.ofp_port);
- ofproto_port_destroy(&ofproto_port);
- }
- iface_clear_db_record(iface->cfg);
- iface_destroy(iface);
- }
+ error = ofproto_port_add(br->ofproto, iface->netdev,
+ &ofp_port);
+ if (!error) {
+ VLOG_INFO("bridge %s: added interface %s (%d)", br->name,
+ iface->name, ofp_port);
+ iface_set_ofp_port(iface, ofp_port);
+ } else {
+ netdev_close(iface->netdev);
+ iface->netdev = NULL;
}
+ }
- if (list_is_empty(&port->ifaces)) {
- if (!need_reconfigure) {
- VLOG_WARN("%s port has no interfaces, dropping", port->name);
- }
- port_destroy(port);
- continue;
+ /* Populate stats columns in new Interface rows. */
+ if (iface->netdev && iface->need_refresh) {
+ iface_refresh_stats(iface);
+ iface_refresh_status(iface);
+ iface->need_refresh = false;
+ }
+
+ /* 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->ofp_port);
+ } else {
+ if (iface->netdev) {
+ VLOG_ERR("bridge %s: missing %s interface, dropping",
+ br->name, iface->name);
+ } else {
+ /* We already reported a related error, don't bother
+ * duplicating it. */
}
+ if (!ofproto_port_query_by_name(br->ofproto, port->name,
+ &ofproto_port)) {
+ VLOG_INFO("bridge %s: removed interface %s (%d)",
+ br->name, port->name, ofproto_port.ofp_port);
+ bridge_ofproto_port_del(br, ofproto_port);
+ ofproto_port_destroy(&ofproto_port);
+ }
+ iface_clear_db_record(iface->cfg);
+ iface_destroy(iface);
+ }
- /* Add bond fake iface if necessary. */
- if (port_is_bond_fake_iface(port)) {
- if (ofproto_port_query_by_name(br->ofproto, port->name,
- &ofproto_port)) {
- struct netdev *netdev;
- int error;
-
- error = netdev_open(port->name, "internal", &netdev);
- if (!error) {
- /* There are unlikely to be a great number of fake
- * interfaces so we don't bother rate limiting their
- * creation. */
- ofproto_port_add(br->ofproto, netdev, NULL);
- netdev_close(netdev);
- } else {
- VLOG_WARN("could not open network device %s (%s)",
- port->name, strerror(error));
- }
+ if (list_is_empty(&port->ifaces)) {
+ port_destroy(port);
+ return;
+ }
+
+ /* Add bond fake iface if necessary. */
+ if (port_is_bond_fake_iface(port)) {
+ if (ofproto_port_query_by_name(br->ofproto, port->name,
+ &ofproto_port)) {
+ struct netdev *netdev;
+ int error;
+
+ error = netdev_open(port->name, "internal", &netdev);
+ if (!error) {
+ ofproto_port_add(br->ofproto, netdev, NULL);
+ netdev_close(netdev);
} else {
- /* Already exists, nothing to do. */
- ofproto_port_destroy(&ofproto_port);
+ VLOG_WARN("could not open network device %s (%s)",
+ port->name, strerror(error));
}
+ } else {
+ /* Already exists, nothing to do. */
+ ofproto_port_destroy(&ofproto_port);
}
}
}
@@ -1929,26 +1994,30 @@ bridge_run_fast(void)
void
bridge_run(void)
{
+ static const struct ovsrec_open_vswitch null_cfg;
const struct ovsrec_open_vswitch *cfg;
bool vlan_splinters_changed;
struct bridge *br;
/* (Re)configure if necessary. */
- 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;
+ if (!reconfiguring) {
+ ovsdb_idl_run(idl);
- VLOG_ERR_RL(&rl, "another ovs-vswitchd process is running, "
- "disabling this process until it goes away");
+ if (ovsdb_idl_is_lock_contended(idl)) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+ struct bridge *br, *next_br;
- HMAP_FOR_EACH_SAFE (br, next_br, node, &all_bridges) {
- bridge_destroy(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;
}
- return;
- } else if (!ovsdb_idl_has_lock(idl)) {
- return;
}
cfg = ovsrec_open_vswitch_first(idl);
@@ -1982,26 +2051,37 @@ bridge_run(void)
}
}
- if (need_reconfigure || ovsdb_idl_get_seqno(idl) != idl_seqno
- || vlan_splinters_changed) {
+ if (ovsdb_idl_get_seqno(idl) != idl_seqno || vlan_splinters_changed) {
idl_seqno = ovsdb_idl_get_seqno(idl);
if (cfg) {
struct ovsdb_idl_txn *txn = ovsdb_idl_txn_create(idl);
bridge_reconfigure(cfg);
- ovsrec_open_vswitch_set_cur_cfg(cfg, cfg->next_cfg);
ovsdb_idl_txn_commit(txn);
ovsdb_idl_txn_destroy(txn); /* XXX */
} else {
/* We still need to reconfigure to avoid dangling pointers to
* now-destroyed ovsrec structures inside bridge data. */
- static const struct ovsrec_open_vswitch null_cfg;
-
bridge_reconfigure(&null_cfg);
}
}
+ if (reconfiguring) {
+ struct ovsdb_idl_txn *txn = ovsdb_idl_txn_create(idl);
+
+ if (cfg) {
+ if (bridge_reconfigure_continue(cfg)) {
+ ovsrec_open_vswitch_set_cur_cfg(cfg, cfg->next_cfg);
+ }
+ } else {
+ bridge_reconfigure_continue(&null_cfg);
+ }
+
+ ovsdb_idl_txn_commit(txn);
+ ovsdb_idl_txn_destroy(txn); /* XXX */
+ }
+
/* Refresh system and interface stats if necessary. */
if (time_msec() >= stats_timer) {
if (cfg) {
@@ -2089,7 +2169,7 @@ bridge_wait(void)
{
ovsdb_idl_wait(idl);
- if (need_reconfigure) {
+ if (reconfiguring) {
poll_immediate_wake();
}
@@ -2223,6 +2303,9 @@ bridge_create(const struct ovsrec_bridge *br_cfg)
hmap_init(&br->iface_by_name);
hmap_init(&br->mirrors);
+ list_init(&br->if_cfg_queue);
+ list_init(&br->ofpp_death_row);
+
hmap_insert(&all_bridges, &br->node, hash_string(br->name, 0));
}
@@ -2232,6 +2315,8 @@ bridge_destroy(struct bridge *br)
if (br) {
struct mirror *mirror, *next_mirror;
struct port *port, *next_port;
+ struct if_cfg *if_cfg, *next_if_cfg;
+ struct ofpp_inmate *inmate, *next_inmate;
HMAP_FOR_EACH_SAFE (port, next_port, hmap_node, &br->ports) {
port_destroy(port);
@@ -2239,6 +2324,19 @@ bridge_destroy(struct bridge *br)
HMAP_FOR_EACH_SAFE (mirror, next_mirror, hmap_node, &br->mirrors) {
mirror_destroy(mirror);
}
+
+ LIST_FOR_EACH_SAFE (inmate, next_inmate, list_node,
+ &br->ofpp_death_row) {
+ list_remove(&inmate->list_node);
+ free(inmate);
+ }
+
+ LIST_FOR_EACH_SAFE (if_cfg, next_if_cfg, list_node,
+ &br->if_cfg_queue) {
+ list_remove(&if_cfg->list_node);
+ free(if_cfg);
+ }
+
hmap_remove(&all_bridges, &br->node);
ofproto_destroy(br->ofproto);
hmap_destroy(&br->ifaces);
@@ -2330,17 +2428,54 @@ bridge_get_controllers(const struct bridge *br,
return n_controllers;
}
-/* Adds and deletes "struct port"s and "struct iface"s under 'br' to match
- * those configured in 'br->cfg'. */
+static void
+bridge_queue_if_cfg(struct bridge *br,
+ const struct ovsrec_interface *cfg,
+ const struct ovsrec_port *parent)
+{
+ struct if_cfg *if_cfg = xmalloc(sizeof *if_cfg);
+
+ if_cfg->cfg = cfg;
+ if_cfg->parent = parent;
+ list_insert(&br->if_cfg_queue, &if_cfg->list_node);
+}
+
+static void
+bridge_update_ifaces(struct bridge *br, struct shash *new_ports)
+{
+ struct shash_node *port_node;
+
+ SHASH_FOR_EACH (port_node, new_ports) {
+ const struct ovsrec_port *port = port_node->data;
+ size_t i;
+
+ for (i = 0; i < port->n_interfaces; i++) {
+ const struct ovsrec_interface *cfg = port->interfaces[i];
+ struct iface *iface = iface_lookup(br, cfg->name);
+
+ if (iface) {
+ iface->cfg = cfg;
+ iface_refresh_type(iface);
+ } else {
+ bridge_queue_if_cfg(br, cfg, port);
+ }
+ }
+ }
+}
+
+/* Deletes "struct port"s and "struct iface"s under 'br' which aren't
+ * consistent with 'br->cfg'. Updates 'br->if_cfg_queue' with interfaces which
+ * 'br' needs to complete it's configuration. */
static void
bridge_add_del_ports(struct bridge *br,
const unsigned long int *splinter_vlans)
{
struct port *port, *next;
- struct shash_node *node;
struct shash new_ports;
size_t i;
+ assert(list_is_empty(&br->if_cfg_queue));
+
/* Collect new ports. */
shash_init(&new_ports);
for (i = 0; i < br->cfg->n_ports; i++) {
@@ -2382,21 +2517,8 @@ bridge_add_del_ports(struct bridge *br,
}
}
- /* Create new ports.
- * Add new interfaces to existing ports. */
- SHASH_FOR_EACH (node, &new_ports) {
- struct port *port = port_lookup(br, node->name);
- if (!port) {
- struct ovsrec_port *cfg = node->data;
- port = port_create(br, cfg);
- }
- port_add_ifaces(port);
- if (list_is_empty(&port->ifaces)) {
- VLOG_WARN("bridge %s: port %s has no interfaces, dropping",
- br->name, port->name);
- port_destroy(port);
- }
- }
+ bridge_update_ifaces(br, &new_ports);
+
shash_destroy(&new_ports);
}
@@ -2718,51 +2840,6 @@ port_del_ifaces(struct port *port)
sset_destroy(&new_ifaces);
}
-/* Adds new interfaces to 'port' and updates 'type' and 'cfg' members of
- * existing ones. */
-static void
-port_add_ifaces(struct port *port)
-{
- struct shash new_ifaces;
- struct shash_node *node;
- size_t i;
-
- /* Collect new ifaces. */
- shash_init(&new_ifaces);
- for (i = 0; i < port->cfg->n_interfaces; i++) {
- const struct ovsrec_interface *cfg = port->cfg->interfaces[i];
- 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_clear_db_record(cfg);
- }
- }
-
- /* Create new interfaces.
- * Update interface types and 'cfg' members. */
- SHASH_FOR_EACH (node, &new_ifaces) {
- const struct ovsrec_interface *cfg = node->data;
- const char *iface_name = node->name;
- struct iface *iface;
-
- iface = iface_lookup(port->bridge, iface_name);
- if (!iface) {
- iface = iface_create(port, cfg);
- } else {
- iface->cfg = cfg;
- }
-
- /* Determine interface type. The local port always has type
- * "internal". Other ports take their type from the database and
- * default to "system" if none is specified. */
- iface->type = (!strcmp(iface_name, port->bridge->name) ? "internal"
- : cfg->type[0] ? cfg->type
- : "system");
- }
- shash_destroy(&new_ifaces);
-}
-
static void
port_destroy(struct port *port)
{
@@ -3016,6 +3093,35 @@ iface_create(struct port *port, const struct
ovsrec_interface *if_cfg)
}
static void
+iface_refresh_type(struct iface *iface)
+{
+ /* Determine interface type. The local port always has type
+ * "internal". Other ports take their type from the database and
+ * default to "system" if none is specified. */
+ iface->type = (!strcmp(iface->name, iface->port->bridge->name) ? "internal"
+ : iface->cfg->type[0] ? iface->cfg->type
+ : "system");
+}
+
+static struct iface *
+iface_from_if_cfg(struct bridge *br, struct if_cfg *if_cfg)
+{
+ struct iface *iface;
+ struct port *port;
+
+ port = port_lookup(br, if_cfg->parent->name);
+ if (!port) {
+ port = port_create(br, if_cfg->parent);
+ }
+
+ assert(!iface_lookup(br, if_cfg->cfg->name));
+ iface = iface_create(port, if_cfg->cfg);
+ iface_refresh_type(iface);
+
+ return iface;
+}
+
+static void
iface_destroy(struct iface *iface)
{
if (iface) {
--
1.7.9.6
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
