From 051fd25fc6feaf56c0fed9c0e9b9ad3c7bd72f7b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Feb 2017 01:30:25 +0100 Subject: [PATCH 01/14] wifi: introduce enum type NMSupplicantInterfaceState instead of plain int Also change the signature of the NM_SUPPLICANT_INTERFACE_STATE signal, to have three "int" type arguments. Thereby also fix the subscribers to this signal that wrongly had type guint32, instead of guint (which happens to be the same underlying type, so no real problem). https://mail.gnome.org/archives/networkmanager-list/2017-February/msg00021.html (cherry picked from commit 5a03de70518bd2f2ed3c6397d09fa9bbfac1608b) --- src/devices/nm-device-ethernet.c | 6 +- src/devices/nm-device-macsec.c | 6 +- src/devices/wifi/nm-device-wifi.c | 40 ++++++------ src/supplicant/nm-supplicant-interface.c | 83 +++++++++--------------- src/supplicant/nm-supplicant-interface.h | 10 +-- 5 files changed, 65 insertions(+), 80 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 5df16df0ea..8dedd7ebfa 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -596,8 +596,8 @@ build_supplicant_config (NMDeviceEthernet *self, static void supplicant_iface_state_cb (NMSupplicantInterface *iface, - guint32 new_state, - guint32 old_state, + int new_state_i, + int old_state_i, int disconnect_reason, gpointer user_data) { @@ -608,6 +608,8 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, gboolean success = FALSE; NMDeviceState devstate; GError *error = NULL; + NMSupplicantInterfaceState new_state = new_state_i; + NMSupplicantInterfaceState old_state = old_state_i; if (new_state == old_state) return; diff --git a/src/devices/nm-device-macsec.c b/src/devices/nm-device-macsec.c index c511a0e721..58ee4fec3f 100644 --- a/src/devices/nm-device-macsec.c +++ b/src/devices/nm-device-macsec.c @@ -412,8 +412,8 @@ time_out: static void supplicant_iface_state_cb (NMSupplicantInterface *iface, - guint32 new_state, - guint32 old_state, + int new_state_i, + int old_state_i, int disconnect_reason, gpointer user_data) { @@ -424,6 +424,8 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, gboolean success = FALSE; NMDeviceState devstate; GError *error = NULL; + NMSupplicantInterfaceState new_state = new_state_i; + NMSupplicantInterfaceState old_state = old_state_i; if (new_state == old_state) return; diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 188ed54eb6..07dc36dc11 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -152,8 +152,8 @@ static void cleanup_association_attempt (NMDeviceWifi * self, gboolean disconnect); static void supplicant_iface_state_cb (NMSupplicantInterface *iface, - guint32 new_state, - guint32 old_state, + int new_state_i, + int old_state_i, int disconnect_reason, gpointer user_data); @@ -428,7 +428,7 @@ periodic_update (NMDeviceWifi *self) guint32 new_rate; int percent; NMDeviceState state; - guint32 supplicant_state; + NMSupplicantInterfaceState supplicant_state; /* BSSID and signal strength have meaningful values only if the device * is activated and not scanning. @@ -955,7 +955,7 @@ is_available (NMDevice *device, NMDeviceCheckDevAvailableFlags flags) { NMDeviceWifi *self = NM_DEVICE_WIFI (device); NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - guint32 state; + NMSupplicantInterfaceState supplicant_state; if (!priv->enabled) return FALSE; @@ -963,9 +963,9 @@ is_available (NMDevice *device, NMDeviceCheckDevAvailableFlags flags) if (!priv->sup_iface) return FALSE; - state = nm_supplicant_interface_get_state (priv->sup_iface); - if ( state < NM_SUPPLICANT_INTERFACE_STATE_READY - || state > NM_SUPPLICANT_INTERFACE_STATE_COMPLETED) + supplicant_state = nm_supplicant_interface_get_state (priv->sup_iface); + if ( supplicant_state < NM_SUPPLICANT_INTERFACE_STATE_READY + || supplicant_state > NM_SUPPLICANT_INTERFACE_STATE_COMPLETED) return FALSE; return TRUE; @@ -1242,7 +1242,7 @@ static gboolean scanning_allowed (NMDeviceWifi *self) { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - guint32 sup_state; + NMSupplicantInterfaceState supplicant_state; NMConnection *connection; g_return_val_if_fail (priv->sup_iface != NULL, FALSE); @@ -1274,11 +1274,11 @@ scanning_allowed (NMDeviceWifi *self) } /* Don't scan if the supplicant is busy */ - sup_state = nm_supplicant_interface_get_state (priv->sup_iface); - if ( sup_state == NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATING - || sup_state == NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATED - || sup_state == NM_SUPPLICANT_INTERFACE_STATE_4WAY_HANDSHAKE - || sup_state == NM_SUPPLICANT_INTERFACE_STATE_GROUP_HANDSHAKE + supplicant_state = nm_supplicant_interface_get_state (priv->sup_iface); + if ( supplicant_state == NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATING + || supplicant_state == NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATED + || supplicant_state == NM_SUPPLICANT_INTERFACE_STATE_4WAY_HANDSHAKE + || supplicant_state == NM_SUPPLICANT_INTERFACE_STATE_GROUP_HANDSHAKE || nm_supplicant_interface_get_scanning (priv->sup_iface)) return FALSE; @@ -1893,7 +1893,7 @@ link_timeout_cb (gpointer user_data) static gboolean need_new_8021x_secrets (NMDeviceWifi *self, - guint32 old_state, + NMSupplicantInterfaceState old_state, const char **setting_name) { NMSetting8021x *s_8021x; @@ -1947,7 +1947,7 @@ need_new_8021x_secrets (NMDeviceWifi *self, static gboolean need_new_wpa_psk (NMDeviceWifi *self, - guint32 old_state, + NMSupplicantInterfaceState old_state, gint disconnect_reason, const char **setting_name) { @@ -1988,8 +1988,8 @@ need_new_wpa_psk (NMDeviceWifi *self, static gboolean handle_8021x_or_psk_auth_fail (NMDeviceWifi *self, - guint32 new_state, - guint32 old_state, + NMSupplicantInterfaceState new_state, + NMSupplicantInterfaceState old_state, int disconnect_reason) { NMDevice *device = NM_DEVICE (self); @@ -2042,8 +2042,8 @@ reacquire_interface_cb (gpointer user_data) static void supplicant_iface_state_cb (NMSupplicantInterface *iface, - guint32 new_state, - guint32 old_state, + int new_state_i, + int old_state_i, int disconnect_reason, gpointer user_data) { @@ -2052,6 +2052,8 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, NMDevice *device = NM_DEVICE (self); NMDeviceState devstate; gboolean scanning; + NMSupplicantInterfaceState new_state = new_state_i; + NMSupplicantInterfaceState old_state = old_state_i; if (new_state == old_state) return; diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index f932fe8b53..334a8d22a9 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -71,7 +71,7 @@ typedef struct { guint32 ready_count; char * object_path; - guint32 state; + NMSupplicantInterfaceState state; int disconnect_reason; gboolean scanning; @@ -238,12 +238,10 @@ handle_new_bss (NMSupplicantInterface *self, const char *object_path) } static void -set_state (NMSupplicantInterface *self, guint32 new_state) +set_state (NMSupplicantInterface *self, NMSupplicantInterfaceState new_state) { NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - guint32 old_state = priv->state; - - g_return_if_fail (new_state < NM_SUPPLICANT_INTERFACE_STATE_LAST); + NMSupplicantInterfaceState old_state = priv->state; if (new_state == priv->state) return; @@ -286,12 +284,12 @@ set_state (NMSupplicantInterface *self, guint32 new_state) priv->disconnect_reason = 0; g_signal_emit (self, signals[STATE], 0, - priv->state, - old_state, - priv->disconnect_reason); + (int) priv->state, + (int) old_state, + (int) priv->disconnect_reason); } -static int +static NMSupplicantInterfaceState wpas_state_string_to_enum (const char *str_state) { if (!strcmp (str_state, "interface_disabled")) @@ -315,20 +313,20 @@ wpas_state_string_to_enum (const char *str_state) else if (!strcmp (str_state, "completed")) return NM_SUPPLICANT_INTERFACE_STATE_COMPLETED; - return -1; + return NM_SUPPLICANT_INTERFACE_STATE_INVALID; } static void set_state_from_string (NMSupplicantInterface *self, const char *new_state) { - int state; + NMSupplicantInterfaceState state; state = wpas_state_string_to_enum (new_state); - if (state == -1) { + if (state == NM_SUPPLICANT_INTERFACE_STATE_INVALID) { _LOGW ("unknown supplicant state '%s'", new_state); return; } - set_state (self, (guint32) state); + set_state (self, state); } static void @@ -1317,7 +1315,7 @@ nm_supplicant_interface_request_scan (NMSupplicantInterface *self, const GPtrArr return TRUE; } -guint32 +NMSupplicantInterfaceState nm_supplicant_interface_get_state (NMSupplicantInterface * self) { g_return_val_if_fail (NM_IS_SUPPLICANT_INTERFACE (self), NM_SUPPLICANT_INTERFACE_STATE_DOWN); @@ -1325,43 +1323,24 @@ nm_supplicant_interface_get_state (NMSupplicantInterface * self) return NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)->state; } -const char * -nm_supplicant_interface_state_to_string (guint32 state) -{ - switch (state) { - case NM_SUPPLICANT_INTERFACE_STATE_INIT: - return "init"; - case NM_SUPPLICANT_INTERFACE_STATE_STARTING: - return "starting"; - case NM_SUPPLICANT_INTERFACE_STATE_READY: - return "ready"; - case NM_SUPPLICANT_INTERFACE_STATE_DISABLED: - return "disabled"; - case NM_SUPPLICANT_INTERFACE_STATE_DISCONNECTED: - return "disconnected"; - case NM_SUPPLICANT_INTERFACE_STATE_INACTIVE: - return "inactive"; - case NM_SUPPLICANT_INTERFACE_STATE_SCANNING: - return "scanning"; - case NM_SUPPLICANT_INTERFACE_STATE_AUTHENTICATING: - return "authenticating"; - case NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATING: - return "associating"; - case NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATED: - return "associated"; - case NM_SUPPLICANT_INTERFACE_STATE_4WAY_HANDSHAKE: - return "4-way handshake"; - case NM_SUPPLICANT_INTERFACE_STATE_GROUP_HANDSHAKE: - return "group handshake"; - case NM_SUPPLICANT_INTERFACE_STATE_COMPLETED: - return "completed"; - case NM_SUPPLICANT_INTERFACE_STATE_DOWN: - return "down"; - default: - break; - } - return "unknown"; -} +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) @@ -1550,7 +1529,7 @@ nm_supplicant_interface_class_init (NMSupplicantInterfaceClass *klass) G_SIGNAL_RUN_LAST, 0, NULL, NULL, NULL, - G_TYPE_NONE, 3, G_TYPE_UINT, G_TYPE_UINT, G_TYPE_INT); + G_TYPE_NONE, 3, G_TYPE_INT, G_TYPE_INT, G_TYPE_INT); signals[REMOVED] = g_signal_new (NM_SUPPLICANT_INTERFACE_REMOVED, diff --git a/src/supplicant/nm-supplicant-interface.h b/src/supplicant/nm-supplicant-interface.h index 2ef63d1e94..d78cd28ecd 100644 --- a/src/supplicant/nm-supplicant-interface.h +++ b/src/supplicant/nm-supplicant-interface.h @@ -28,7 +28,8 @@ * Supplicant interface states * A mix of wpa_supplicant interface states and internal states. */ -enum { +typedef enum { + NM_SUPPLICANT_INTERFACE_STATE_INVALID = -1, NM_SUPPLICANT_INTERFACE_STATE_INIT = 0, NM_SUPPLICANT_INTERFACE_STATE_STARTING, NM_SUPPLICANT_INTERFACE_STATE_READY, @@ -43,8 +44,7 @@ enum { NM_SUPPLICANT_INTERFACE_STATE_GROUP_HANDSHAKE, NM_SUPPLICANT_INTERFACE_STATE_COMPLETED, NM_SUPPLICANT_INTERFACE_STATE_DOWN, - NM_SUPPLICANT_INTERFACE_STATE_LAST -}; +} NMSupplicantInterfaceState; #define NM_TYPE_SUPPLICANT_INTERFACE (nm_supplicant_interface_get_type ()) #define NM_SUPPLICANT_INTERFACE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_SUPPLICANT_INTERFACE, NMSupplicantInterface)) @@ -93,9 +93,9 @@ const char *nm_supplicant_interface_get_object_path (NMSupplicantInterface * ifa gboolean nm_supplicant_interface_request_scan (NMSupplicantInterface * self, const GPtrArray *ssids); -guint32 nm_supplicant_interface_get_state (NMSupplicantInterface * self); +NMSupplicantInterfaceState nm_supplicant_interface_get_state (NMSupplicantInterface * self); -const char *nm_supplicant_interface_state_to_string (guint32 state); +const char *nm_supplicant_interface_state_to_string (NMSupplicantInterfaceState state); gboolean nm_supplicant_interface_get_scanning (NMSupplicantInterface *self); From 2735e614b704af9043f4ff39e342e87554c04411 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Feb 2017 19:30:21 +0100 Subject: [PATCH 02/14] 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. (cherry picked from commit 66c45d0fdcbd9e87b5164c5f77f914141cbb9419) --- 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 8dedd7ebfa..1059c2b8e5 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 58ee4fec3f..e1a5a1388f 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 07dc36dc11..95868170b0 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); /*****************************************************************************/ @@ -1761,21 +1759,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); } @@ -2082,8 +2071,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. @@ -2173,35 +2162,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, @@ -2620,7 +2594,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); @@ -2684,20 +2659,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 334a8d22a9..acbc605895 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -39,6 +39,15 @@ /*****************************************************************************/ +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 */ @@ -46,7 +55,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 }; @@ -80,15 +88,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 { @@ -126,18 +134,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 990810ed7c48c80443e89a2a3a55e8cb71de0c85 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 00:11:16 +0100 Subject: [PATCH 03/14] supplicant/trivial: move code around (cherry picked from commit e16bf4f3db3da36f8194195e766029d1b751b5ec) --- 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 acbc605895..35fe7db04e 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -134,6 +134,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 6ce8cf361ad9ca5f18e1df2d6fb1eb6790f25bc8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 00:16:38 +0100 Subject: [PATCH 04/14] supplicant: use nm_clear_g_cancellable() helper (cherry picked from commit da34034b95651d8b68eaa19d7bf115ffc1cd1fb5) --- 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 35fe7db04e..725f6d4b3f 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 a6d3d17f5e6676183bae0cabcfe330249cb0b51b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 00:22:46 +0100 Subject: [PATCH 05/14] supplicant: remove unused return value from nm_supplicant_interface_request_scan() It cannot fail, remove code that anticipates a failure of request-scan. (cherry picked from commit dce13b6f11105422d54ee3aa3781ae77c875ae0f) --- 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 95868170b0..33daa26641 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1422,9 +1422,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); @@ -1434,6 +1432,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) { @@ -1471,22 +1471,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 725f6d4b3f..ebedad6fe0 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 fbe98ba107ebe47df1be4838e6ba0e027e093d23 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 10:14:54 +0100 Subject: [PATCH 06/14] 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. (cherry picked from commit c47026715ec6a9612752f469358a7251aabbdee7) --- 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 ebedad6fe0..44bd27a80d 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -39,15 +39,24 @@ /*****************************************************************************/ +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 c961c36c8f50bf084a057187e1bdcd72ee9038a1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 11:17:55 +0100 Subject: [PATCH 07/14] 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. (cherry picked from commit 29a53b1cd7fb48984221c71b38b049c3ced9b560) --- 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 33daa26641..eaea9a27f0 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), @@ -1629,14 +1620,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; @@ -1651,40 +1641,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. @@ -1695,32 +1685,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, @@ -1734,19 +1698,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 44bd27a80d..7942bec140 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -60,8 +60,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_data (G_OBJECT (proxy), BSS_PROXY_INITED, 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_data (G_OBJECT (bss_proxy), BSS_PROXY_INITED)) { 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 13b17d44dc2b6d4d66edf934d97adaaee37b5c62 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 11:50:59 +0100 Subject: [PATCH 08/14] 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(). (cherry picked from commit e3a489180b83d55c796d2162eecae01b7351327a) --- 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 b4fe07e96ee7a53537effeedb0cadf63a2d65e9a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 11:05:47 +0100 Subject: [PATCH 09/14] 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 (cherry picked from commit e0f96770188eeaada70a299bd6dab7a50ec34a53) --- src/supplicant/nm-supplicant-interface.c | 168 +++++++++++++++-------- 1 file changed, 114 insertions(+), 54 deletions(-) diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 7942bec140..eb22733099 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -39,6 +39,11 @@ /*****************************************************************************/ +typedef struct { + GDBusProxy *proxy; + gulong change_id; +} BssData; + struct _AddNetworkData; typedef struct { @@ -90,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; @@ -142,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"), @@ -164,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); @@ -181,63 +203,75 @@ 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); } #define BSS_PROXY_INITED "bss-proxy-inited" 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_data (G_OBJECT (proxy), BSS_PROXY_INITED, 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 +285,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 +598,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 +641,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_data (G_OBJECT (bss_proxy), BSS_PROXY_INITED)) { - 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 +660,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 +670,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 +725,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 +1577,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 7f7bed473624e3d24db7273f95d64314e1b8acf8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 13:20:06 +0100 Subject: [PATCH 10/14] 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. (cherry picked from commit c9dc0eba65565d361e32200894386da5e5c2d001) --- 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 eaea9a27f0..6085b3732a 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); @@ -1643,7 +1646,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; @@ -1673,7 +1677,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 @@ -1707,9 +1710,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); } @@ -2500,7 +2503,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 @@ -2849,6 +2853,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 @@ -2861,13 +2867,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 b8af01b455448f2f746f8bd77b63b4f9f6fd072a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Feb 2017 13:26:54 +0100 Subject: [PATCH 11/14] wifi: also show the NM D-Bus path for the Wi-Fi AP in nm_wifi_ap_to_string() (cherry picked from commit 257484e7bac2813d0cf1bc922ef57e6b781b1c96) --- 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 da37c8ac6c17118bab9cc511114572cabf0c384b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Feb 2017 13:57:57 +0100 Subject: [PATCH 12/14] 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. (cherry picked from commit 2f9166e6b9c18af4801fd5cc6c01f7eaaeb9d538) --- 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 2a2d276d92..7f015c3502 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 7dfaa5abf8..6be0e2f078 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -4240,7 +4240,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 4bf493fcafe94ae497a104d3afc273904a3cecab Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Feb 2017 12:45:38 +0100 Subject: [PATCH 13/14] 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. (cherry picked from commit 6eaded9071fbf868476255adb8ee5f416e7ad134) --- 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 7f015c3502..96fa9af0b2 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 f270bc34b4e503d5ba79d6aad1129fb4f49fee05 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Feb 2017 15:10:36 +0100 Subject: [PATCH 14/14] 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 (cherry picked from commit 2ab2254dd7336b9b7baa03ea1eb1f1c72f7ab6a8) --- 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 6085b3732a..944139f576 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, @@ -3184,6 +3198,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;