From 66c45d0fdcbd9e87b5164c5f77f914141cbb9419 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Feb 2017 19:30:21 +0100 Subject: [PATCH 01/13] supplicant: rework nm_supplicant_interface_set_config() to invoke result callback Instead of having a NM_SUPPLICANT_INTERFACE_CONNECTION_ERROR signal to notify about failures during AddNetwork/SelectNetwork, accept a callback to report success/failure. Thereby, rename nm_supplicant_interface_set_config() to nm_supplicant_interface_assoc(). The async callback is guaranteed to: - be invoked exactly once, signalling success or failure - always being invoked asyncronously. The pending request can be (synchronously) cancelled via nm_supplicant_interface_disconnect() or by disposing the interface instance. In those cases the callback will be invoked too, with error code cancelled/disposing. --- src/devices/nm-device-ethernet.c | 68 ++--- src/devices/nm-device-macsec.c | 56 +--- src/devices/wifi/nm-device-wifi.c | 63 +---- src/supplicant/nm-supplicant-interface.c | 312 +++++++++++++---------- src/supplicant/nm-supplicant-interface.h | 13 +- 5 files changed, 236 insertions(+), 276 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 92f9653c89..9a2fd8c2f8 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -71,7 +71,6 @@ typedef struct Supplicant { NMSupplicantInterface *iface; /* signal handler ids */ - gulong iface_error_id; gulong iface_state_id; /* Timeouts and idles */ @@ -423,22 +422,12 @@ check_connection_compatible (NMDevice *device, NMConnection *connection) /* 802.1X */ static void -supplicant_interface_clear_handlers (NMDeviceEthernet *self) +supplicant_interface_release (NMDeviceEthernet *self) { NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); nm_clear_g_source (&priv->supplicant_timeout_id); nm_clear_g_source (&priv->supplicant.con_timeout_id); - nm_clear_g_signal_handler (priv->supplicant.iface, &priv->supplicant.iface_error_id); -} - -static void -supplicant_interface_release (NMDeviceEthernet *self) -{ - NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); - - supplicant_interface_clear_handlers (self); - nm_clear_g_signal_handler (priv->supplicant.iface, &priv->supplicant.iface_state_id); if (priv->supplicant.iface) { @@ -594,6 +583,21 @@ build_supplicant_config (NMDeviceEthernet *self, return config; } +static void +supplicant_iface_assoc_cb (NMSupplicantInterface *iface, + GError *error, + gpointer user_data) +{ + NMDeviceEthernet *self = NM_DEVICE_ETHERNET (user_data); + + if (error && !nm_utils_error_is_cancelled (error, TRUE)) { + supplicant_interface_release (self); + nm_device_queue_state (NM_DEVICE (self), + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_SUPPLICANT_CONFIG_FAILED); + } +} + static void supplicant_iface_state_cb (NMSupplicantInterface *iface, int new_state_i, @@ -605,7 +609,6 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); NMDevice *device = NM_DEVICE (self); NMSupplicantConfig *config; - gboolean success = FALSE; NMDeviceState devstate; GError *error = NULL; NMSupplicantInterfaceState new_state = new_state_i; @@ -624,30 +627,23 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, case NM_SUPPLICANT_INTERFACE_STATE_READY: config = build_supplicant_config (self, &error); if (config) { - success = nm_supplicant_interface_set_config (priv->supplicant.iface, config, &error); + nm_supplicant_interface_assoc (priv->supplicant.iface, config, + supplicant_iface_assoc_cb, self); g_object_unref (config); - - if (!success) { - _LOGE (LOGD_DEVICE | LOGD_ETHER, - "Activation: (ethernet) couldn't send security configuration to the supplicant: %s", - error->message); - g_clear_error (&error); - } } else { _LOGE (LOGD_DEVICE | LOGD_ETHER, "Activation: (ethernet) couldn't build security configuration: %s", error->message); g_clear_error (&error); - } - if (!success) { nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_SUPPLICANT_CONFIG_FAILED); } break; case NM_SUPPLICANT_INTERFACE_STATE_COMPLETED: - supplicant_interface_clear_handlers (self); + nm_clear_g_source (&priv->supplicant_timeout_id); + nm_clear_g_source (&priv->supplicant.con_timeout_id); /* If this is the initial association during device activation, * schedule the next activation stage. @@ -679,24 +675,6 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, } } -static void -supplicant_iface_connection_error_cb (NMSupplicantInterface *iface, - const char *name, - const char *message, - gpointer user_data) -{ - NMDeviceEthernet *self = NM_DEVICE_ETHERNET (user_data); - - _LOGW (LOGD_DEVICE | LOGD_ETHER, - "Activation: (ethernet) association request to the supplicant failed: %s - %s", - name, message); - - supplicant_interface_release (self); - nm_device_queue_state (NM_DEVICE (self), - NM_DEVICE_STATE_FAILED, - NM_DEVICE_STATE_REASON_SUPPLICANT_CONFIG_FAILED); -} - static NMActStageReturn handle_auth_or_fail (NMDeviceEthernet *self, NMActRequest *req, @@ -792,12 +770,6 @@ supplicant_interface_init (NMDeviceEthernet *self) G_CALLBACK (supplicant_iface_state_cb), self); - /* Hook up error signal handler to capture association errors */ - priv->supplicant.iface_error_id = g_signal_connect (priv->supplicant.iface, - NM_SUPPLICANT_INTERFACE_CONNECTION_ERROR, - G_CALLBACK (supplicant_iface_connection_error_cb), - self); - /* Set up a timeout on the connection attempt to fail it after 25 seconds */ priv->supplicant.con_timeout_id = g_timeout_add_seconds (25, supplicant_connection_timeout_cb, self); diff --git a/src/devices/nm-device-macsec.c b/src/devices/nm-device-macsec.c index ff65178fdf..6c8b98cba3 100644 --- a/src/devices/nm-device-macsec.c +++ b/src/devices/nm-device-macsec.c @@ -45,7 +45,6 @@ typedef struct Supplicant { NMSupplicantInterface *iface; /* signal handler ids */ - gulong iface_error_id; gulong iface_state_id; /* Timeouts and idles */ @@ -248,22 +247,12 @@ build_supplicant_config (NMDeviceMacsec *self, GError **error) } static void -supplicant_interface_clear_handlers (NMDeviceMacsec *self) +supplicant_interface_release (NMDeviceMacsec *self) { NMDeviceMacsecPrivate *priv = NM_DEVICE_MACSEC_GET_PRIVATE (self); nm_clear_g_source (&priv->supplicant_timeout_id); nm_clear_g_source (&priv->supplicant.con_timeout_id); - nm_clear_g_signal_handler (priv->supplicant.iface, &priv->supplicant.iface_error_id); -} - -static void -supplicant_interface_release (NMDeviceMacsec *self) -{ - NMDeviceMacsecPrivate *priv = NM_DEVICE_MACSEC_GET_PRIVATE (self); - - supplicant_interface_clear_handlers (self); - nm_clear_g_signal_handler (priv->supplicant.iface, &priv->supplicant.iface_state_id); if (priv->supplicant.iface) { @@ -273,21 +262,18 @@ supplicant_interface_release (NMDeviceMacsec *self) } static void -supplicant_iface_connection_error_cb (NMSupplicantInterface *iface, - const char *name, - const char *message, - gpointer user_data) +supplicant_iface_assoc_cb (NMSupplicantInterface *iface, + GError *error, + gpointer user_data) { NMDeviceMacsec *self = NM_DEVICE_MACSEC (user_data); - _LOGW (LOGD_DEVICE, - "Activation: association request to the supplicant failed: %s - %s", - name, message); - - supplicant_interface_release (self); - nm_device_queue_state (NM_DEVICE (self), - NM_DEVICE_STATE_FAILED, - NM_DEVICE_STATE_REASON_SUPPLICANT_CONFIG_FAILED); + if (error && !nm_utils_error_is_cancelled (error, TRUE)) { + supplicant_interface_release (self); + nm_device_queue_state (NM_DEVICE (self), + NM_DEVICE_STATE_FAILED, + NM_DEVICE_STATE_REASON_SUPPLICANT_CONFIG_FAILED); + } } static void @@ -421,7 +407,6 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, NMDeviceMacsecPrivate *priv = NM_DEVICE_MACSEC_GET_PRIVATE (self); NMDevice *device = NM_DEVICE (self); NMSupplicantConfig *config; - gboolean success = FALSE; NMDeviceState devstate; GError *error = NULL; NMSupplicantInterfaceState new_state = new_state_i; @@ -440,30 +425,23 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, case NM_SUPPLICANT_INTERFACE_STATE_READY: config = build_supplicant_config (self, &error); if (config) { - success = nm_supplicant_interface_set_config (priv->supplicant.iface, config, &error); + nm_supplicant_interface_assoc (priv->supplicant.iface, config, + supplicant_iface_assoc_cb, self); g_object_unref (config); - - if (!success) { - _LOGE (LOGD_DEVICE, - "Activation: couldn't send security configuration to the supplicant: %s", - error->message); - g_clear_error (&error); - } } else { _LOGE (LOGD_DEVICE, "Activation: couldn't build security configuration: %s", error->message); g_clear_error (&error); - } - if (!success) { nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_SUPPLICANT_CONFIG_FAILED); } break; case NM_SUPPLICANT_INTERFACE_STATE_COMPLETED: - supplicant_interface_clear_handlers (self); + nm_clear_g_source (&priv->supplicant_timeout_id); + nm_clear_g_source (&priv->supplicant.con_timeout_id); nm_device_bring_up (device, TRUE, NULL); /* If this is the initial association during device activation, @@ -595,12 +573,6 @@ supplicant_interface_init (NMDeviceMacsec *self) G_CALLBACK (supplicant_iface_state_cb), self); - /* Hook up error signal handler to capture association errors */ - priv->supplicant.iface_error_id = g_signal_connect (priv->supplicant.iface, - NM_SUPPLICANT_INTERFACE_CONNECTION_ERROR, - G_CALLBACK (supplicant_iface_connection_error_cb), - self); - /* Set up a timeout on the connection attempt to fail it after 25 seconds */ priv->supplicant.con_timeout_id = g_timeout_add_seconds (25, supplicant_connection_timeout_cb, self); diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 097e8fe963..0afd8034ca 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -190,8 +190,6 @@ static void ap_add_remove (NMDeviceWifi *self, NMWifiAP *ap, gboolean recheck_available_connections); -static void remove_supplicant_interface_error_handler (NMDeviceWifi *self); - static void _hw_addr_set_scanning (NMDeviceWifi *self, gboolean do_reset); /*****************************************************************************/ @@ -1766,21 +1764,12 @@ supplicant_iface_bss_removed_cb (NMSupplicantInterface *iface, } static void -remove_supplicant_timeouts (NMDeviceWifi *self) +cleanup_association_attempt (NMDeviceWifi *self, gboolean disconnect) { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); nm_clear_g_source (&priv->sup_timeout_id); nm_clear_g_source (&priv->link_timeout_id); -} - -static void -cleanup_association_attempt (NMDeviceWifi *self, gboolean disconnect) -{ - NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - - remove_supplicant_interface_error_handler (self); - remove_supplicant_timeouts (self); if (disconnect && priv->sup_iface) nm_supplicant_interface_disconnect (priv->sup_iface); } @@ -2087,8 +2076,8 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, nm_device_remove_pending_action (device, NM_PENDING_ACTION_WAITING_FOR_SUPPLICANT, TRUE); break; case NM_SUPPLICANT_INTERFACE_STATE_COMPLETED: - remove_supplicant_interface_error_handler (self); - remove_supplicant_timeouts (self); + nm_clear_g_source (&priv->sup_timeout_id); + nm_clear_g_source (&priv->link_timeout_id); /* If this is the initial association during device activation, * schedule the next activation stage. @@ -2178,35 +2167,20 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, } static void -supplicant_iface_connection_error_cb (NMSupplicantInterface *iface, - const char *name, - const char *message, - NMDeviceWifi *self) +supplicant_iface_assoc_cb (NMSupplicantInterface *iface, + GError *error, + gpointer user_data) { + NMDeviceWifi *self = NM_DEVICE_WIFI (user_data); NMDevice *device = NM_DEVICE (self); - if (nm_device_is_activating (device)) { - _LOGW (LOGD_DEVICE | LOGD_WIFI, - "Activation: (wifi) supplicant association failed: %s - %s", - name, message); - + if ( error && !nm_utils_error_is_cancelled (error, TRUE) + && nm_device_is_activating (device)) { cleanup_association_attempt (self, TRUE); nm_device_queue_state (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); } } -static void -remove_supplicant_interface_error_handler (NMDeviceWifi *self) -{ - NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - - if (priv->sup_iface) { - g_signal_handlers_disconnect_by_func (priv->sup_iface, - supplicant_iface_connection_error_cb, - self); - } -} - static void supplicant_iface_notify_scanning_cb (NMSupplicantInterface *iface, GParamSpec *pspec, @@ -2625,7 +2599,8 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *reason) g_return_val_if_fail (reason != NULL, NM_ACT_STAGE_RETURN_FAILURE); - remove_supplicant_timeouts (self); + nm_clear_g_source (&priv->sup_timeout_id); + nm_clear_g_source (&priv->link_timeout_id); req = nm_device_get_act_request (device); g_assert (req); @@ -2689,20 +2664,8 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *reason) goto out; } - /* Hook up error signal handler to capture association errors */ - g_signal_connect (priv->sup_iface, - NM_SUPPLICANT_INTERFACE_CONNECTION_ERROR, - G_CALLBACK (supplicant_iface_connection_error_cb), - self); - - if (!nm_supplicant_interface_set_config (priv->sup_iface, config, &error)) { - _LOGE (LOGD_DEVICE | LOGD_WIFI, - "Activation: (wifi) couldn't send wireless configuration to the supplicant: %s", - error->message); - g_clear_error (&error); - *reason = NM_DEVICE_STATE_REASON_SUPPLICANT_CONFIG_FAILED; - goto out; - } + nm_supplicant_interface_assoc (priv->sup_iface, config, + supplicant_iface_assoc_cb, self); /* Set up a timeout on the association attempt to fail after 25 seconds */ priv->sup_timeout_id = g_timeout_add_seconds (25, supplicant_connection_timeout_cb, self); diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 93f7216feb..17f2dab95d 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -41,6 +41,15 @@ static NM_CACHED_QUARK_FCN ("bss-proxy-inited", bss_proxy_inited_quark) /*****************************************************************************/ +typedef struct { + NMSupplicantConfig *cfg; + GCancellable *cancellable; + NMSupplicantInterfaceAssocCb callback; + gpointer user_data; + guint fail_on_idle_id; + guint blobs_left; +} AssocData; + enum { STATE, /* change in the interface's state */ REMOVED, /* interface was removed by the supplicant */ @@ -48,7 +57,6 @@ enum { BSS_UPDATED, /* a BSS property changed */ BSS_REMOVED, /* supplicant removed BSS from its scan list */ SCAN_DONE, /* wifi scan is complete */ - CONNECTION_ERROR, /* an error occurred during a connection request */ CREDENTIALS_REQUEST, /* 802.1x identity or password requested */ LAST_SIGNAL }; @@ -82,15 +90,15 @@ typedef struct { GCancellable * init_cancellable; GDBusProxy * iface_proxy; GCancellable * other_cancellable; - GCancellable * assoc_cancellable; + + AssocData * assoc_data; + char * net_path; - guint32 blobs_left; GHashTable * bss_proxies; char * current_bss; gint32 last_scan; /* timestamp as returned by nm_utils_get_monotonic_timestamp_s() */ - NMSupplicantConfig *cfg; } NMSupplicantInterfacePrivate; struct _NMSupplicantInterface { @@ -128,18 +136,6 @@ G_DEFINE_TYPE (NMSupplicantInterface, nm_supplicant_interface, G_TYPE_OBJECT) /*****************************************************************************/ -static void -emit_error_helper (NMSupplicantInterface *self, GError *error) -{ - char *name = NULL; - - if (g_dbus_error_is_remote_error (error)) - name = g_dbus_error_get_remote_error (error); - - g_signal_emit (self, signals[CONNECTION_ERROR], 0, name, error->message); - g_free (name); -} - static void bss_props_changed_cb (GDBusProxy *proxy, GVariant *changed_properties, @@ -1007,6 +1003,34 @@ log_result_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) } } +/*****************************************************************************/ + +static void +assoc_return (NMSupplicantInterface *self, GError *error, const char *message) +{ + NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); + AssocData *assoc_data; + + assoc_data = g_steal_pointer (&priv->assoc_data); + if (!assoc_data) + return; + + if (error) { + g_dbus_error_strip_remote_error (error); + _LOGW ("assoc[%p]: %s: %s", assoc_data, message, error->message); + } else + _LOGD ("assoc[%p]: association request successful", assoc_data); + + nm_clear_g_source (&assoc_data->fail_on_idle_id); + nm_clear_g_cancellable (&assoc_data->cancellable); + + if (assoc_data->callback) + assoc_data->callback (self, error, assoc_data->user_data); + + g_object_unref (assoc_data->cfg); + g_slice_free (AssocData, assoc_data); +} + void nm_supplicant_interface_disconnect (NMSupplicantInterface * self) { @@ -1017,9 +1041,11 @@ nm_supplicant_interface_disconnect (NMSupplicantInterface * self) priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); /* Cancel all pending calls related to a prior connection attempt */ - if (priv->assoc_cancellable) { - g_cancellable_cancel (priv->assoc_cancellable); - g_clear_object (&priv->assoc_cancellable); + if (priv->assoc_data) { + gs_free GError *error = NULL; + + nm_utils_error_set_cancelled (&error, FALSE, "NMSupplicantInterface"); + assoc_return (self, error, "abort due to disconnect"); } /* Don't do anything if there is no connection to the supplicant yet. */ @@ -1055,42 +1081,40 @@ nm_supplicant_interface_disconnect (NMSupplicantInterface * self) } static void -select_network_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +assoc_select_network_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) { NMSupplicantInterface *self; gs_unref_variant GVariant *reply = NULL; gs_free_error GError *error = NULL; reply = g_dbus_proxy_call_finish (proxy, result, &error); - if ( !reply - && !g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { - self = NM_SUPPLICANT_INTERFACE (user_data); - g_dbus_error_strip_remote_error (error); - _LOGW ("couldn't select network config: %s", error->message); - emit_error_helper (self, error); - } + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_SUPPLICANT_INTERFACE (user_data); + if (error) + assoc_return (self, error, "failure to select network config"); + else + assoc_return (self, NULL, NULL); } static void -call_select_network (NMSupplicantInterface *self) +assoc_call_select_network (NMSupplicantInterface *self) { NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - /* We only select the network after all blobs (if any) have been set */ - if (priv->blobs_left == 0) { - g_dbus_proxy_call (priv->iface_proxy, - "SelectNetwork", - g_variant_new ("(o)", priv->net_path), - G_DBUS_CALL_FLAGS_NONE, - -1, - priv->assoc_cancellable, - (GAsyncReadyCallback) select_network_cb, - self); - } + g_dbus_proxy_call (priv->iface_proxy, + "SelectNetwork", + g_variant_new ("(o)", priv->net_path), + G_DBUS_CALL_FLAGS_NONE, + -1, + priv->assoc_data->cancellable, + (GAsyncReadyCallback) assoc_select_network_cb, + self); } static void -add_blob_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +assoc_add_blob_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) { NMSupplicantInterface *self; NMSupplicantInterfacePrivate *priv; @@ -1104,18 +1128,19 @@ add_blob_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) self = NM_SUPPLICANT_INTERFACE (user_data); priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - priv->blobs_left--; - if (reply) - call_select_network (self); - else { - g_dbus_error_strip_remote_error (error); - _LOGW ("couldn't set network certificates: %s", error->message); - emit_error_helper (self, error); + if (error) { + assoc_return (self, error, "failure to set network certificates"); + return; } + + priv->assoc_data->blobs_left--; + _LOGT ("assoc[%p]: blob added (%u left)", priv->assoc_data, priv->assoc_data->blobs_left); + if (priv->assoc_data->blobs_left == 0) + assoc_call_select_network (self); } static void -add_network_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +assoc_add_network_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) { NMSupplicantInterface *self; NMSupplicantInterfacePrivate *priv; @@ -1135,57 +1160,43 @@ add_network_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) self = NM_SUPPLICANT_INTERFACE (user_data); priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - g_free (priv->net_path); - priv->net_path = NULL; + nm_clear_g_free (&priv->net_path); if (error) { - g_dbus_error_strip_remote_error (error); - _LOGW ("adding network to supplicant failed: %s", error->message); - emit_error_helper (self, error); + assoc_return (self, error, "failure to add network"); return; } g_variant_get (reply, "(o)", &priv->net_path); /* Send blobs first; otherwise jump to selecting the network */ - blobs = nm_supplicant_config_get_blobs (priv->cfg); - priv->blobs_left = g_hash_table_size (blobs); + blobs = nm_supplicant_config_get_blobs (priv->assoc_data->cfg); + priv->assoc_data->blobs_left = g_hash_table_size (blobs); - g_hash_table_iter_init (&iter, blobs); - while (g_hash_table_iter_next (&iter, (gpointer) &blob_name, (gpointer) &blob_data)) { - g_dbus_proxy_call (priv->iface_proxy, - "AddBlob", - g_variant_new ("(s@ay)", - blob_name, - g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, - blob_data->data, blob_data->len, 1)), - G_DBUS_CALL_FLAGS_NONE, - -1, - priv->assoc_cancellable, - (GAsyncReadyCallback) add_blob_cb, - self); + _LOGT ("assoc[%p]: network added (%s) (%u blobs left)", priv->assoc_data, priv->net_path, priv->assoc_data->blobs_left); + + if (priv->assoc_data->blobs_left == 0) + assoc_call_select_network (self); + else { + g_hash_table_iter_init (&iter, blobs); + while (g_hash_table_iter_next (&iter, (gpointer) &blob_name, (gpointer) &blob_data)) { + g_dbus_proxy_call (priv->iface_proxy, + "AddBlob", + g_variant_new ("(s@ay)", + blob_name, + g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, + blob_data->data, blob_data->len, 1)), + G_DBUS_CALL_FLAGS_NONE, + -1, + priv->assoc_data->cancellable, + (GAsyncReadyCallback) assoc_add_blob_cb, + self); + } } - - call_select_network (self); } static void -add_network (NMSupplicantInterface *self) -{ - NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - - g_dbus_proxy_call (priv->iface_proxy, - "AddNetwork", - g_variant_new ("(@a{sv})", nm_supplicant_config_to_variant (priv->cfg)), - G_DBUS_CALL_FLAGS_NONE, - -1, - priv->assoc_cancellable, - (GAsyncReadyCallback) add_network_cb, - self); -} - -static void -set_ap_scan_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +assoc_set_ap_scan_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) { NMSupplicantInterface *self; NMSupplicantInterfacePrivate *priv; @@ -1199,62 +1210,103 @@ set_ap_scan_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) self = NM_SUPPLICANT_INTERFACE (user_data); priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - if (!reply) { - g_dbus_error_strip_remote_error (error); - _LOGW ("couldn't send AP scan mode to the supplicant interface: %s", - error->message); - emit_error_helper (self, error); + if (error) { + assoc_return (self, error, "failure to set AP scan mode"); return; } - _LOGI ("config: set interface ap_scan to %d", - nm_supplicant_config_get_ap_scan (priv->cfg)); + _LOGT ("assoc[%p]: set interface ap_scan to %d", + priv->assoc_data, + nm_supplicant_config_get_ap_scan (priv->assoc_data->cfg)); - add_network (self); + g_dbus_proxy_call (priv->iface_proxy, + "AddNetwork", + g_variant_new ("(@a{sv})", nm_supplicant_config_to_variant (priv->assoc_data->cfg)), + G_DBUS_CALL_FLAGS_NONE, + -1, + priv->assoc_data->cancellable, + (GAsyncReadyCallback) assoc_add_network_cb, + self); } -gboolean -nm_supplicant_interface_set_config (NMSupplicantInterface *self, - NMSupplicantConfig *cfg, - GError **error) +static gboolean +assoc_fail_on_idle_cb (gpointer user_data) +{ + NMSupplicantInterface *self = user_data; + NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); + gs_free_error GError *error = NULL; + + priv->assoc_data->fail_on_idle_id = 0; + g_set_error (&error, NM_SUPPLICANT_ERROR, NM_SUPPLICANT_ERROR_CONFIG, + "EAP-FAST is not supported by the supplicant"); + assoc_return (self, error, "failure due to missing supplicant support"); + return G_SOURCE_REMOVE; +} + +/** + * nm_supplicant_interface_assoc: + * @self: the supplicant interface instance + * @cfg: the configuration with the data for the association + * @callback: callback invoked when the association completes or fails. + * @user_data: data for the callback. + * + * Calls AddNetwork and SelectNetwork to start associating according to @cfg. + * + * The callback is invoked exactly once (always) and always asynchronously. + * The pending association can be aborted via nm_supplicant_interface_disconnect() + * or by destroying @self. In that case, the @callback is invoked synchornously with + * an error reason indicating cancellation/disposing (see nm_utils_error_is_cancelled()). + */ +void +nm_supplicant_interface_assoc (NMSupplicantInterface *self, + NMSupplicantConfig *cfg, + NMSupplicantInterfaceAssocCb callback, + gpointer user_data) { NMSupplicantInterfacePrivate *priv; + AssocData *assoc_data; - g_return_val_if_fail (NM_IS_SUPPLICANT_INTERFACE (self), FALSE); + g_return_if_fail (NM_IS_SUPPLICANT_INTERFACE (self)); + g_return_if_fail (NM_IS_SUPPLICANT_CONFIG (cfg)); priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); nm_supplicant_interface_disconnect (self); + assoc_data = g_slice_new0 (AssocData); + priv->assoc_data = assoc_data; + + assoc_data->cfg = g_object_ref (cfg); + assoc_data->callback = callback; + assoc_data->user_data = user_data; + + _LOGD ("assoc[%p]: starting association...", assoc_data); + /* Make sure the supplicant supports EAP-FAST before trying to send * it an EAP-FAST configuration. */ if ( priv->fast_support == NM_SUPPLICANT_FEATURE_NO && nm_supplicant_config_fast_required (cfg)) { - g_set_error (error, NM_SUPPLICANT_ERROR, NM_SUPPLICANT_ERROR_CONFIG, - "EAP-FAST is not supported by the supplicant"); - return FALSE; + assoc_data->fail_on_idle_id = g_idle_add (assoc_fail_on_idle_cb, self); + return; } - g_clear_object (&priv->cfg); - if (cfg) { - priv->assoc_cancellable = g_cancellable_new (); - priv->cfg = g_object_ref (cfg); - g_dbus_proxy_call (priv->iface_proxy, - DBUS_INTERFACE_PROPERTIES ".Set", - g_variant_new ("(ssv)", - WPAS_DBUS_IFACE_INTERFACE, - "ApScan", - g_variant_new_uint32 (nm_supplicant_config_get_ap_scan (priv->cfg))), - G_DBUS_CALL_FLAGS_NONE, - -1, - priv->assoc_cancellable, - (GAsyncReadyCallback) set_ap_scan_cb, - self); - } - return TRUE; + assoc_data->cancellable = g_cancellable_new(); + g_dbus_proxy_call (priv->iface_proxy, + DBUS_INTERFACE_PROPERTIES ".Set", + g_variant_new ("(ssv)", + WPAS_DBUS_IFACE_INTERFACE, + "ApScan", + g_variant_new_uint32 (nm_supplicant_config_get_ap_scan (priv->assoc_data->cfg))), + G_DBUS_CALL_FLAGS_NONE, + -1, + priv->assoc_data->cancellable, + (GAsyncReadyCallback) assoc_set_ap_scan_cb, + self); } +/*****************************************************************************/ + static void scan_request_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) { @@ -1449,7 +1501,15 @@ get_property (GObject *object, static void dispose (GObject *object) { - NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE ((NMSupplicantInterface *) object); + NMSupplicantInterface *self = NM_SUPPLICANT_INTERFACE (object); + NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); + + if (priv->assoc_data) { + gs_free GError *error = NULL; + + nm_utils_error_set_cancelled (&error, TRUE, "NMSupplicantInterface"); + assoc_return (self, error, "cancelled due to dispose of supplicant interface"); + } if (priv->iface_proxy) g_signal_handlers_disconnect_by_data (priv->iface_proxy, object); @@ -1457,7 +1517,6 @@ dispose (GObject *object) nm_clear_g_cancellable (&priv->init_cancellable); nm_clear_g_cancellable (&priv->other_cancellable); - nm_clear_g_cancellable (&priv->assoc_cancellable); g_clear_object (&priv->wpas_proxy); g_clear_pointer (&priv->bss_proxies, (GDestroyNotify) g_hash_table_destroy); @@ -1467,9 +1526,6 @@ dispose (GObject *object) g_clear_pointer (&priv->object_path, g_free); g_clear_pointer (&priv->current_bss, g_free); - g_clear_object (&priv->cfg); - - /* Chain up to the parent class */ G_OBJECT_CLASS (nm_supplicant_interface_parent_class)->dispose (object); } @@ -1571,14 +1627,6 @@ nm_supplicant_interface_class_init (NMSupplicantInterfaceClass *klass) NULL, NULL, NULL, G_TYPE_NONE, 1, G_TYPE_BOOLEAN); - signals[CONNECTION_ERROR] = - g_signal_new (NM_SUPPLICANT_INTERFACE_CONNECTION_ERROR, - G_OBJECT_CLASS_TYPE (object_class), - G_SIGNAL_RUN_LAST, - 0, - NULL, NULL, NULL, - G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_STRING); - signals[CREDENTIALS_REQUEST] = g_signal_new (NM_SUPPLICANT_INTERFACE_CREDENTIALS_REQUEST, G_OBJECT_CLASS_TYPE (object_class), diff --git a/src/supplicant/nm-supplicant-interface.h b/src/supplicant/nm-supplicant-interface.h index d78cd28ecd..3498bb5925 100644 --- a/src/supplicant/nm-supplicant-interface.h +++ b/src/supplicant/nm-supplicant-interface.h @@ -68,7 +68,6 @@ typedef enum { #define NM_SUPPLICANT_INTERFACE_BSS_UPDATED "bss-updated" #define NM_SUPPLICANT_INTERFACE_BSS_REMOVED "bss-removed" #define NM_SUPPLICANT_INTERFACE_SCAN_DONE "scan-done" -#define NM_SUPPLICANT_INTERFACE_CONNECTION_ERROR "connection-error" #define NM_SUPPLICANT_INTERFACE_CREDENTIALS_REQUEST "credentials-request" typedef struct _NMSupplicantInterfaceClass NMSupplicantInterfaceClass; @@ -83,9 +82,15 @@ NMSupplicantInterface * nm_supplicant_interface_new (const char *ifname, void nm_supplicant_interface_set_supplicant_available (NMSupplicantInterface *self, gboolean available); -gboolean nm_supplicant_interface_set_config (NMSupplicantInterface * iface, - NMSupplicantConfig * cfg, - GError **error); +typedef void (*NMSupplicantInterfaceAssocCb) (NMSupplicantInterface *iface, + GError *error, + gpointer user_data); + +void +nm_supplicant_interface_assoc (NMSupplicantInterface *self, + NMSupplicantConfig *cfg, + NMSupplicantInterfaceAssocCb callback, + gpointer user_data); void nm_supplicant_interface_disconnect (NMSupplicantInterface * iface); From e16bf4f3db3da36f8194195e766029d1b751b5ec Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 00:11:16 +0100 Subject: [PATCH 02/13] supplicant/trivial: move code around --- src/supplicant/nm-supplicant-interface.c | 114 ++++++++++++----------- 1 file changed, 59 insertions(+), 55 deletions(-) diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 17f2dab95d..ac92e52db0 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -136,6 +136,27 @@ G_DEFINE_TYPE (NMSupplicantInterface, nm_supplicant_interface, G_TYPE_OBJECT) /*****************************************************************************/ +NM_UTILS_LOOKUP_STR_DEFINE (nm_supplicant_interface_state_to_string, NMSupplicantInterfaceState, + NM_UTILS_LOOKUP_DEFAULT_WARN ("unknown"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_INVALID, "invalid"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_INIT, "init"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_STARTING, "starting"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_READY, "ready"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_DISABLED, "disabled"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_DISCONNECTED, "disconnected"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_INACTIVE, "inactive"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_SCANNING, "scanning"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_AUTHENTICATING, "authenticating"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATING, "associating"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATED, "associated"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_4WAY_HANDSHAKE, "4-way handshake"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_GROUP_HANDSHAKE, "group handshake"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_COMPLETED, "completed"), + NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_DOWN, "down"), +); + +/*****************************************************************************/ + static void bss_props_changed_cb (GDBusProxy *proxy, GVariant *changed_properties, @@ -1367,6 +1388,8 @@ nm_supplicant_interface_request_scan (NMSupplicantInterface *self, const GPtrArr return TRUE; } +/*****************************************************************************/ + NMSupplicantInterfaceState nm_supplicant_interface_get_state (NMSupplicantInterface * self) { @@ -1375,25 +1398,6 @@ nm_supplicant_interface_get_state (NMSupplicantInterface * self) return NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)->state; } -NM_UTILS_LOOKUP_STR_DEFINE (nm_supplicant_interface_state_to_string, NMSupplicantInterfaceState, - NM_UTILS_LOOKUP_DEFAULT_WARN ("unknown"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_INVALID, "invalid"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_INIT, "init"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_STARTING, "starting"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_READY, "ready"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_DISABLED, "disabled"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_DISCONNECTED, "disconnected"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_INACTIVE, "inactive"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_SCANNING, "scanning"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_AUTHENTICATING, "authenticating"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATING, "associating"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATED, "associated"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_4WAY_HANDSHAKE, "4-way handshake"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_GROUP_HANDSHAKE, "group handshake"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_COMPLETED, "completed"), - NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_DOWN, "down"), -); - const char * nm_supplicant_interface_get_object_path (NMSupplicantInterface *self) { @@ -1420,29 +1424,25 @@ nm_supplicant_interface_get_max_scan_ssids (NMSupplicantInterface *self) /*****************************************************************************/ -NMSupplicantInterface * -nm_supplicant_interface_new (const char *ifname, - NMSupplicantDriver driver, - NMSupplicantFeature fast_support, - NMSupplicantFeature ap_support) -{ - g_return_val_if_fail (ifname != NULL, NULL); - - return g_object_new (NM_TYPE_SUPPLICANT_INTERFACE, - NM_SUPPLICANT_INTERFACE_IFACE, ifname, - NM_SUPPLICANT_INTERFACE_DRIVER, (guint) driver, - NM_SUPPLICANT_INTERFACE_FAST_SUPPORT, (int) fast_support, - NM_SUPPLICANT_INTERFACE_AP_SUPPORT, (int) ap_support, - NULL); -} - static void -nm_supplicant_interface_init (NMSupplicantInterface * self) +get_property (GObject *object, + guint prop_id, + GValue *value, + GParamSpec *pspec) { - NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); + NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE ((NMSupplicantInterface *) object); - priv->state = NM_SUPPLICANT_INTERFACE_STATE_INIT; - priv->bss_proxies = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_object_unref); + switch (prop_id) { + case PROP_SCANNING: + g_value_set_boolean (value, priv->scanning); + break; + case PROP_CURRENT_BSS: + g_value_set_string (value, priv->current_bss); + break; + default: + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); + break; + } } static void @@ -1478,24 +1478,28 @@ set_property (GObject *object, } static void -get_property (GObject *object, - guint prop_id, - GValue *value, - GParamSpec *pspec) +nm_supplicant_interface_init (NMSupplicantInterface * self) { - NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE ((NMSupplicantInterface *) object); + NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - switch (prop_id) { - case PROP_SCANNING: - g_value_set_boolean (value, priv->scanning); - break; - case PROP_CURRENT_BSS: - g_value_set_string (value, priv->current_bss); - break; - default: - G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); - break; - } + priv->state = NM_SUPPLICANT_INTERFACE_STATE_INIT; + priv->bss_proxies = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_object_unref); +} + +NMSupplicantInterface * +nm_supplicant_interface_new (const char *ifname, + NMSupplicantDriver driver, + NMSupplicantFeature fast_support, + NMSupplicantFeature ap_support) +{ + g_return_val_if_fail (ifname != NULL, NULL); + + return g_object_new (NM_TYPE_SUPPLICANT_INTERFACE, + NM_SUPPLICANT_INTERFACE_IFACE, ifname, + NM_SUPPLICANT_INTERFACE_DRIVER, (guint) driver, + NM_SUPPLICANT_INTERFACE_FAST_SUPPORT, (int) fast_support, + NM_SUPPLICANT_INTERFACE_AP_SUPPORT, (int) ap_support, + NULL); } static void From da34034b95651d8b68eaa19d7bf115ffc1cd1fb5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 00:16:38 +0100 Subject: [PATCH 03/13] supplicant: use nm_clear_g_cancellable() helper --- src/supplicant/nm-supplicant-interface.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index ac92e52db0..cafa54c503 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -271,20 +271,11 @@ set_state (NMSupplicantInterface *self, NMSupplicantInterfaceState new_state) g_return_if_fail (new_state > NM_SUPPLICANT_INTERFACE_STATE_READY); if (new_state == NM_SUPPLICANT_INTERFACE_STATE_READY) { - if (priv->other_cancellable) { - g_warn_if_fail (priv->other_cancellable == NULL); - g_cancellable_cancel (priv->other_cancellable); - g_clear_object (&priv->other_cancellable); - } + nm_clear_g_cancellable (&priv->other_cancellable); priv->other_cancellable = g_cancellable_new (); } else if (new_state == NM_SUPPLICANT_INTERFACE_STATE_DOWN) { - if (priv->init_cancellable) - g_cancellable_cancel (priv->init_cancellable); - g_clear_object (&priv->init_cancellable); - - if (priv->other_cancellable) - g_cancellable_cancel (priv->other_cancellable); - g_clear_object (&priv->other_cancellable); + nm_clear_g_cancellable (&priv->init_cancellable); + nm_clear_g_cancellable (&priv->other_cancellable); if (priv->iface_proxy) g_signal_handlers_disconnect_by_data (priv->iface_proxy, self); @@ -970,8 +961,7 @@ interface_add (NMSupplicantInterface *self) /* Move to starting to prevent double-calls of interface_add() */ set_state (self, NM_SUPPLICANT_INTERFACE_STATE_STARTING); - g_warn_if_fail (priv->init_cancellable == NULL); - g_clear_object (&priv->init_cancellable); + nm_clear_g_cancellable (&priv->init_cancellable); priv->init_cancellable = g_cancellable_new (); g_dbus_proxy_new_for_bus (G_BUS_TYPE_SYSTEM, From dce13b6f11105422d54ee3aa3781ae77c875ae0f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 00:22:46 +0100 Subject: [PATCH 04/13] supplicant: remove unused return value from nm_supplicant_interface_request_scan() It cannot fail, remove code that anticipates a failure of request-scan. --- src/devices/wifi/nm-device-wifi.c | 22 +++++++--------------- src/supplicant/nm-supplicant-interface.c | 5 ++--- src/supplicant/nm-supplicant-interface.h | 2 +- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 0afd8034ca..88560a1896 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1427,9 +1427,7 @@ static void request_wireless_scan (NMDeviceWifi *self, gboolean force_if_scanning, GVariant *scan_options) { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - gboolean backoff = FALSE; - GPtrArray *ssids = NULL; - gboolean new_scan_requested = FALSE; + gboolean request_started = FALSE; nm_clear_g_source (&priv->pending_scan_id); @@ -1439,6 +1437,8 @@ request_wireless_scan (NMDeviceWifi *self, gboolean force_if_scanning, GVariant } if (check_scanning_allowed (self)) { + gs_unref_ptrarray GPtrArray *ssids = NULL; + _LOGD (LOGD_WIFI, "wifi-scan: scanning requested"); if (scan_options) { @@ -1476,22 +1476,14 @@ request_wireless_scan (NMDeviceWifi *self, gboolean force_if_scanning, GVariant _hw_addr_set_scanning (self, FALSE); - if (nm_supplicant_interface_request_scan (priv->sup_iface, ssids)) { - /* success */ - backoff = TRUE; - _requested_scan_set (self, TRUE); - new_scan_requested = TRUE; - } - - if (ssids) - g_ptr_array_unref (ssids); + nm_supplicant_interface_request_scan (priv->sup_iface, ssids); + request_started = TRUE; } else _LOGD (LOGD_WIFI, "wifi-scan: scanning requested but not allowed at this time"); - if (!new_scan_requested) - _requested_scan_set (self, FALSE); + _requested_scan_set (self, request_started); - schedule_scan (self, backoff); + schedule_scan (self, request_started); } static gboolean diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index cafa54c503..ecd7828a65 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -1340,14 +1340,14 @@ scan_request_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) } } -gboolean +void nm_supplicant_interface_request_scan (NMSupplicantInterface *self, const GPtrArray *ssids) { NMSupplicantInterfacePrivate *priv; GVariantBuilder builder; guint i; - g_return_val_if_fail (NM_IS_SUPPLICANT_INTERFACE (self), FALSE); + g_return_if_fail (NM_IS_SUPPLICANT_INTERFACE (self)); priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); @@ -1375,7 +1375,6 @@ nm_supplicant_interface_request_scan (NMSupplicantInterface *self, const GPtrArr priv->other_cancellable, (GAsyncReadyCallback) scan_request_cb, self); - return TRUE; } /*****************************************************************************/ diff --git a/src/supplicant/nm-supplicant-interface.h b/src/supplicant/nm-supplicant-interface.h index 3498bb5925..97ee38e689 100644 --- a/src/supplicant/nm-supplicant-interface.h +++ b/src/supplicant/nm-supplicant-interface.h @@ -96,7 +96,7 @@ void nm_supplicant_interface_disconnect (NMSupplicantInterface * iface); const char *nm_supplicant_interface_get_object_path (NMSupplicantInterface * iface); -gboolean nm_supplicant_interface_request_scan (NMSupplicantInterface * self, const GPtrArray *ssids); +void nm_supplicant_interface_request_scan (NMSupplicantInterface * self, const GPtrArray *ssids); NMSupplicantInterfaceState nm_supplicant_interface_get_state (NMSupplicantInterface * self); From c47026715ec6a9612752f469358a7251aabbdee7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 10:14:54 +0100 Subject: [PATCH 05/13] supplicant: cleanup network when cancelling "AddNetwork" request If the assoc-request is cancelled while an "AddNetwork" request is pending, we must cleanup the added network when the request succeeds. The issue can also happen when NetworkManager shuts down and exits the mainloop. This scenario is unsolved as the cleanup action "RemoveNetwork" has no chance to run. "AddBlob" works differently in that blogs are added with a specific name, not like "AddNetwork", where a new D-Bus path is created. Maybe we should also cleanup blobs that were added by us, but currently we don't. --- src/supplicant/nm-supplicant-interface.c | 58 +++++++++++++++++++++--- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index ecd7828a65..62cf96be63 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -41,15 +41,24 @@ static NM_CACHED_QUARK_FCN ("bss-proxy-inited", bss_proxy_inited_quark) /*****************************************************************************/ +struct _AddNetworkData; + typedef struct { + NMSupplicantInterface *self; NMSupplicantConfig *cfg; GCancellable *cancellable; NMSupplicantInterfaceAssocCb callback; gpointer user_data; guint fail_on_idle_id; guint blobs_left; + struct _AddNetworkData *add_network_data; } AssocData; +typedef struct _AddNetworkData { + /* the assoc_data at the time when doing the call. */ + AssocData *assoc_data; +} AddNetworkData; + enum { STATE, /* change in the interface's state */ REMOVED, /* interface was removed by the supplicant */ @@ -1032,6 +1041,11 @@ assoc_return (NMSupplicantInterface *self, GError *error, const char *message) } else _LOGD ("assoc[%p]: association request successful", assoc_data); + if (assoc_data->add_network_data) { + /* signal that this request already completed */ + assoc_data->add_network_data->assoc_data = NULL; + } + nm_clear_g_source (&assoc_data->fail_on_idle_id); nm_clear_g_cancellable (&assoc_data->cancellable); @@ -1153,6 +1167,8 @@ assoc_add_blob_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) static void assoc_add_network_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) { + AddNetworkData *add_network_data = user_data; + AssocData *assoc_data; NMSupplicantInterface *self; NMSupplicantInterfacePrivate *priv; gs_unref_variant GVariant *reply = NULL; @@ -1162,17 +1178,40 @@ assoc_add_network_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_dat const char *blob_name; GByteArray *blob_data; + assoc_data = add_network_data->assoc_data; + if (assoc_data) + assoc_data->add_network_data = NULL; + g_slice_free (AddNetworkData, add_network_data); + reply = _nm_dbus_proxy_call_finish (proxy, result, G_VARIANT_TYPE ("(o)"), &error); - if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + + if (!assoc_data) { + if (!error) { + gs_free char *net_path = NULL; + + /* the assoc-request was already cancelled, but the AddNetwork request succeeded. + * Cleanup the created network. + * + * This cleanup action does not work when NetworkManager is about to exit + * and leaves the mainloop. During program shutdown, we may orphan networks. */ + g_variant_get (reply, "(o)", &net_path); + g_dbus_proxy_call (proxy, + "RemoveNetwork", + g_variant_new ("(o)", net_path), + G_DBUS_CALL_FLAGS_NONE, + -1, + NULL, + NULL, + NULL); + } return; + } - self = NM_SUPPLICANT_INTERFACE (user_data); + self = NM_SUPPLICANT_INTERFACE (assoc_data->self); priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - nm_clear_g_free (&priv->net_path); - if (error) { assoc_return (self, error, "failure to add network"); return; @@ -1213,6 +1252,7 @@ assoc_set_ap_scan_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_dat NMSupplicantInterfacePrivate *priv; gs_unref_variant GVariant *reply = NULL; gs_free_error GError *error = NULL; + AddNetworkData *add_network_data; reply = g_dbus_proxy_call_finish (proxy, result, &error); if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) @@ -1230,14 +1270,19 @@ assoc_set_ap_scan_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_dat priv->assoc_data, nm_supplicant_config_get_ap_scan (priv->assoc_data->cfg)); + add_network_data = g_slice_new0 (AddNetworkData); + priv->assoc_data->add_network_data = add_network_data; + + add_network_data->assoc_data = priv->assoc_data; + g_dbus_proxy_call (priv->iface_proxy, "AddNetwork", g_variant_new ("(@a{sv})", nm_supplicant_config_to_variant (priv->assoc_data->cfg)), G_DBUS_CALL_FLAGS_NONE, -1, - priv->assoc_data->cancellable, + NULL, (GAsyncReadyCallback) assoc_add_network_cb, - self); + add_network_data); } static gboolean @@ -1287,6 +1332,7 @@ nm_supplicant_interface_assoc (NMSupplicantInterface *self, assoc_data = g_slice_new0 (AssocData); priv->assoc_data = assoc_data; + assoc_data->self = self; assoc_data->cfg = g_object_ref (cfg); assoc_data->callback = callback; assoc_data->user_data = user_data; From 29a53b1cd7fb48984221c71b38b049c3ced9b560 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 11:17:55 +0100 Subject: [PATCH 06/13] supplicant: merge NEW_BSS signal with BSS_UPDATED Before, the NEW_BSS signal was not careful to emit the signal only when the BSS is seen for the first time. Consequently, supplicant_iface_new_bss_cb() checked whether it already knows about the new BSS. Merge NEW_BSS and BSS_UPDATED. Now we emit BSS_UPDATED when either the BSS is new or changed. Also, in supplicant_iface_new_bss_cb() (now supplicant_iface_bss_updated_cb()) no longer constructs an @ap instance if we have a @found_ap. In some situations there can be a value of having a separate ADD signal. But only when there the consumers care, and if the consumers can trust that ADD is not just an UPDATE. The only consumer doesn't care and it not not be trusted, so merge the signals. --- src/devices/wifi/nm-device-wifi.c | 129 +++++++++-------------- src/supplicant/nm-supplicant-interface.c | 17 +-- src/supplicant/nm-supplicant-interface.h | 1 - 3 files changed, 51 insertions(+), 96 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 88560a1896..6af76b598a 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -157,11 +157,6 @@ static void supplicant_iface_state_cb (NMSupplicantInterface *iface, int disconnect_reason, gpointer user_data); -static void supplicant_iface_new_bss_cb (NMSupplicantInterface * iface, - const char *object_path, - GVariant *properties, - NMDeviceWifi * self); - static void supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface, const char *object_path, GVariant *properties, @@ -260,10 +255,6 @@ supplicant_interface_acquire (NMDeviceWifi *self) NM_SUPPLICANT_INTERFACE_STATE, G_CALLBACK (supplicant_iface_state_cb), self); - g_signal_connect (priv->sup_iface, - NM_SUPPLICANT_INTERFACE_NEW_BSS, - G_CALLBACK (supplicant_iface_new_bss_cb), - self); g_signal_connect (priv->sup_iface, NM_SUPPLICANT_INTERFACE_BSS_UPDATED, G_CALLBACK (supplicant_iface_bss_updated_cb), @@ -1634,14 +1625,13 @@ try_fill_ssid_for_hidden_ap (NMDeviceWifi *self, } static void -supplicant_iface_new_bss_cb (NMSupplicantInterface *iface, - const char *object_path, - GVariant *properties, - NMDeviceWifi *self) +supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface, + const char *object_path, + GVariant *properties, + NMDeviceWifi *self) { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); NMDeviceState state; - NMWifiAP *ap; NMWifiAP *found_ap = NULL; const GByteArray *ssid; @@ -1656,40 +1646,40 @@ supplicant_iface_new_bss_cb (NMSupplicantInterface *iface, if (NM_DEVICE_WIFI_GET_PRIVATE (self)->mode == NM_802_11_MODE_AP) return; - ap = nm_wifi_ap_new_from_properties (object_path, properties); - if (!ap) { - _LOGD (LOGD_WIFI, "invalid AP properties received for %s", object_path); - return; - } - - /* Let the manager try to fill in the SSID from seen-bssids lists */ - ssid = nm_wifi_ap_get_ssid (ap); - if (!ssid || nm_utils_is_empty_ssid (ssid->data, ssid->len)) { - /* Try to fill the SSID from the AP database */ - try_fill_ssid_for_hidden_ap (self, ap); - - ssid = nm_wifi_ap_get_ssid (ap); - if (ssid && (nm_utils_is_empty_ssid (ssid->data, ssid->len) == FALSE)) { - /* Yay, matched it, no longer treat as hidden */ - _LOGD (LOGD_WIFI, "matched hidden AP %s => '%s'", - nm_wifi_ap_get_address (ap), nm_utils_escape_ssid (ssid->data, ssid->len)); - } else { - /* Didn't have an entry for this AP in the database */ - _LOGD (LOGD_WIFI, "failed to match hidden AP %s", - nm_wifi_ap_get_address (ap)); - } - } - found_ap = get_ap_by_supplicant_path (self, object_path); if (found_ap) { - _ap_dump (self, ap, "updated", 0); nm_wifi_ap_update_from_properties (found_ap, object_path, properties); + _ap_dump (self, found_ap, "updated", 0); } else { - _ap_dump (self, ap, "added", 0); - ap_add_remove (self, ACCESS_POINT_ADDED, ap, TRUE); - } + gs_unref_object NMWifiAP *ap = NULL; - g_object_unref (ap); + ap = nm_wifi_ap_new_from_properties (object_path, properties); + if (!ap) { + _LOGD (LOGD_WIFI, "invalid AP properties received for %s", object_path); + return; + } + + /* Let the manager try to fill in the SSID from seen-bssids lists */ + ssid = nm_wifi_ap_get_ssid (ap); + if (!ssid || nm_utils_is_empty_ssid (ssid->data, ssid->len)) { + /* Try to fill the SSID from the AP database */ + try_fill_ssid_for_hidden_ap (self, ap); + + ssid = nm_wifi_ap_get_ssid (ap); + if (ssid && (nm_utils_is_empty_ssid (ssid->data, ssid->len) == FALSE)) { + /* Yay, matched it, no longer treat as hidden */ + _LOGD (LOGD_WIFI, "matched hidden AP %s => '%s'", + nm_wifi_ap_get_address (ap), nm_utils_escape_ssid (ssid->data, ssid->len)); + } else { + /* Didn't have an entry for this AP in the database */ + _LOGD (LOGD_WIFI, "failed to match hidden AP %s", + nm_wifi_ap_get_address (ap)); + } + } + + ap_add_remove (self, ACCESS_POINT_ADDED, ap, TRUE); + _ap_dump (self, ap, "added", 0); + } /* Update the current AP if the supplicant notified a current BSS change * before it sent the current BSS's scan result. @@ -1700,32 +1690,6 @@ supplicant_iface_new_bss_cb (NMSupplicantInterface *iface, schedule_ap_list_dump (self); } -static void -supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface, - const char *object_path, - GVariant *properties, - NMDeviceWifi *self) -{ - NMDeviceState state; - NMWifiAP *ap; - - g_return_if_fail (self != NULL); - g_return_if_fail (object_path != NULL); - g_return_if_fail (properties != NULL); - - /* Ignore new APs when unavailable or unmanaged */ - state = nm_device_get_state (NM_DEVICE (self)); - if (state <= NM_DEVICE_STATE_UNAVAILABLE) - return; - - ap = get_ap_by_supplicant_path (self, object_path); - if (ap) { - _ap_dump (self, ap, "updated", 0); - nm_wifi_ap_update_from_properties (ap, object_path, properties); - schedule_ap_list_dump (self); - } -} - static void supplicant_iface_bss_removed_cb (NMSupplicantInterface *iface, const char *object_path, @@ -1739,19 +1703,20 @@ supplicant_iface_bss_removed_cb (NMSupplicantInterface *iface, priv = NM_DEVICE_WIFI_GET_PRIVATE (self); ap = get_ap_by_supplicant_path (self, object_path); - if (ap) { - if (ap == priv->current_ap) { - /* The current AP cannot be removed (to prevent NM indicating that - * it is connected, but to nothing), but it must be removed later - * when the current AP is changed or cleared. Set 'fake' to - * indicate that this AP is now unknown to the supplicant. - */ - nm_wifi_ap_set_fake (ap, TRUE); - } else { - _ap_dump (self, ap, "removed", 0); - ap_add_remove (self, ACCESS_POINT_REMOVED, ap, TRUE); - schedule_ap_list_dump (self); - } + if (!ap) + return; + + if (ap == priv->current_ap) { + /* The current AP cannot be removed (to prevent NM indicating that + * it is connected, but to nothing), but it must be removed later + * when the current AP is changed or cleared. Set 'fake' to + * indicate that this AP is now unknown to the supplicant. + */ + nm_wifi_ap_set_fake (ap, TRUE); + } else { + _ap_dump (self, ap, "removed", 0); + ap_add_remove (self, ACCESS_POINT_REMOVED, ap, TRUE); + schedule_ap_list_dump (self); } } diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 62cf96be63..2590b1de44 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -62,8 +62,7 @@ typedef struct _AddNetworkData { enum { STATE, /* change in the interface's state */ REMOVED, /* interface was removed by the supplicant */ - NEW_BSS, /* interface saw a new access point from a scan */ - BSS_UPDATED, /* a BSS property changed */ + BSS_UPDATED, /* a new BSS appeared or an existing had properties changed */ BSS_REMOVED, /* supplicant removed BSS from its scan list */ SCAN_DONE, /* wifi scan is complete */ CREDENTIALS_REQUEST, /* 802.1x identity or password requested */ @@ -229,7 +228,7 @@ on_bss_proxy_acquired (GDBusProxy *proxy, GAsyncResult *result, gpointer user_da g_object_set_qdata (G_OBJECT (proxy), bss_proxy_inited_quark (), GUINT_TO_POINTER (TRUE)); - g_signal_emit (self, signals[NEW_BSS], 0, + g_signal_emit (self, signals[BSS_UPDATED], 0, g_dbus_proxy_get_object_path (proxy), g_variant_ref_sink (props)); } @@ -577,13 +576,13 @@ wpas_iface_scan_done (GDBusProxy *proxy, /* Cache last scan completed time */ priv->last_scan = nm_utils_get_monotonic_timestamp_s (); - /* Emit NEW_BSS so that wifi device has the APs (in case it removed them) */ + /* Emit BSS_UPDATED so that wifi device has the APs (in case it removed them) */ g_hash_table_iter_init (&iter, priv->bss_proxies); while (g_hash_table_iter_next (&iter, (gpointer) &bss_path, (gpointer) &bss_proxy)) { if (g_object_get_qdata (G_OBJECT (bss_proxy), bss_proxy_inited_quark ())) { props = _get_bss_proxy_properties (self, bss_proxy); if (props) { - g_signal_emit (self, signals[NEW_BSS], 0, + g_signal_emit (self, signals[BSS_UPDATED], 0, bss_path, g_variant_ref_sink (props)); g_variant_unref (props); @@ -1634,14 +1633,6 @@ nm_supplicant_interface_class_init (NMSupplicantInterfaceClass *klass) NULL, NULL, NULL, G_TYPE_NONE, 0); - signals[NEW_BSS] = - g_signal_new (NM_SUPPLICANT_INTERFACE_NEW_BSS, - G_OBJECT_CLASS_TYPE (object_class), - G_SIGNAL_RUN_LAST, - 0, - NULL, NULL, NULL, - G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_VARIANT); - signals[BSS_UPDATED] = g_signal_new (NM_SUPPLICANT_INTERFACE_BSS_UPDATED, G_OBJECT_CLASS_TYPE (object_class), diff --git a/src/supplicant/nm-supplicant-interface.h b/src/supplicant/nm-supplicant-interface.h index 97ee38e689..d60d4a544c 100644 --- a/src/supplicant/nm-supplicant-interface.h +++ b/src/supplicant/nm-supplicant-interface.h @@ -64,7 +64,6 @@ typedef enum { /* Signals */ #define NM_SUPPLICANT_INTERFACE_STATE "state" #define NM_SUPPLICANT_INTERFACE_REMOVED "removed" -#define NM_SUPPLICANT_INTERFACE_NEW_BSS "new-bss" #define NM_SUPPLICANT_INTERFACE_BSS_UPDATED "bss-updated" #define NM_SUPPLICANT_INTERFACE_BSS_REMOVED "bss-removed" #define NM_SUPPLICANT_INTERFACE_SCAN_DONE "scan-done" From e3a489180b83d55c796d2162eecae01b7351327a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 11:50:59 +0100 Subject: [PATCH 07/13] wifi: check for invalid BSSID in nm_wifi_ap_update_from_properties() In nm_wifi_ap_new_from_properties(), we checked that the BSSID is valid and bailed out otherwise. Since we call nm_wifi_ap_update_from_properties() on a created BSSID, we should ensure there too that an update does not cause the address to become invalid. In the unlikely case where an update would change a previously valid address to an invalid one, we would ignore the update. Thus, move the check for addresses inside nm_wifi_ap_update_from_properties(). --- src/devices/wifi/nm-wifi-ap.c | 44 +++++++++++++++++------------------ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index 96e9d19c5f..8c041374fa 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -29,6 +29,7 @@ #include "NetworkManagerUtils.h" #include "nm-utils.h" #include "nm-core-internal.h" +#include "platform/nm-platform.h" #include "nm-setting-wireless.h" @@ -202,10 +203,24 @@ nm_wifi_ap_get_address (const NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->address; } +static void +nm_wifi_ap_set_address_bin (NMWifiAP *ap, const guint8 *addr /* ETH_ALEN bytes */) +{ + NMWifiAPPrivate *priv; + + priv = NM_WIFI_AP_GET_PRIVATE (ap); + + if ( !priv->address + || !nm_utils_hwaddr_matches (addr, ETH_ALEN, priv->address, -1)) { + g_free (priv->address); + priv->address = nm_utils_hwaddr_ntoa (addr, ETH_ALEN); + _notify (ap, PROP_HW_ADDRESS); + } +} + void nm_wifi_ap_set_address (NMWifiAP *ap, const char *addr) { - NMWifiAPPrivate *priv; guint8 addr_buf[ETH_ALEN]; g_return_if_fail (NM_IS_WIFI_AP (ap)); @@ -213,14 +228,7 @@ nm_wifi_ap_set_address (NMWifiAP *ap, const char *addr) || !nm_utils_hwaddr_aton (addr, addr_buf, sizeof (addr_buf))) g_return_if_reached (); - priv = NM_WIFI_AP_GET_PRIVATE (ap); - - if ( !priv->address - || !nm_utils_hwaddr_matches (addr_buf, sizeof (addr_buf), priv->address, -1)) { - g_free (priv->address); - priv->address = nm_utils_hwaddr_ntoa (addr_buf, sizeof (addr_buf)); - _notify (ap, PROP_HW_ADDRESS); - } + nm_wifi_ap_set_address_bin (ap, addr_buf); } NM80211Mode @@ -406,7 +414,6 @@ nm_wifi_ap_update_from_properties (NMWifiAP *ap, GVariant *properties) { NMWifiAPPrivate *priv; - char *addr; const guint8 *bytes; GVariant *v; gsize len; @@ -454,11 +461,10 @@ nm_wifi_ap_update_from_properties (NMWifiAP *ap, v = g_variant_lookup_value (properties, "BSSID", G_VARIANT_TYPE_BYTESTRING); if (v) { bytes = g_variant_get_fixed_array (v, &len, 1); - if (len == ETH_ALEN) { - addr = nm_utils_hwaddr_ntoa (bytes, len); - nm_wifi_ap_set_address (ap, addr); - g_free (addr); - } + if ( len == ETH_ALEN + && memcmp (bytes, nm_ip_addr_zero.addr_eth, ETH_ALEN) != 0 + && memcmp (bytes, (char[ETH_ALEN]) { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }, ETH_ALEN) != 0) + nm_wifi_ap_set_address_bin (ap, bytes); g_variant_unref (v); } @@ -793,10 +799,7 @@ nm_wifi_ap_init (NMWifiAP *ap) NMWifiAP * nm_wifi_ap_new_from_properties (const char *supplicant_path, GVariant *properties) { - const char bad_bssid1[ETH_ALEN] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }; - const char bad_bssid2[ETH_ALEN] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }; NMWifiAP *ap; - const char *addr; g_return_val_if_fail (supplicant_path != NULL, NULL); g_return_val_if_fail (properties != NULL, NULL); @@ -805,10 +808,7 @@ nm_wifi_ap_new_from_properties (const char *supplicant_path, GVariant *propertie nm_wifi_ap_update_from_properties (ap, supplicant_path, properties); /* ignore APs with invalid or missing BSSIDs */ - addr = nm_wifi_ap_get_address (ap); - if ( !addr - || nm_utils_hwaddr_matches (addr, -1, bad_bssid1, ETH_ALEN) - || nm_utils_hwaddr_matches (addr, -1, bad_bssid2, ETH_ALEN)) { + if (!nm_wifi_ap_get_address (ap)) { g_object_unref (ap); return NULL; } From e0f96770188eeaada70a299bd6dab7a50ec34a53 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 11:05:47 +0100 Subject: [PATCH 08/13] supplicant: delay SCAN_DONE signal until all BSS are initialized We initialize the BSS asyncronously. Don't declare SCAN_DONE until all BSS are up. Otherwise, especially during the very first scan we declare SCAN_DONE when having no BSS yet. This wrongly removes the pending action "wifi-scan", while "autoconnect" cannot happen as there are not BSS yet. Thus we declare "startup-complete" too early. Another issue is that we may start autoconnecting with an incomplete scan list, and thus pick a non-preferred connections. https://bugzilla.gnome.org/show_bug.cgi?id=777831 --- src/supplicant/nm-supplicant-interface.c | 170 +++++++++++++++-------- 1 file changed, 114 insertions(+), 56 deletions(-) diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 2590b1de44..9376037652 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -37,10 +37,13 @@ #define WPAS_ERROR_INVALID_IFACE WPAS_DBUS_INTERFACE ".InvalidInterface" #define WPAS_ERROR_EXISTS_ERROR WPAS_DBUS_INTERFACE ".InterfaceExists" -static NM_CACHED_QUARK_FCN ("bss-proxy-inited", bss_proxy_inited_quark) - /*****************************************************************************/ +typedef struct { + GDBusProxy *proxy; + gulong change_id; +} BssData; + struct _AddNetworkData; typedef struct { @@ -92,7 +95,10 @@ typedef struct { NMSupplicantInterfaceState state; int disconnect_reason; - gboolean scanning; + gboolean scanning:1; + + bool scan_done_pending:1; + bool scan_done_success:1; GDBusProxy * wpas_proxy; GCancellable * init_cancellable; @@ -144,6 +150,10 @@ G_DEFINE_TYPE (NMSupplicantInterface, nm_supplicant_interface, G_TYPE_OBJECT) /*****************************************************************************/ +static void scan_done_emit_signal (NMSupplicantInterface *self); + +/*****************************************************************************/ + NM_UTILS_LOOKUP_STR_DEFINE (nm_supplicant_interface_state_to_string, NMSupplicantInterfaceState, NM_UTILS_LOOKUP_DEFAULT_WARN ("unknown"), NM_UTILS_LOOKUP_STR_ITEM (NM_SUPPLICANT_INTERFACE_STATE_INVALID, "invalid"), @@ -166,10 +176,20 @@ NM_UTILS_LOOKUP_STR_DEFINE (nm_supplicant_interface_state_to_string, NMSupplican /*****************************************************************************/ static void -bss_props_changed_cb (GDBusProxy *proxy, - GVariant *changed_properties, - char **invalidated_properties, - gpointer user_data) +bss_data_destroy (gpointer user_data) +{ + BssData *bss_data = user_data; + + nm_clear_g_signal_handler (bss_data->proxy, &bss_data->change_id); + g_object_unref (bss_data->proxy); + g_slice_free (BssData, bss_data); +} + +static void +bss_proxy_properties_changed_cb (GDBusProxy *proxy, + GVariant *changed_properties, + char **invalidated_properties, + gpointer user_data) { NMSupplicantInterface *self = NM_SUPPLICANT_INTERFACE (user_data); NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); @@ -183,61 +203,73 @@ bss_props_changed_cb (GDBusProxy *proxy, } static GVariant * -_get_bss_proxy_properties (NMSupplicantInterface *self, GDBusProxy *proxy) +bss_proxy_get_properties (NMSupplicantInterface *self, GDBusProxy *proxy) { gs_strfreev char **properties = NULL; GVariantBuilder builder; char **iter; iter = properties = g_dbus_proxy_get_cached_property_names (proxy); - if (!iter) - return NULL; g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}")); - while (*iter) { - GVariant *copy = g_dbus_proxy_get_cached_property (proxy, *iter); + if (iter) { + while (*iter) { + GVariant *copy = g_dbus_proxy_get_cached_property (proxy, *iter); - g_variant_builder_add (&builder, "{sv}", *iter++, copy); - g_variant_unref (copy); + g_variant_builder_add (&builder, "{sv}", *iter++, copy); + g_variant_unref (copy); + } } - return g_variant_builder_end (&builder); } static void -on_bss_proxy_acquired (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) +bss_proxy_acquired_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) { NMSupplicantInterface *self; + NMSupplicantInterfacePrivate *priv; gs_free_error GError *error = NULL; - gs_unref_variant GVariant *props = NULL; + GVariant *props = NULL; + const char *object_path; + BssData *bss_data; - if (!g_async_initable_init_finish (G_ASYNC_INITABLE (proxy), result, &error)) { - if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) { - self = NM_SUPPLICANT_INTERFACE (user_data); - _LOGD ("failed to acquire BSS proxy: (%s)", error->message); - g_hash_table_remove (NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)->bss_proxies, - g_dbus_proxy_get_object_path (proxy)); - } + g_async_initable_init_finish (G_ASYNC_INITABLE (proxy), result, &error); + if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + return; + + self = NM_SUPPLICANT_INTERFACE (user_data); + priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); + + if (error) { + _LOGD ("failed to acquire BSS proxy: (%s)", error->message); + g_hash_table_remove (priv->bss_proxies, + g_dbus_proxy_get_object_path (proxy)); return; } - self = NM_SUPPLICANT_INTERFACE (user_data); - props = _get_bss_proxy_properties (self, proxy); - if (!props) + object_path = g_dbus_proxy_get_object_path (proxy); + bss_data = g_hash_table_lookup (priv->bss_proxies, object_path); + if (!bss_data) return; - g_object_set_qdata (G_OBJECT (proxy), bss_proxy_inited_quark (), GUINT_TO_POINTER (TRUE)); + bss_data->change_id = g_signal_connect (proxy, "g-properties-changed", G_CALLBACK (bss_proxy_properties_changed_cb), self); + props = bss_proxy_get_properties (self, proxy); g_signal_emit (self, signals[BSS_UPDATED], 0, g_dbus_proxy_get_object_path (proxy), g_variant_ref_sink (props)); + g_variant_unref (props); + + if (priv->scan_done_pending) + scan_done_emit_signal (self); } static void -handle_new_bss (NMSupplicantInterface *self, const char *object_path) +bss_add_new (NMSupplicantInterface *self, const char *object_path) { NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); GDBusProxy *bss_proxy; + BssData *bss_data; g_return_if_fail (object_path != NULL); @@ -251,17 +283,20 @@ handle_new_bss (NMSupplicantInterface *self, const char *object_path) "g-object-path", object_path, "g-interface-name", WPAS_DBUS_IFACE_BSS, NULL); + bss_data = g_slice_new0 (BssData); + bss_data->proxy = bss_proxy; g_hash_table_insert (priv->bss_proxies, (char *) g_dbus_proxy_get_object_path (bss_proxy), - bss_proxy); - g_signal_connect (bss_proxy, "g-properties-changed", G_CALLBACK (bss_props_changed_cb), self); + bss_data); g_async_initable_init_async (G_ASYNC_INITABLE (bss_proxy), G_PRIORITY_DEFAULT, priv->other_cancellable, - (GAsyncReadyCallback) on_bss_proxy_acquired, + (GAsyncReadyCallback) bss_proxy_acquired_cb, self); } +/*****************************************************************************/ + static void set_state (NMSupplicantInterface *self, NMSupplicantInterfaceState new_state) { @@ -561,6 +596,42 @@ iface_introspect_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data iface_check_ready (self); } +static void +scan_done_emit_signal (NMSupplicantInterface *self) +{ + NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); + const char *object_path; + BssData *bss_data; + gboolean success; + GHashTableIter iter; + + g_hash_table_iter_init (&iter, priv->bss_proxies); + while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &bss_data)) { + /* we have some BSS' that need to be initialized first. Delay + * emitting signal. */ + if (!bss_data->change_id) { + priv->scan_done_pending = TRUE; + return; + } + } + + /* Emit BSS_UPDATED so that wifi device has the APs (in case it removed them) */ + g_hash_table_iter_init (&iter, priv->bss_proxies); + while (g_hash_table_iter_next (&iter, (gpointer *) &object_path, (gpointer *) &bss_data)) { + gs_unref_variant GVariant *props = NULL; + + props = bss_proxy_get_properties (self, bss_data->proxy); + g_signal_emit (self, signals[BSS_UPDATED], 0, + object_path, + g_variant_ref_sink (props)); + } + + success = priv->scan_done_success; + priv->scan_done_success = FALSE; + priv->scan_done_pending = FALSE; + g_signal_emit (self, signals[SCAN_DONE], 0, success); +} + static void wpas_iface_scan_done (GDBusProxy *proxy, gboolean success, @@ -568,29 +639,11 @@ wpas_iface_scan_done (GDBusProxy *proxy, { NMSupplicantInterface *self = NM_SUPPLICANT_INTERFACE (user_data); NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - GVariant *props; - GHashTableIter iter; - char *bss_path; - GDBusProxy *bss_proxy; /* Cache last scan completed time */ priv->last_scan = nm_utils_get_monotonic_timestamp_s (); - - /* Emit BSS_UPDATED so that wifi device has the APs (in case it removed them) */ - g_hash_table_iter_init (&iter, priv->bss_proxies); - while (g_hash_table_iter_next (&iter, (gpointer) &bss_path, (gpointer) &bss_proxy)) { - if (g_object_get_qdata (G_OBJECT (bss_proxy), bss_proxy_inited_quark ())) { - props = _get_bss_proxy_properties (self, bss_proxy); - if (props) { - g_signal_emit (self, signals[BSS_UPDATED], 0, - bss_path, - g_variant_ref_sink (props)); - g_variant_unref (props); - } - } - } - - g_signal_emit (self, signals[SCAN_DONE], 0, success); + priv->scan_done_success |= success; + scan_done_emit_signal (self); } static void @@ -605,7 +658,7 @@ wpas_iface_bss_added (GDBusProxy *proxy, if (priv->scanning) priv->last_scan = nm_utils_get_monotonic_timestamp_s (); - handle_new_bss (self, path); + bss_add_new (self, path); } static void @@ -615,9 +668,14 @@ wpas_iface_bss_removed (GDBusProxy *proxy, { NMSupplicantInterface *self = NM_SUPPLICANT_INTERFACE (user_data); NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); + BssData *bss_data; + bss_data = g_hash_table_lookup (priv->bss_proxies, path); + if (!bss_data) + return; + g_hash_table_steal (priv->bss_proxies, path); g_signal_emit (self, signals[BSS_REMOVED], 0, path); - g_hash_table_remove (priv->bss_proxies, path); + bss_data_destroy (bss_data); } static void @@ -665,7 +723,7 @@ props_changed_cb (GDBusProxy *proxy, if (g_variant_lookup (changed_properties, "BSSs", "^a&o", &array)) { iter = array; while (*iter) - handle_new_bss (self, *iter++); + bss_add_new (self, *iter++); g_free (array); } @@ -1517,7 +1575,7 @@ nm_supplicant_interface_init (NMSupplicantInterface * self) NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); priv->state = NM_SUPPLICANT_INTERFACE_STATE_INIT; - priv->bss_proxies = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_object_unref); + priv->bss_proxies = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, bss_data_destroy); } NMSupplicantInterface * From c9dc0eba65565d361e32200894386da5e5c2d001 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 13:20:06 +0100 Subject: [PATCH 09/13] wifi: only react on AP update signal when there are actual changes Since we emit BSS_UPDATED signal before SCAN_DONE, it is very likely that nothing actually changed. This clutters the logs with update messages. Also move the added/removed logging messages inside ap_add_remove(). We would call ap_add_remove() at several places without logging the change. --- src/devices/wifi/nm-device-wifi.c | 29 ++++--- src/devices/wifi/nm-wifi-ap.c | 133 +++++++++++++++++++----------- src/devices/wifi/nm-wifi-ap.h | 14 ++-- 3 files changed, 109 insertions(+), 67 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 6af76b598a..1d1eb47d7e 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -444,7 +444,8 @@ periodic_update (NMDeviceWifi *self) /* Smooth out the strength to work around crappy drivers */ percent = nm_platform_wifi_get_quality (NM_PLATFORM_GET, ifindex); if (percent >= 0 || ++priv->invalid_strength_counter > 3) { - nm_wifi_ap_set_strength (priv->current_ap, (gint8) percent); + if (nm_wifi_ap_set_strength (priv->current_ap, (gint8) percent)) + _ap_dump (self, priv->current_ap, "updated", 0); priv->invalid_strength_counter = 0; } } @@ -477,7 +478,9 @@ ap_add_remove (NMDeviceWifi *self, g_hash_table_insert (priv->aps, (gpointer) nm_exported_object_export ((NMExportedObject *) ap), g_object_ref (ap)); - } + _ap_dump (self, ap, "added", 0); + } else + _ap_dump (self, ap, "removed", 0); g_signal_emit (self, signals[signum], 0, ap); @@ -1648,7 +1651,8 @@ supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface, found_ap = get_ap_by_supplicant_path (self, object_path); if (found_ap) { - nm_wifi_ap_update_from_properties (found_ap, object_path, properties); + if (!nm_wifi_ap_update_from_properties (found_ap, object_path, properties)) + return; _ap_dump (self, found_ap, "updated", 0); } else { gs_unref_object NMWifiAP *ap = NULL; @@ -1678,7 +1682,6 @@ supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface, } ap_add_remove (self, ACCESS_POINT_ADDED, ap, TRUE); - _ap_dump (self, ap, "added", 0); } /* Update the current AP if the supplicant notified a current BSS change @@ -1712,9 +1715,9 @@ supplicant_iface_bss_removed_cb (NMSupplicantInterface *iface, * when the current AP is changed or cleared. Set 'fake' to * indicate that this AP is now unknown to the supplicant. */ - nm_wifi_ap_set_fake (ap, TRUE); + if (nm_wifi_ap_set_fake (ap, TRUE)) + _ap_dump (self, ap, "updated", 0); } else { - _ap_dump (self, ap, "removed", 0); ap_add_remove (self, ACCESS_POINT_REMOVED, ap, TRUE); schedule_ap_list_dump (self); } @@ -2505,7 +2508,8 @@ ensure_hotspot_frequency (NMDeviceWifi *self, if (!freq) freq = (g_strcmp0 (band, "a") == 0) ? 5180 : 2462; - nm_wifi_ap_set_freq (ap, freq); + if (nm_wifi_ap_set_freq (ap, freq)) + _ap_dump (self, ap, "updated", 0); } static void @@ -2854,6 +2858,8 @@ activation_success_handler (NMDevice *device) g_warn_if_fail (priv->current_ap); if (priv->current_ap) { if (nm_wifi_ap_get_fake (priv->current_ap)) { + gboolean ap_changed = FALSE; + /* If the activation AP hasn't been seen by the supplicant in a scan * yet, it will be "fake". This usually happens for Ad-Hoc and * AP-mode connections. Fill in the details from the device itself @@ -2866,13 +2872,16 @@ activation_success_handler (NMDevice *device) if ( nm_platform_wifi_get_bssid (NM_PLATFORM_GET, ifindex, bssid) && nm_ethernet_address_is_valid (bssid, ETH_ALEN)) { bssid_str = nm_utils_hwaddr_ntoa (bssid, ETH_ALEN); - nm_wifi_ap_set_address (priv->current_ap, bssid_str); + ap_changed |= nm_wifi_ap_set_address (priv->current_ap, bssid_str); } } if (!nm_wifi_ap_get_freq (priv->current_ap)) - nm_wifi_ap_set_freq (priv->current_ap, nm_platform_wifi_get_frequency (NM_PLATFORM_GET, ifindex)); + ap_changed |= nm_wifi_ap_set_freq (priv->current_ap, nm_platform_wifi_get_frequency (NM_PLATFORM_GET, ifindex)); if (!nm_wifi_ap_get_max_bitrate (priv->current_ap)) - nm_wifi_ap_set_max_bitrate (priv->current_ap, nm_platform_wifi_get_rate (NM_PLATFORM_GET, ifindex)); + ap_changed |= nm_wifi_ap_set_max_bitrate (priv->current_ap, nm_platform_wifi_get_rate (NM_PLATFORM_GET, ifindex)); + + if (ap_changed) + _ap_dump (self, priv->current_ap, "updated", 0); } nm_active_connection_set_specific_object (NM_ACTIVE_CONNECTION (req), diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index 8c041374fa..b60020ac3c 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -69,8 +69,8 @@ typedef struct { NM80211ApSecurityFlags rsn_flags; /* RSN (WPA2) -related flags */ /* Non-scanned attributes */ - bool fake; /* Whether or not the AP is from a scan */ - bool hotspot; /* Whether the AP is a local device's hotspot network */ + bool fake:1; /* Whether or not the AP is from a scan */ + bool hotspot:1; /* Whether the AP is a local device's hotspot network */ gint32 last_seen; /* Timestamp when the AP was seen lastly (obtained via nm_utils_get_monotonic_timestamp_s()) */ } NMWifiAPPrivate; @@ -123,20 +123,20 @@ const GByteArray * nm_wifi_ap_get_ssid (const NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->ssid; } -void +gboolean nm_wifi_ap_set_ssid (NMWifiAP *ap, const guint8 *ssid, gsize len) { NMWifiAPPrivate *priv; - g_return_if_fail (NM_IS_WIFI_AP (ap)); - g_return_if_fail (ssid == NULL || len > 0); + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); + g_return_val_if_fail (ssid == NULL || len > 0, FALSE); priv = NM_WIFI_AP_GET_PRIVATE (ap); /* same SSID */ if ((ssid && priv->ssid) && (len == priv->ssid->len)) { if (!memcmp (ssid, priv->ssid->data, len)) - return; + return FALSE; } if (priv->ssid) { @@ -150,49 +150,56 @@ nm_wifi_ap_set_ssid (NMWifiAP *ap, const guint8 *ssid, gsize len) } _notify (ap, PROP_SSID); + return TRUE; } -static void +static gboolean nm_wifi_ap_set_flags (NMWifiAP *ap, NM80211ApFlags flags) { NMWifiAPPrivate *priv; - g_return_if_fail (NM_IS_WIFI_AP (ap)); + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); priv = NM_WIFI_AP_GET_PRIVATE (ap); if (priv->flags != flags) { priv->flags = flags; _notify (ap, PROP_FLAGS); + return TRUE; } + return FALSE; } -static void +static gboolean nm_wifi_ap_set_wpa_flags (NMWifiAP *ap, NM80211ApSecurityFlags flags) { NMWifiAPPrivate *priv; - g_return_if_fail (NM_IS_WIFI_AP (ap)); + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); priv = NM_WIFI_AP_GET_PRIVATE (ap); if (priv->wpa_flags != flags) { priv->wpa_flags = flags; _notify (ap, PROP_WPA_FLAGS); + return TRUE; } + return FALSE; } -static void +static gboolean nm_wifi_ap_set_rsn_flags (NMWifiAP *ap, NM80211ApSecurityFlags flags) { NMWifiAPPrivate *priv; - g_return_if_fail (NM_IS_WIFI_AP (ap)); + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); priv = NM_WIFI_AP_GET_PRIVATE (ap); if (priv->rsn_flags != flags) { priv->rsn_flags = flags; _notify (ap, PROP_RSN_FLAGS); + return TRUE; } + return FALSE; } const char * @@ -203,7 +210,7 @@ nm_wifi_ap_get_address (const NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->address; } -static void +static gboolean nm_wifi_ap_set_address_bin (NMWifiAP *ap, const guint8 *addr /* ETH_ALEN bytes */) { NMWifiAPPrivate *priv; @@ -215,20 +222,22 @@ nm_wifi_ap_set_address_bin (NMWifiAP *ap, const guint8 *addr /* ETH_ALEN bytes * g_free (priv->address); priv->address = nm_utils_hwaddr_ntoa (addr, ETH_ALEN); _notify (ap, PROP_HW_ADDRESS); + return TRUE; } + return FALSE; } -void +gboolean nm_wifi_ap_set_address (NMWifiAP *ap, const char *addr) { guint8 addr_buf[ETH_ALEN]; - g_return_if_fail (NM_IS_WIFI_AP (ap)); + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); if ( !addr || !nm_utils_hwaddr_aton (addr, addr_buf, sizeof (addr_buf))) - g_return_if_reached (); + g_return_val_if_reached (FALSE); - nm_wifi_ap_set_address_bin (ap, addr_buf); + return nm_wifi_ap_set_address_bin (ap, addr_buf); } NM80211Mode @@ -239,21 +248,23 @@ nm_wifi_ap_get_mode (NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->mode; } -static void +static gboolean nm_wifi_ap_set_mode (NMWifiAP *ap, const NM80211Mode mode) { NMWifiAPPrivate *priv; - g_return_if_fail (NM_IS_WIFI_AP (ap)); - g_return_if_fail ( mode == NM_802_11_MODE_ADHOC - || mode == NM_802_11_MODE_INFRA); + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); + g_return_val_if_fail ( mode == NM_802_11_MODE_ADHOC + || mode == NM_802_11_MODE_INFRA, FALSE); priv = NM_WIFI_AP_GET_PRIVATE (ap); if (priv->mode != mode) { priv->mode = mode; _notify (ap, PROP_MODE); + return TRUE; } + return FALSE; } gboolean @@ -272,19 +283,21 @@ nm_wifi_ap_get_strength (NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->strength; } -void +gboolean nm_wifi_ap_set_strength (NMWifiAP *ap, const gint8 strength) { NMWifiAPPrivate *priv; - g_return_if_fail (NM_IS_WIFI_AP (ap)); + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); priv = NM_WIFI_AP_GET_PRIVATE (ap); if (priv->strength != strength) { priv->strength = strength; _notify (ap, PROP_STRENGTH); + return TRUE; } + return FALSE; } guint32 @@ -295,20 +308,22 @@ nm_wifi_ap_get_freq (NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->freq; } -void +gboolean nm_wifi_ap_set_freq (NMWifiAP *ap, const guint32 freq) { NMWifiAPPrivate *priv; - g_return_if_fail (NM_IS_WIFI_AP (ap)); + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); priv = NM_WIFI_AP_GET_PRIVATE (ap); if (priv->freq != freq) { priv->freq = freq; _notify (ap, PROP_FREQUENCY); + return TRUE; } + return FALSE; } guint32 @@ -320,19 +335,21 @@ nm_wifi_ap_get_max_bitrate (NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->max_bitrate; } -void +gboolean nm_wifi_ap_set_max_bitrate (NMWifiAP *ap, guint32 bitrate) { NMWifiAPPrivate *priv; - g_return_if_fail (NM_IS_WIFI_AP (ap)); + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); priv = NM_WIFI_AP_GET_PRIVATE (ap); if (priv->max_bitrate != bitrate) { priv->max_bitrate = bitrate; _notify (ap, PROP_MAX_BITRATE); + return TRUE; } + return FALSE; } gboolean @@ -343,27 +360,37 @@ nm_wifi_ap_get_fake (const NMWifiAP *ap) return NM_WIFI_AP_GET_PRIVATE (ap)->fake; } -void +gboolean nm_wifi_ap_set_fake (NMWifiAP *ap, gboolean fake) { - g_return_if_fail (NM_IS_WIFI_AP (ap)); + NMWifiAPPrivate *priv; - NM_WIFI_AP_GET_PRIVATE (ap)->fake = fake; + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); + + priv = NM_WIFI_AP_GET_PRIVATE (ap); + + if (priv->fake != !!fake) { + priv->fake = fake; + return TRUE; + } + return FALSE; } -static void +static gboolean nm_wifi_ap_set_last_seen (NMWifiAP *ap, gint32 last_seen) { NMWifiAPPrivate *priv; - g_return_if_fail (NM_IS_WIFI_AP (ap)); + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); priv = NM_WIFI_AP_GET_PRIVATE (ap); if (priv->last_seen != last_seen) { priv->last_seen = last_seen; _notify (ap, PROP_LAST_SEEN); + return TRUE; } + return FALSE; } /*****************************************************************************/ @@ -408,7 +435,7 @@ security_from_vardict (GVariant *security) return flags; } -void +gboolean nm_wifi_ap_update_from_properties (NMWifiAP *ap, const char *supplicant_path, GVariant *properties) @@ -421,28 +448,30 @@ nm_wifi_ap_update_from_properties (NMWifiAP *ap, const char *s; gint16 i16; guint16 u16; + gboolean changed = FALSE; + + g_return_val_if_fail (NM_IS_WIFI_AP (ap), FALSE); + g_return_val_if_fail (properties, FALSE); - g_return_if_fail (ap != NULL); - g_return_if_fail (properties != NULL); priv = NM_WIFI_AP_GET_PRIVATE (ap); g_object_freeze_notify (G_OBJECT (ap)); if (g_variant_lookup (properties, "Privacy", "b", &b) && b) - nm_wifi_ap_set_flags (ap, priv->flags | NM_802_11_AP_FLAGS_PRIVACY); + changed |= nm_wifi_ap_set_flags (ap, priv->flags | NM_802_11_AP_FLAGS_PRIVACY); if (g_variant_lookup (properties, "Mode", "&s", &s)) { if (!g_strcmp0 (s, "infrastructure")) - nm_wifi_ap_set_mode (ap, NM_802_11_MODE_INFRA); + changed |= nm_wifi_ap_set_mode (ap, NM_802_11_MODE_INFRA); else if (!g_strcmp0 (s, "ad-hoc")) - nm_wifi_ap_set_mode (ap, NM_802_11_MODE_ADHOC); + changed |= nm_wifi_ap_set_mode (ap, NM_802_11_MODE_ADHOC); } if (g_variant_lookup (properties, "Signal", "n", &i16)) - nm_wifi_ap_set_strength (ap, nm_wifi_utils_level_to_quality (i16)); + changed |= nm_wifi_ap_set_strength (ap, nm_wifi_utils_level_to_quality (i16)); if (g_variant_lookup (properties, "Frequency", "q", &u16)) - nm_wifi_ap_set_freq (ap, u16); + changed |= nm_wifi_ap_set_freq (ap, u16); v = g_variant_lookup_value (properties, "SSID", G_VARIANT_TYPE_BYTESTRING); if (v) { @@ -453,7 +482,7 @@ nm_wifi_ap_update_from_properties (NMWifiAP *ap, if ( bytes && len && !(((len == 8) || (len == 9)) && !memcmp (bytes, "", 8)) && !nm_utils_is_empty_ssid (bytes, len)) - nm_wifi_ap_set_ssid (ap, bytes, len); + changed |= nm_wifi_ap_set_ssid (ap, bytes, len); g_variant_unref (v); } @@ -464,7 +493,7 @@ nm_wifi_ap_update_from_properties (NMWifiAP *ap, if ( len == ETH_ALEN && memcmp (bytes, nm_ip_addr_zero.addr_eth, ETH_ALEN) != 0 && memcmp (bytes, (char[ETH_ALEN]) { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF }, ETH_ALEN) != 0) - nm_wifi_ap_set_address_bin (ap, bytes); + changed |= nm_wifi_ap_set_address_bin (ap, bytes); g_variant_unref (v); } @@ -476,33 +505,37 @@ nm_wifi_ap_update_from_properties (NMWifiAP *ap, /* Find the max AP rate */ for (i = 0; i < len; i++) { - if (rates[i] > maxrate) { + if (rates[i] > maxrate) maxrate = rates[i]; - nm_wifi_ap_set_max_bitrate (ap, rates[i] / 1000); - } } + if (maxrate) + changed |= nm_wifi_ap_set_max_bitrate (ap, maxrate / 1000); g_variant_unref (v); } v = g_variant_lookup_value (properties, "WPA", G_VARIANT_TYPE_VARDICT); if (v) { - nm_wifi_ap_set_wpa_flags (ap, priv->wpa_flags | security_from_vardict (v)); + changed |= nm_wifi_ap_set_wpa_flags (ap, priv->wpa_flags | security_from_vardict (v)); g_variant_unref (v); } v = g_variant_lookup_value (properties, "RSN", G_VARIANT_TYPE_VARDICT); if (v) { - nm_wifi_ap_set_rsn_flags (ap, priv->rsn_flags | security_from_vardict (v)); + changed |= nm_wifi_ap_set_rsn_flags (ap, priv->rsn_flags | security_from_vardict (v)); g_variant_unref (v); } - if (!priv->supplicant_path) + if (!priv->supplicant_path) { priv->supplicant_path = g_strdup (supplicant_path); + changed = TRUE; + } - nm_wifi_ap_set_last_seen (ap, nm_utils_get_monotonic_timestamp_s ()); - priv->fake = FALSE; + changed |= nm_wifi_ap_set_last_seen (ap, nm_utils_get_monotonic_timestamp_s ()); + changed |= nm_wifi_ap_set_fake (ap, FALSE); g_object_thaw_notify (G_OBJECT (ap)); + + return changed; } static gboolean diff --git a/src/devices/wifi/nm-wifi-ap.h b/src/devices/wifi/nm-wifi-ap.h index a68aece8a8..5e64087cca 100644 --- a/src/devices/wifi/nm-wifi-ap.h +++ b/src/devices/wifi/nm-wifi-ap.h @@ -53,7 +53,7 @@ NMWifiAP * nm_wifi_ap_new_from_properties (const char *supplicant_path, GVariant *properties); NMWifiAP * nm_wifi_ap_new_fake_from_connection (NMConnection *connection); -void nm_wifi_ap_update_from_properties (NMWifiAP *ap, +gboolean nm_wifi_ap_update_from_properties (NMWifiAP *ap, const char *supplicant_path, GVariant *properties); @@ -68,25 +68,25 @@ gboolean nm_wifi_ap_complete_connection (NMWifiAP *self, const char * nm_wifi_ap_get_supplicant_path (NMWifiAP *ap); guint64 nm_wifi_ap_get_id (NMWifiAP *ap); const GByteArray *nm_wifi_ap_get_ssid (const NMWifiAP *ap); -void nm_wifi_ap_set_ssid (NMWifiAP *ap, +gboolean nm_wifi_ap_set_ssid (NMWifiAP *ap, const guint8 *ssid, gsize len); const char * nm_wifi_ap_get_address (const NMWifiAP *ap); -void nm_wifi_ap_set_address (NMWifiAP *ap, +gboolean nm_wifi_ap_set_address (NMWifiAP *ap, const char *addr); NM80211Mode nm_wifi_ap_get_mode (NMWifiAP *ap); gboolean nm_wifi_ap_is_hotspot (NMWifiAP *ap); gint8 nm_wifi_ap_get_strength (NMWifiAP *ap); -void nm_wifi_ap_set_strength (NMWifiAP *ap, +gboolean nm_wifi_ap_set_strength (NMWifiAP *ap, gint8 strength); guint32 nm_wifi_ap_get_freq (NMWifiAP *ap); -void nm_wifi_ap_set_freq (NMWifiAP *ap, +gboolean nm_wifi_ap_set_freq (NMWifiAP *ap, guint32 freq); guint32 nm_wifi_ap_get_max_bitrate (NMWifiAP *ap); -void nm_wifi_ap_set_max_bitrate (NMWifiAP *ap, +gboolean nm_wifi_ap_set_max_bitrate (NMWifiAP *ap, guint32 bitrate); gboolean nm_wifi_ap_get_fake (const NMWifiAP *ap); -void nm_wifi_ap_set_fake (NMWifiAP *ap, +gboolean nm_wifi_ap_set_fake (NMWifiAP *ap, gboolean fake); const char *nm_wifi_ap_to_string (const NMWifiAP *self, From 257484e7bac2813d0cf1bc922ef57e6b781b1c96 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 13:26:54 +0100 Subject: [PATCH 10/13] wifi: also show the NM D-Bus path for the Wi-Fi AP in nm_wifi_ap_to_string() --- src/devices/wifi/nm-wifi-ap.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index b60020ac3c..7de0838f97 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -622,6 +622,7 @@ nm_wifi_ap_to_string (const NMWifiAP *self, { const NMWifiAPPrivate *priv; const char *supplicant_id = "-"; + const char *export_path; guint32 chan; char b1[200]; @@ -630,10 +631,16 @@ nm_wifi_ap_to_string (const NMWifiAP *self, priv = NM_WIFI_AP_GET_PRIVATE (self); chan = nm_utils_wifi_freq_to_channel (priv->freq); if (priv->supplicant_path) - supplicant_id = strrchr (priv->supplicant_path, '/'); + supplicant_id = strrchr (priv->supplicant_path, '/') ?: supplicant_id; + + export_path = nm_exported_object_get_path (NM_EXPORTED_OBJECT (self)); + if (export_path) + export_path = strrchr (export_path, '/') ?: export_path; + else + export_path = "/"; g_snprintf (str_buf, buf_len, - "%17s %-32s [ %c %3u %3u%% %c W:%04X R:%04X ] %3us %s", + "%17s %-32s [ %c %3u %3u%% %c W:%04X R:%04X ] %3us sup:%s [nm:%s]", priv->address ?: "(none)", nm_sprintf_buf (b1, "%s%s%s", NM_PRINT_FMT_QUOTED (priv->ssid, "\"", nm_utils_escape_ssid (priv->ssid->data, priv->ssid->len), "\"", "(none)")), @@ -650,7 +657,8 @@ nm_wifi_ap_to_string (const NMWifiAP *self, priv->wpa_flags & 0xFFFF, priv->rsn_flags & 0xFFFF, priv->last_seen > 0 ? ((now_s > 0 ? now_s : nm_utils_get_monotonic_timestamp_s ()) - priv->last_seen) : -1, - supplicant_id); + supplicant_id, + export_path); return str_buf; } From 2f9166e6b9c18af4801fd5cc6c01f7eaaeb9d538 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Feb 2017 13:57:57 +0100 Subject: [PATCH 11/13] device: separately handle NMDevice's autoconnect by user and internal decision The NMDevice's autoconnect property is settable via D-Bus and is is also modified by internal decision, like when no PIN is available. Certain internal actions cause clearing the internal autoconnect flag, but they should not override the user desicion. For example, when NM awaks from sleep it would reenable autoconnect, but it should not reenable it for devices where the user explicitly said that autoconnect is to be disabled. Similarly, activating a device alone is not yet an instruction to re-enable autoconnect. If the user consciously disables autoconnect, it should stay enabled. On the other hand, activating a device should reenable autoconnect if it was blocked by internal decision. We need to track these two flags separately, and set them accordingly. --- src/devices/bluetooth/nm-device-bt.c | 2 +- src/devices/nm-device.c | 56 ++++++++++++++++++++-------- src/devices/nm-device.h | 2 +- src/devices/wwan/nm-device-modem.c | 2 +- src/nm-manager.c | 2 +- 5 files changed, 44 insertions(+), 20 deletions(-) diff --git a/src/devices/bluetooth/nm-device-bt.c b/src/devices/bluetooth/nm-device-bt.c index 31b9bbf927..12660701f5 100644 --- a/src/devices/bluetooth/nm-device-bt.c +++ b/src/devices/bluetooth/nm-device-bt.c @@ -483,8 +483,8 @@ modem_prepare_result (NMModem *modem, * the device to be auto-activated anymore, which would risk locking * the SIM if the incorrect PIN continues to be used. */ - nm_device_set_autoconnect (device, FALSE); _LOGI (LOGD_MB, "disabling autoconnect due to failed SIM PIN"); + nm_device_set_autoconnect_intern (device, FALSE); } nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, reason); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 46a1cfd48c..d063bdf607 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -416,7 +416,8 @@ typedef struct _NMDevicePrivate { gboolean needs_ip6_subnet; /* allow autoconnect feature */ - bool autoconnect; + bool autoconnect_intern:1; + bool autoconnect_user:1; /* master interface for bridge/bond/team slave */ NMDevice * master; @@ -482,6 +483,9 @@ static NMActStageReturn linklocal6_start (NMDevice *self); static void _carrier_wait_check_queued_act_request (NMDevice *self); +static void nm_device_set_autoconnect_both (NMDevice *self, gboolean autoconnect); +static void nm_device_set_autoconnect_full (NMDevice *self, int autoconnect_intern, int autoconnect_user); + static const char *_activation_func_to_string (ActivationHandleFunc func); static void activation_source_handle_cb (NMDevice *self, int family); @@ -2652,6 +2656,8 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink) if (real_rate) priv->stats.timeout_id = g_timeout_add (real_rate, _stats_timeout_cb, self); + nm_device_set_autoconnect_full (self, !!DEFAULT_AUTOCONNECT, TRUE); + klass->realize_start_notify (self, plink); /* Do not manage externally created software devices until they are IFF_UP @@ -2846,7 +2852,7 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error) priv->real = FALSE; _notify (self, PROP_REAL); - nm_device_set_autoconnect (self, DEFAULT_AUTOCONNECT); + nm_device_set_autoconnect_both (self, FALSE); g_object_thaw_notify (G_OBJECT (self)); @@ -3398,25 +3404,44 @@ nm_device_set_enabled (NMDevice *self, gboolean enabled) gboolean nm_device_get_autoconnect (NMDevice *self) { + NMDevicePrivate *priv; + g_return_val_if_fail (NM_IS_DEVICE (self), FALSE); - return NM_DEVICE_GET_PRIVATE (self)->autoconnect; + priv = NM_DEVICE_GET_PRIVATE (self); + return priv->autoconnect_intern && priv->autoconnect_user; } -void -nm_device_set_autoconnect (NMDevice *self, gboolean autoconnect) +static void +nm_device_set_autoconnect_full (NMDevice *self, int autoconnect_intern, int autoconnect_user) { NMDevicePrivate *priv; + gboolean old_value; g_return_if_fail (NM_IS_DEVICE (self)); - autoconnect = !!autoconnect; - priv = NM_DEVICE_GET_PRIVATE (self); - if (priv->autoconnect != autoconnect) { - priv->autoconnect = autoconnect; + + old_value = nm_device_get_autoconnect (self); + if (autoconnect_intern != -1) + priv->autoconnect_intern = autoconnect_intern; + if (autoconnect_user != -1) + priv->autoconnect_user = autoconnect_user; + if (old_value != nm_device_get_autoconnect (self)) _notify (self, PROP_AUTOCONNECT); - } +} + +void +nm_device_set_autoconnect_intern (NMDevice *self, gboolean autoconnect) +{ + nm_device_set_autoconnect_full (self, !!autoconnect, -1); +} + +static void +nm_device_set_autoconnect_both (NMDevice *self, gboolean autoconnect) +{ + autoconnect = !!autoconnect; + nm_device_set_autoconnect_full (self, autoconnect, autoconnect); } static gboolean @@ -3444,7 +3469,7 @@ nm_device_autoconnect_allowed (NMDevice *self) GValue instance = G_VALUE_INIT; GValue retval = G_VALUE_INIT; - if (!priv->autoconnect) + if (!nm_device_get_autoconnect (self)) return FALSE; /* Unrealized devices can always autoconnect. */ @@ -8832,7 +8857,7 @@ disconnect_cb (NMDevice *self, nm_audit_log_device_op (NM_AUDIT_OP_DEVICE_DISCONNECT, self, FALSE, subject, local->message); g_dbus_method_invocation_take_error (context, local); } else { - nm_device_set_autoconnect (self, FALSE); + nm_device_set_autoconnect_intern (self, FALSE); nm_device_state_changed (self, NM_DEVICE_STATE_DEACTIVATING, @@ -11996,7 +12021,7 @@ _set_state_full (NMDevice *self, /* Reset autoconnect flag when the device is activating or connected. */ if ( state >= NM_DEVICE_STATE_PREPARE && state <= NM_DEVICE_STATE_ACTIVATED) - nm_device_set_autoconnect (self, TRUE); + nm_device_set_autoconnect_intern (self, TRUE); _notify (self, PROP_STATE); _notify (self, PROP_STATE_REASON); @@ -12950,7 +12975,6 @@ nm_device_init (NMDevice *self) priv->state_reason = NM_DEVICE_STATE_REASON_NONE; priv->dhcp_timeout = 0; priv->rfkill_type = RFKILL_TYPE_UNKNOWN; - priv->autoconnect = DEFAULT_AUTOCONNECT; priv->unmanaged_flags = NM_UNMANAGED_PLATFORM_INIT; priv->unmanaged_mask = priv->unmanaged_flags; priv->available_connections = g_hash_table_new_full (g_direct_hash, g_direct_equal, g_object_unref, NULL); @@ -13222,7 +13246,7 @@ set_property (GObject *object, guint prop_id, } break; case PROP_AUTOCONNECT: - nm_device_set_autoconnect (self, g_value_get_boolean (value)); + nm_device_set_autoconnect_both (self, g_value_get_boolean (value)); break; case PROP_FIRMWARE_MISSING: /* construct-only */ @@ -13348,7 +13372,7 @@ get_property (GObject *object, guint prop_id, g_value_set_boolean (value, nm_device_get_state (self) > NM_DEVICE_STATE_UNMANAGED); break; case PROP_AUTOCONNECT: - g_value_set_boolean (value, priv->autoconnect); + g_value_set_boolean (value, nm_device_get_autoconnect (self)); break; case PROP_FIRMWARE_MISSING: g_value_set_boolean (value, priv->firmware_missing); diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index e512f2ca7d..1b239c2e04 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -577,7 +577,7 @@ gboolean nm_device_unrealize (NMDevice *device, GError **error); gboolean nm_device_get_autoconnect (NMDevice *device); -void nm_device_set_autoconnect (NMDevice *device, gboolean autoconnect); +void nm_device_set_autoconnect_intern (NMDevice *device, gboolean autoconnect); void nm_device_emit_recheck_auto_activate (NMDevice *device); void nm_device_state_changed (NMDevice *device, diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index 7a70a0bf43..ee263d9ca4 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -128,7 +128,7 @@ modem_prepare_result (NMModem *modem, * the device to be auto-activated anymore, which would risk locking * the SIM if the incorrect PIN continues to be used. */ - nm_device_set_autoconnect (device, FALSE); + nm_device_set_autoconnect_intern (device, FALSE); _LOGI (LOGD_MB, "disabling autoconnect due to failed SIM PIN"); } diff --git a/src/nm-manager.c b/src/nm-manager.c index 0f021fede4..5588552389 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -4276,7 +4276,7 @@ do_sleep_wake (NMManager *self, gboolean sleeping_changed) nm_device_set_enabled (device, enabled); } - nm_device_set_autoconnect (device, TRUE); + nm_device_set_autoconnect_intern (device, TRUE); nm_device_set_unmanaged_by_flags (device, NM_UNMANAGED_SLEEPING, FALSE, NM_DEVICE_STATE_REASON_NOW_MANAGED); } From 6eaded9071fbf868476255adb8ee5f416e7ad134 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Feb 2017 12:45:38 +0100 Subject: [PATCH 12/13] device: add get_autoconnect_allowed() virtual function It allows derived classes to override the autoconnect-allowed state. We already have - NM_DEVICE_AUTOCONNECT property, which is two parts: - NMDevicePrivate::autoconnect_user, which is settable via D-Bus by the use, to allow the device to autoconnect. - NMDevicePrivate::autoconnect_intern, which is set by internal decision. - NM_DEVICE_AUTOCONNECT_ALLOWED signal, where other devices can subscribe to block autoconnect. Currently that is only used by NMDeviceOlpcMesh. These two make up for nm_device_autoconnect_allowed(). Add another way to allow derived classes to disable autoconnect temporarily. This could also be achieved by having the device subscribe to NM_DEVICE_AUTOCONNECT_ALLOWED of self, or by adding a signal slot. But a plain function pointer seems easier. --- src/devices/nm-device.c | 11 ++++++++++- src/devices/nm-device.h | 5 +++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index d063bdf607..e70594d17d 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3444,6 +3444,12 @@ nm_device_set_autoconnect_both (NMDevice *self, gboolean autoconnect) nm_device_set_autoconnect_full (self, autoconnect, autoconnect); } +static gboolean +get_autoconnect_allowed (NMDevice *self) +{ + return TRUE; +} + static gboolean autoconnect_allowed_accumulator (GSignalInvocationHint *ihint, GValue *return_accu, @@ -3466,10 +3472,12 @@ gboolean nm_device_autoconnect_allowed (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + NMDeviceClass *klass = NM_DEVICE_GET_CLASS (self); GValue instance = G_VALUE_INIT; GValue retval = G_VALUE_INIT; - if (!nm_device_get_autoconnect (self)) + if ( !nm_device_get_autoconnect (self) + || !klass->get_autoconnect_allowed (self)) return FALSE; /* Unrealized devices can always autoconnect. */ @@ -13499,6 +13507,7 @@ nm_device_class_init (NMDeviceClass *klass) klass->have_any_ready_slaves = have_any_ready_slaves; klass->get_type_description = get_type_description; + klass->get_autoconnect_allowed = get_autoconnect_allowed; klass->can_auto_connect = can_auto_connect; klass->check_connection_compatible = check_connection_compatible; klass->check_connection_available = check_connection_available; diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 1b239c2e04..eb527cc374 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -243,6 +243,11 @@ typedef struct { void (* set_enabled) (NMDevice *self, gboolean enabled); + /* allow derived classes to override the result of nm_device_autoconnect_allowed(). + * If the value changes, the class should call nm_device_emit_recheck_auto_activate(), + * which emits NM_DEVICE_RECHECK_AUTO_ACTIVATE signal. */ + gboolean (* get_autoconnect_allowed) (NMDevice *self); + gboolean (* can_auto_connect) (NMDevice *self, NMConnection *connection, char **specific_object); From 2ab2254dd7336b9b7baa03ea1eb1f1c72f7ab6a8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Feb 2017 15:10:36 +0100 Subject: [PATCH 13/13] device/wifi: block autoconnect while scanning is in progress We should only start autoconnecting after the scan is complete. Otherwise, we might activate a shared connection or pick a connection based on an incomplete scan list. https://bugzilla.gnome.org/show_bug.cgi?id=770938 --- src/devices/wifi/nm-device-wifi.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 1d1eb47d7e..01621e8bfa 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -295,8 +295,10 @@ _requested_scan_set (NMDeviceWifi *self, gboolean value) priv->requested_scan = value; if (value) nm_device_add_pending_action ((NMDevice *) self, NM_PENDING_ACTION_WIFI_SCAN, TRUE); - else + else { + nm_device_emit_recheck_auto_activate (NM_DEVICE (self)); nm_device_remove_pending_action ((NMDevice *) self, NM_PENDING_ACTION_WIFI_SCAN, TRUE); + } } static void @@ -963,6 +965,18 @@ is_available (NMDevice *device, NMDeviceCheckDevAvailableFlags flags) return TRUE; } +static gboolean +get_autoconnect_allowed (NMDevice *device) +{ + NMDeviceWifiPrivate *priv; + + if (!NM_DEVICE_CLASS (nm_device_wifi_parent_class)->get_autoconnect_allowed (device)) + return FALSE; + + priv = NM_DEVICE_WIFI_GET_PRIVATE (NM_DEVICE_WIFI (device)); + return !priv->requested_scan; +} + static gboolean can_auto_connect (NMDevice *device, NMConnection *connection, @@ -3189,6 +3203,7 @@ nm_device_wifi_class_init (NMDeviceWifiClass *klass) object_class->finalize = finalize; parent_class->can_auto_connect = can_auto_connect; + parent_class->get_autoconnect_allowed = get_autoconnect_allowed; parent_class->is_available = is_available; parent_class->check_connection_compatible = check_connection_compatible; parent_class->check_connection_available = check_connection_available;