On Wed, Apr 11, 2012 at 01:04:37PM -0700, Ethan Jackson wrote:
> 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 <et...@nicira.com>
> ---
>
> I've tested this version on my virtual machine, but still need to try it on a
> real server before merging it. I don't expect it to need to change so I'm
> sending it out for review now.
It looks OK to me.
I couldn't resist the urge to tinker with the details, so here's an
incremental. Feel free to take it or leave it as you like.
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 314fc71..57a1b89 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -167,9 +167,9 @@ static size_t bridge_get_controllers(const struct bridge
*br,
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);
+ long long int *port_action_timer);
static void bridge_del_ofproto_ports(struct bridge *,
- long long int port_action_timer);
+ 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 *);
@@ -416,8 +416,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
*ovs_cfg)
COVERAGE_INC(bridge_reconfigure);
- time_refresh();
- port_action_timer = time_msec() + OFP_PORT_ACTION_WINDOW;
+ port_action_timer = LLONG_MAX;
need_reconfigure = false;
/* Create and destroy "struct bridge"s, "struct port"s, and "struct
@@ -442,7 +441,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
*ovs_cfg)
bridge_del_ofprotos();
HMAP_FOR_EACH (br, node, &all_bridges) {
if (br->ofproto) {
- bridge_del_ofproto_ports(br, port_action_timer);
+ bridge_del_ofproto_ports(br, &port_action_timer);
}
}
@@ -455,7 +454,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
*ovs_cfg)
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);
+ bridge_del_ofproto_ports(br, &port_action_timer);
} else {
bridge_destroy(br);
}
@@ -463,7 +462,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
*ovs_cfg)
}
HMAP_FOR_EACH (br, node, &all_bridges) {
bridge_refresh_ofp_port(br);
- bridge_add_ofproto_ports(br, port_action_timer);
+ bridge_add_ofproto_ports(br, &port_action_timer);
}
/* Complete the configuration. */
@@ -1053,23 +1052,20 @@ add_del_bridges(const struct ovsrec_open_vswitch *cfg)
}
static bool
-may_port_action(long long int port_action_timer)
+may_port_action(long long int *port_action_timer)
{
- if (time_msec() < port_action_timer) {
- time_refresh();
- }
-
- if (time_msec() < port_action_timer) {
- return true;
+ if (need_reconfigure) {
+ return false;
}
- /* Guarantee at least one port is acted on per run loop. */
- if (!need_reconfigure) {
+ 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 true;
+ return false;
}
-
- return false;
+ return true;
}
/* Delete each ofproto port on 'br' that doesn't have a corresponding "struct
@@ -1080,7 +1076,7 @@ may_port_action(long long int port_action_timer)
* deletions before any port additions. */
static void
bridge_del_ofproto_ports(struct bridge *br,
- long long int port_action_timer)
+ long long int *port_action_timer)
{
struct ofproto_port_dump dump;
struct ofproto_port ofproto_port;
@@ -1179,7 +1175,7 @@ bridge_refresh_ofp_port(struct bridge *br)
* 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)
+ long long int *port_action_timer)
{
struct port *port, *next_port;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
