[ovs-dev] [PATCH] bridge: Rate limit port creations and deletions.
Ethan Jackson
ethan at nicira.com
Wed Apr 11 07:10:13 UTC 2012
In some datapaths, adding or deleting OpenFlow ports can take quite
a bit of time. If there are lots of OpenFlow ports which needed to
be added in a run loop, this can cause Open vSwitch to lock up and
stop setting up flows while trying to catch up. This patch lessons
the severity of the problem by only doing a few OpenFlow port adds
or deletions per run loop allowing other work to be done in
between.
Bug #10672.
Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
vswitchd/bridge.c | 66 +++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 54 insertions(+), 12 deletions(-)
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index bbfa116..18e30ec 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -140,6 +140,17 @@ static long long int stats_timer = LLONG_MIN;
#define DB_LIMIT_INTERVAL (1 * 1000) /* In milliseconds. */
static long long int db_limiter = LLONG_MIN;
+/* In some datapaths, creating and destroying OpenFlow ports can be extremely
+ * expensive. This can cause bridge_reconfigure() to take a long time during
+ * which no other work can be done. To deal with this problem, we limit port
+ * adds and deletions to at most MAX_OFP_PORT_ACTIONS per run loop. If there
+ * is more work to do after the limit is reached, 'need_reconfigure', is
+ * flagged and it's done on the next loop. This allows the rest of the code to
+ * catch up on important things like forwarding packets. */
+#define MAX_OFP_PORT_ACTIONS 5
+static bool need_reconfigure = false;
+static int ofp_port_action_count = 0;
+
static void add_del_bridges(const struct ovsrec_open_vswitch *);
static void bridge_del_ofprotos(void);
static bool bridge_add_ofprotos(struct bridge *);
@@ -398,6 +409,9 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
COVERAGE_INC(bridge_reconfigure);
+ ofp_port_action_count = 0;
+ need_reconfigure = false;
+
/* Create and destroy "struct bridge"s, "struct port"s, and "struct
* iface"s according to 'ovs_cfg', with only very minimal configuration
* otherwise.
@@ -477,6 +491,10 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
}
free(managers);
+ if (ofp_port_action_count >= MAX_OFP_PORT_ACTIONS) {
+ need_reconfigure = true;
+ }
+
/* ovs-vswitchd has completed initialization, so allow the process that
* forked us to exit successfully. */
daemonize_complete();
@@ -1066,10 +1084,17 @@ bridge_del_ofproto_ports(struct bridge *br)
|| !strcmp(netdev_get_type(iface->netdev), type))) {
continue;
}
- 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));
+ if (ofp_port_action_count < MAX_OFP_PORT_ACTIONS) {
+ ofp_port_action_count++;
+
+ 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("Destroyed OpenFlow Port %d.",
+ ofproto_port.ofp_port);
+ }
}
if (iface) {
netdev_close(iface->netdev);
@@ -1138,6 +1163,13 @@ bridge_add_ofproto_ports(struct bridge *br)
LIST_FOR_EACH_SAFE (iface, next_iface, port_elem, &port->ifaces) {
int error;
+ if (ofp_port_action_count >= MAX_OFP_PORT_ACTIONS
+ && iface->ofp_port < 0) {
+ iface_clear_db_record(iface->cfg);
+ iface_destroy(iface);
+ continue;
+ }
+
/* Open the netdev. */
if (!iface->netdev) {
error = netdev_open(iface->name, iface->type, &iface->netdev);
@@ -1179,9 +1211,12 @@ bridge_add_ofproto_ports(struct bridge *br)
uint16_t ofp_port;
int error;
+ ofp_port_action_count++;
error = ofproto_port_add(br->ofproto, iface->netdev,
&ofp_port);
if (!error) {
+ VLOG_INFO("Created OpenFlow port %d named %s.", ofp_port,
+ iface->name);
iface_set_ofp_port(iface, ofp_port);
} else {
netdev_close(iface->netdev);
@@ -1210,6 +1245,9 @@ bridge_add_ofproto_ports(struct bridge *br)
}
if (!ofproto_port_query_by_name(br->ofproto, port->name,
&ofproto_port)) {
+ ofp_port_action_count++;
+ VLOG_INFO("Destroyed OpenFlow Port %d.",
+ ofproto_port.ofp_port);
ofproto_port_del(br->ofproto, ofproto_port.ofp_port);
ofproto_port_destroy(&ofproto_port);
}
@@ -1218,7 +1256,9 @@ bridge_add_ofproto_ports(struct bridge *br)
}
}
if (list_is_empty(&port->ifaces)) {
- VLOG_WARN("%s port has no interfaces, dropping", port->name);
+ if (ofp_port_action_count < MAX_OFP_PORT_ACTIONS) {
+ VLOG_WARN("%s port has no interfaces, dropping", port->name);
+ }
port_destroy(port);
continue;
}
@@ -1232,6 +1272,9 @@ bridge_add_ofproto_ports(struct bridge *br)
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 {
@@ -1920,7 +1963,7 @@ bridge_run(void)
}
}
- if (database_changed || vlan_splinters_changed) {
+ if (need_reconfigure || database_changed || vlan_splinters_changed) {
if (cfg) {
struct ovsdb_idl_txn *txn = ovsdb_idl_txn_create(idl);
@@ -2024,6 +2067,11 @@ void
bridge_wait(void)
{
ovsdb_idl_wait(idl);
+
+ if (need_reconfigure) {
+ poll_immediate_wake();
+ }
+
if (!hmap_is_empty(&all_bridges)) {
struct bridge *br;
@@ -2614,9 +2662,6 @@ port_create(struct bridge *br, const struct ovsrec_port *cfg)
list_init(&port->ifaces);
hmap_insert(&br->ports, &port->hmap_node, hash_string(port->name, 0));
-
- VLOG_INFO("created port %s on bridge %s", port->name, br->name);
-
return port;
}
@@ -2709,9 +2754,6 @@ port_destroy(struct port *port)
}
hmap_remove(&br->ports, &port->hmap_node);
-
- VLOG_INFO("destroyed port %s on bridge %s", port->name, br->name);
-
free(port->name);
free(port);
}
--
1.7.9.6
More information about the dev
mailing list