From ef9674ce751b9da5c2b0b0d407359760fdcce239 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sat, 4 Mar 2006 06:44:05 +0000 Subject: [PATCH] 2006-03-04 Dan Williams Clean up activation cancellation. Should be a lot faster now. Observed an issue with wireless devices between stage 2 and 3 of activation, where activation would be cancelled, but the device thread wouldn't notice until the supplicant association timed out. Reorganize activation such that a cancellation handler gets immediately scheduled in the device's thread, and devices have a chance to perform any custom cleanup too. * src/nm-device.[ch] - (activation_cancel_handler): new device-type-specific function for cleaning up device-type-specific stuff on cancellation - (cancel_activation): removed - (nm_device_activation_cancel): subsume functionality of real_cancel_activation, but instead of doing anything, punt operation to a handler that's run in device-thread context - (nm_device_schedule_activation_handle_cancel): fix spelling of a warning message - (activation_handle_cancel_helper): cancellation handler run in device-thread context, calls device-type-specific cancelation, then tears down the activation request - (real_activation_cancel_handler): generic cancellation handler, deals with cancelling any in-process DHCP request - (nm_device_activate_stage1_device_prepare, nm_device_activate_stage2_device_config, nm_device_activate_stage3_ip_config_start, nm_device_activate_stage4_ip_config_get, nm_device_activate_stage4_ip_config_timeout, nm_device_activate_stage5_ip_commit): don't call nm_device_schedule_activation_handle_cancel() any more, since cancellation will have been already scheduled for us by nm_device_activation_cancel(). Just exit the function and assume that the cancel handler will be called next. * src/nm-device-802-3-ethernet.c - (real_act_stage2_config): remove; didn't do anything anyway * src/nm-device-802-11-wireless.c - (supplicant_status_cb): ensure we don't do anything if the activation got cancelled - (real_activation_cancel_handler): implement; cancel user key request on activation cancellation git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@1549 4912f4e0-d625-0410-9fb7-b9a5a253dbdc --- ChangeLog | 43 ++++++++++++ src/nm-device-802-11-wireless.c | 28 ++++++++ src/nm-device-802-3-ethernet.c | 15 ----- src/nm-device.c | 116 ++++++++++---------------------- src/nm-device.h | 3 +- 5 files changed, 107 insertions(+), 98 deletions(-) diff --git a/ChangeLog b/ChangeLog index f11db60ad5..42db58e547 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,46 @@ +2006-03-04 Dan Williams + + Clean up activation cancellation. Should be a lot faster now. Observed + an issue with wireless devices between stage 2 and 3 of activation, where + activation would be cancelled, but the device thread wouldn't notice until + the supplicant association timed out. Reorganize activation such that + a cancellation handler gets immediately scheduled in the device's thread, + and devices have a chance to perform any custom cleanup too. + + * src/nm-device.[ch] + - (activation_cancel_handler): new device-type-specific function + for cleaning up device-type-specific stuff on cancellation + - (cancel_activation): removed + - (nm_device_activation_cancel): subsume functionality of + real_cancel_activation, but instead of doing anything, punt + operation to a handler that's run in device-thread context + - (nm_device_schedule_activation_handle_cancel): fix spelling of + a warning message + - (activation_handle_cancel_helper): cancellation handler run in + device-thread context, calls device-type-specific cancelation, + then tears down the activation request + - (real_activation_cancel_handler): generic cancellation handler, + deals with cancelling any in-process DHCP request + - (nm_device_activate_stage1_device_prepare, + nm_device_activate_stage2_device_config, + nm_device_activate_stage3_ip_config_start, + nm_device_activate_stage4_ip_config_get, + nm_device_activate_stage4_ip_config_timeout, + nm_device_activate_stage5_ip_commit): don't call + nm_device_schedule_activation_handle_cancel() any more, since + cancellation will have been already scheduled for us by + nm_device_activation_cancel(). Just exit the function and + assume that the cancel handler will be called next. + + * src/nm-device-802-3-ethernet.c + - (real_act_stage2_config): remove; didn't do anything anyway + + * src/nm-device-802-11-wireless.c + - (supplicant_status_cb): ensure we don't do anything if the activation + got cancelled + - (real_activation_cancel_handler): implement; cancel user key request + on activation cancellation + 2006-03-04 Dan Williams * src/nm-device-802-11-wireless.c diff --git a/src/nm-device-802-11-wireless.c b/src/nm-device-802-11-wireless.c index aa65967900..79f0dbc085 100644 --- a/src/nm-device-802-11-wireless.c +++ b/src/nm-device-802-11-wireless.c @@ -2192,6 +2192,12 @@ supplicant_status_cb (GIOChannel *source, g_assert (self); + /* Do nothing if we're supposed to be canceling activation. + * We'll get cleaned up by the cancellation handlers later. + */ + if (nm_device_activation_should_cancel (dev)) + return TRUE; + ctrl = self->priv->supplicant.ctrl; g_return_val_if_fail (ctrl != NULL, FALSE); @@ -2830,6 +2836,27 @@ real_activation_failure_handler (NMDevice *dev, ap ? nm_ap_get_essid (ap) : "(none)"); } +static void +real_activation_cancel_handler (NMDevice *dev, + NMActRequest *req) +{ + NMDevice80211Wireless * self = NM_DEVICE_802_11_WIRELESS (dev); + NMDevice80211WirelessClass * klass; + NMDeviceClass * parent_class; + + /* Chain up to parent first */ + klass = NM_DEVICE_802_11_WIRELESS_GET_CLASS (self); + parent_class = NM_DEVICE_CLASS (g_type_class_peek_parent (klass)); + parent_class->activation_cancel_handler (dev, req); + + if (nm_act_request_get_stage (req) == NM_ACT_STAGE_NEED_USER_KEY) + { + NMData *data = nm_device_get_app_data (dev); + nm_dbus_cancel_get_user_key_for_network (data->dbus_connection, req); + } +} + + static gboolean real_can_interrupt_activation (NMDevice *dev) { @@ -2922,6 +2949,7 @@ nm_device_802_11_wireless_class_init (NMDevice80211WirelessClass *klass) parent_class->activation_failure_handler = real_activation_failure_handler; parent_class->activation_success_handler = real_activation_success_handler; + parent_class->activation_cancel_handler = real_activation_cancel_handler; g_type_class_add_private (object_class, sizeof (NMDevice80211WirelessPrivate)); } diff --git a/src/nm-device-802-3-ethernet.c b/src/nm-device-802-3-ethernet.c index 3355646584..e57c481a04 100644 --- a/src/nm-device-802-3-ethernet.c +++ b/src/nm-device-802-3-ethernet.c @@ -213,19 +213,6 @@ real_get_generic_capabilities (NMDevice *dev) return caps; } -static NMActStageReturn -real_act_stage2_config (NMDevice *dev, NMActRequest *req) -{ - NMData * data; - - g_assert (req); - data = nm_act_request_get_data (req); - g_assert (data); - - return TRUE; -} - - static void nm_device_802_3_ethernet_dispose (GObject *object) { @@ -280,8 +267,6 @@ nm_device_802_3_ethernet_class_init (NMDevice8023EthernetClass *klass) parent_class->start = real_start; parent_class->update_link = real_update_link; - parent_class->act_stage2_config = real_act_stage2_config; - g_type_class_add_private (object_class, sizeof (NMDevice8023EthernetPrivate)); } diff --git a/src/nm-device.c b/src/nm-device.c index d2f9152921..77eeb2db43 100644 --- a/src/nm-device.c +++ b/src/nm-device.c @@ -75,9 +75,6 @@ static gpointer nm_device_worker (gpointer user_data); static void nm_device_activate_schedule_stage5_ip_config_commit (NMActRequest *req); -static void nm_device_schedule_activation_handle_cancel (NMActRequest *req); - - /* * nm_device_test_wireless_extensions * @@ -678,10 +675,7 @@ nm_device_activate_stage1_device_prepare (NMActRequest *req) g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); if (nm_device_activation_should_cancel (self)) - { - nm_device_schedule_activation_handle_cancel (req); goto out; - } nm_device_activate_schedule_stage2_device_config (req); @@ -762,10 +756,7 @@ nm_device_activate_stage2_device_config (NMActRequest *req) nm_device_bring_up (self); if (nm_device_activation_should_cancel (self)) - { - nm_device_schedule_activation_handle_cancel (req); goto out; - } ret = NM_DEVICE_GET_CLASS (self)->act_stage2_config (self, req); if (ret == NM_ACT_STAGE_RETURN_POSTPONE) @@ -780,10 +771,7 @@ nm_device_activate_stage2_device_config (NMActRequest *req) nm_info ("Activation (%s) Stage 2 of 5 (Device Configure) successful.", iface); if (nm_device_activation_should_cancel (self)) - { - nm_device_schedule_activation_handle_cancel (req); goto out; - } nm_device_activate_schedule_stage3_ip_config_start (req); @@ -873,10 +861,7 @@ nm_device_activate_stage3_ip_config_start (NMActRequest *req) nm_info ("Activation (%s) Stage 3 of 5 (IP Configure Start) started...", iface); if (nm_device_activation_should_cancel (self)) - { - nm_device_schedule_activation_handle_cancel (req); goto out; - } ret = NM_DEVICE_GET_CLASS (self)->act_stage3_ip_config_start (self, req); if (ret == NM_ACT_STAGE_RETURN_POSTPONE) @@ -889,10 +874,7 @@ nm_device_activate_stage3_ip_config_start (NMActRequest *req) g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); if (nm_device_activation_should_cancel (self)) - { - nm_device_schedule_activation_handle_cancel (req); goto out; - } nm_device_activate_schedule_stage4_ip_config_get (req); @@ -1019,18 +1001,12 @@ nm_device_activate_stage4_ip_config_get (NMActRequest *req) nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Get) started...", nm_device_get_iface (self)); if (nm_device_activation_should_cancel (self)) - { - nm_device_schedule_activation_handle_cancel (req); goto out; - } ret = NM_DEVICE_GET_CLASS (self)->act_stage4_get_ip4_config (self, req, &ip4_config); if (nm_device_activation_should_cancel (self)) - { - nm_device_schedule_activation_handle_cancel (req); goto out; - } if (ret == NM_ACT_STAGE_RETURN_POSTPONE) goto out; @@ -1042,10 +1018,7 @@ nm_device_activate_stage4_ip_config_get (NMActRequest *req) g_assert (ret == NM_ACT_STAGE_RETURN_SUCCESS); if (nm_device_activation_should_cancel (self)) - { - nm_device_schedule_activation_handle_cancel (req); goto out; - } nm_act_request_set_ip4_config (req, ip4_config); nm_device_activate_schedule_stage5_ip_config_commit (req); @@ -1129,10 +1102,7 @@ nm_device_activate_stage4_ip_config_timeout (NMActRequest *req) nm_info ("Activation (%s) Stage 4 of 5 (IP Configure Timeout) started...", iface); if (nm_device_activation_should_cancel (self)) - { - nm_device_schedule_activation_handle_cancel (req); goto out; - } ret = NM_DEVICE_GET_CLASS (self)->act_stage4_ip_config_timeout (self, req, &ip4_config); if (ret == NM_ACT_STAGE_RETURN_POSTPONE) @@ -1209,10 +1179,7 @@ nm_device_activate_stage5_ip_config_commit (NMActRequest *req) nm_device_get_iface (self)); if (nm_device_activation_should_cancel (self)) - { - nm_device_schedule_activation_handle_cancel (req); goto out; - } nm_device_set_ip4_config (self, ip4_config); if (nm_system_device_set_from_ip4_config (self)) @@ -1262,27 +1229,39 @@ nm_device_activate_schedule_stage5_ip_config_commit (NMActRequest *req) } +static void +real_activation_cancel_handler (NMDevice *self, + NMActRequest *req) +{ + g_return_if_fail (self != NULL); + g_return_if_fail (req != NULL); + + if (nm_act_request_get_stage (req) == NM_ACT_STAGE_IP_CONFIG_START) + nm_dhcp_manager_cancel_transaction (self->priv->app_data->dhcp_manager, req); +} + /* - * nm_device_activation_handle_cancel + * activation_handle_cancel_helper * - * Cancel activation on a device and clean up. + * Allow specific device types to clean up their own cancellation * */ static gboolean -nm_device_activation_handle_cancel (NMActRequest *req) +activation_handle_cancel_helper (NMActRequest *req) { - NMDevice * self; - NMData * data; + NMDevice * self; + NMDeviceClass *klass; - g_return_val_if_fail (req != NULL, FALSE); - - data = nm_act_request_get_data (req); - g_assert (data); + g_assert (req); self = nm_act_request_get_dev (req); g_assert (self); - if ((req = nm_device_get_act_request (self)) && nm_device_is_activating (self)) + klass = NM_DEVICE_CLASS (g_type_class_peek (NM_TYPE_DEVICE)); + if (klass->activation_cancel_handler) + klass->activation_cancel_handler (self, req); + + if ((req = nm_device_get_act_request (self))) { self->priv->act_request = NULL; nm_act_request_unref (req); @@ -1317,29 +1296,13 @@ nm_device_schedule_activation_handle_cancel (NMActRequest *req) nm_info ("Activation (%s) cancellation handler scheduled...", nm_device_get_iface (self)); source = g_idle_source_new (); - g_source_set_callback (source, (GSourceFunc) nm_device_activation_handle_cancel, req, NULL); + g_source_set_callback (source, (GSourceFunc) activation_handle_cancel_helper, req, NULL); + g_source_set_priority (source, G_PRIORITY_HIGH_IDLE); g_source_attach (source, self->priv->context); g_source_unref (source); } -/* - * nm_device_activation_cancel - * - * Signal activation worker that it should stop and die. - * - */ -void -nm_device_activation_cancel (NMDevice *self) -{ - NMDeviceClass * klass; - - g_return_if_fail (self != NULL); - - klass = NM_DEVICE_CLASS (g_type_class_peek (NM_TYPE_DEVICE)); - klass->cancel_activation (self); -} - static gboolean nm_ac_test (int tries, nm_completion_args args) @@ -1358,8 +1321,14 @@ gboolean nm_ac_test (int tries, return TRUE; } -static void -real_cancel_activation (NMDevice *self) +/* + * nm_device_activation_cancel + * + * Signal activation worker that it should stop and die. + * + */ +void +nm_device_activation_cancel (NMDevice *self) { nm_completion_args args; NMData * app_data; @@ -1372,28 +1341,11 @@ real_cancel_activation (NMDevice *self) if (nm_device_is_activating (self)) { NMActRequest * req = nm_device_get_act_request (self); - gboolean clear_act_request = FALSE; nm_info ("Activation (%s): cancelling...", nm_device_get_iface (self)); self->priv->quit_activation = TRUE; - /* If the device is waiting for DHCP or a user key, force its current request to stop. */ - if (nm_act_request_get_stage (req) == NM_ACT_STAGE_NEED_USER_KEY) - { - nm_dbus_cancel_get_user_key_for_network (app_data->dbus_connection, req); - clear_act_request = TRUE; - } - else if (nm_act_request_get_stage (req) == NM_ACT_STAGE_IP_CONFIG_START) - { - nm_dhcp_manager_cancel_transaction (app_data->dhcp_manager, req); - clear_act_request = TRUE; - } - - if (clear_act_request) - { - self->priv->act_request = NULL; - nm_act_request_unref (req); - } + nm_device_schedule_activation_handle_cancel (req); /* Spin until cancelled. Possible race conditions or deadlocks here. * The other problem with waiting here is that we hold up dbus traffic @@ -1972,7 +1924,7 @@ nm_device_class_init (NMDeviceClass *klass) object_class->finalize = nm_device_finalize; klass->is_test_device = real_is_test_device; - klass->cancel_activation = real_cancel_activation; + klass->activation_cancel_handler = real_activation_cancel_handler; klass->get_type_capabilities = real_get_type_capabilities; klass->get_generic_capabilities = real_get_generic_capabilities; klass->start = real_start; diff --git a/src/nm-device.h b/src/nm-device.h index 1689fe19b5..f413c8800c 100644 --- a/src/nm-device.h +++ b/src/nm-device.h @@ -102,12 +102,13 @@ struct _NMDeviceClass NMIP4Config **config); void (* deactivate) (NMDevice *self); void (* deactivate_quickly) (NMDevice *self); - void (* cancel_activation) (NMDevice *self); void (* activation_failure_handler) (NMDevice *self, struct NMActRequest *req); void (* activation_success_handler) (NMDevice *self, struct NMActRequest *req); + void (* activation_cancel_handler) (NMDevice *self, + struct NMActRequest *req); gboolean (* can_interrupt_activation) (NMDevice *self); };