This is cleaner than using the need_reconfigure flag to guarantee at
least one port modification so I've folded it in.
I'll merge this when I've had a chance to test it.
Ethan
On Thu, Apr 12, 2012 at 16:31, Ben Pfaff <b...@nicira.com> wrote:
> 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
