From e3dc1a0cc5b3de4e10c58a8f40be63466a9d249f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 18:33:56 +0100 Subject: [PATCH 01/21] core: use define for GObject property name NM_SUPPLICANT_INTERFACE_SCANNING grep-ing for '\' yields 42 hits under src. But only 2 are actual references to the "scanning" GObject property of NMDeviceWifi. Use a #define with a unique name where we mean NMDeviceWifi's property. (cherry picked from commit 95a95b3e480244f79623246e41cab33865fe40c3) --- src/devices/wifi/nm-device-wifi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index ed5d2c6252..5ba9b127af 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -262,7 +262,7 @@ supplicant_interface_acquire (NMDeviceWifi *self) G_CALLBACK (supplicant_iface_scan_done_cb), self); g_signal_connect (priv->sup_iface, - "notify::scanning", + "notify::"NM_SUPPLICANT_INTERFACE_SCANNING, G_CALLBACK (supplicant_iface_notify_scanning_cb), self); g_signal_connect (priv->sup_iface, From 45ac6d8c12efd264078928a0ffd2e4aeb8528cb5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 27 Jan 2017 13:52:26 +0100 Subject: [PATCH 02/21] device: add pending-action "recheck-available" Startup-complete means that all devices have settled in a state and no further activation is pending. When we have a recheck-available scheduled, we clearly should not yet declare startup-complete. Add a new pending-action "recheck-available" to avoid: [1485520408.3920] device (wlp2s0): supplicant interface state: starting -> ready [1485520408.3920] device[0x563abbcca400] (wlp2s0): remove_pending_action (0): 'waiting for supplicant' [1485520408.3920] manager: startup complete [1485520408.3924] device[0x563abbcca400] (wlp2s0): add_pending_action (1): 'queued state change to disconnected' (cherry picked from commit 11744b090ea0f6e7d1f71c936e76588b4c7ea347) --- src/devices/nm-device.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index adce2fcf9c..28e798b867 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -143,6 +143,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDevice, #define PENDING_ACTION_DHCP4 "dhcp4" #define PENDING_ACTION_DHCP6 "dhcp6" #define PENDING_ACTION_AUTOCONF6 "autoconf6" +#define PENDING_ACTION_RECHECK_AVAILABLE "recheck-available" #define DHCP_RESTART_TIMEOUT 120 #define DHCP_NUM_TRIES_MAX 3 @@ -3921,6 +3922,9 @@ recheck_available (gpointer user_data) priv->recheck_available.unavailable_reason = NM_DEVICE_STATE_REASON_NONE; } + if (priv->recheck_available.call_id == 0) + nm_device_remove_pending_action (self, PENDING_ACTION_RECHECK_AVAILABLE, TRUE); + return G_SOURCE_REMOVE; } @@ -3933,8 +3937,12 @@ nm_device_queue_recheck_available (NMDevice *self, priv->recheck_available.available_reason = available_reason; priv->recheck_available.unavailable_reason = unavailable_reason; - if (!priv->recheck_available.call_id) + if (!priv->recheck_available.call_id) { priv->recheck_available.call_id = g_idle_add (recheck_available, self); + nm_device_add_pending_action (self, PENDING_ACTION_RECHECK_AVAILABLE, + FALSE /* cannot assert, because of how recheck_available() first clears + the call-id and postpones removing the pending-action. */); + } } void From 3a438a1431252f6c09de432c968744c743127dac Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 27 Jan 2017 13:55:41 +0100 Subject: [PATCH 03/21] device: queue recheck-available before removing "wait for supplicant" pending action While we still recheck-available, we want to queue a pending action to block startup-complete. However, we have to queue that before removing the pending action for "wait for supplicant". [...] device[0x563abbcca400] (wlp2s0): remove_pending_action (0): 'waiting for supplicant' [...] manager: startup complete [...] device[0x563abbcca400] (wlp2s0): add_pending_action (1): 'queued state change to disconnected' (cherry picked from commit 252e95c1130d6864489c6b27005566d02303090f) --- src/devices/wifi/nm-device-wifi.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 5ba9b127af..1489ef5120 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -2022,7 +2022,6 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, NMDevice *device = NM_DEVICE (self); NMDeviceState devstate; gboolean scanning; - gboolean recheck_available = FALSE; if (new_state == old_state) return; @@ -2043,7 +2042,9 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, switch (new_state) { case NM_SUPPLICANT_INTERFACE_STATE_READY: _LOGD (LOGD_WIFI_SCAN, "supplicant ready"); - recheck_available = TRUE; + nm_device_queue_recheck_available (NM_DEVICE (device), + NM_DEVICE_STATE_REASON_SUPPLICANT_AVAILABLE, + NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); priv->scan_interval = SCAN_INTERVAL_MIN; if (old_state < NM_SUPPLICANT_INTERFACE_STATE_READY) nm_device_remove_pending_action (device, "waiting for supplicant", TRUE); @@ -2104,7 +2105,9 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, } break; case NM_SUPPLICANT_INTERFACE_STATE_DOWN: - recheck_available = TRUE; + nm_device_queue_recheck_available (NM_DEVICE (device), + NM_DEVICE_STATE_REASON_SUPPLICANT_AVAILABLE, + NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); cleanup_association_attempt (self, FALSE); if (old_state < NM_SUPPLICANT_INTERFACE_STATE_READY) @@ -2129,12 +2132,6 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, break; } - if (recheck_available) { - nm_device_queue_recheck_available (NM_DEVICE (device), - NM_DEVICE_STATE_REASON_SUPPLICANT_AVAILABLE, - NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); - } - /* Signal scanning state changes */ if ( new_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING || old_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING) From 9822299d415ded7114e209c53c5dc2065a3b1b72 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 27 Jan 2017 13:57:15 +0100 Subject: [PATCH 04/21] device: refactor pending-action strings as named defines (cherry picked from commit e234673a4aab53675374e35eedf02ff9949157f4) --- src/devices/nm-device.c | 67 ++++++++++++-------------- src/devices/nm-device.h | 15 ++++++ src/devices/wifi/nm-device-olpc-mesh.c | 6 +-- src/devices/wifi/nm-device-wifi.c | 10 ++-- src/nm-active-connection.c | 2 +- src/nm-policy.c | 4 +- 6 files changed, 56 insertions(+), 48 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 28e798b867..ef9d85c963 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -140,11 +140,6 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDevice, /*****************************************************************************/ -#define PENDING_ACTION_DHCP4 "dhcp4" -#define PENDING_ACTION_DHCP6 "dhcp6" -#define PENDING_ACTION_AUTOCONF6 "autoconf6" -#define PENDING_ACTION_RECHECK_AVAILABLE "recheck-available" - #define DHCP_RESTART_TIMEOUT 120 #define DHCP_NUM_TRIES_MAX 3 @@ -505,22 +500,20 @@ static void _cancel_activation (NMDevice *self); /*****************************************************************************/ -#define QUEUED_PREFIX "queued state change to " - static const char *state_table[] = { - [NM_DEVICE_STATE_UNKNOWN] = QUEUED_PREFIX "unknown", - [NM_DEVICE_STATE_UNMANAGED] = QUEUED_PREFIX "unmanaged", - [NM_DEVICE_STATE_UNAVAILABLE] = QUEUED_PREFIX "unavailable", - [NM_DEVICE_STATE_DISCONNECTED] = QUEUED_PREFIX "disconnected", - [NM_DEVICE_STATE_PREPARE] = QUEUED_PREFIX "prepare", - [NM_DEVICE_STATE_CONFIG] = QUEUED_PREFIX "config", - [NM_DEVICE_STATE_NEED_AUTH] = QUEUED_PREFIX "need-auth", - [NM_DEVICE_STATE_IP_CONFIG] = QUEUED_PREFIX "ip-config", - [NM_DEVICE_STATE_IP_CHECK] = QUEUED_PREFIX "ip-check", - [NM_DEVICE_STATE_SECONDARIES] = QUEUED_PREFIX "secondaries", - [NM_DEVICE_STATE_ACTIVATED] = QUEUED_PREFIX "activated", - [NM_DEVICE_STATE_DEACTIVATING] = QUEUED_PREFIX "deactivating", - [NM_DEVICE_STATE_FAILED] = QUEUED_PREFIX "failed", + [NM_DEVICE_STATE_UNKNOWN] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "unknown", + [NM_DEVICE_STATE_UNMANAGED] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "unmanaged", + [NM_DEVICE_STATE_UNAVAILABLE] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "unavailable", + [NM_DEVICE_STATE_DISCONNECTED] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "disconnected", + [NM_DEVICE_STATE_PREPARE] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "prepare", + [NM_DEVICE_STATE_CONFIG] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "config", + [NM_DEVICE_STATE_NEED_AUTH] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "need-auth", + [NM_DEVICE_STATE_IP_CONFIG] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "ip-config", + [NM_DEVICE_STATE_IP_CHECK] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "ip-check", + [NM_DEVICE_STATE_SECONDARIES] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "secondaries", + [NM_DEVICE_STATE_ACTIVATED] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "activated", + [NM_DEVICE_STATE_DEACTIVATING] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "deactivating", + [NM_DEVICE_STATE_FAILED] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "failed", }; static const char * @@ -534,7 +527,7 @@ queued_state_to_string (NMDeviceState state) static const char * state_to_string (NMDeviceState state) { - return queued_state_to_string (state) + strlen (QUEUED_PREFIX); + return queued_state_to_string (state) + NM_STRLEN (NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE); } NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_reason_to_string, NMDeviceStateReason, @@ -1990,7 +1983,7 @@ nm_device_set_carrier (NMDevice *self, gboolean carrier) klass->carrier_changed (self, TRUE); if (nm_clear_g_source (&priv->carrier_wait_id)) { - nm_device_remove_pending_action (self, "carrier wait", TRUE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, TRUE); _carrier_wait_check_queued_act_request (self); } } else if ( state <= NM_DEVICE_STATE_DISCONNECTED @@ -3923,7 +3916,7 @@ recheck_available (gpointer user_data) } if (priv->recheck_available.call_id == 0) - nm_device_remove_pending_action (self, PENDING_ACTION_RECHECK_AVAILABLE, TRUE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_RECHECK_AVAILABLE, TRUE); return G_SOURCE_REMOVE; } @@ -3939,7 +3932,7 @@ nm_device_queue_recheck_available (NMDevice *self, priv->recheck_available.unavailable_reason = unavailable_reason; if (!priv->recheck_available.call_id) { priv->recheck_available.call_id = g_idle_add (recheck_available, self); - nm_device_add_pending_action (self, PENDING_ACTION_RECHECK_AVAILABLE, + nm_device_add_pending_action (self, NM_PENDING_ACTION_RECHECK_AVAILABLE, FALSE /* cannot assert, because of how recheck_available() first clears the call-id and postpones removing the pending-action. */); } @@ -4979,7 +4972,7 @@ dhcp4_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release) /* Stop any ongoing DHCP transaction on this device */ nm_clear_g_signal_handler (priv->dhcp4.client, &priv->dhcp4.state_sigid); - nm_device_remove_pending_action (self, PENDING_ACTION_DHCP4, FALSE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_DHCP4, FALSE); if ( cleanup_type == CLEANUP_TYPE_DECONFIGURE || cleanup_type == CLEANUP_TYPE_REMOVED) @@ -5207,7 +5200,7 @@ dhcp4_lease_change (NMDevice *self, NMIP4Config *config) NULL, NULL); - nm_device_remove_pending_action (self, PENDING_ACTION_DHCP4, FALSE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_DHCP4, FALSE); return TRUE; } @@ -5432,7 +5425,7 @@ dhcp4_start (NMDevice *self, G_CALLBACK (dhcp4_state_changed), self); - nm_device_add_pending_action (self, PENDING_ACTION_DHCP4, TRUE); + nm_device_add_pending_action (self, NM_PENDING_ACTION_DHCP4, TRUE); /* DHCP devices will be notified by the DHCP manager when stuff happens */ return NM_ACT_STAGE_RETURN_POSTPONE; @@ -5734,7 +5727,7 @@ dhcp6_cleanup (NMDevice *self, CleanupType cleanup_type, gboolean release) g_clear_object (&priv->dhcp6.client); } - nm_device_remove_pending_action (self, PENDING_ACTION_DHCP6, FALSE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_DHCP6, FALSE); if (priv->dhcp6.config) { nm_exported_object_clear_and_unexport (&priv->dhcp6.config); @@ -5978,7 +5971,7 @@ dhcp6_lease_change (NMDevice *self) nm_device_get_applied_connection (self), self, NULL, NULL, NULL); - nm_device_remove_pending_action (self, PENDING_ACTION_DHCP6, FALSE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_DHCP6, FALSE); return TRUE; } @@ -6264,7 +6257,7 @@ dhcp6_start (NMDevice *self, gboolean wait_for_ll, NMDeviceStateReason *reason) s_ip6 = nm_connection_get_setting_ip6_config (connection); if (!nm_setting_ip_config_get_may_fail (s_ip6) || !strcmp (nm_setting_ip_config_get_method (s_ip6), NM_SETTING_IP6_CONFIG_METHOD_DHCP)) - nm_device_add_pending_action (self, PENDING_ACTION_DHCP6, TRUE); + nm_device_add_pending_action (self, NM_PENDING_ACTION_DHCP6, TRUE); if (wait_for_ll) { NMActStageReturn ret; @@ -7039,7 +7032,7 @@ addrconf6_start (NMDevice *self, NMSettingIP6ConfigPrivacy use_tempaddr) } if (!nm_setting_ip_config_get_may_fail (nm_connection_get_setting_ip6_config (connection))) - nm_device_add_pending_action (self, PENDING_ACTION_AUTOCONF6, TRUE); + nm_device_add_pending_action (self, NM_PENDING_ACTION_AUTOCONF6, TRUE); /* ensure link local is ready... */ ret = linklocal6_start (self); @@ -7061,7 +7054,7 @@ addrconf6_cleanup (NMDevice *self) nm_clear_g_signal_handler (priv->ndisc, &priv->ndisc_changed_id); nm_clear_g_signal_handler (priv->ndisc, &priv->ndisc_timeout_id); - nm_device_remove_pending_action (self, PENDING_ACTION_AUTOCONF6, FALSE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_AUTOCONF6, FALSE); g_clear_object (&priv->ac_ip6_config); g_clear_object (&priv->ndisc); @@ -7929,7 +7922,7 @@ activate_stage5_ip4_config_commit (NMDevice *self) arp_announce (self); - nm_device_remove_pending_action (self, PENDING_ACTION_DHCP4, FALSE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_DHCP4, FALSE); /* Enter the IP_CHECK state if this is the first method to complete */ _set_ip_state (self, AF_INET, IP_DONE); @@ -8066,8 +8059,8 @@ activate_stage5_ip6_config_commit (NMDevice *self) return; } } - nm_device_remove_pending_action (self, PENDING_ACTION_DHCP6, FALSE); - nm_device_remove_pending_action (self, PENDING_ACTION_AUTOCONF6, FALSE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_DHCP6, FALSE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_AUTOCONF6, FALSE); /* Start IPv6 forwarding if we need it */ method = nm_utils_get_ip_config_method (connection, NM_TYPE_SETTING_IP6_CONFIG); @@ -9753,7 +9746,7 @@ carrier_wait_timeout (gpointer user_data) NMDevice *self = NM_DEVICE (user_data); NM_DEVICE_GET_PRIVATE (self)->carrier_wait_id = 0; - nm_device_remove_pending_action (self, "carrier wait", TRUE); + nm_device_remove_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, TRUE); _carrier_wait_check_queued_act_request (self); @@ -9835,7 +9828,7 @@ nm_device_bring_up (NMDevice *self, gboolean block, gboolean *no_firmware) */ if (nm_device_has_capability (self, NM_DEVICE_CAP_CARRIER_DETECT)) { if (!nm_clear_g_source (&priv->carrier_wait_id)) - nm_device_add_pending_action (self, "carrier wait", TRUE); + nm_device_add_pending_action (self, NM_PENDING_ACTION_CARRIER_WAIT, TRUE); priv->carrier_wait_id = g_timeout_add_seconds (5, carrier_wait_timeout, self); } diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 311b5a2c77..e512f2ca7d 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -30,6 +30,21 @@ #include "nm-rfkill-manager.h" #include "NetworkManagerUtils.h" + +#define NM_PENDING_ACTION_AUTOACTIVATE "autoactivate" +#define NM_PENDING_ACTION_DHCP4 "dhcp4" +#define NM_PENDING_ACTION_DHCP6 "dhcp6" +#define NM_PENDING_ACTION_AUTOCONF6 "autoconf6" +#define NM_PENDING_ACTION_RECHECK_AVAILABLE "recheck-available" +#define NM_PENDING_ACTION_CARRIER_WAIT "carrier-wait" +#define NM_PENDING_ACTION_WAITING_FOR_SUPPLICANT "waiting-for-supplicant" +#define NM_PENDING_ACTION_WIFI_SCAN "wifi-scan" +#define NM_PENDING_ACTION_WAITING_FOR_COMPANION "waiting-for-companion" + +#define NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "queued-state-change-" +#define NM_PENDING_ACTIONPREFIX_ACTIVATION "activation-" + + /* Properties */ #define NM_DEVICE_UDI "udi" #define NM_DEVICE_IFACE "interface" diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index e0258523c0..0fdccb8ae5 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -377,7 +377,7 @@ device_added_cb (NMManager *manager, NMDevice *other, gpointer user_data) nm_device_queue_recheck_available (NM_DEVICE (self), NM_DEVICE_STATE_REASON_NONE, NM_DEVICE_STATE_REASON_NONE); - nm_device_remove_pending_action (NM_DEVICE (self), "waiting for companion", TRUE); + nm_device_remove_pending_action (NM_DEVICE (self), NM_PENDING_ACTION_WAITING_FOR_COMPANION, TRUE); } } @@ -399,7 +399,7 @@ find_companion (NMDeviceOlpcMesh *self) if (priv->companion) return; - nm_device_add_pending_action (NM_DEVICE (self), "waiting for companion", TRUE); + nm_device_add_pending_action (NM_DEVICE (self), NM_PENDING_ACTION_WAITING_FOR_COMPANION, TRUE); /* Try to find the companion if it's already known to the NMManager */ for (list = nm_manager_get_devices (priv->manager); list ; list = g_slist_next (list)) { @@ -407,7 +407,7 @@ find_companion (NMDeviceOlpcMesh *self) nm_device_queue_recheck_available (NM_DEVICE (self), NM_DEVICE_STATE_REASON_NONE, NM_DEVICE_STATE_REASON_NONE); - nm_device_remove_pending_action (NM_DEVICE (self), "waiting for companion", TRUE); + nm_device_remove_pending_action (NM_DEVICE (self), NM_PENDING_ACTION_WAITING_FOR_COMPANION, TRUE); break; } } diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 1489ef5120..fdd8886755 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -239,7 +239,7 @@ supplicant_interface_acquire (NMDeviceWifi *self) } if (nm_supplicant_interface_get_state (priv->sup_iface) < NM_SUPPLICANT_INTERFACE_STATE_READY) - nm_device_add_pending_action (NM_DEVICE (self), "waiting for supplicant", FALSE); + nm_device_add_pending_action (NM_DEVICE (self), NM_PENDING_ACTION_WAITING_FOR_SUPPLICANT, FALSE); g_signal_connect (priv->sup_iface, NM_SUPPLICANT_INTERFACE_STATE, @@ -286,9 +286,9 @@ _requested_scan_set (NMDeviceWifi *self, gboolean value) priv->requested_scan = value; if (value) - nm_device_add_pending_action ((NMDevice *) self, "scan", TRUE); + nm_device_add_pending_action ((NMDevice *) self, NM_PENDING_ACTION_WIFI_SCAN, TRUE); else - nm_device_remove_pending_action ((NMDevice *) self, "scan", TRUE); + nm_device_remove_pending_action ((NMDevice *) self, NM_PENDING_ACTION_WIFI_SCAN, TRUE); } static void @@ -2047,7 +2047,7 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); priv->scan_interval = SCAN_INTERVAL_MIN; if (old_state < NM_SUPPLICANT_INTERFACE_STATE_READY) - nm_device_remove_pending_action (device, "waiting for supplicant", TRUE); + 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); @@ -2111,7 +2111,7 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, cleanup_association_attempt (self, FALSE); if (old_state < NM_SUPPLICANT_INTERFACE_STATE_READY) - nm_device_remove_pending_action (device, "waiting for supplicant", TRUE); + nm_device_remove_pending_action (device, NM_PENDING_ACTION_WAITING_FOR_SUPPLICANT, TRUE); /* If the device is already in UNAVAILABLE state then the state change * is a NOP and the interface won't be re-acquired in the device state diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 38aadd2714..068bc85ea5 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -567,7 +567,7 @@ nm_active_connection_set_device (NMActiveConnection *self, NMDevice *device) G_CALLBACK (device_metered_changed), self); if (!priv->assumed) { - priv->pending_activation_id = g_strdup_printf ("activation::%p", (void *)self); + priv->pending_activation_id = g_strdup_printf (NM_PENDING_ACTIONPREFIX_ACTIVATION"%p", (void *)self); nm_device_add_pending_action (device, priv->pending_activation_id, TRUE); } } else { diff --git a/src/nm-policy.c b/src/nm-policy.c index a8719d9f37..a2ff2945b2 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -943,7 +943,7 @@ activate_data_free (ActivateData *data) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (data->policy); - nm_device_remove_pending_action (data->device, "autoactivate", TRUE); + nm_device_remove_pending_action (data->device, NM_PENDING_ACTION_AUTOACTIVATE, TRUE); priv->pending_activation_checks = g_slist_remove (priv->pending_activation_checks, data); if (data->autoactivate_id) @@ -1253,7 +1253,7 @@ schedule_activate_check (NMPolicy *self, NMDevice *device) return; } - nm_device_add_pending_action (device, "autoactivate", TRUE); + nm_device_add_pending_action (device, NM_PENDING_ACTION_AUTOACTIVATE, TRUE); data = g_slice_new0 (ActivateData); data->policy = self; From 78eaabf98f7a98d65f7f8592bc983b4ee22ffafb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 27 Jan 2017 14:13:09 +0100 Subject: [PATCH 05/21] device: implement queued_state_to_string() via NM_UTILS_LOOKUP_STR_DEFINE_STATIC() (cherry picked from commit 8ac14b5400a2017a8383c5352973b14666023ae6) --- src/devices/nm-device.c | 39 ++++++++++++++++----------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index ef9d85c963..0151dbc4de 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -500,29 +500,22 @@ static void _cancel_activation (NMDevice *self); /*****************************************************************************/ -static const char *state_table[] = { - [NM_DEVICE_STATE_UNKNOWN] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "unknown", - [NM_DEVICE_STATE_UNMANAGED] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "unmanaged", - [NM_DEVICE_STATE_UNAVAILABLE] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "unavailable", - [NM_DEVICE_STATE_DISCONNECTED] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "disconnected", - [NM_DEVICE_STATE_PREPARE] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "prepare", - [NM_DEVICE_STATE_CONFIG] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "config", - [NM_DEVICE_STATE_NEED_AUTH] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "need-auth", - [NM_DEVICE_STATE_IP_CONFIG] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "ip-config", - [NM_DEVICE_STATE_IP_CHECK] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "ip-check", - [NM_DEVICE_STATE_SECONDARIES] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "secondaries", - [NM_DEVICE_STATE_ACTIVATED] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "activated", - [NM_DEVICE_STATE_DEACTIVATING] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "deactivating", - [NM_DEVICE_STATE_FAILED] = NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "failed", -}; - -static const char * -queued_state_to_string (NMDeviceState state) -{ - if ((gsize) state < G_N_ELEMENTS (state_table)) - return state_table[state]; - return state_table[NM_DEVICE_STATE_UNKNOWN]; -} +NM_UTILS_LOOKUP_STR_DEFINE_STATIC (queued_state_to_string, NMDeviceState, + NM_UTILS_LOOKUP_DEFAULT ( NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "???"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_UNKNOWN, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "unknown"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_UNMANAGED, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "unmanaged"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_UNAVAILABLE, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "unavailable"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_DISCONNECTED, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "disconnected"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_PREPARE, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "prepare"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_CONFIG, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "config"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_NEED_AUTH, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "need-auth"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_IP_CONFIG, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "ip-config"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_IP_CHECK, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "ip-check"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_SECONDARIES, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "secondaries"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_ACTIVATED, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "activated"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_DEACTIVATING, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "deactivating"), + NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_STATE_FAILED, NM_PENDING_ACTIONPREFIX_QUEUED_STATE_CHANGE "failed"), +); static const char * state_to_string (NMDeviceState state) From aebff6a1ac9cbed768c9cc96d97245cd7e9ba916 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 27 Jan 2017 15:41:25 +0100 Subject: [PATCH 06/21] device: don't clone pending-action string All callers either use a static @action argument or keep a clone of the string that lives as long as the action is pending. So, save cloning the string. (cherry picked from commit 3bc1e02adf6f12dc5fd0fc5b5266ed444296de9d) --- src/devices/nm-device.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 0151dbc4de..b26c5d10af 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -11235,7 +11235,9 @@ nm_device_supports_vlans (NMDevice *self) /** * nm_device_add_pending_action(): * @self: the #NMDevice to add the pending action to - * @action: a static string that identifies the action + * @action: a static string that identifies the action. The string instance must + * stay valid until the pending action is removed (that is, the string is + * not cloned, but ownership stays with the caller). * @assert_not_yet_pending: if %TRUE, assert that the @action is currently not yet pending. * Otherwise, ignore duplicate scheduling of the same action silently. * @@ -11270,7 +11272,7 @@ nm_device_add_pending_action (NMDevice *self, const char *action, gboolean asser count++; } - priv->pending_actions = g_slist_append (priv->pending_actions, g_strdup (action)); + priv->pending_actions = g_slist_append (priv->pending_actions, (char *) action); count++; _LOGD (LOGD_DEVICE, "add_pending_action (%d): '%s'", count, action); @@ -11284,7 +11286,7 @@ nm_device_add_pending_action (NMDevice *self, const char *action, gboolean asser /** * nm_device_remove_pending_action(): * @self: the #NMDevice to remove the pending action from - * @action: a static string that identifies the action + * @action: a string that identifies the action. * @assert_is_pending: if %TRUE, assert that the @action is pending. * If %FALSE, don't do anything if the current action is not pending and * return %FALSE. @@ -11311,7 +11313,6 @@ nm_device_remove_pending_action (NMDevice *self, const char *action, gboolean as _LOGD (LOGD_DEVICE, "remove_pending_action (%d): '%s'", count + g_slist_length (iter->next), /* length excluding 'iter' */ action); - g_free (iter->data); priv->pending_actions = g_slist_delete_link (priv->pending_actions, iter); if (priv->pending_actions == NULL) _notify (self, PROP_HAS_PENDING_ACTION); @@ -13137,7 +13138,7 @@ finalize (GObject *object) g_free (priv->hw_addr); g_free (priv->hw_addr_perm); g_free (priv->hw_addr_initial); - g_slist_free_full (priv->pending_actions, g_free); + g_slist_free (priv->pending_actions); g_slist_free_full (priv->dad6_failed_addrs, g_free); g_clear_pointer (&priv->physical_port_id, g_free); g_free (priv->udi); From b1be460863ccdc46170f7a768728b9133b1942c1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 12:09:48 +0100 Subject: [PATCH 07/21] device: prepend pending actions list The order doesn't matter, so prepend instead of append new items to the pending-actions list. (cherry picked from commit e347d965963962b5f00f7c3a9558a7750e08a7a5) --- src/devices/nm-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index b26c5d10af..33c5a0981c 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -11272,7 +11272,7 @@ nm_device_add_pending_action (NMDevice *self, const char *action, gboolean asser count++; } - priv->pending_actions = g_slist_append (priv->pending_actions, (char *) action); + priv->pending_actions = g_slist_prepend (priv->pending_actions, (char *) action); count++; _LOGD (LOGD_DEVICE, "add_pending_action (%d): '%s'", count, action); From e61e87257434d6e21ea457c2af96478a1f003cca Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 12:28:32 +0100 Subject: [PATCH 08/21] device: inline device helper structs QueuedState and PingInfo These two structs are only used at exactly one place: as the type for a field in NMDevicePrivate. Having additional structs (that are only used at one place) only add noise. Also, there are already prior-acts of using unnamed structs in NMDevicePrivate in case of structs that only serve to group/namespace a set of fields. (cherry picked from commit 5151a6071f023dc8dc2c4e64cfe84a1f8af740e8) --- src/devices/nm-device.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 33c5a0981c..2d9116ffca 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -164,12 +164,6 @@ typedef enum { IP_FAIL } IpState; -typedef struct { - NMDeviceState state; - NMDeviceStateReason reason; - guint id; -} QueuedState; - typedef struct { NMDevice *slave; gulong watch_id; @@ -177,16 +171,6 @@ typedef struct { bool configure; } SlaveInfo; -typedef struct { - NMLogDomain log_domain; - guint timeout; - guint watch; - GPid pid; - const char *binary; - const char *address; - guint deadline; -} PingInfo; - typedef struct { NMDevice *device; guint idle_add_id; @@ -216,7 +200,11 @@ typedef struct _NMDevicePrivate { NMDeviceState state; NMDeviceStateReason state_reason; - QueuedState queued_state; + struct { + NMDeviceState state; + NMDeviceStateReason reason; + guint id; + } queued_state; guint queued_ip4_config_id; guint queued_ip6_config_id; GSList *pending_actions; @@ -354,7 +342,15 @@ typedef struct _NMDevicePrivate { guint num_tries_left; } dhcp4; - PingInfo gw_ping; + struct { + NMLogDomain log_domain; + guint timeout; + guint watch; + GPid pid; + const char *binary; + const char *address; + guint deadline; + } gw_ping; /* dnsmasq stuff for shared connections */ NMDnsMasqManager *dnsmasq_manager; From dc7e9844222876ff53e2c6407f62ceb34ba4dadf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 12:34:30 +0100 Subject: [PATCH 09/21] device: change hw_addr_type field to bitfield type We don't want to waste a full "int" size to store the @hw_addr_type in NMDevicePrivate. Previously, that was hacked around by using guint8. Now, instead use a bitfield which has the right type. (cherry picked from commit cac07387238cbefdc3ccda768a2d2c2da4a9868c) --- src/devices/nm-device.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 2d9116ffca..a09ec8a34a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -222,7 +222,8 @@ typedef struct _NMDevicePrivate { const guint8 hw_addr_len; /* read-only */ guint8 hw_addr_len_; }; - guint8 /*HwAddrType*/ hw_addr_type; + + HwAddrType hw_addr_type:5; bool real:1; @@ -12575,7 +12576,9 @@ nm_device_hw_addr_is_explict (NMDevice *self) g_return_val_if_fail (NM_IS_DEVICE (self), FALSE); priv = NM_DEVICE_GET_PRIVATE (self); - return !NM_IN_SET (priv->hw_addr_type, HW_ADDR_TYPE_PERMANENT, HW_ADDR_TYPE_UNSET); + return !NM_IN_SET ((HwAddrType) priv->hw_addr_type, + HW_ADDR_TYPE_PERMANENT, + HW_ADDR_TYPE_UNSET); } static gboolean From 57111e356dfd0bc0798df818e884933239caaaf9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 12:38:35 +0100 Subject: [PATCH 10/21] device/trivial: reorder defines in "nm-device.c" Reorder code to be like in other source files: - first includes and generic defines - then various helper structs - then GObject related declarations, with first signal and property enums, then the private data, then the G_DEFINE_TYPE() itself. - finally, forward declarations for functions. (cherry picked from commit f97d8b86fb1163c402c019ad0d74688c13f17089) --- src/devices/nm-device.c | 121 +++++++++++++++++++++------------------- 1 file changed, 63 insertions(+), 58 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index a09ec8a34a..b1d13c229e 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -73,9 +73,64 @@ _LOG_DECLARE_SELF (NMDevice); #include "introspection/org.freedesktop.NetworkManager.Device.h" #include "introspection/org.freedesktop.NetworkManager.Device.Statistics.h" -G_DEFINE_ABSTRACT_TYPE (NMDevice, nm_device, NM_TYPE_EXPORTED_OBJECT) +/*****************************************************************************/ -#define NM_DEVICE_GET_PRIVATE(self) _NM_GET_PRIVATE_PTR(self, NMDevice, NM_IS_DEVICE) +#define DHCP_RESTART_TIMEOUT 120 +#define DHCP_NUM_TRIES_MAX 3 +#define DEFAULT_AUTOCONNECT TRUE + +/*****************************************************************************/ + +typedef void (*ActivationHandleFunc) (NMDevice *self); + +typedef struct { + ActivationHandleFunc func; + guint id; +} ActivationHandleData; + +typedef enum { + CLEANUP_TYPE_KEEP, + CLEANUP_TYPE_REMOVED, + CLEANUP_TYPE_DECONFIGURE, +} CleanupType; + +typedef enum { + IP_NONE = 0, + IP_WAIT, + IP_CONF, + IP_DONE, + IP_FAIL +} IpState; + +typedef struct { + NMDevice *slave; + gulong watch_id; + bool slave_is_enslaved; + bool configure; +} SlaveInfo; + +typedef struct { + NMDevice *device; + guint idle_add_id; + int ifindex; +} DeleteOnDeactivateData; + +typedef void (*ArpingCallback) (NMDevice *, NMIP4Config **, gboolean); + +typedef struct { + ArpingCallback callback; + NMDevice *device; + NMIP4Config **configs; +} ArpingData; + +typedef enum { + HW_ADDR_TYPE_UNSET = 0, + HW_ADDR_TYPE_PERMANENT, + HW_ADDR_TYPE_EXPLICIT, + HW_ADDR_TYPE_GENERATED, +} HwAddrType; + +/*****************************************************************************/ enum { STATE_CHANGED, @@ -136,62 +191,6 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDevice, PROP_RX_BYTES, ); -#define DEFAULT_AUTOCONNECT TRUE - -/*****************************************************************************/ - -#define DHCP_RESTART_TIMEOUT 120 -#define DHCP_NUM_TRIES_MAX 3 - -typedef void (*ActivationHandleFunc) (NMDevice *self); - -typedef struct { - ActivationHandleFunc func; - guint id; -} ActivationHandleData; - -typedef enum { - CLEANUP_TYPE_KEEP, - CLEANUP_TYPE_REMOVED, - CLEANUP_TYPE_DECONFIGURE, -} CleanupType; - -typedef enum { - IP_NONE = 0, - IP_WAIT, - IP_CONF, - IP_DONE, - IP_FAIL -} IpState; - -typedef struct { - NMDevice *slave; - gulong watch_id; - bool slave_is_enslaved; - bool configure; -} SlaveInfo; - -typedef struct { - NMDevice *device; - guint idle_add_id; - int ifindex; -} DeleteOnDeactivateData; - -typedef void (*ArpingCallback) (NMDevice *, NMIP4Config **, gboolean); - -typedef struct { - ArpingCallback callback; - NMDevice *device; - NMIP4Config **configs; -} ArpingData; - -typedef enum { - HW_ADDR_TYPE_UNSET = 0, - HW_ADDR_TYPE_PERMANENT, - HW_ADDR_TYPE_EXPLICIT, - HW_ADDR_TYPE_GENERATED, -} HwAddrType; - typedef struct _NMDevicePrivate { bool in_state_changed; @@ -444,6 +443,12 @@ typedef struct _NMDevicePrivate { } NMDevicePrivate; +G_DEFINE_ABSTRACT_TYPE (NMDevice, nm_device, NM_TYPE_EXPORTED_OBJECT) + +#define NM_DEVICE_GET_PRIVATE(self) _NM_GET_PRIVATE_PTR(self, NMDevice, NM_IS_DEVICE) + +/*****************************************************************************/ + static void nm_device_set_proxy_config (NMDevice *self, GHashTable *options); static gboolean nm_device_set_ip4_config (NMDevice *self, From 8b455abab28d015b99ba90164b6442876eb39a13 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 12:39:07 +0100 Subject: [PATCH 11/21] device: cleanup handling queued state change in NMDevice - no longer bother clearing .state and .reason when the .id field is unset. The fields just don't matter and no user accesses these fields when the glib source id is not set. - unify logging and give them all a prefix "queue-state[%s, %s, %u]: ". - drop nm_device_queued_state_peek(), it only had one caller, thus inline the trivial check. - make nm_device_queued_state_clear() a static function queued_state_clear() - rename queued_set_state() to queued_state_set(). (cherry picked from commit 96b167cd978ceec7bcf67bec4d9f210d302c8a22) --- src/devices/nm-device-private.h | 4 -- src/devices/nm-device.c | 116 +++++++++++++++++--------------- 2 files changed, 60 insertions(+), 60 deletions(-) diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index c6d2a931d4..2023feb577 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -91,10 +91,6 @@ gboolean nm_device_dhcp6_renew (NMDevice *device, gboolean release); void nm_device_recheck_available_connections (NMDevice *device); -void nm_device_queued_state_clear (NMDevice *device); - -NMDeviceState nm_device_queued_state_peek (NMDevice *device); - gboolean nm_device_get_enslaved (NMDevice *device); NMDevice *nm_device_master_get_slave_by_ifindex (NMDevice *dev, int ifindex); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index b1d13c229e..2a2d276d92 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -200,9 +200,11 @@ typedef struct _NMDevicePrivate { NMDeviceState state; NMDeviceStateReason state_reason; struct { + guint id; + + /* The @state/@reason is only valid, when @id is set. */ NMDeviceState state; NMDeviceStateReason reason; - guint id; } queued_state; guint queued_ip4_config_id; guint queued_ip6_config_id; @@ -487,7 +489,7 @@ static void _set_state_full (NMDevice *self, NMDeviceState state, NMDeviceStateReason reason, gboolean quitting); - +static void queued_state_clear (NMDevice *device); static gboolean queued_ip4_config_change (gpointer user_data); static gboolean queued_ip6_config_change (gpointer user_data); static void ip_check_ping_watch_cb (GPid pid, gint status, gpointer user_data); @@ -1921,8 +1923,9 @@ carrier_changed (NMDevice *self, gboolean carrier) } } else { if (priv->state == NM_DEVICE_STATE_UNAVAILABLE) { - if (nm_device_queued_state_peek (self) >= NM_DEVICE_STATE_DISCONNECTED) - nm_device_queued_state_clear (self); + if ( priv->queued_state.id + && priv->queued_state.state >= NM_DEVICE_STATE_DISCONNECTED) + queued_state_clear (self); } else { nm_device_queue_state (self, NM_DEVICE_STATE_UNAVAILABLE, NM_DEVICE_STATE_REASON_CARRIER); @@ -11380,8 +11383,7 @@ _cleanup_generic_pre (NMDevice *self, CleanupType cleanup_type) NULL); } - /* Clear any queued transitions */ - nm_device_queued_state_clear (self); + queued_state_clear (self); _cleanup_ip4_pre (self, cleanup_type); _cleanup_ip6_pre (self, cleanup_type); @@ -11886,8 +11888,7 @@ _set_state_full (NMDevice *self, priv->state = state; priv->state_reason = reason; - /* Clear any queued transitions */ - nm_device_queued_state_clear (self); + queued_state_clear (self); dispatcher_cleanup (self); if (priv->deactivating_cancellable) @@ -12199,33 +12200,32 @@ nm_device_state_changed (NMDevice *self, } static gboolean -queued_set_state (gpointer user_data) +queued_state_set (gpointer user_data) { NMDevice *self = NM_DEVICE (user_data); NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMDeviceState new_state; NMDeviceStateReason new_reason; - if (priv->queued_state.id) { - _LOGD (LOGD_DEVICE, "running queued state change to %s (id %d)", - state_to_string (priv->queued_state.state), - priv->queued_state.id); + nm_assert (priv->queued_state.id); - /* Clear queued state struct before triggering state change, since - * the state change may queue another state. - */ - priv->queued_state.id = 0; - new_state = priv->queued_state.state; - new_reason = priv->queued_state.reason; - nm_device_queued_state_clear (self); + _LOGD (LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s", + state_to_string (priv->queued_state.state), + reason_to_string (priv->queued_state.reason), + priv->queued_state.id, + "change state"); - nm_device_state_changed (self, new_state, new_reason); - nm_device_remove_pending_action (self, queued_state_to_string (new_state), TRUE); - } else { - g_warn_if_fail (priv->queued_state.state == NM_DEVICE_STATE_UNKNOWN); - g_warn_if_fail (priv->queued_state.reason == NM_DEVICE_STATE_REASON_NONE); - } - return FALSE; + /* Clear queued state struct before triggering state change, since + * the state change may queue another state. + */ + priv->queued_state.id = 0; + new_state = priv->queued_state.state; + new_reason = priv->queued_state.reason; + + nm_device_state_changed (self, new_state, new_reason); + nm_device_remove_pending_action (self, queued_state_to_string (new_state), TRUE); + + return G_SOURCE_REMOVE; } void @@ -12239,8 +12239,16 @@ nm_device_queue_state (NMDevice *self, priv = NM_DEVICE_GET_PRIVATE (self); - if (priv->queued_state.id && priv->queued_state.state == state) + if (priv->queued_state.id && priv->queued_state.state == state) { + _LOGD (LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s%s%s%s", + state_to_string (priv->queued_state.state), + reason_to_string (priv->queued_state.reason), + priv->queued_state.id, + "ignore queuing same state change", + NM_PRINT_FMT_QUOTED (priv->queued_state.reason != reason, + " (reason differs: ", reason_to_string (reason), ")", "")); return; + } /* Add pending action for the new state before clearing the queued states, so * that we don't accidently pop all pending states and reach 'startup complete' */ @@ -12248,45 +12256,41 @@ nm_device_queue_state (NMDevice *self, /* We should only ever have one delayed state transition at a time */ if (priv->queued_state.id) { - _LOGW (LOGD_DEVICE, "overwriting previously queued state change to %s (%s)", + _LOGW (LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s", state_to_string (priv->queued_state.state), - reason_to_string (priv->queued_state.reason)); - nm_device_queued_state_clear (self); + reason_to_string (priv->queued_state.reason), + priv->queued_state.id, + "replace previously queued state change"); + nm_clear_g_source (&priv->queued_state.id); + nm_device_remove_pending_action (self, queued_state_to_string (priv->queued_state.state), TRUE); } priv->queued_state.state = state; priv->queued_state.reason = reason; - priv->queued_state.id = g_idle_add (queued_set_state, self); + priv->queued_state.id = g_idle_add (queued_state_set, self); - _LOGD (LOGD_DEVICE, "queued state change to %s due to %s (id %d)", - state_to_string (state), reason_to_string (reason), - priv->queued_state.id); + _LOGD (LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s", + state_to_string (state), + reason_to_string (reason), + priv->queued_state.id, + "queue state change"); } -NMDeviceState -nm_device_queued_state_peek (NMDevice *self) -{ - NMDevicePrivate *priv; - - g_return_val_if_fail (NM_IS_DEVICE (self), NM_DEVICE_STATE_UNKNOWN); - - priv = NM_DEVICE_GET_PRIVATE (self); - - return priv->queued_state.id ? priv->queued_state.state : NM_DEVICE_STATE_UNKNOWN; -} - -void -nm_device_queued_state_clear (NMDevice *self) +static void +queued_state_clear (NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - if (priv->queued_state.id) { - _LOGD (LOGD_DEVICE, "clearing queued state transition (id %d)", - priv->queued_state.id); - nm_clear_g_source (&priv->queued_state.id); - nm_device_remove_pending_action (self, queued_state_to_string (priv->queued_state.state), TRUE); - } - memset (&priv->queued_state, 0, sizeof (priv->queued_state)); + if (!priv->queued_state.id) + return; + + _LOGD (LOGD_DEVICE, "queue-state[%s, reason:%s, id:%u]: %s", + state_to_string (priv->queued_state.state), + reason_to_string (priv->queued_state.reason), + priv->queued_state.id, + "clear queued state change"); + nm_clear_g_source (&priv->queued_state.id); + nm_device_remove_pending_action (self, queued_state_to_string (priv->queued_state.state), TRUE); } NMDeviceState From 36196b5a82627b26362b64893308202e07b4723f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 14:40:40 +0100 Subject: [PATCH 12/21] device/wifi: refactor logging of Wi-Fi AP by preserving logging context of device The _LOGD() macros of NMDeviceWifi print a logging context for each line, that is, they add a prefix with the device name. Replace nm_wifi_ap_dump() by nm_wifi_ap_to_string() and let device log a message about the AP. Also, update the format for printing the AP. Now, all fields are separated by space. (cherry picked from commit d98fa31dddf2f64faf5b77c97d72537105c5a3ba) --- src/devices/wifi/nm-device-wifi.c | 23 +++++++++--- src/devices/wifi/nm-wifi-ap.c | 58 ++++++++++++++----------------- src/devices/wifi/nm-wifi-ap.h | 6 ++-- 3 files changed, 48 insertions(+), 39 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index fdd8886755..11905d7d6f 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -195,6 +195,19 @@ static void _hw_addr_set_scanning (NMDeviceWifi *self, gboolean do_reset); /*****************************************************************************/ +static void +_ap_dump (NMDeviceWifi *self, + const NMWifiAP *ap, + const char *prefix) +{ + char buf[1024]; + + buf[0] = '\0'; + _LOGD (LOGD_WIFI_SCAN, "wifi-ap: %-7s %s", + prefix, + nm_wifi_ap_to_string (ap, buf, sizeof (buf))); +} + static void constructed (GObject *object) { @@ -1559,7 +1572,7 @@ ap_list_dump (gpointer user_data) priv->scheduled_scan_time); list = ap_list_get_sorted (self, TRUE); for (i = 0; list[i]; i++) - nm_wifi_ap_dump (list[i], "dump ", nm_device_get_iface (NM_DEVICE (self))); + _ap_dump (self, list[i], "dump"); return G_SOURCE_REMOVE; } @@ -1657,10 +1670,10 @@ supplicant_iface_new_bss_cb (NMSupplicantInterface *iface, found_ap = get_ap_by_supplicant_path (self, object_path); if (found_ap) { - nm_wifi_ap_dump (ap, "updated ", nm_device_get_iface (NM_DEVICE (self))); + _ap_dump (self, ap, "updated"); nm_wifi_ap_update_from_properties (found_ap, object_path, properties); } else { - nm_wifi_ap_dump (ap, "added ", nm_device_get_iface (NM_DEVICE (self))); + _ap_dump (self, ap, "added"); ap_add_remove (self, ACCESS_POINT_ADDED, ap, TRUE); } @@ -1695,7 +1708,7 @@ supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface, ap = get_ap_by_supplicant_path (self, object_path); if (ap) { - nm_wifi_ap_dump (ap, "updated ", nm_device_get_iface (NM_DEVICE (self))); + _ap_dump (self, ap, "updated"); nm_wifi_ap_update_from_properties (ap, object_path, properties); schedule_ap_list_dump (self); } @@ -1723,7 +1736,7 @@ supplicant_iface_bss_removed_cb (NMSupplicantInterface *iface, */ nm_wifi_ap_set_fake (ap, TRUE); } else { - nm_wifi_ap_dump (ap, "removed ", nm_device_get_iface (NM_DEVICE (self))); + _ap_dump (self, ap, "removed"); ap_add_remove (self, ACCESS_POINT_REMOVED, ap, TRUE); schedule_ap_list_dump (self); } diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index 9d8ba42c61..702e5e8c31 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -206,16 +206,19 @@ 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)); - g_return_if_fail (addr != NULL); - g_return_if_fail (nm_utils_hwaddr_valid (addr, ETH_ALEN)); + if ( !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, -1, priv->address, -1)) { + if ( !priv->address + || !nm_utils_hwaddr_matches (addr_buf, sizeof (addr_buf), priv->address, -1)) { g_free (priv->address); - priv->address = g_strdup (addr); + priv->address = nm_utils_hwaddr_ntoa (addr_buf, sizeof (addr_buf)); _notify (ap, PROP_HW_ADDRESS); } } @@ -572,50 +575,43 @@ add_group_ciphers (NMWifiAP *ap, NMSettingWirelessSecurity *sec) nm_wifi_ap_set_rsn_flags (ap, priv->rsn_flags | flags); } -static char -mode_to_char (NMWifiAP *self) +const char * +nm_wifi_ap_to_string (const NMWifiAP *self, + char *str_buf, + gulong buf_len) { - NMWifiAPPrivate *priv = NM_WIFI_AP_GET_PRIVATE (self); - - if (priv->mode == NM_802_11_MODE_ADHOC) - return '*'; - if (priv->hotspot) - return '#'; - if (priv->fake) - return '-'; - return ' '; -} - -void -nm_wifi_ap_dump (NMWifiAP *self, - const char *prefix, - const char *ifname) -{ - NMWifiAPPrivate *priv; + const NMWifiAPPrivate *priv; const char *supplicant_id = "-"; guint32 chan; + char b1[200]; - g_return_if_fail (NM_IS_WIFI_AP (self)); + g_return_val_if_fail (NM_IS_WIFI_AP (self), NULL); 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, '/'); - nm_log_dbg (LOGD_WIFI_SCAN, "%s[%s%c] %-32s[%s%u %3u%% %c W:%04X R:%04X] [%3u] %s%s", - prefix, + g_snprintf (str_buf, buf_len, + "%17s %-32s [ %c %3u %3u%% %c W:%04X R:%04X ] %3us %s", priv->address ?: "(none)", - mode_to_char (self), - priv->ssid ? nm_utils_escape_ssid (priv->ssid->data, priv->ssid->len) : "(none)", - chan > 99 ? "" : (chan > 9 ? " " : " "), + nm_sprintf_buf (b1, "%s%s%s", + NM_PRINT_FMT_QUOTED (priv->ssid, "\"", nm_utils_escape_ssid (priv->ssid->data, priv->ssid->len), "\"", "(none)")), + (priv->mode == NM_802_11_MODE_ADHOC + ? '*' + : (priv->hotspot + ? '#' + : (priv->fake + ? 'f' + : 'a'))), chan, priv->strength, - priv->flags & NM_802_11_AP_FLAGS_PRIVACY ? 'P' : ' ', + priv->flags & NM_802_11_AP_FLAGS_PRIVACY ? 'P' : '_', priv->wpa_flags & 0xFFFF, priv->rsn_flags & 0xFFFF, priv->last_seen > 0 ? (nm_utils_get_monotonic_timestamp_s () - priv->last_seen) : -1, - ifname, supplicant_id); + return str_buf; } static guint diff --git a/src/devices/wifi/nm-wifi-ap.h b/src/devices/wifi/nm-wifi-ap.h index 5a2a2e58b5..818e651dbc 100644 --- a/src/devices/wifi/nm-wifi-ap.h +++ b/src/devices/wifi/nm-wifi-ap.h @@ -89,8 +89,8 @@ gboolean nm_wifi_ap_get_fake (const NMWifiAP *ap); void nm_wifi_ap_set_fake (NMWifiAP *ap, gboolean fake); -void nm_wifi_ap_dump (NMWifiAP *self, - const char *prefix, - const char *ifname); +const char *nm_wifi_ap_to_string (const NMWifiAP *self, + char *str_buf, + gulong buf_len); #endif /* __NM_WIFI_AP_H__ */ From 1272f3cc04f8b755bbc0531c2635bc8a9548a12c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 16:08:20 +0100 Subject: [PATCH 13/21] device/wifi: log scanning related messages with LOGD_WIFI instead of LOGD_WIFI_SCAN LOGD_WIFI_SCAN is there to avoid flodding the log with continous scan results. It should not be used for messages related to scheduling scan requests. This is especially important, because LOGD_WIFI_SCAN domain is not included in LOGD_DEFAULT. (cherry picked from commit d5657d003cdfa2fc798c05212c8e8fd8a8e7023c) --- src/devices/wifi/nm-device-wifi.c | 55 ++++++++++++++++--------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 11905d7d6f..d5446dde63 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -319,7 +319,7 @@ supplicant_interface_release (NMDeviceWifi *self) /* Reset the scan interval to be pretty frequent when disconnected */ priv->scan_interval = SCAN_INTERVAL_MIN + SCAN_INTERVAL_STEP; - _LOGD (LOGD_WIFI_SCAN, "reset scanning interval to %d seconds", + _LOGD (LOGD_WIFI, "reset scanning interval to %d seconds", priv->scan_interval); nm_clear_g_source (&priv->ap_dump_id); @@ -1427,7 +1427,7 @@ request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) } if (check_scanning_allowed (self)) { - _LOGD (LOGD_WIFI_SCAN, "scanning requested"); + _LOGD (LOGD_WIFI, "scanning requested"); if (scan_options) { GVariant *val = g_variant_lookup_value (scan_options, "ssids", NULL); @@ -1436,14 +1436,14 @@ request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) if (g_variant_is_of_type (val, G_VARIANT_TYPE ("aay"))) ssids = ssids_options_to_ptrarray (val); else - _LOGD (LOGD_WIFI_SCAN, "ignoring invalid 'ssids' scan option"); + _LOGD (LOGD_WIFI, "ignoring invalid 'ssids' scan option"); g_variant_unref (val); } } if (!ssids) ssids = build_hidden_probe_list (self); - if (nm_logging_enabled (LOGL_DEBUG, LOGD_WIFI_SCAN)) { + if (_LOGD_ENABLED (LOGD_WIFI)) { if (ssids) { const GByteArray *ssid; guint i; @@ -1454,12 +1454,12 @@ request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) foo = ssid->len > 0 ? nm_utils_ssid_to_utf8 (ssid->data, ssid->len) : NULL; - _LOGD (LOGD_WIFI_SCAN, "(%d) probe scanning SSID '%s'", + _LOGD (LOGD_WIFI, "(%d) probe scanning SSID '%s'", i, foo ? foo : ""); g_free (foo); } } else - _LOGD (LOGD_WIFI_SCAN, "no SSIDs to probe scan"); + _LOGD (LOGD_WIFI, "no SSIDs to probe scan"); } _hw_addr_set_scanning (self, FALSE); @@ -1473,7 +1473,7 @@ request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) if (ssids) g_ptr_array_unref (ssids); } else - _LOGD (LOGD_WIFI_SCAN, "scan requested but not allowed at this time"); + _LOGD (LOGD_WIFI, "scan requested but not allowed at this time"); schedule_scan (self, backoff); } @@ -1532,7 +1532,7 @@ schedule_scan (NMDeviceWifi *self, gboolean backoff) priv->scan_interval = 5; } - _LOGD (LOGD_WIFI_SCAN, "scheduled scan in %d seconds (interval now %d seconds)", + _LOGD (LOGD_WIFI, "scheduled scan in %d seconds (interval now %d seconds)", next_scan, priv->scan_interval); } } @@ -1544,7 +1544,7 @@ supplicant_iface_scan_done_cb (NMSupplicantInterface *iface, { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - _LOGD (LOGD_WIFI_SCAN, "scan %s", success ? "successful" : "failed"); + _LOGD (LOGD_WIFI, "scan %s", success ? "successful" : "failed"); priv->last_scan = nm_utils_get_monotonic_timestamp_s (); schedule_scan (self, success); @@ -1562,17 +1562,21 @@ ap_list_dump (gpointer user_data) { NMDeviceWifi *self = NM_DEVICE_WIFI (user_data); NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - gs_free NMWifiAP **list = NULL; - gsize i; priv->ap_dump_id = 0; - _LOGD (LOGD_WIFI_SCAN, "APs: [now:%u last:%u next:%u]", - nm_utils_get_monotonic_timestamp_s (), - priv->last_scan, - priv->scheduled_scan_time); - list = ap_list_get_sorted (self, TRUE); - for (i = 0; list[i]; i++) - _ap_dump (self, list[i], "dump"); + + if (_LOGD_ENABLED (LOGD_WIFI_SCAN)) { + gs_free NMWifiAP **list = NULL; + gsize i; + + _LOGD (LOGD_WIFI_SCAN, "APs: [now:%u last:%u next:%u]", + nm_utils_get_monotonic_timestamp_s (), + priv->last_scan, + priv->scheduled_scan_time); + list = ap_list_get_sorted (self, TRUE); + for (i = 0; list[i]; i++) + _ap_dump (self, list[i], "dump"); + } return G_SOURCE_REMOVE; } @@ -1581,10 +1585,9 @@ schedule_ap_list_dump (NMDeviceWifi *self) { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - if (!nm_logging_enabled (LOGL_DEBUG, LOGD_WIFI_SCAN)) - return; nm_clear_g_source (&priv->ap_dump_id); - priv->ap_dump_id = g_timeout_add_seconds (1, ap_list_dump, self); + if (_LOGD_ENABLED (LOGD_WIFI_SCAN)) + priv->ap_dump_id = g_timeout_add_seconds (1, ap_list_dump, self); } static void @@ -1646,7 +1649,7 @@ supplicant_iface_new_bss_cb (NMSupplicantInterface *iface, ap = nm_wifi_ap_new_from_properties (object_path, properties); if (!ap) { - _LOGD (LOGD_WIFI_SCAN, "invalid AP properties received for %s", object_path); + _LOGD (LOGD_WIFI, "invalid AP properties received for %s", object_path); return; } @@ -1659,11 +1662,11 @@ supplicant_iface_new_bss_cb (NMSupplicantInterface *iface, 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_SCAN, "matched hidden AP %s => '%s'", + _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_SCAN, "failed to match hidden AP %s", + _LOGD (LOGD_WIFI, "failed to match hidden AP %s", nm_wifi_ap_get_address (ap)); } } @@ -2054,7 +2057,7 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, switch (new_state) { case NM_SUPPLICANT_INTERFACE_STATE_READY: - _LOGD (LOGD_WIFI_SCAN, "supplicant ready"); + _LOGD (LOGD_WIFI, "supplicant ready"); nm_device_queue_recheck_available (NM_DEVICE (device), NM_DEVICE_STATE_REASON_SUPPLICANT_AVAILABLE, NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); @@ -2190,7 +2193,7 @@ supplicant_iface_notify_scanning_cb (NMSupplicantInterface *iface, gboolean scanning; scanning = nm_supplicant_interface_get_scanning (iface); - _LOGD (LOGD_WIFI_SCAN, "now %s", scanning ? "scanning" : "idle"); + _LOGD (LOGD_WIFI, "now %s", scanning ? "scanning" : "idle"); _notify (self, PROP_SCANNING); From 023bceb41ce9f0cd5abeb65974792d06daa9e6ab Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 16:22:57 +0100 Subject: [PATCH 14/21] device/wifi: ensure consistent timestamp for dumping Wi-Fi AP When we dump a list of APs, determine one timestamp for "now", instead of re-evaluating it every time. This ensures that all APs are printed with the same understanding of the current timestamp. (cherry picked from commit 5e4d13271cfa5d13f7d847a2cdd269cd11cb56cd) --- src/devices/wifi/nm-device-wifi.c | 18 ++++++++++-------- src/devices/wifi/nm-wifi-ap.c | 5 +++-- src/devices/wifi/nm-wifi-ap.h | 3 ++- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index d5446dde63..64b55e3bde 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -198,14 +198,15 @@ static void _hw_addr_set_scanning (NMDeviceWifi *self, gboolean do_reset); static void _ap_dump (NMDeviceWifi *self, const NMWifiAP *ap, - const char *prefix) + const char *prefix, + gint32 now_s) { char buf[1024]; buf[0] = '\0'; _LOGD (LOGD_WIFI_SCAN, "wifi-ap: %-7s %s", prefix, - nm_wifi_ap_to_string (ap, buf, sizeof (buf))); + nm_wifi_ap_to_string (ap, buf, sizeof (buf), now_s)); } static void @@ -1568,14 +1569,15 @@ ap_list_dump (gpointer user_data) if (_LOGD_ENABLED (LOGD_WIFI_SCAN)) { gs_free NMWifiAP **list = NULL; gsize i; + gint32 now_s = nm_utils_get_monotonic_timestamp_s (); _LOGD (LOGD_WIFI_SCAN, "APs: [now:%u last:%u next:%u]", - nm_utils_get_monotonic_timestamp_s (), + now_s, priv->last_scan, priv->scheduled_scan_time); list = ap_list_get_sorted (self, TRUE); for (i = 0; list[i]; i++) - _ap_dump (self, list[i], "dump"); + _ap_dump (self, list[i], "dump", now_s); } return G_SOURCE_REMOVE; } @@ -1673,10 +1675,10 @@ supplicant_iface_new_bss_cb (NMSupplicantInterface *iface, found_ap = get_ap_by_supplicant_path (self, object_path); if (found_ap) { - _ap_dump (self, ap, "updated"); + _ap_dump (self, ap, "updated", 0); nm_wifi_ap_update_from_properties (found_ap, object_path, properties); } else { - _ap_dump (self, ap, "added"); + _ap_dump (self, ap, "added", 0); ap_add_remove (self, ACCESS_POINT_ADDED, ap, TRUE); } @@ -1711,7 +1713,7 @@ supplicant_iface_bss_updated_cb (NMSupplicantInterface *iface, ap = get_ap_by_supplicant_path (self, object_path); if (ap) { - _ap_dump (self, ap, "updated"); + _ap_dump (self, ap, "updated", 0); nm_wifi_ap_update_from_properties (ap, object_path, properties); schedule_ap_list_dump (self); } @@ -1739,7 +1741,7 @@ supplicant_iface_bss_removed_cb (NMSupplicantInterface *iface, */ nm_wifi_ap_set_fake (ap, TRUE); } else { - _ap_dump (self, ap, "removed"); + _ap_dump (self, ap, "removed", 0); ap_add_remove (self, ACCESS_POINT_REMOVED, ap, TRUE); schedule_ap_list_dump (self); } diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index 702e5e8c31..96e9d19c5f 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -578,7 +578,8 @@ add_group_ciphers (NMWifiAP *ap, NMSettingWirelessSecurity *sec) const char * nm_wifi_ap_to_string (const NMWifiAP *self, char *str_buf, - gulong buf_len) + gulong buf_len, + gint32 now_s) { const NMWifiAPPrivate *priv; const char *supplicant_id = "-"; @@ -609,7 +610,7 @@ nm_wifi_ap_to_string (const NMWifiAP *self, priv->flags & NM_802_11_AP_FLAGS_PRIVACY ? 'P' : '_', priv->wpa_flags & 0xFFFF, priv->rsn_flags & 0xFFFF, - priv->last_seen > 0 ? (nm_utils_get_monotonic_timestamp_s () - priv->last_seen) : -1, + priv->last_seen > 0 ? ((now_s > 0 ? now_s : nm_utils_get_monotonic_timestamp_s ()) - priv->last_seen) : -1, supplicant_id); return str_buf; } diff --git a/src/devices/wifi/nm-wifi-ap.h b/src/devices/wifi/nm-wifi-ap.h index 818e651dbc..a68aece8a8 100644 --- a/src/devices/wifi/nm-wifi-ap.h +++ b/src/devices/wifi/nm-wifi-ap.h @@ -91,6 +91,7 @@ void nm_wifi_ap_set_fake (NMWifiAP *ap, const char *nm_wifi_ap_to_string (const NMWifiAP *self, char *str_buf, - gulong buf_len); + gulong buf_len, + gint32 now_s); #endif /* __NM_WIFI_AP_H__ */ From b8f3039d8738c9c73bc02e7b03da7d6ef661d72c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 16:49:57 +0100 Subject: [PATCH 15/21] device/wifi: give wifi-scan related logging message common prefix Add a prefix "wifi-scan: " to related logging messages for easier searching the logfiles. (cherry picked from commit a2798c18b6c4a000ebc5d8b4da3afd0e0245bc65) --- src/devices/wifi/nm-device-wifi.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 64b55e3bde..bd5adae1ae 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -320,8 +320,8 @@ supplicant_interface_release (NMDeviceWifi *self) /* Reset the scan interval to be pretty frequent when disconnected */ priv->scan_interval = SCAN_INTERVAL_MIN + SCAN_INTERVAL_STEP; - _LOGD (LOGD_WIFI, "reset scanning interval to %d seconds", - priv->scan_interval); + _LOGD (LOGD_WIFI, "wifi-scan: reset interval to %u seconds", + (unsigned) priv->scan_interval); nm_clear_g_source (&priv->ap_dump_id); @@ -1428,7 +1428,7 @@ request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) } if (check_scanning_allowed (self)) { - _LOGD (LOGD_WIFI, "scanning requested"); + _LOGD (LOGD_WIFI, "wifi-scan: scanning requested"); if (scan_options) { GVariant *val = g_variant_lookup_value (scan_options, "ssids", NULL); @@ -1437,7 +1437,7 @@ request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) if (g_variant_is_of_type (val, G_VARIANT_TYPE ("aay"))) ssids = ssids_options_to_ptrarray (val); else - _LOGD (LOGD_WIFI, "ignoring invalid 'ssids' scan option"); + _LOGD (LOGD_WIFI, "wifi-scan: ignoring invalid 'ssids' scan option"); g_variant_unref (val); } } @@ -1455,12 +1455,12 @@ request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) foo = ssid->len > 0 ? nm_utils_ssid_to_utf8 (ssid->data, ssid->len) : NULL; - _LOGD (LOGD_WIFI, "(%d) probe scanning SSID '%s'", - i, foo ? foo : ""); + _LOGD (LOGD_WIFI, "wifi-scan: (%u) probe scanning SSID %s%s%s", + i, NM_PRINT_FMT_QUOTED (foo, "\"", foo, "\"", "")); g_free (foo); } } else - _LOGD (LOGD_WIFI, "no SSIDs to probe scan"); + _LOGD (LOGD_WIFI, "wifi-scan: no SSIDs to probe scan"); } _hw_addr_set_scanning (self, FALSE); @@ -1474,7 +1474,7 @@ request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) if (ssids) g_ptr_array_unref (ssids); } else - _LOGD (LOGD_WIFI, "scan requested but not allowed at this time"); + _LOGD (LOGD_WIFI, "wifi-scan: scanning requested but not allowed at this time"); schedule_scan (self, backoff); } @@ -1533,7 +1533,7 @@ schedule_scan (NMDeviceWifi *self, gboolean backoff) priv->scan_interval = 5; } - _LOGD (LOGD_WIFI, "scheduled scan in %d seconds (interval now %d seconds)", + _LOGD (LOGD_WIFI, "wifi-scan: scheduled in %d seconds (interval now %d seconds)", next_scan, priv->scan_interval); } } @@ -1545,7 +1545,7 @@ supplicant_iface_scan_done_cb (NMSupplicantInterface *iface, { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - _LOGD (LOGD_WIFI, "scan %s", success ? "successful" : "failed"); + _LOGD (LOGD_WIFI, "wifi-scan: done [%s]", success ? "successful" : "failed"); priv->last_scan = nm_utils_get_monotonic_timestamp_s (); schedule_scan (self, success); @@ -2195,7 +2195,7 @@ supplicant_iface_notify_scanning_cb (NMSupplicantInterface *iface, gboolean scanning; scanning = nm_supplicant_interface_get_scanning (iface); - _LOGD (LOGD_WIFI, "now %s", scanning ? "scanning" : "idle"); + _LOGD (LOGD_WIFI, "wifi-scan: now %s", scanning ? "scanning" : "idle"); _notify (self, PROP_SCANNING); From a57a1940c208c75bcee239e87a8f16771c655aeb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 12:38:35 +0100 Subject: [PATCH 16/21] device/wifi/trivial: reorder code in "nm-device-wifi.c" (cherry picked from commit 1b7f03d1a554c91ac7df2412b8e0f3449512cb74) --- src/devices/wifi/nm-device-wifi.c | 147 +++++++++++++++--------------- 1 file changed, 74 insertions(+), 73 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index bd5adae1ae..64804760db 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -209,21 +209,6 @@ _ap_dump (NMDeviceWifi *self, nm_wifi_ap_to_string (ap, buf, sizeof (buf), now_s)); } -static void -constructed (GObject *object) -{ - NMDeviceWifi *self = NM_DEVICE_WIFI (object); - NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - - G_OBJECT_CLASS (nm_device_wifi_parent_class)->constructed (object); - - if (priv->capabilities & NM_WIFI_DEVICE_CAP_AP) - _LOGI (LOGD_PLATFORM | LOGD_WIFI, "driver supports Access Point (AP) mode"); - - /* Connect to the supplicant manager */ - priv->sup_mgr = g_object_ref (nm_supplicant_manager_get ()); -} - static gboolean unmanaged_on_quit (NMDevice *self) { @@ -3100,64 +3085,6 @@ set_enabled (NMDevice *device, gboolean enabled) /*****************************************************************************/ -NMDevice * -nm_device_wifi_new (const char *iface, NMDeviceWifiCapabilities capabilities) -{ - return g_object_new (NM_TYPE_DEVICE_WIFI, - NM_DEVICE_IFACE, iface, - NM_DEVICE_TYPE_DESC, "802.11 WiFi", - NM_DEVICE_DEVICE_TYPE, NM_DEVICE_TYPE_WIFI, - NM_DEVICE_LINK_TYPE, NM_LINK_TYPE_WIFI, - NM_DEVICE_RFKILL_TYPE, RFKILL_TYPE_WLAN, - NM_DEVICE_WIFI_CAPABILITIES, (guint) capabilities, - NULL); -} - -static void -nm_device_wifi_init (NMDeviceWifi *self) -{ - NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - - priv->mode = NM_802_11_MODE_INFRA; - priv->aps = g_hash_table_new (g_str_hash, g_str_equal); -} - -static void -dispose (GObject *object) -{ - NMDeviceWifi *self = NM_DEVICE_WIFI (object); - NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - - nm_clear_g_source (&priv->periodic_source_id); - - wifi_secrets_cancel (self); - - cleanup_association_attempt (self, TRUE); - supplicant_interface_release (self); - cleanup_supplicant_failures (self); - - g_clear_object (&priv->sup_mgr); - - remove_all_aps (self); - - G_OBJECT_CLASS (nm_device_wifi_parent_class)->dispose (object); -} - -static void -finalize (GObject *object) -{ - NMDeviceWifi *self = NM_DEVICE_WIFI (object); - NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - - nm_assert (g_hash_table_size (priv->aps) == 0); - - g_hash_table_unref (priv->aps); - - g_free (priv->hw_addr_scan); - - G_OBJECT_CLASS (nm_device_wifi_parent_class)->finalize (object); -} - static void get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) @@ -3213,6 +3140,80 @@ set_property (GObject *object, guint prop_id, } } +/*****************************************************************************/ + +static void +nm_device_wifi_init (NMDeviceWifi *self) +{ + NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); + + priv->mode = NM_802_11_MODE_INFRA; + priv->aps = g_hash_table_new (g_str_hash, g_str_equal); +} + +static void +constructed (GObject *object) +{ + NMDeviceWifi *self = NM_DEVICE_WIFI (object); + NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); + + G_OBJECT_CLASS (nm_device_wifi_parent_class)->constructed (object); + + if (priv->capabilities & NM_WIFI_DEVICE_CAP_AP) + _LOGI (LOGD_PLATFORM | LOGD_WIFI, "driver supports Access Point (AP) mode"); + + /* Connect to the supplicant manager */ + priv->sup_mgr = g_object_ref (nm_supplicant_manager_get ()); +} + +NMDevice * +nm_device_wifi_new (const char *iface, NMDeviceWifiCapabilities capabilities) +{ + return g_object_new (NM_TYPE_DEVICE_WIFI, + NM_DEVICE_IFACE, iface, + NM_DEVICE_TYPE_DESC, "802.11 WiFi", + NM_DEVICE_DEVICE_TYPE, NM_DEVICE_TYPE_WIFI, + NM_DEVICE_LINK_TYPE, NM_LINK_TYPE_WIFI, + NM_DEVICE_RFKILL_TYPE, RFKILL_TYPE_WLAN, + NM_DEVICE_WIFI_CAPABILITIES, (guint) capabilities, + NULL); +} + +static void +dispose (GObject *object) +{ + NMDeviceWifi *self = NM_DEVICE_WIFI (object); + NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); + + nm_clear_g_source (&priv->periodic_source_id); + + wifi_secrets_cancel (self); + + cleanup_association_attempt (self, TRUE); + supplicant_interface_release (self); + cleanup_supplicant_failures (self); + + g_clear_object (&priv->sup_mgr); + + remove_all_aps (self); + + G_OBJECT_CLASS (nm_device_wifi_parent_class)->dispose (object); +} + +static void +finalize (GObject *object) +{ + NMDeviceWifi *self = NM_DEVICE_WIFI (object); + NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); + + nm_assert (g_hash_table_size (priv->aps) == 0); + + g_hash_table_unref (priv->aps); + + g_free (priv->hw_addr_scan); + + G_OBJECT_CLASS (nm_device_wifi_parent_class)->finalize (object); +} static void nm_device_wifi_class_init (NMDeviceWifiClass *klass) From f9c95bdd2303aff7541f7e793679d1174b8b364e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 17:16:27 +0100 Subject: [PATCH 17/21] device/wifi: cache GObject property "scanning" Cache the value for accessing the GObject property NM_DEVICE_WIFI_SCANNING. Re-evaluating the property every time by checking the supplicant interface is ugly because it might change under the hood. It should only change if (and only if) we emit a notify changed signal. Also, avoid accessing nm_supplicant_interface_get_scanning (priv->sup_iface) without checking whether priv->sup_iface is not NULL. (cherry picked from commit 4e84472b471cf82baea3cf3f61562575928f0e0c) --- src/devices/wifi/nm-device-wifi.c | 40 +++++++++++++++++------- src/supplicant/nm-supplicant-interface.c | 2 +- 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 64804760db..33847b8866 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -95,6 +95,7 @@ typedef struct { bool enabled:1; /* rfkilled or not */ bool requested_scan:1; bool ssid_found:1; + bool is_scanning:1; gint32 last_scan; gint32 scheduled_scan_time; @@ -209,6 +210,23 @@ _ap_dump (NMDeviceWifi *self, nm_wifi_ap_to_string (ap, buf, sizeof (buf), now_s)); } +static void +_notify_scanning (NMDeviceWifi *self) +{ + NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); + gboolean scanning; + + scanning = priv->sup_iface + && nm_supplicant_interface_get_scanning (priv->sup_iface); + + if (scanning == priv->is_scanning) + return; + + _LOGD (LOGD_WIFI, "wifi-scan: scanning-state: %s", scanning ? "scanning" : "idle"); + priv->is_scanning = scanning; + _notify (self, PROP_SCANNING); +} + static gboolean unmanaged_on_quit (NMDevice *self) { @@ -269,6 +287,8 @@ supplicant_interface_acquire (NMDeviceWifi *self) G_CALLBACK (supplicant_iface_notify_current_bss), self); + _notify_scanning (self); + return TRUE; } @@ -319,6 +339,8 @@ supplicant_interface_release (NMDeviceWifi *self) g_clear_object (&priv->sup_iface); } + + _notify_scanning (self); } static NMWifiAP * @@ -1530,7 +1552,7 @@ supplicant_iface_scan_done_cb (NMSupplicantInterface *iface, { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - _LOGD (LOGD_WIFI, "wifi-scan: done [%s]", success ? "successful" : "failed"); + _LOGD (LOGD_WIFI, "wifi-scan: scan-done callback: %s", success ? "successful" : "failed"); priv->last_scan = nm_utils_get_monotonic_timestamp_s (); schedule_scan (self, success); @@ -2138,7 +2160,7 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, /* Signal scanning state changes */ if ( new_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING || old_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING) - _notify (self, PROP_SCANNING); + _notify_scanning (self); } static void @@ -2176,17 +2198,11 @@ supplicant_iface_notify_scanning_cb (NMSupplicantInterface *iface, GParamSpec *pspec, NMDeviceWifi *self) { - NMDeviceState state; - gboolean scanning; - - scanning = nm_supplicant_interface_get_scanning (iface); - _LOGD (LOGD_WIFI, "wifi-scan: now %s", scanning ? "scanning" : "idle"); - - _notify (self, PROP_SCANNING); + _notify_scanning (self); /* Run a quick update of current AP when coming out of a scan */ - state = nm_device_get_state (NM_DEVICE (self)); - if (!scanning && state == NM_DEVICE_STATE_ACTIVATED) + if ( !NM_DEVICE_WIFI_GET_PRIVATE (self)->is_scanning + && nm_device_get_state (NM_DEVICE (self)) == NM_DEVICE_STATE_ACTIVATED) periodic_update (self); } @@ -3114,7 +3130,7 @@ get_property (GObject *object, guint prop_id, nm_utils_g_value_set_object_path (value, priv->current_ap); break; case PROP_SCANNING: - g_value_set_boolean (value, nm_supplicant_interface_get_scanning (priv->sup_iface)); + g_value_set_boolean (value, priv->is_scanning); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index c913a6960c..8bed7eed4c 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -352,7 +352,7 @@ nm_supplicant_interface_get_scanning (NMSupplicantInterface *self) { NMSupplicantInterfacePrivate *priv; - g_return_val_if_fail (self != NULL, FALSE); + g_return_val_if_fail (self, FALSE); priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); if (priv->scanning) From 442ea0348deb63548d7e6c97e7b727e6e606d8bb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 18:39:38 +0100 Subject: [PATCH 18/21] device/wifi: don't reschedule idle handler for schedule_ap_list_dump() (cherry picked from commit e4a9942ba8a24744d043ec0b9dece3d347b0fc64) --- src/devices/wifi/nm-device-wifi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 33847b8866..3891de7ead 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1594,8 +1594,8 @@ schedule_ap_list_dump (NMDeviceWifi *self) { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - nm_clear_g_source (&priv->ap_dump_id); - if (_LOGD_ENABLED (LOGD_WIFI_SCAN)) + if ( !priv->ap_dump_id + && _LOGD_ENABLED (LOGD_WIFI_SCAN)) priv->ap_dump_id = g_timeout_add_seconds (1, ap_list_dump, self); } From 9977609b7ae4412d5c69303acda387a4f623fdde Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 22:30:37 +0100 Subject: [PATCH 19/21] device/wifi: first emit NEW_BSS signals before SCAN_DONE In the SCAN_DONE handler, NMDeviceWifi resets the flag that indicates that a current scan request is pending. We need to first obtain the new APs (NEW_BSS) before signalling SCAN_DONE. (cherry picked from commit 40a4cc5b2dcb2de6643ac36ff6efd0a1262093f5) --- src/supplicant/nm-supplicant-interface.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 8bed7eed4c..2698b74f7e 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -562,8 +562,6 @@ wpas_iface_scan_done (GDBusProxy *proxy, /* Cache last scan completed time */ priv->last_scan = nm_utils_get_monotonic_timestamp_s (); - g_signal_emit (self, signals[SCAN_DONE], 0, success); - /* Emit NEW_BSS 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)) { @@ -577,6 +575,8 @@ wpas_iface_scan_done (GDBusProxy *proxy, } } } + + g_signal_emit (self, signals[SCAN_DONE], 0, success); } static void From 29b6ba0cd4a81df73f0a44366e7380101de6f5c0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Feb 2017 22:36:11 +0100 Subject: [PATCH 20/21] device/wifi: don't emit wrong SCAN_DONE signal when "Scan" request completes scan_request_cb() handles the answer from the D-Bus "Scan" method. At that point, the scan is not yet done, it merely started. It is wrong to already signal SCAN_DONE. The only place where we want to signal SCAN_DONE is when we actually receive the "ScanDone" D-Bus signal. (cherry picked from commit 75356841fb85c5691c8694bcc8cc0a77bdf08b6b) --- src/supplicant/nm-supplicant-interface.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 2698b74f7e..f932fe8b53 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -1269,7 +1269,6 @@ scan_request_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) return; self = NM_SUPPLICANT_INTERFACE (user_data); - if (error) { if (_nm_dbus_error_has_name (error, "fi.w1.wpa_supplicant1.Interface.ScanError")) _LOGD ("could not get scan request result: %s", error->message); @@ -1278,7 +1277,6 @@ scan_request_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_data) _LOGW ("could not get scan request result: %s", error->message); } } - g_signal_emit (self, signals[SCAN_DONE], 0, error ? FALSE : TRUE); } gboolean From 8b95169791fa9763c9dd409231f2a77dd5ac5c08 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 5 Feb 2017 21:32:28 +0100 Subject: [PATCH 21/21] device/wifi: prevent clearing pending wifi scan action during inactive supplicant [1486325858.0691] device[0x563b8fba42e0] (wlp3s0): wifi-scan: scan-done callback: successful [1486325858.0692] device[0x563b8fba42e0] (wlp3s0): wifi-scan: scheduled in 23 seconds (interval now 33 seconds) [1486325858.0692] device[0x563b8fba42e0] (wlp3s0): remove_pending_action (0): 'wifi-scan' [1486325858.0692] properties-changed[0x563b8fba42e0]: ignoring notification for prop has-pending-action on type NMDeviceWifi [1486325858.0692] manager: check_if_startup_complete returns FALSE because of enp0s25 [1486325858.0697] device (wlp3s0): supplicant interface state: ready -> inactive [1486325858.0698] device[0x563b8fba42e0] (wlp3s0): wifi-scan: scanning requested [1486325858.0698] device[0x563b8fba42e0] (wlp3s0): wifi-scan: (0) probe scanning SSID [1486325858.0698] device[0x563b8fba42e0] (wlp3s0): wifi-scan: (1) probe scanning SSID "aaa" [1486325858.0699] device[0x563b8fba42e0] (wlp3s0): set-hw-addr: no MAC address change needed (2A:71:5D:54:85:1F) [1486325858.0699] device[0x563b8fba42e0] (wlp3s0): add_pending_action (1): 'wifi-scan' (cherry picked from commit 94127d3f9eff46af68273e844caca3dd44e7f051) --- src/devices/wifi/nm-device-wifi.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 3891de7ead..188ed54eb6 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -183,7 +183,7 @@ static void supplicant_iface_notify_current_bss (NMSupplicantInterface *iface, GParamSpec *pspec, NMDeviceWifi *self); -static void request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options); +static void request_wireless_scan (NMDeviceWifi *self, gboolean force_if_scanning, GVariant *scan_options); static void ap_add_remove (NMDeviceWifi *self, guint signum, @@ -562,7 +562,7 @@ deactivate (NMDevice *device) /* Ensure we trigger a scan after deactivating a Hotspot */ if (old_mode == NM_802_11_MODE_AP) - request_wireless_scan (self, NULL); + request_wireless_scan (self, FALSE, NULL); } static void @@ -1186,7 +1186,7 @@ request_scan_cb (NMDevice *device, priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - request_wireless_scan (self, new_scan_options); + request_wireless_scan (self, FALSE, new_scan_options); g_dbus_method_invocation_return_value (context, NULL); } @@ -1421,15 +1421,16 @@ ssids_options_to_ptrarray (GVariant *value) } static void -request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) +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; nm_clear_g_source (&priv->pending_scan_id); - if (priv->requested_scan) { + if (!force_if_scanning && priv->requested_scan) { /* There's already a scan in progress */ return; } @@ -1476,6 +1477,7 @@ request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) /* success */ backoff = TRUE; _requested_scan_set (self, TRUE); + new_scan_requested = TRUE; } if (ssids) @@ -1483,6 +1485,9 @@ request_wireless_scan (NMDeviceWifi *self, GVariant *scan_options) } else _LOGD (LOGD_WIFI, "wifi-scan: scanning requested but not allowed at this time"); + if (!new_scan_requested) + _requested_scan_set (self, FALSE); + schedule_scan (self, backoff); } @@ -1493,7 +1498,7 @@ request_wireless_scan_periodic (gpointer user_data) NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); priv->pending_scan_id = 0; - request_wireless_scan (self, NULL); + request_wireless_scan (self, FALSE, NULL); return G_SOURCE_REMOVE; } @@ -2150,8 +2155,10 @@ supplicant_iface_state_cb (NMSupplicantInterface *iface, _LOGI (LOGD_DEVICE | LOGD_WIFI, "supplicant interface keeps failing, giving up"); break; case NM_SUPPLICANT_INTERFACE_STATE_INACTIVE: - _requested_scan_set (self, FALSE); - request_wireless_scan (self, NULL); + /* we would clear _requested_scan_set() and trigger a new scan. + * However, we don't want to cancel the current pending action, so force + * a new scan request. */ + request_wireless_scan (self, TRUE, NULL); break; default: break; @@ -3027,7 +3034,7 @@ device_state_changed (NMDevice *device, case NM_DEVICE_STATE_DISCONNECTED: /* Kick off a scan to get latest results */ priv->scan_interval = SCAN_INTERVAL_MIN; - request_wireless_scan (self, NULL); + request_wireless_scan (self, FALSE, NULL); break; default: break;