From 962ad7f8506ed54073b51bb5485484d532ba92c6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 24 Mar 2020 11:07:32 +0100 Subject: [PATCH 01/19] shared: accept empty buffer for nm_hash_update() There is no need to reject empty buffers. c_siphash_append() handles them gracefully. --- shared/nm-glib-aux/nm-hash-utils.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shared/nm-glib-aux/nm-hash-utils.h b/shared/nm-glib-aux/nm-hash-utils.h index 21c5e58416..0e64738fb1 100644 --- a/shared/nm-glib-aux/nm-hash-utils.h +++ b/shared/nm-glib-aux/nm-hash-utils.h @@ -95,8 +95,7 @@ static inline void nm_hash_update (NMHashState *state, const void *ptr, gsize n) { nm_assert (state); - nm_assert (ptr); - nm_assert (n > 0); + nm_assert (n == 0 || ptr); /* Note: the data passed in here might be sensitive data (secrets), * that we should nm_explicty_zero() afterwards. However, since From b1503d8a724e09ff9fa7ab4ddf70ce1cdfb7462a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Mar 2020 21:04:57 +0100 Subject: [PATCH 02/19] shared: add nm_hash_mem() helper --- shared/nm-glib-aux/nm-hash-utils.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/shared/nm-glib-aux/nm-hash-utils.h b/shared/nm-glib-aux/nm-hash-utils.h index 0e64738fb1..1f29413bd9 100644 --- a/shared/nm-glib-aux/nm-hash-utils.h +++ b/shared/nm-glib-aux/nm-hash-utils.h @@ -266,6 +266,18 @@ guint nm_str_hash (gconstpointer str); nm_hash_complete (&_h); \ }) +static inline guint +nm_hash_mem (guint static_seed, const void *ptr, gsize n) +{ + NMHashState h; + + if (n == 0) + return nm_hash_static (static_seed); + nm_hash_init (&h, static_seed); + nm_hash_update (&h, ptr, n); + return nm_hash_complete (&h); +} + /*****************************************************************************/ /* nm_pstr_*() are for hashing keys that are pointers to strings, From 573b02f7d7451e89009f5cb9c2eb28dac85ffe50 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Mar 2020 22:47:40 +0100 Subject: [PATCH 03/19] shared: add nm_pgbytes_hash()/nm_pgbytes_equal() For hashing of a pointer to a GBytes*. This is useful if your key is a GBytes array, and the first field in your to be hashed struct. --- shared/nm-glib-aux/nm-hash-utils.c | 22 ++++++++++++++++++++++ shared/nm-glib-aux/nm-hash-utils.h | 5 +++++ 2 files changed, 27 insertions(+) diff --git a/shared/nm-glib-aux/nm-hash-utils.c b/shared/nm-glib-aux/nm-hash-utils.c index cd51bbaf63..dcf662053a 100644 --- a/shared/nm-glib-aux/nm-hash-utils.c +++ b/shared/nm-glib-aux/nm-hash-utils.c @@ -257,3 +257,25 @@ nm_ppdirect_equal (gconstpointer a, gconstpointer b) return **s1 == **s2; } + +/*****************************************************************************/ + +guint +nm_pgbytes_hash (gconstpointer p) +{ + GBytes *const*ptr = p; + gconstpointer arr; + gsize len; + + arr = g_bytes_get_data (*ptr, &len); + return nm_hash_mem (1470631313u, arr, len); +} + +gboolean +nm_pgbytes_equal (gconstpointer a, gconstpointer b) +{ + GBytes *const*ptr_a = a; + GBytes *const*ptr_b = b; + + return g_bytes_equal (*ptr_a, *ptr_b); +} diff --git a/shared/nm-glib-aux/nm-hash-utils.h b/shared/nm-glib-aux/nm-hash-utils.h index 1f29413bd9..9879dc477b 100644 --- a/shared/nm-glib-aux/nm-hash-utils.h +++ b/shared/nm-glib-aux/nm-hash-utils.h @@ -314,6 +314,11 @@ gboolean nm_ppdirect_equal (gconstpointer a, gconstpointer b); /*****************************************************************************/ +guint nm_pgbytes_hash (gconstpointer p); +gboolean nm_pgbytes_equal (gconstpointer a, gconstpointer b); + +/*****************************************************************************/ + #define NM_HASH_OBFUSCATE_PTR_FMT "%016" G_GINT64_MODIFIER "x" /* sometimes we want to log a pointer directly, for providing context/information about From b6fdc30a887df55238e264e879e4f26e6a9634b3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 24 Mar 2020 11:31:27 +0100 Subject: [PATCH 04/19] shared: cleanup _get_hash_key_init() and better explain the reasoning - add more code comments - refactor the code flow in _get_hash_key_init() to follow a simpler code path. - use c_siphash_hash() instead of 3 separate steps. - Drop "?: static_seed" from nm_hash_static(). It's not useful, because the only _get_hash_key() for which _get_hash_key()^static_seed is zero is ~static_seed. That means, only one value of all the static seeds can result in zero here. At that point, we can just coerce that value to 3679500967u directly. --- shared/nm-glib-aux/nm-hash-utils.c | 92 ++++++++++++++++-------------- shared/nm-glib-aux/nm-hash-utils.h | 3 +- 2 files changed, 50 insertions(+), 45 deletions(-) diff --git a/shared/nm-glib-aux/nm-hash-utils.c b/shared/nm-glib-aux/nm-hash-utils.c index dcf662053a..0a701d06e1 100644 --- a/shared/nm-glib-aux/nm-hash-utils.c +++ b/shared/nm-glib-aux/nm-hash-utils.c @@ -24,7 +24,6 @@ static const guint8 *volatile global_seed = NULL; static const guint8 * _get_hash_key_init (void) { - static gsize g_lock; /* the returned hash is aligned to guin64, hence, it is safe * to use it as guint* or guint64* pointer. */ static union { @@ -34,50 +33,49 @@ _get_hash_key_init (void) guint64 _align_as_uint64; } g_arr; const guint8 *g; - union { - guint8 v8[HASH_KEY_SIZE]; - guint vuint; - } t_arr; again: g = g_atomic_pointer_get (&global_seed); - if (G_LIKELY (g != NULL)) { - nm_assert (g == g_arr.v8); - return g; - } - - { - CSipHash siph_state; + if (!G_UNLIKELY (g)) { + static gsize g_lock; uint64_t h; - - /* initialize a random key in t_arr. */ + union { + guint vuint; + guint8 v8[HASH_KEY_SIZE]; + guint8 _extra_entropy[3 * HASH_KEY_SIZE]; + } t_arr; nm_utils_random_bytes (&t_arr, sizeof (t_arr)); - /* use siphash() of the key-size, to mangle the first guint. Otherwise, - * the first guint has only the entropy that nm_utils_random_bytes() - * generated for the first 4 bytes and relies on a good random generator. + /* We only initialize one random hash key. So we can spend some effort + * of getting this right. For one, we collect more random bytes than + * necessary. * - * The first int is especially interesting for nm_hash_static() below, and we - * want to have it all the entropy of t_arr. */ - c_siphash_init (&siph_state, t_arr.v8); - c_siphash_append (&siph_state, (const guint8 *) &t_arr, sizeof (t_arr)); - h = c_siphash_finalize (&siph_state); - if (sizeof (guint) < sizeof (h)) - t_arr.vuint = t_arr.vuint ^ ((guint) (h & 0xFFFFFFFFu)) ^ ((guint) (h >> 32)); + * Then, the first guint of the seed should have all the entropy that we could + * obtain in sizeof(t_arr). For that, siphash(t_arr) and xor the first guint + * with hash. + * The first guint is especially interesting for nm_hash_static() below that + * doesn't use siphash itself. */ + h = c_siphash_hash (t_arr.v8, + (const guint8 *) &t_arr, + sizeof (t_arr)); + if (sizeof (h) > sizeof (guint)) + t_arr.vuint = t_arr.vuint ^ ((guint) (h & G_MAXUINT)) ^ ((guint) (h >> 32)); else - t_arr.vuint = t_arr.vuint ^ ((guint) (h & 0xFFFFFFFFu)); + t_arr.vuint = t_arr.vuint ^ ((guint) (h & G_MAXUINT)); + + if (!g_once_init_enter (&g_lock)) { + /* lost a race. The random key is already initialized. */ + goto again; + } + + memcpy (g_arr.v8, t_arr.v8, HASH_KEY_SIZE); + g = g_arr.v8; + g_atomic_pointer_set (&global_seed, g); + g_once_init_leave (&g_lock, 1); } - if (!g_once_init_enter (&g_lock)) { - /* lost a race. The random key is already initialized. */ - goto again; - } - - memcpy (g_arr.v8, t_arr.v8, HASH_KEY_SIZE); - g = g_arr.v8; - g_atomic_pointer_set (&global_seed, g); - g_once_init_leave (&g_lock, 1); + nm_assert (g == g_arr.v8); return g; } @@ -94,18 +92,24 @@ again: guint nm_hash_static (guint static_seed) { - /* note that we only xor the static_seed with the key. - * We don't use siphash, which would mix the bits better. - * Note that this doesn't matter, because static_seed is not - * supposed to be a value that you are hashing (for that, use - * full siphash). - * Instead, different callers may set a different static_seed - * so that nm_hash_str(NULL) != nm_hash_ptr(NULL). + /* Note that we only xor the static_seed with the first guint of the key. * - * Also, ensure that we don't return zero. + * We don't use siphash, which would mix the bits better with _get_hash_key(). + * Note that nm_hash_static() isn't used to hash the static_seed. Instead, it + * is used to get a unique hash value in a static context. That means, every + * caller is responsible to choose a static_seed that is sufficiently + * distinct from all other callers. In other words, static_seed should be a + * unique constant with good entropy. + * + * Note that _get_hash_key_init() already xored the first guint of the + * key with the siphash of the entire static key. That means, even if + * we got bad randomness for the first guint, the first guint is also + * mixed with the randomness of the entire random key. + * + * Also, ensure that we don't return zero (like for nm_hash_complete()). */ - return ((*((const guint *) _get_hash_key ())) ^ static_seed) - ?: static_seed ?: 3679500967u; + return ((*((const guint *) _get_hash_key ())) ^ static_seed) + ?: 3679500967u; } void diff --git a/shared/nm-glib-aux/nm-hash-utils.h b/shared/nm-glib-aux/nm-hash-utils.h index 9879dc477b..9f2e976678 100644 --- a/shared/nm-glib-aux/nm-hash-utils.h +++ b/shared/nm-glib-aux/nm-hash-utils.h @@ -88,7 +88,8 @@ nm_hash_complete (NMHashState *state) /* we don't ever want to return a zero hash. * * NMPObject requires that in _idx_obj_part(), and it's just a good idea. */ - return (((guint) (h >> 32)) ^ ((guint) h)) ?: 1396707757u; + return (((guint) (h >> 32)) ^ ((guint) h)) + ?: 1396707757u; } static inline void From 93a6bcc8a28dedbd5b0f266e545141cf8d55a309 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 2 Apr 2020 12:01:18 +0200 Subject: [PATCH 05/19] cli: fix `nmcli device wifi list --rescan=yes` to wait Fixes: db396cea9d37 ('cli: rework do_device_wifi_list() to scan and print Wi-Fi list') --- clients/cli/devices.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clients/cli/devices.c b/clients/cli/devices.c index a4b7ec18af..d829368304 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -2922,7 +2922,6 @@ wifi_list_finish (WifiListData *wifi_list_data, guint i; if ( !force_finished - && scan_info->rescan_cutoff_msec != G_MAXINT64 && scan_info->rescan_cutoff_msec > _device_wifi_get_last_scan (wifi_list_data->wifi)) { /* wait longer... */ return; @@ -3115,7 +3114,7 @@ do_device_wifi_list (NmCli *nmc, int argc, char **argv) else if (nm_streq (rescan, "no")) rescan_cutoff_msec = G_MININT64; else if (nm_streq (rescan, "yes")) - rescan_cutoff_msec = G_MAXINT64; + rescan_cutoff_msec = nm_utils_get_timestamp_msec (); else { g_string_printf (nmc->return_text, _("Error: invalid rescan argument: '%s' not among [auto, no, yes]"), rescan); return NMC_RESULT_ERROR_USER_INPUT; From ebedd0d792b9005aed482006111ea0f9d64cc9e7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 24 Mar 2020 11:52:01 +0100 Subject: [PATCH 06/19] supplicant: log message whenever we request scanning It's important to clearly see in the log when we actually request a scan. --- src/supplicant/nm-supplicant-interface.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 9725b983dd..12d808a832 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -2318,12 +2318,13 @@ scan_request_cb (GObject *source, GAsyncResult *result, gpointer user_data) 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); + _LOGD ("request-scan: could not get scan request result: %s", error->message); else { g_dbus_error_strip_remote_error (error); - _LOGW ("could not get scan request result: %s", error->message); + _LOGW ("request-scan: could not get scan request result: %s", error->message); } - } + } else + _LOGT ("request-scan: request scanning success"); } void @@ -2339,7 +2340,8 @@ nm_supplicant_interface_request_scan (NMSupplicantInterface *self, priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - /* Scan parameters */ + _LOGT ("request-scan: request scanning (%u ssids)...", ssids_len); + g_variant_builder_init (&builder, G_VARIANT_TYPE_VARDICT); g_variant_builder_add (&builder, "{sv}", "Type", g_variant_new_string ("active")); g_variant_builder_add (&builder, "{sv}", "AllowRoam", g_variant_new_boolean (FALSE)); From b480cda59678447d87d4bc7c7055d7e02d4a5ec6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Mar 2020 11:22:24 +0100 Subject: [PATCH 07/19] supplicant: cleanup notify signals for combined properties in supplicant Certain properties (for example "scanning") are combined from multiple other properties. So, we want to notify a changed signal, exactly when something relevant changes. We also may not want to emit a signal while we are still in the middle of changing multiple properties together. Only at certain places we want to check and emit the signal. Simplify the implementation for that by tracking the property value that we currently expose, and keeping state about when it changes. --- src/supplicant/nm-supplicant-interface.c | 73 ++++++++++++++---------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index 12d808a832..b97a7e4b0f 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -141,9 +141,11 @@ typedef struct _NMSupplicantInterfacePrivate { NMSupplicantInterfaceState state; NMSupplicantInterfaceState supp_state; - bool scanning:1; + bool scanning_property:1; + bool scanning_cached:1; - bool p2p_capable:1; + bool p2p_capable_property:1; + bool p2p_capable_cached:1; bool p2p_group_is_owner:1; @@ -433,11 +435,37 @@ _remove_network (NMSupplicantInterface *self) /*****************************************************************************/ -static gboolean -_prop_p2p_available_get (NMSupplicantInterfacePrivate *priv) +static void +_notify_maybe_scanning (NMSupplicantInterface *self) { - return priv->is_ready_p2p_device - && priv->p2p_capable; + NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); + gboolean scanning; + + scanning = NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (priv->state) + && ( priv->scanning_property + || priv->supp_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING); + + if (priv->scanning_cached == scanning) + return; + + priv->scanning_cached = scanning; + _notify (self, PROP_SCANNING); +} + +static void +_notify_maybe_p2p_available (NMSupplicantInterface *self) +{ + NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); + gboolean value; + + value = priv->is_ready_p2p_device + && priv->p2p_capable_property; + + if (priv->p2p_capable_cached == value) + return; + + priv->p2p_capable_cached = value; + _notify (self, PROP_P2P_AVAILABLE); } /*****************************************************************************/ @@ -1069,20 +1097,12 @@ nm_supplicant_interface_get_current_bss (NMSupplicantInterface *self) return NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)->current_bss; } -static inline gboolean -_prop_scanning_get (NMSupplicantInterfacePrivate *priv) -{ - return ( priv->scanning - || priv->supp_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING) - && NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (priv->state); -} - gboolean nm_supplicant_interface_get_scanning (NMSupplicantInterface *self) { g_return_val_if_fail (NM_IS_SUPPLICANT_INTERFACE (self), FALSE); - return _prop_scanning_get (NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)); + return NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)->scanning_cached; } gint64 @@ -1095,7 +1115,7 @@ nm_supplicant_interface_get_last_scan (NMSupplicantInterface *self) priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); /* returns -1 if we are currently scanning. */ - return _prop_scanning_get (priv) + return priv->scanning_cached ? -1 : priv->last_scan_msec; } @@ -1129,7 +1149,7 @@ parse_capabilities (NMSupplicantInterface *self, GVariant *capabilities) /* Setting p2p_capable might toggle _prop_p2p_available_get(). However, * we don't need to check for a property changed notification, because * the caller did g_object_freeze_notify() and will perform the check. */ - priv->p2p_capable = g_strv_contains (array, "p2p"); + priv->p2p_capable_property = g_strv_contains (array, "p2p"); g_free (array); } @@ -1763,10 +1783,10 @@ _properties_changed_main (NMSupplicantInterface *self, } if (nm_g_variant_lookup (properties, "Scanning", "b", &v_b)) { - if (priv->scanning != (!!v_b)) { - if (priv->scanning) + if (priv->scanning_cached != (!!v_b)) { + if (priv->scanning_cached) priv->last_scan_msec = nm_utils_get_monotonic_timestamp_msec (); - priv->scanning = v_b; + priv->scanning_cached = v_b; } } @@ -2520,8 +2540,6 @@ _properties_changed (NMSupplicantInterface *self, { NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); gboolean is_main; - gboolean old_val_scanning; - gboolean old_val_p2p_available; nm_assert (!properties || g_variant_is_of_type (properties, G_VARIANT_TYPE ("a{sv}"))); @@ -2542,9 +2560,6 @@ _properties_changed (NMSupplicantInterface *self, priv->starting_pending_count++; - old_val_scanning = _prop_scanning_get (priv); - old_val_p2p_available = _prop_p2p_available_get (priv); - if (is_main) { priv->is_ready_main = TRUE; _properties_changed_main (self, properties); @@ -2556,10 +2571,8 @@ _properties_changed (NMSupplicantInterface *self, priv->starting_pending_count--; _starting_check_ready (self); - if (old_val_scanning != _prop_scanning_get (priv)) - _notify (self, PROP_SCANNING); - if (old_val_p2p_available != _prop_p2p_available_get (priv)) - _notify (self, PROP_P2P_AVAILABLE); + _notify_maybe_scanning (self); + _notify_maybe_p2p_available (self); g_object_thaw_notify (G_OBJECT (self)); } @@ -2878,7 +2891,7 @@ _signal_cb (GDBusConnection *connection, gboolean nm_supplicant_interface_get_p2p_available (NMSupplicantInterface *self) { - return _prop_p2p_available_get (NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)); + return NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)->p2p_capable_cached; } gboolean From 4a302e28f56967b495fd9affaef030ce2640e74f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Mar 2020 13:22:40 +0100 Subject: [PATCH 08/19] supplicant: cleanup notify signals for combined properties in supplicant (2) --- src/supplicant/nm-supplicant-interface.c | 79 +++++++++++------------- 1 file changed, 37 insertions(+), 42 deletions(-) diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index b97a7e4b0f..ec1afa25ec 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -147,7 +147,10 @@ typedef struct _NMSupplicantInterfacePrivate { bool p2p_capable_property:1; bool p2p_capable_cached:1; - bool p2p_group_is_owner:1; + bool p2p_group_owner_property:1; + bool p2p_group_owner_cached:1; + + bool p2p_group_joined_cached:1; bool is_ready_main:1; bool is_ready_p2p_device:1; @@ -468,6 +471,32 @@ _notify_maybe_p2p_available (NMSupplicantInterface *self) _notify (self, PROP_P2P_AVAILABLE); } +static void +_notify_maybe_p2p_group (NMSupplicantInterface *self) +{ + NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); + gboolean value_joined; + gboolean value_owner; + gboolean joined_changed; + gboolean owner_changed; + + value_joined = priv->p2p_group_path + && !priv->p2p_group_properties_cancellable; + value_owner = value_joined + && priv->p2p_group_owner_property; + + if ((joined_changed = (priv->p2p_group_joined_cached != value_joined))) + priv->p2p_group_joined_cached = value_joined; + + if ((owner_changed = (priv->p2p_group_owner_cached != value_owner))) + priv->p2p_group_owner_cached = value_owner; + + if (joined_changed) + _notify (self, PROP_P2P_GROUP_JOINED); + if (owner_changed) + _notify (self, PROP_P2P_GROUP_OWNER); +} + /*****************************************************************************/ static void @@ -1284,37 +1313,19 @@ nm_supplicant_interface_get_auth_state (NMSupplicantInterface *self) /*****************************************************************************/ -static gboolean -_prop_p2p_group_joined_get (NMSupplicantInterfacePrivate *priv) -{ - return priv->p2p_group_path - && !priv->p2p_group_properties_cancellable; -} - -static gboolean -_prop_p2p_group_is_owner_get (NMSupplicantInterfacePrivate *priv) -{ - return _prop_p2p_group_joined_get (priv) - && priv->p2p_group_is_owner; -} - static void _p2p_group_properties_changed (NMSupplicantInterface *self, GVariant *properties) { NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - gboolean old_val_p2p_group_is_owner; const char *s; - old_val_p2p_group_is_owner = _prop_p2p_group_is_owner_get (priv); - if (!properties) - priv->p2p_group_is_owner = FALSE; + priv->p2p_group_owner_property = FALSE; else if (g_variant_lookup (properties, "Role", "&s", &s)) - priv->p2p_group_is_owner = nm_streq (s, "GO"); + priv->p2p_group_owner_property = nm_streq (s, "GO"); - if (old_val_p2p_group_is_owner != _prop_p2p_group_is_owner_get (priv)) - _notify (self, PROP_P2P_GROUP_OWNER); + _notify_maybe_p2p_group (self); } static void @@ -1351,8 +1362,6 @@ _p2p_group_properties_get_all_cb (GVariant *result, { NMSupplicantInterface *self; NMSupplicantInterfacePrivate *priv; - gboolean old_val_p2p_group_joined; - gboolean old_val_p2p_group_is_owner; gs_unref_variant GVariant *properties = NULL; if (nm_utils_error_is_cancelled (error)) @@ -1363,9 +1372,6 @@ _p2p_group_properties_get_all_cb (GVariant *result, g_object_freeze_notify (G_OBJECT (self)); - old_val_p2p_group_joined = _prop_p2p_group_joined_get (priv); - old_val_p2p_group_is_owner = _prop_p2p_group_is_owner_get (priv); - nm_clear_g_cancellable (&priv->p2p_group_properties_cancellable); if (result) @@ -1375,10 +1381,7 @@ _p2p_group_properties_get_all_cb (GVariant *result, _starting_check_ready (self); - if (old_val_p2p_group_joined != _prop_p2p_group_joined_get (priv)) - _notify (self, PROP_P2P_GROUP_JOINED); - if (old_val_p2p_group_is_owner != _prop_p2p_group_is_owner_get (priv)) - _notify (self, PROP_P2P_GROUP_OWNER); + _notify_maybe_p2p_group (self); g_object_thaw_notify (G_OBJECT (self)); } @@ -1389,17 +1392,12 @@ _p2p_group_set_path (NMSupplicantInterface *self, { NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); nm_auto_ref_string NMRefString *group_path = NULL; - gboolean old_val_p2p_group_joined; - gboolean old_val_p2p_group_is_owner; group_path = nm_ref_string_new (nm_dbus_path_not_empty (path)); if (priv->p2p_group_path == group_path) return; - old_val_p2p_group_joined = _prop_p2p_group_joined_get (priv); - old_val_p2p_group_is_owner = _prop_p2p_group_is_owner_get (priv); - nm_clear_g_dbus_connection_signal (priv->dbus_connection, &priv->p2p_group_properties_changed_id); nm_clear_g_cancellable (&priv->p2p_group_properties_cancellable); @@ -1427,10 +1425,7 @@ _p2p_group_set_path (NMSupplicantInterface *self, } _notify (self, PROP_P2P_GROUP_PATH); - if (old_val_p2p_group_joined != _prop_p2p_group_joined_get (priv)) - _notify (self, PROP_P2P_GROUP_JOINED); - if (old_val_p2p_group_is_owner != _prop_p2p_group_is_owner_get (priv)) - _notify (self, PROP_P2P_GROUP_OWNER); + _notify_maybe_p2p_group (self); nm_assert_starting_has_pending_count (priv->starting_pending_count); } @@ -2897,7 +2892,7 @@ nm_supplicant_interface_get_p2p_available (NMSupplicantInterface *self) gboolean nm_supplicant_interface_get_p2p_group_joined (NMSupplicantInterface *self) { - return _prop_p2p_group_joined_get (NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)); + return NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)->p2p_group_joined_cached; } const char* @@ -2909,7 +2904,7 @@ nm_supplicant_interface_get_p2p_group_path (NMSupplicantInterface *self) gboolean nm_supplicant_interface_get_p2p_group_owner (NMSupplicantInterface *self) { - return _prop_p2p_group_is_owner_get (NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)); + return NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)->p2p_group_owner_cached; } /*****************************************************************************/ From 68395ce7d534deb7e6aa8b46bce0076b7bae0aff Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Mar 2020 19:44:13 +0100 Subject: [PATCH 09/19] wifi/trivial: rename field NMDeviceWifiPrivate.last_scan to last_scan_msec --- src/devices/wifi/nm-device-wifi.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index ce5cf1a466..889cd39f96 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -85,7 +85,8 @@ typedef struct { bool is_scanning:1; bool hidden_probe_scan_warn:1; - gint64 last_scan; /* milliseconds */ + gint64 last_scan_msec; + gint32 scheduled_scan_time; /* seconds */ guint8 scan_interval; /* seconds */ guint pending_scan_id; @@ -1512,7 +1513,7 @@ supplicant_iface_scan_done_cb (NMSupplicantInterface *iface, _LOGD (LOGD_WIFI, "wifi-scan: scan-done callback"); - priv->last_scan = nm_utils_get_monotonic_timestamp_msec (); + priv->last_scan_msec = nm_utils_get_monotonic_timestamp_msec (); _notify (self, PROP_LAST_SCAN); schedule_scan (self, TRUE); @@ -1538,7 +1539,7 @@ ap_list_dump (gpointer user_data) _LOGD (LOGD_WIFI_SCAN, "APs: [now:%u last:%" G_GINT64_FORMAT " next:%u]", now_s, - priv->last_scan / NM_UTILS_MSEC_PER_SEC, + priv->last_scan_msec / NM_UTILS_MSEC_PER_SEC, priv->scheduled_scan_time); c_list_for_each_entry (ap, &priv->aps_lst_head, aps_lst) _ap_dump (self, LOGL_DEBUG, ap, "dump", now_s); @@ -3270,9 +3271,9 @@ get_property (GObject *object, guint prop_id, break; case PROP_LAST_SCAN: g_value_set_int64 (value, - priv->last_scan > 0 - ? nm_utils_monotonic_timestamp_as_boottime (priv->last_scan, NM_UTILS_NSEC_PER_MSEC) - : (gint64) -1); + priv->last_scan_msec > 0 + ? nm_utils_monotonic_timestamp_as_boottime (priv->last_scan_msec, NM_UTILS_NSEC_PER_MSEC) + : (gint64) -1); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); From 85bccb6d28db0c59f520a1b8b3f0f7bc6503c8fe Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 19 Mar 2020 19:53:34 +0100 Subject: [PATCH 10/19] wifi: print age of Wi-Fi access point with milliseconds precision For a computer a second is a really long time. Rounding times to seconds feels unnecessarily inaccurate. --- src/devices/wifi/nm-device-iwd.c | 11 +++++------ src/devices/wifi/nm-device-wifi.c | 21 ++++++++++++++------- src/devices/wifi/nm-wifi-ap.c | 14 ++++++++++---- src/devices/wifi/nm-wifi-ap.h | 2 +- 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 49fe07fb08..72c83cf0e2 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -101,15 +101,14 @@ static void _ap_dump (NMDeviceIwd *self, NMLogLevel log_level, const NMWifiAP *ap, - const char *prefix, - gint32 now_s) + const char *prefix) { char buf[1024]; buf[0] = '\0'; _NMLOG (log_level, LOGD_WIFI_SCAN, "wifi-ap: %-7s %s", prefix, - nm_wifi_ap_to_string (ap, buf, sizeof (buf), now_s)); + nm_wifi_ap_to_string (ap, buf, sizeof (buf), 0)); } /* Callers ensure we're not removing current_ap */ @@ -126,12 +125,12 @@ ap_add_remove (NMDeviceIwd *self, ap->wifi_device = NM_DEVICE (self); c_list_link_tail (&priv->aps_lst_head, &ap->aps_lst); nm_dbus_object_export (NM_DBUS_OBJECT (ap)); - _ap_dump (self, LOGL_DEBUG, ap, "added", 0); + _ap_dump (self, LOGL_DEBUG, ap, "added"); nm_device_wifi_emit_signal_access_point (NM_DEVICE (self), ap, TRUE); } else { ap->wifi_device = NULL; c_list_unlink (&ap->aps_lst); - _ap_dump (self, LOGL_DEBUG, ap, "removed", 0); + _ap_dump (self, LOGL_DEBUG, ap, "removed"); } _notify (self, PROP_ACCESS_POINTS); @@ -357,7 +356,7 @@ get_ordered_networks_cb (GObject *source, GAsyncResult *res, gpointer user_data) nm_wifi_ap_get_supplicant_path (ap)); if (new_ap) { if (nm_wifi_ap_set_strength (ap, nm_wifi_ap_get_strength (new_ap))) { - _ap_dump (self, LOGL_TRACE, ap, "updated", 0); + _ap_dump (self, LOGL_TRACE, ap, "updated"); changed = TRUE; } g_hash_table_remove (new_aps, diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 889cd39f96..ed9d5c3df8 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -205,14 +205,14 @@ _ap_dump (NMDeviceWifi *self, NMLogLevel log_level, const NMWifiAP *ap, const char *prefix, - gint32 now_s) + gint64 now_msec) { char buf[1024]; buf[0] = '\0'; _NMLOG (log_level, LOGD_WIFI_SCAN, "wifi-ap: %-7s %s", prefix, - nm_wifi_ap_to_string (ap, buf, sizeof (buf), now_s)); + nm_wifi_ap_to_string (ap, buf, sizeof (buf), now_msec)); } static void @@ -1535,14 +1535,21 @@ ap_list_dump (gpointer user_data) if (_LOGD_ENABLED (LOGD_WIFI_SCAN)) { NMWifiAP *ap; - gint32 now_s = nm_utils_get_monotonic_timestamp_sec (); + gint64 now_msec = nm_utils_get_monotonic_timestamp_msec (); + char str_buf[100]; - _LOGD (LOGD_WIFI_SCAN, "APs: [now:%u last:%" G_GINT64_FORMAT " next:%u]", - now_s, - priv->last_scan_msec / NM_UTILS_MSEC_PER_SEC, + _LOGD (LOGD_WIFI_SCAN, "APs: [now:%u.%03u, last:%s, next:%u]", + (guint) (now_msec / NM_UTILS_MSEC_PER_SEC), + (guint) (now_msec % NM_UTILS_MSEC_PER_SEC), + priv->last_scan_msec > 0 + ? nm_sprintf_buf (str_buf, + "%u.%03u", + (guint) (priv->last_scan_msec / NM_UTILS_MSEC_PER_SEC), + (guint) (priv->last_scan_msec % NM_UTILS_MSEC_PER_SEC)) + : "-1", priv->scheduled_scan_time); c_list_for_each_entry (ap, &priv->aps_lst_head, aps_lst) - _ap_dump (self, LOGL_DEBUG, ap, "dump", now_s); + _ap_dump (self, LOGL_DEBUG, ap, "dump", now_msec); } return G_SOURCE_REMOVE; } diff --git a/src/devices/wifi/nm-wifi-ap.c b/src/devices/wifi/nm-wifi-ap.c index 038d847807..e427c86fbd 100644 --- a/src/devices/wifi/nm-wifi-ap.c +++ b/src/devices/wifi/nm-wifi-ap.c @@ -503,13 +503,14 @@ const char * nm_wifi_ap_to_string (const NMWifiAP *self, char *str_buf, gulong buf_len, - gint32 now_s) + gint64 now_msec) { const NMWifiAPPrivate *priv; const char *supplicant_id = "-"; const char *export_path; guint32 chan; gs_free char *ssid_to_free = NULL; + char str_buf_ts[100]; g_return_val_if_fail (NM_IS_WIFI_AP (self), NULL); @@ -525,8 +526,10 @@ nm_wifi_ap_to_string (const NMWifiAP *self, else export_path = "/"; + nm_utils_get_monotonic_timestamp_msec_cached (&now_msec); + g_snprintf (str_buf, buf_len, - "%17s %-35s [ %c %3u %3u%% %c%c W:%04X R:%04X ] %3us sup:%s [nm:%s]", + "%17s %-35s [ %c %3u %3u%% %c%c W:%04X R:%04X ] %s sup:%s [nm:%s]", priv->address ?: "(none)", (ssid_to_free = _nm_utils_ssid_to_string (priv->ssid)), (priv->mode == NM_802_11_MODE_ADHOC @@ -545,8 +548,11 @@ nm_wifi_ap_to_string (const NMWifiAP *self, priv->wpa_flags & 0xFFFF, priv->rsn_flags & 0xFFFF, priv->last_seen_msec != G_MININT64 - ? (int) ((now_s > 0 ? now_s : nm_utils_get_monotonic_timestamp_sec ()) - (priv->last_seen_msec / 1000)) - : -1, + ? nm_sprintf_buf (str_buf_ts, + "%3u.%03us", + (guint) ((now_msec - priv->last_seen_msec) / 1000), + (guint) ((now_msec - priv->last_seen_msec) % 1000)) + : " ", supplicant_id, export_path); return str_buf; diff --git a/src/devices/wifi/nm-wifi-ap.h b/src/devices/wifi/nm-wifi-ap.h index 2b3864a767..1bf4e604fd 100644 --- a/src/devices/wifi/nm-wifi-ap.h +++ b/src/devices/wifi/nm-wifi-ap.h @@ -94,7 +94,7 @@ gboolean nm_wifi_ap_get_metered (const NMWifiAP *self); const char *nm_wifi_ap_to_string (const NMWifiAP *self, char *str_buf, gulong buf_len, - gint32 now_s); + gint64 now_msec); const char **nm_wifi_aps_get_paths (const CList *aps_lst_head, gboolean include_without_ssid); From 80e7e8845abbfebc3631a762949aa746bad56bb8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 18 Mar 2020 12:59:49 +0100 Subject: [PATCH 11/19] wifi: fix and improve handling of Wi-Fi scanning state In NMSupplicantInterface, we determine whether we currently are scanning both on the "scanning" supplicant state and the "Scanning" property. Extend that. If we currently are scanning and are about to clear the scanning state, then pretend to still scan as long as we are still initializing BSS instances. What otherwise happens is that we declare that we finished scanning, but the NMWifiAP instances are not yet ready. The result is, that `nmcli device wifi` will already start printing the scan list, when we didn't yet fully process all access points. Now, _notify_maybe_scanning() will delay switching the scanning state to disabled, as long as we have BSS initializing (bss_initializing_lst_head). Also, ignore the "ScanDone" signal. It's redundant to the "Scanning" property anyway. Also, only set priv->last_scan_msec when we switch the scanning state off. That is the right (and only) place where the last-scan timestamp needs updating. --- src/devices/wifi/nm-device-wifi.c | 82 ++++++++++++------------ src/supplicant/nm-supplicant-interface.c | 70 +++++++++----------- src/supplicant/nm-supplicant-interface.h | 1 - 3 files changed, 72 insertions(+), 81 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index ed9d5c3df8..9ea16f25d7 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -165,17 +165,10 @@ static void supplicant_iface_bss_changed_cb (NMSupplicantInterface *iface, gboolean is_present, NMDeviceWifi *self); -static void supplicant_iface_scan_done_cb (NMSupplicantInterface * iface, - NMDeviceWifi * self); - static void supplicant_iface_wps_credentials_cb (NMSupplicantInterface *iface, GVariant *credentials, NMDeviceWifi *self); -static void supplicant_iface_notify_scanning_cb (NMSupplicantInterface * iface, - GParamSpec * pspec, - NMDeviceWifi * self); - static void supplicant_iface_notify_current_bss (NMSupplicantInterface *iface, GParamSpec *pspec, NMDeviceWifi *self); @@ -184,6 +177,10 @@ static void supplicant_iface_notify_p2p_available (NMSupplicantInterface *iface, GParamSpec *pspec, NMDeviceWifi *self); +static void _requested_scan_set (NMDeviceWifi *self, gboolean value); + +static void periodic_update (NMDeviceWifi *self); + static void request_wireless_scan (NMDeviceWifi *self, gboolean periodic, gboolean force_if_scanning, @@ -220,6 +217,7 @@ _notify_scanning (NMDeviceWifi *self) { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); gboolean scanning; + gboolean last_scan_changed = FALSE; scanning = priv->sup_iface && nm_supplicant_interface_get_scanning (priv->sup_iface); @@ -229,7 +227,36 @@ _notify_scanning (NMDeviceWifi *self) _LOGD (LOGD_WIFI, "wifi-scan: scanning-state: %s", scanning ? "scanning" : "idle"); priv->is_scanning = scanning; - _notify (self, PROP_SCANNING); + + if ( !scanning + || priv->last_scan_msec == 0) { + last_scan_changed = TRUE; + priv->last_scan_msec = nm_utils_get_monotonic_timestamp_msec (); + } + + schedule_scan (self, TRUE); + + nm_gobject_notify_together (self, + PROP_SCANNING, + last_scan_changed + ? PROP_LAST_SCAN + : PROP_0); + + if (!priv->is_scanning) { + _requested_scan_set (self, FALSE); + if (nm_device_get_state (NM_DEVICE (self)) == NM_DEVICE_STATE_ACTIVATED) { + /* Run a quick update of current AP when coming out of a scan */ + periodic_update (self); + } + } +} + +static void +supplicant_iface_notify_scanning_cb (NMSupplicantInterface *iface, + GParamSpec *pspec, + NMDeviceWifi *self) +{ + _notify_scanning (self); } static gboolean @@ -279,10 +306,6 @@ supplicant_interface_acquire_cb (NMSupplicantManager *supplicant_manager, NM_SUPPLICANT_INTERFACE_BSS_CHANGED, G_CALLBACK (supplicant_iface_bss_changed_cb), self); - g_signal_connect (priv->sup_iface, - NM_SUPPLICANT_INTERFACE_SCAN_DONE, - G_CALLBACK (supplicant_iface_scan_done_cb), - self); g_signal_connect (priv->sup_iface, NM_SUPPLICANT_INTERFACE_WPS_CREDENTIALS, G_CALLBACK (supplicant_iface_wps_credentials_cb), @@ -1236,7 +1259,12 @@ _nm_device_wifi_request_scan (NMDeviceWifi *self, last_scan = nm_supplicant_interface_get_last_scan (priv->sup_iface); if ( last_scan > 0 - && (nm_utils_get_monotonic_timestamp_msec () - last_scan) < 10 * NM_UTILS_MSEC_PER_SEC) { + && nm_utils_get_monotonic_timestamp_msec () < last_scan + (10 * NM_UTILS_MSEC_PER_SEC)) { + /* FIXME: we really should not outright reject a scan request in this case. We should + * ensure to start a scan request soon, possibly with rate limiting. And there is no + * need to tell the caller that we aren't going to scan... + * + * Same above, if we are currently scanning... */ g_dbus_method_invocation_return_error_literal (invocation, NM_DEVICE_ERROR, NM_DEVICE_ERROR_NOT_ALLOWED, @@ -1505,21 +1533,6 @@ schedule_scan (NMDeviceWifi *self, gboolean backoff) } } -static void -supplicant_iface_scan_done_cb (NMSupplicantInterface *iface, - NMDeviceWifi *self) -{ - NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - - _LOGD (LOGD_WIFI, "wifi-scan: scan-done callback"); - - priv->last_scan_msec = nm_utils_get_monotonic_timestamp_msec (); - _notify (self, PROP_LAST_SCAN); - schedule_scan (self, TRUE); - - _requested_scan_set (self, FALSE); -} - /**************************************************************************** * WPA Supplicant control stuff * @@ -2211,19 +2224,6 @@ supplicant_iface_assoc_cb (NMSupplicantInterface *iface, } } -static void -supplicant_iface_notify_scanning_cb (NMSupplicantInterface *iface, - GParamSpec *pspec, - NMDeviceWifi *self) -{ - _notify_scanning (self); - - /* Run a quick update of current AP when coming out of a scan */ - if ( !NM_DEVICE_WIFI_GET_PRIVATE (self)->is_scanning - && nm_device_get_state (NM_DEVICE (self)) == NM_DEVICE_STATE_ACTIVATED) - periodic_update (self); -} - static void supplicant_iface_notify_current_bss (NMSupplicantInterface *iface, GParamSpec *pspec, diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index ec1afa25ec..b72962804f 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -59,7 +59,6 @@ enum { STATE, /* change in the interface's state */ BSS_CHANGED, /* a new BSS appeared, was updated, or was removed. */ PEER_CHANGED, /* a new Peer appeared, was updated, or was removed */ - SCAN_DONE, /* wifi scan is complete */ WPS_CREDENTIALS, /* WPS credentials received */ GROUP_STARTED, /* a new Group (interface) was created */ GROUP_FINISHED, /* a Group (interface) has been finished */ @@ -451,6 +450,22 @@ _notify_maybe_scanning (NMSupplicantInterface *self) if (priv->scanning_cached == scanning) return; + if ( !scanning + && !c_list_is_empty (&priv->bss_initializing_lst_head)) { + /* we would change state to indicate we no longer scan. However, + * we still have BSS instances to be initialized. Delay the + * state change further. */ + return; + } + + _LOGT ("scanning: %s", scanning ? "yes" : "no"); + + if (!scanning) + priv->last_scan_msec = nm_utils_get_monotonic_timestamp_msec (); + else { + /* while we are scanning, we set the timestamp to -1. */ + priv->last_scan_msec = -1; + } priv->scanning_cached = scanning; _notify (self, PROP_SCANNING); } @@ -514,6 +529,9 @@ _bss_info_changed_emit (NMSupplicantInterface *self, NMSupplicantBssInfo *bss_info, gboolean is_present) { + _LOGT ("BSS %s %s", + bss_info->bss_path->str, + is_present ? "updated" : "deleted"); g_signal_emit (self, signals[BSS_CHANGED], 0, @@ -744,6 +762,8 @@ _bss_info_get_all_cb (GVariant *result, _bss_info_properties_changed (self, bss_info, properties, TRUE); _starting_check_ready (self); + + _notify_maybe_scanning (self); } static void @@ -1083,6 +1103,8 @@ set_state_down (NMSupplicantInterface *self, _remove_network (self); nm_clear_pointer (&priv->current_bss, nm_ref_string_unref); + + _notify_maybe_scanning (self); } static void @@ -1105,9 +1127,6 @@ set_state (NMSupplicantInterface *self, NMSupplicantInterfaceState new_state) nm_supplicant_interface_state_to_string (new_state), nm_supplicant_interface_state_to_string (priv->state)); - if (old_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING) - priv->last_scan_msec = nm_utils_get_monotonic_timestamp_msec (); - priv->state = new_state; _emit_signal_state (self, @@ -1137,16 +1156,9 @@ nm_supplicant_interface_get_scanning (NMSupplicantInterface *self) gint64 nm_supplicant_interface_get_last_scan (NMSupplicantInterface *self) { - NMSupplicantInterfacePrivate *priv; - g_return_val_if_fail (NM_IS_SUPPLICANT_INTERFACE (self), FALSE); - priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); - - /* returns -1 if we are currently scanning. */ - return priv->scanning_cached - ? -1 - : priv->last_scan_msec; + return NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self)->last_scan_msec; } #define MATCH_PROPERTY(p, n, v, t) (!strcmp (p, n) && g_variant_is_of_type (v, t)) @@ -1777,13 +1789,8 @@ _properties_changed_main (NMSupplicantInterface *self, g_variant_unref (v_v); } - if (nm_g_variant_lookup (properties, "Scanning", "b", &v_b)) { - if (priv->scanning_cached != (!!v_b)) { - if (priv->scanning_cached) - priv->last_scan_msec = nm_utils_get_monotonic_timestamp_msec (); - priv->scanning_cached = v_b; - } - } + if (nm_g_variant_lookup (properties, "Scanning", "b", &v_b)) + priv->scanning_property = v_b; if (nm_g_variant_lookup (properties, "Ifname", "&s", &v_s)) { if (nm_utils_strdup_reset (&priv->ifname, v_s)) @@ -1870,6 +1877,8 @@ _properties_changed_main (NMSupplicantInterface *self, if (do_set_state) set_state (self, priv->supp_state); + + _notify_maybe_scanning (self); } static void @@ -2717,16 +2726,6 @@ _signal_handle (NMSupplicantInterface *self, if (!priv->is_ready_main) return; - if (nm_streq (signal_name, "ScanDone")) { - priv->last_scan_msec = nm_utils_get_monotonic_timestamp_msec (); - _LOGT ("ScanDone signal received"); - if (priv->state > NM_SUPPLICANT_INTERFACE_STATE_STARTING) { - nm_assert (priv->state < NM_SUPPLICANT_INTERFACE_STATE_DOWN); - g_signal_emit (self, signals[SCAN_DONE], 0); - } - return; - } - if (nm_streq (signal_name, "BSSAdded")) { if (!g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(oa{sv})"))) return; @@ -2874,11 +2873,11 @@ _signal_cb (GDBusConnection *connection, NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); priv->starting_pending_count++; - _signal_handle (self, signal_interface_name, signal_name, parameters); - priv->starting_pending_count--; _starting_check_ready (self); + + _notify_maybe_scanning (self); } /*****************************************************************************/ @@ -3003,6 +3002,7 @@ nm_supplicant_interface_init (NMSupplicantInterface * self) priv->state = NM_SUPPLICANT_INTERFACE_STATE_STARTING; priv->supp_state = NM_SUPPLICANT_INTERFACE_STATE_INVALID; + priv->last_scan_msec = -1; c_list_init (&self->supp_lst); @@ -3298,14 +3298,6 @@ nm_supplicant_interface_class_init (NMSupplicantInterfaceClass *klass) NULL, NULL, NULL, G_TYPE_NONE, 2, G_TYPE_POINTER, G_TYPE_BOOLEAN); - signals[SCAN_DONE] = - g_signal_new (NM_SUPPLICANT_INTERFACE_SCAN_DONE, - G_OBJECT_CLASS_TYPE (object_class), - G_SIGNAL_RUN_LAST, - 0, - NULL, NULL, NULL, - G_TYPE_NONE, 0); - signals[WPS_CREDENTIALS] = g_signal_new (NM_SUPPLICANT_INTERFACE_WPS_CREDENTIALS, G_OBJECT_CLASS_TYPE (object_class), diff --git a/src/supplicant/nm-supplicant-interface.h b/src/supplicant/nm-supplicant-interface.h index 8964a4754f..cfc898d80f 100644 --- a/src/supplicant/nm-supplicant-interface.h +++ b/src/supplicant/nm-supplicant-interface.h @@ -71,7 +71,6 @@ typedef enum { #define NM_SUPPLICANT_INTERFACE_STATE "state" #define NM_SUPPLICANT_INTERFACE_BSS_CHANGED "bss-changed" #define NM_SUPPLICANT_INTERFACE_PEER_CHANGED "peer-changed" -#define NM_SUPPLICANT_INTERFACE_SCAN_DONE "scan-done" #define NM_SUPPLICANT_INTERFACE_WPS_CREDENTIALS "wps-credentials" #define NM_SUPPLICANT_INTERFACE_GROUP_STARTED "group-started" #define NM_SUPPLICANT_INTERFACE_GROUP_FINISHED "group-finished" From b10c382b1d78175687d6fe034213385492ff5937 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Apr 2020 11:56:21 +0200 Subject: [PATCH 12/19] wifi/trivial: rename function nm_supplicant_interface_state_is_operational() from upper case name --- src/devices/nm-device-ethernet.c | 4 ++-- src/devices/nm-device-macsec.c | 4 ++-- src/devices/wifi/nm-device-wifi-p2p.c | 8 ++++---- src/supplicant/nm-supplicant-interface.c | 6 +++--- src/supplicant/nm-supplicant-interface.h | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index dc44c084bd..7556bb27ba 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -811,7 +811,7 @@ supplicant_connection_timeout_cb (gpointer user_data) state = nm_supplicant_interface_get_state (priv->supplicant.iface); if (state != NM_SUPPLICANT_INTERFACE_STATE_COMPLETED - && NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (state)) + && nm_supplicant_interface_state_is_operational (state)) priv->supplicant.lnk_timeout_id = g_timeout_add_seconds (SUPPLICANT_LNK_TIMEOUT_SEC, supplicant_lnk_timeout_cb, self); } @@ -862,7 +862,7 @@ supplicant_interface_create_cb (NMSupplicantManager *supplicant_manager, supplicant_connection_timeout_cb, self); - if (NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (nm_supplicant_interface_get_state (iface))) + if (nm_supplicant_interface_state_is_operational (nm_supplicant_interface_get_state (iface))) supplicant_iface_start (self); } diff --git a/src/devices/nm-device-macsec.c b/src/devices/nm-device-macsec.c index 4d620831ac..f1878078b2 100644 --- a/src/devices/nm-device-macsec.c +++ b/src/devices/nm-device-macsec.c @@ -584,7 +584,7 @@ supplicant_connection_timeout_cb (gpointer user_data) state = nm_supplicant_interface_get_state (priv->supplicant.iface); if (state != NM_SUPPLICANT_INTERFACE_STATE_COMPLETED - && NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (state)) + && nm_supplicant_interface_state_is_operational (state)) priv->supplicant.lnk_timeout_id = g_timeout_add_seconds (SUPPLICANT_LNK_TIMEOUT_SEC, supplicant_lnk_timeout_cb, self); } @@ -636,7 +636,7 @@ supplicant_interface_create_cb (NMSupplicantManager *supplicant_manager, supplicant_connection_timeout_cb, self); - if (NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (nm_supplicant_interface_get_state (iface))) + if (nm_supplicant_interface_state_is_operational (nm_supplicant_interface_get_state (iface))) supplicant_iface_start (self); } diff --git a/src/devices/wifi/nm-device-wifi-p2p.c b/src/devices/wifi/nm-device-wifi-p2p.c index a655c95025..497bc43603 100644 --- a/src/devices/wifi/nm-device-wifi-p2p.c +++ b/src/devices/wifi/nm-device-wifi-p2p.c @@ -228,7 +228,7 @@ is_available (NMDevice *device, NMDeviceCheckDevAvailableFlags flags) return FALSE; supplicant_state = nm_supplicant_interface_get_state (priv->mgmt_iface); - return NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (supplicant_state); + return nm_supplicant_interface_state_is_operational (supplicant_state); } static gboolean @@ -712,7 +712,7 @@ check_group_iface_ready (NMDeviceWifiP2P *self) if (!priv->group_iface) return; - if (!NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (nm_supplicant_interface_get_state (priv->group_iface))) + if (!nm_supplicant_interface_state_is_operational (nm_supplicant_interface_get_state (priv->group_iface))) return; if (!nm_supplicant_interface_get_p2p_group_joined (priv->group_iface)) @@ -910,7 +910,7 @@ device_state_changed (NMDevice *device, break; case NM_DEVICE_STATE_UNAVAILABLE: if ( !priv->mgmt_iface - || !NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (nm_supplicant_interface_get_state (priv->mgmt_iface))) + || !nm_supplicant_interface_state_is_operational (nm_supplicant_interface_get_state (priv->mgmt_iface))) _set_is_waiting_for_supplicant (self, TRUE); break; case NM_DEVICE_STATE_NEED_AUTH: @@ -1080,7 +1080,7 @@ done: NM_DEVICE_STATE_REASON_SUPPLICANT_FAILED); _set_is_waiting_for_supplicant (self, !priv->mgmt_iface - || !NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (nm_supplicant_interface_get_state (priv->mgmt_iface))); + || !nm_supplicant_interface_state_is_operational (nm_supplicant_interface_get_state (priv->mgmt_iface))); } void diff --git a/src/supplicant/nm-supplicant-interface.c b/src/supplicant/nm-supplicant-interface.c index b72962804f..cc3d109e35 100644 --- a/src/supplicant/nm-supplicant-interface.c +++ b/src/supplicant/nm-supplicant-interface.c @@ -443,7 +443,7 @@ _notify_maybe_scanning (NMSupplicantInterface *self) NMSupplicantInterfacePrivate *priv = NM_SUPPLICANT_INTERFACE_GET_PRIVATE (self); gboolean scanning; - scanning = NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (priv->state) + scanning = nm_supplicant_interface_state_is_operational (priv->state) && ( priv->scanning_property || priv->supp_state == NM_SUPPLICANT_INTERFACE_STATE_SCANNING); @@ -1115,7 +1115,7 @@ set_state (NMSupplicantInterface *self, NMSupplicantInterfaceState new_state) nm_assert (new_state > NM_SUPPLICANT_INTERFACE_STATE_STARTING); nm_assert (new_state < NM_SUPPLICANT_INTERFACE_STATE_DOWN); - nm_assert (NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (new_state)); + nm_assert (nm_supplicant_interface_state_is_operational (new_state)); nm_assert (priv->state >= NM_SUPPLICANT_INTERFACE_STATE_STARTING); nm_assert (priv->state < NM_SUPPLICANT_INTERFACE_STATE_DOWN); @@ -1237,7 +1237,7 @@ _starting_check_ready (NMSupplicantInterface *self) nm_assert (priv->state == NM_SUPPLICANT_INTERFACE_STATE_STARTING); - if (!NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (priv->supp_state)) { + if (!nm_supplicant_interface_state_is_operational (priv->supp_state)) { _LOGW ("Supplicant state is unknown during initialization. Destroy the interface"); set_state_down (self, TRUE, "failure to get valid interface state"); return; diff --git a/src/supplicant/nm-supplicant-interface.h b/src/supplicant/nm-supplicant-interface.h index cfc898d80f..774b56c6e4 100644 --- a/src/supplicant/nm-supplicant-interface.h +++ b/src/supplicant/nm-supplicant-interface.h @@ -35,7 +35,7 @@ typedef enum { } NMSupplicantInterfaceState; static inline gboolean -NM_SUPPLICANT_INTERFACE_STATE_IS_OPERATIONAL (NMSupplicantInterfaceState state) +nm_supplicant_interface_state_is_operational (NMSupplicantInterfaceState state) { return state > NM_SUPPLICANT_INTERFACE_STATE_STARTING && state < NM_SUPPLICANT_INTERFACE_STATE_DOWN; From 2011392fb77cc9108dfb25366f3744241672f4b9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Mar 2020 12:22:00 +0100 Subject: [PATCH 13/19] wifi: cleanup periodic_update() in "nm-device-wifi.c" --- src/devices/wifi/nm-device-wifi.c | 64 +++++++++++++----------- src/supplicant/nm-supplicant-interface.h | 7 +++ 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 9ea16f25d7..6355c2153f 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -101,7 +101,7 @@ typedef struct { NMActRequestGetSecretsCallId *wifi_secrets_id; - guint periodic_source_id; + guint periodic_update_id; guint link_timeout_id; guint32 failed_iface_count; guint reacquire_iface_id; @@ -466,38 +466,44 @@ set_current_ap (NMDeviceWifi *self, NMWifiAP *new_ap, gboolean recheck_available static void periodic_update (NMDeviceWifi *self) { - NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - int ifindex = nm_device_get_ifindex (NM_DEVICE (self)); + NMDeviceWifiPrivate *priv; + int ifindex; guint32 new_rate; - int percent; - NMDeviceState state; - NMSupplicantInterfaceState supplicant_state; - /* BSSID and signal strength have meaningful values only if the device - * is activated and not scanning. - */ - state = nm_device_get_state (NM_DEVICE (self)); - if (state != NM_DEVICE_STATE_ACTIVATED) + if (nm_device_get_state (NM_DEVICE (self)) != NM_DEVICE_STATE_ACTIVATED) { + /* BSSID and signal strength have meaningful values only if the device + * is activated and not scanning. + */ return; + } - /* Only update current AP if we're actually talking to something, otherwise - * assume the old one (if any) is still valid until we're told otherwise or - * the connection fails. - */ - supplicant_state = nm_supplicant_interface_get_state (priv->sup_iface); - if ( supplicant_state < NM_SUPPLICANT_INTERFACE_STATE_AUTHENTICATING - || supplicant_state > NM_SUPPLICANT_INTERFACE_STATE_COMPLETED - || nm_supplicant_interface_get_scanning (priv->sup_iface)) - return; + priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - /* In AP mode we currently have nothing to do. */ - if (priv->mode == NM_802_11_MODE_AP) + if ( !nm_supplicant_interface_state_is_associated (nm_supplicant_interface_get_state (priv->sup_iface)) + || nm_supplicant_interface_get_scanning (priv->sup_iface)) { + /* Only update current AP if we're actually talking to something, otherwise + * assume the old one (if any) is still valid until we're told otherwise or + * the connection fails. + */ return; + } + + if (priv->mode == NM_802_11_MODE_AP) { + /* In AP mode we currently have nothing to do. */ + return; + } + + ifindex = nm_device_get_ifindex (NM_DEVICE (self)); + if (ifindex <= 0) + g_return_if_reached (); if (priv->current_ap) { + int percent; + /* Smooth out the strength to work around crappy drivers */ percent = nm_platform_wifi_get_quality (nm_device_get_platform (NM_DEVICE (self)), ifindex); - if (percent >= 0 || ++priv->invalid_strength_counter > 3) { + if ( percent >= 0 + || ++priv->invalid_strength_counter > 3) { if (nm_wifi_ap_set_strength (priv->current_ap, (gint8) percent)) { #if NM_MORE_LOGGING _ap_dump (self, LOGL_TRACE, priv->current_ap, "updated", 0); @@ -517,7 +523,7 @@ periodic_update (NMDeviceWifi *self) static gboolean periodic_update_cb (gpointer user_data) { - periodic_update (NM_DEVICE_WIFI (user_data)); + periodic_update (user_data); return TRUE; } @@ -653,7 +659,7 @@ deactivate (NMDevice *device) int ifindex = nm_device_get_ifindex (device); NM80211Mode old_mode = priv->mode; - nm_clear_g_source (&priv->periodic_source_id); + nm_clear_g_source (&priv->periodic_update_id); cleanup_association_attempt (self, TRUE); @@ -2882,8 +2888,8 @@ act_stage2_config (NMDevice *device, NMDeviceStateReason *out_failure_reason) supplicant_connection_timeout_cb, self); - if (!priv->periodic_source_id) - priv->periodic_source_id = g_timeout_add_seconds (6, periodic_update_cb, self); + if (!priv->periodic_update_id) + priv->periodic_update_id = g_timeout_add_seconds (6, periodic_update_cb, self); /* We'll get stage3 started when the supplicant connects */ return NM_ACT_STAGE_RETURN_POSTPONE; @@ -3080,7 +3086,7 @@ device_state_changed (NMDevice *device, */ supplicant_interface_release (self); - nm_clear_g_source (&priv->periodic_source_id); + nm_clear_g_source (&priv->periodic_update_id); cleanup_association_attempt (self, TRUE); cleanup_supplicant_failures (self); @@ -3355,7 +3361,7 @@ 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); + nm_clear_g_source (&priv->periodic_update_id); wifi_secrets_cancel (self); diff --git a/src/supplicant/nm-supplicant-interface.h b/src/supplicant/nm-supplicant-interface.h index 774b56c6e4..eb414f26f5 100644 --- a/src/supplicant/nm-supplicant-interface.h +++ b/src/supplicant/nm-supplicant-interface.h @@ -41,6 +41,13 @@ nm_supplicant_interface_state_is_operational (NMSupplicantInterfaceState state) && state < NM_SUPPLICANT_INTERFACE_STATE_DOWN; } +static inline gboolean +nm_supplicant_interface_state_is_associated (NMSupplicantInterfaceState state) +{ + return state >= NM_SUPPLICANT_INTERFACE_STATE_AUTHENTICATING + && state <= NM_SUPPLICANT_INTERFACE_STATE_COMPLETED; +} + typedef enum { NM_SUPPLICANT_AUTH_STATE_UNKNOWN, NM_SUPPLICANT_AUTH_STATE_STARTED, From c1f473d3428acbdc524aa4bc3741e4107e78e8b5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Mar 2020 12:24:16 +0100 Subject: [PATCH 14/19] wifi: drop workaround for bad values in nm_platform_wifi_get_quality() This was first introduced by commit 4ed4b491fa75 ('2005-12-31 Dan Williams '), a very long time ago. It got reworked several times, but I don't think this code makes sense anymore. So, if nm_platform_wifi_get_quality() returns an error, we would ignore it for three times, until we would set the strength to the error code (presumably -1). Why? If we cannot read the strength via nl80211/WEXT, then we should just keep whatever we got from supplicant. Drop this. Also, only accept the percentage if it is in a valid range from 0 to 100%. If the driver (or platform code) gives us numbers out of that range, we have no idea what their meaning is. In that case, the value must be fixed in the lower layers, that knows how to convert the value from the actual meaning to the requested percentage. --- src/devices/wifi/nm-device-wifi.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 6355c2153f..54352f297f 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -72,8 +72,6 @@ enum { static guint signals[LAST_SIGNAL] = { 0 }; typedef struct { - gint8 invalid_strength_counter; - CList aps_lst_head; GHashTable *aps_idx_by_supplicant_path; @@ -500,16 +498,14 @@ periodic_update (NMDeviceWifi *self) if (priv->current_ap) { int percent; - /* Smooth out the strength to work around crappy drivers */ percent = nm_platform_wifi_get_quality (nm_device_get_platform (NM_DEVICE (self)), ifindex); - if ( percent >= 0 - || ++priv->invalid_strength_counter > 3) { + if ( percent >= 0 + && percent <= 100) { if (nm_wifi_ap_set_strength (priv->current_ap, (gint8) percent)) { #if NM_MORE_LOGGING _ap_dump (self, LOGL_TRACE, priv->current_ap, "updated", 0); #endif } - priv->invalid_strength_counter = 0; } } From cf30e15ababae2e67c05cc0a0f4a0bf2f209ec3c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Mar 2020 12:43:43 +0100 Subject: [PATCH 15/19] wifi: parse RequestScan D-Bus arguments before authenticating request It feels better to first parse input arguments before authenticating. One argument for otherwise would be that we shouldn't reveal any information about the request before authenticating it. Meaning: every request (even with invalid arguments) should fail with permission-denied. However, I prefer this for minor reasons: - what makes a valid request is no secret. And if somebody makes an invalid request, it should fail with invalid-arguments first. - we possibly can short cut the expensive authentication process, where we ask PolicyKit. - by extracting the options variant early and only pass on the SSIDs array, we handle the encoding of the options array earlier and where it belongs: closer to the D-Bus request that defines the meaning of the argument. Also, change the failure reason to return invalid-argument. --- src/devices/wifi/nm-device-wifi.c | 54 +++++++++++++++---------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 54352f297f..f605c7a9fd 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1146,7 +1146,7 @@ ssids_options_to_ptrarray (GVariant *value, GError **error) if (num_ssids > 32) { g_set_error_literal (error, NM_DEVICE_ERROR, - NM_DEVICE_ERROR_NOT_ALLOWED, + NM_DEVICE_ERROR_INVALID_ARGUMENT, "too many SSIDs requested to scan"); return NULL; } @@ -1163,7 +1163,7 @@ ssids_options_to_ptrarray (GVariant *value, GError **error) if (len > 32) { g_set_error (error, NM_DEVICE_ERROR, - NM_DEVICE_ERROR_NOT_ALLOWED, + NM_DEVICE_ERROR_INVALID_ARGUMENT, "SSID at index %d more than 32 bytes", (int) i); return NULL; } @@ -1189,8 +1189,7 @@ dbus_request_scan_cb (NMDevice *device, gpointer user_data) { NMDeviceWifi *self = NM_DEVICE_WIFI (device); - gs_unref_variant GVariant *scan_options = user_data; - gs_unref_ptrarray GPtrArray *ssids = NULL; + gs_unref_ptrarray GPtrArray *ssids = user_data; if (error) { g_dbus_method_invocation_return_gerror (context, error); @@ -1205,28 +1204,6 @@ dbus_request_scan_cb (NMDevice *device, return; } - if (scan_options) { - gs_unref_variant GVariant *val = g_variant_lookup_value (scan_options, "ssids", NULL); - - if (val) { - gs_free_error GError *ssid_error = NULL; - - if (!g_variant_is_of_type (val, G_VARIANT_TYPE ("aay"))) { - g_dbus_method_invocation_return_error_literal (context, - NM_DEVICE_ERROR, - NM_DEVICE_ERROR_NOT_ALLOWED, - "Invalid 'ssid' scan option"); - return; - } - - ssids = ssids_options_to_ptrarray (val, &ssid_error); - if (ssid_error) { - g_dbus_method_invocation_return_gerror (context, ssid_error); - return; - } - } - } - request_wireless_scan (self, FALSE, FALSE, ssids); g_dbus_method_invocation_return_value (context, NULL); } @@ -1239,6 +1216,29 @@ _nm_device_wifi_request_scan (NMDeviceWifi *self, NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); NMDevice *device = NM_DEVICE (self); gint64 last_scan; + gs_unref_ptrarray GPtrArray *ssids = NULL; + + if (options) { + gs_unref_variant GVariant *val = g_variant_lookup_value (options, "ssids", NULL); + + if (val) { + gs_free_error GError *ssid_error = NULL; + + if (!g_variant_is_of_type (val, G_VARIANT_TYPE ("aay"))) { + g_dbus_method_invocation_return_error_literal (invocation, + NM_DEVICE_ERROR, + NM_DEVICE_ERROR_INVALID_ARGUMENT, + "Invalid 'ssid' scan option"); + return; + } + + ssids = ssids_options_to_ptrarray (val, &ssid_error); + if (ssid_error) { + g_dbus_method_invocation_return_gerror (invocation, ssid_error); + return; + } + } + } if ( !priv->enabled || !priv->sup_iface @@ -1281,7 +1281,7 @@ _nm_device_wifi_request_scan (NMDeviceWifi *self, NM_AUTH_PERMISSION_WIFI_SCAN, TRUE, dbus_request_scan_cb, - options ? g_variant_ref (options) : NULL); + g_steal_pointer (&ssids)); } static gboolean From 8cc78565b1437439407cb423e22267f65a8e503f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Mar 2020 13:04:05 +0100 Subject: [PATCH 16/19] wifi: rename scan-interval variable to indicate they are in seconds --- src/devices/wifi/nm-device-wifi.c | 42 +++++++++++++++---------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index f605c7a9fd..9959be858b 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -43,10 +43,9 @@ #include "devices/nm-device-logging.h" _LOG_DECLARE_SELF(NMDeviceWifi); -/* All of these are in seconds */ -#define SCAN_INTERVAL_MIN 3 -#define SCAN_INTERVAL_STEP 20 -#define SCAN_INTERVAL_MAX 120 +#define SCAN_INTERVAL_SEC_MIN 3 +#define SCAN_INTERVAL_SEC_STEP 20 +#define SCAN_INTERVAL_SEC_MAX 120 #define SCAN_RAND_MAC_ADDRESS_EXPIRE_MIN 5 @@ -86,7 +85,7 @@ typedef struct { gint64 last_scan_msec; gint32 scheduled_scan_time; /* seconds */ - guint8 scan_interval; /* seconds */ + guint8 scan_interval_sec; guint pending_scan_id; guint ap_dump_id; @@ -382,9 +381,9 @@ supplicant_interface_release (NMDeviceWifi *self) nm_clear_g_source (&priv->pending_scan_id); /* Reset the scan interval to be pretty frequent when disconnected */ - priv->scan_interval = SCAN_INTERVAL_MIN + SCAN_INTERVAL_STEP; + priv->scan_interval_sec = SCAN_INTERVAL_SEC_MIN + SCAN_INTERVAL_SEC_STEP; _LOGD (LOGD_WIFI, "wifi-scan: reset interval to %u seconds", - (unsigned) priv->scan_interval); + (unsigned) priv->scan_interval_sec); nm_clear_g_source (&priv->ap_dump_id); @@ -1499,14 +1498,15 @@ schedule_scan (NMDeviceWifi *self, gboolean backoff) NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); gint32 now = nm_utils_get_monotonic_timestamp_sec (); - /* Cancel the pending scan if it would happen later than (now + the scan_interval) */ + /* Cancel the pending scan if it would happen later than (now + the scan_interval_sec) */ if (priv->pending_scan_id) { - if (now + priv->scan_interval < priv->scheduled_scan_time) + if (now + priv->scan_interval_sec < priv->scheduled_scan_time) nm_clear_g_source (&priv->pending_scan_id); } if (!priv->pending_scan_id) { - guint factor = 2, next_scan = priv->scan_interval; + guint factor = 2; + guint next_scan = priv->scan_interval_sec; if ( nm_device_is_activating (NM_DEVICE (self)) || (nm_device_get_state (NM_DEVICE (self)) == NM_DEVICE_STATE_ACTIVATED)) @@ -1516,22 +1516,22 @@ schedule_scan (NMDeviceWifi *self, gboolean backoff) request_wireless_scan_periodic, self); - priv->scheduled_scan_time = now + priv->scan_interval; - if (backoff && (priv->scan_interval < (SCAN_INTERVAL_MAX / factor))) { - priv->scan_interval += (SCAN_INTERVAL_STEP / factor); + priv->scheduled_scan_time = now + priv->scan_interval_sec; + if (backoff && (priv->scan_interval_sec < (SCAN_INTERVAL_SEC_MAX / factor))) { + priv->scan_interval_sec += (SCAN_INTERVAL_SEC_STEP / factor); /* Ensure the scan interval will never be less than 20s... */ - priv->scan_interval = MAX(priv->scan_interval, SCAN_INTERVAL_MIN + SCAN_INTERVAL_STEP); + priv->scan_interval_sec = MAX(priv->scan_interval_sec, SCAN_INTERVAL_SEC_MIN + SCAN_INTERVAL_SEC_STEP); /* ... or more than 120s */ - priv->scan_interval = MIN(priv->scan_interval, SCAN_INTERVAL_MAX); - } else if (!backoff && (priv->scan_interval == 0)) { + priv->scan_interval_sec = MIN(priv->scan_interval_sec, SCAN_INTERVAL_SEC_MAX); + } else if (!backoff && (priv->scan_interval_sec == 0)) { /* Invalid combination; would cause continual rescheduling of * the scan and hog CPU. Reset to something minimally sane. */ - priv->scan_interval = 5; + priv->scan_interval_sec = 5; } _LOGD (LOGD_WIFI, "wifi-scan: scheduled in %d seconds (interval now %d seconds)", - next_scan, priv->scan_interval); + next_scan, priv->scan_interval_sec); } } @@ -2114,7 +2114,7 @@ supplicant_iface_state (NMDeviceWifi *self, 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; + priv->scan_interval_sec = SCAN_INTERVAL_SEC_MIN; } /* In these states we know the supplicant is actually talking to something */ @@ -3060,7 +3060,7 @@ activation_success_handler (NMDevice *device) update_seen_bssids_cache (self, priv->current_ap); /* Reset scan interval to something reasonable */ - priv->scan_interval = SCAN_INTERVAL_MIN + (SCAN_INTERVAL_STEP * 2); + priv->scan_interval_sec = SCAN_INTERVAL_SEC_MIN + (SCAN_INTERVAL_SEC_STEP * 2); } static void @@ -3121,7 +3121,7 @@ device_state_changed (NMDevice *device, break; case NM_DEVICE_STATE_DISCONNECTED: /* Kick off a scan to get latest results */ - priv->scan_interval = SCAN_INTERVAL_MIN; + priv->scan_interval_sec = SCAN_INTERVAL_SEC_MIN; request_wireless_scan (self, FALSE, FALSE, NULL); break; default: From d2ee666dd74a74b26021d32ccd280306a1680944 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Mar 2020 13:40:30 +0100 Subject: [PATCH 17/19] wifi/iwd: drop unused signal NM_DEVICE_IWD_SCANNING_PROHIBITED --- src/devices/wifi/nm-device-iwd.c | 34 +++----------------------------- src/devices/wifi/nm-device-iwd.h | 2 -- 2 files changed, 3 insertions(+), 33 deletions(-) diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c index 72c83cf0e2..3c1d5b2298 100644 --- a/src/devices/wifi/nm-device-iwd.c +++ b/src/devices/wifi/nm-device-iwd.c @@ -43,14 +43,6 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDeviceIwd, PROP_LAST_SCAN, ); -enum { - SCANNING_PROHIBITED, - - LAST_SIGNAL -}; - -static guint signals[LAST_SIGNAL] = { 0 }; - typedef struct { GDBusObject * dbus_obj; GDBusProxy * dbus_device_proxy; @@ -79,9 +71,6 @@ struct _NMDeviceIwd { struct _NMDeviceIwdClass { NMDeviceClass parent; - - /* Signals */ - gboolean (*scanning_prohibited) (NMDeviceIwd *device, gboolean periodic); }; /*****************************************************************************/ @@ -95,6 +84,8 @@ G_DEFINE_TYPE (NMDeviceIwd, nm_device_iwd, NM_TYPE_DEVICE) static void schedule_periodic_scan (NMDeviceIwd *self, gboolean initial_scan); +static gboolean check_scanning_prohibited (NMDeviceIwd *self, gboolean periodic); + /*****************************************************************************/ static void @@ -972,15 +963,6 @@ _nm_device_iwd_get_aps (NMDeviceIwd *self) return &NM_DEVICE_IWD_GET_PRIVATE (self)->aps_lst_head; } -static gboolean -check_scanning_prohibited (NMDeviceIwd *self, gboolean periodic) -{ - gboolean prohibited = FALSE; - - g_signal_emit (self, signals[SCANNING_PROHIBITED], 0, periodic, &prohibited); - return prohibited; -} - static void scan_cb (GObject *source, GAsyncResult *res, gpointer user_data) { @@ -1090,7 +1072,7 @@ _nm_device_iwd_request_scan (NMDeviceIwd *self, } static gboolean -scanning_prohibited (NMDeviceIwd *self, gboolean periodic) +check_scanning_prohibited (NMDeviceIwd *self, gboolean periodic) { NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE (self); @@ -2580,8 +2562,6 @@ nm_device_iwd_class_init (NMDeviceIwdClass *klass) device_class->state_changed = device_state_changed; - klass->scanning_prohibited = scanning_prohibited; - obj_properties[PROP_MODE] = g_param_spec_uint (NM_DEVICE_IWD_MODE, "", "", NM_802_11_MODE_UNKNOWN, @@ -2626,12 +2606,4 @@ nm_device_iwd_class_init (NMDeviceIwdClass *klass) G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties); - - signals[SCANNING_PROHIBITED] = - g_signal_new (NM_DEVICE_IWD_SCANNING_PROHIBITED, - G_OBJECT_CLASS_TYPE (object_class), - G_SIGNAL_RUN_LAST, - G_STRUCT_OFFSET (NMDeviceIwdClass, scanning_prohibited), - NULL, NULL, NULL, - G_TYPE_BOOLEAN, 1, G_TYPE_BOOLEAN); } diff --git a/src/devices/wifi/nm-device-iwd.h b/src/devices/wifi/nm-device-iwd.h index 586e02f4b3..1f15d3f4bb 100644 --- a/src/devices/wifi/nm-device-iwd.h +++ b/src/devices/wifi/nm-device-iwd.h @@ -25,8 +25,6 @@ #define NM_DEVICE_IWD_SCANNING NM_DEVICE_WIFI_SCANNING #define NM_DEVICE_IWD_LAST_SCAN NM_DEVICE_WIFI_LAST_SCAN -#define NM_DEVICE_IWD_SCANNING_PROHIBITED NM_DEVICE_WIFI_SCANNING_PROHIBITED - typedef struct _NMDeviceIwd NMDeviceIwd; typedef struct _NMDeviceIwdClass NMDeviceIwdClass; From f1da7cb452e880c373e519201e8e2cd19d787c44 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Mar 2020 17:03:00 +0100 Subject: [PATCH 18/19] wifi: add and use nm_device_wifi_get_scanning() Don't read GObject properties. It's inefficient and harder to track who calls who. --- src/devices/wifi/nm-device-olpc-mesh.c | 11 +++++------ src/devices/wifi/nm-device-wifi.c | 10 +++++++++- src/devices/wifi/nm-device-wifi.h | 2 ++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index 2850682b00..bcb75f84b2 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -130,7 +130,6 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) { NMDeviceOlpcMesh *self = NM_DEVICE_OLPC_MESH (device); NMDeviceOlpcMeshPrivate *priv = NM_DEVICE_OLPC_MESH_GET_PRIVATE (self); - gboolean scanning; /* disconnect companion device, if it is connected */ if (nm_device_get_act_request (NM_DEVICE (priv->companion))) { @@ -145,8 +144,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason) } /* wait with continuing configuration until the companion device is done scanning */ - g_object_get (priv->companion, NM_DEVICE_WIFI_SCANNING, &scanning, NULL); - if (scanning) { + if (nm_device_wifi_get_scanning (NM_DEVICE_WIFI (priv->companion))) { priv->stage1_waiting = TRUE; return NM_ACT_STAGE_RETURN_POSTPONE; } @@ -248,13 +246,14 @@ companion_notify_cb (NMDeviceWifi *companion, GParamSpec *pspec, gpointer user_d { NMDeviceOlpcMesh *self = NM_DEVICE_OLPC_MESH (user_data); NMDeviceOlpcMeshPrivate *priv = NM_DEVICE_OLPC_MESH_GET_PRIVATE (self); - gboolean scanning; + + nm_assert (NM_IS_DEVICE_WIFI (companion)); + nm_assert (priv->companion == (gpointer) companion); if (!priv->stage1_waiting) return; - g_object_get (companion, NM_DEVICE_WIFI_SCANNING, &scanning, NULL); - if (!scanning) { + if (!nm_device_wifi_get_scanning (NM_DEVICE_WIFI (companion))) { priv->stage1_waiting = FALSE; nm_device_activate_schedule_stage1_device_prepare (NM_DEVICE (self), FALSE); } diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 9959be858b..0885f2a5ae 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -209,6 +209,14 @@ _ap_dump (NMDeviceWifi *self, nm_wifi_ap_to_string (ap, buf, sizeof (buf), now_msec)); } +gboolean +nm_device_wifi_get_scanning (NMDeviceWifi *self) +{ + g_return_val_if_fail (NM_IS_DEVICE_WIFI (self), FALSE); + + return NM_DEVICE_WIFI_GET_PRIVATE (self)->is_scanning; +} + static void _notify_scanning (NMDeviceWifi *self) { @@ -3276,7 +3284,7 @@ get_property (GObject *object, guint prop_id, nm_dbus_utils_g_value_set_object_path (value, priv->current_ap); break; case PROP_SCANNING: - g_value_set_boolean (value, priv->is_scanning); + g_value_set_boolean (value, nm_device_wifi_get_scanning (self)); break; case PROP_LAST_SCAN: g_value_set_int64 (value, diff --git a/src/devices/wifi/nm-device-wifi.h b/src/devices/wifi/nm-device-wifi.h index d861d24712..280856efaf 100644 --- a/src/devices/wifi/nm-device-wifi.h +++ b/src/devices/wifi/nm-device-wifi.h @@ -42,4 +42,6 @@ void _nm_device_wifi_request_scan (NMDeviceWifi *self, GPtrArray *nmtst_ssids_options_to_ptrarray (GVariant *value, GError **error); +gboolean nm_device_wifi_get_scanning (NMDeviceWifi *self); + #endif /* __NETWORKMANAGER_DEVICE_WIFI_H__ */ From fa86fe4ee23067870e2daa6966e8b59817cbd18d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Mar 2020 17:53:34 +0100 Subject: [PATCH 19/19] wifi: rework scanning-prohibited tracking for Wi-Fi companion and OLPC device This was previously tracked via a signal "scanning-prohibited". However, I think it was buggy, because the signal didn't specify a GSignalAccumulator, so when a NMDeviceOlpcMesh registered a handler, NMDeviceWifi.scanning_prohibited() was ignored. In theory, a GObject signal decouples the target and source of the signal and is more abstract. But more abstraction is worse, if there is exactly one target who cares about this signal: the OLPC mesh. And that target is well known at compile time. So, don't pretend that NMDeviceWifi or NMDeviceOlpcMesh aren't aware that they are together in this. Another downside of the signal is that you don't know when scanning gets unblocked. You can only poll and asked whether it is blocked, but there was no mechanism how NMDeviceWifi would be notified when scanning is no longer blocked. Rework this. Instead, the OLPC mesh explicitly registers and unregisters its blocking state with nm_device_wifi_scanning_prohibited_track(). --- src/devices/wifi/nm-device-olpc-mesh.c | 36 +++++----- src/devices/wifi/nm-device-wifi.c | 93 +++++++++++++++++--------- src/devices/wifi/nm-device-wifi.h | 5 +- 3 files changed, 86 insertions(+), 48 deletions(-) diff --git a/src/devices/wifi/nm-device-olpc-mesh.c b/src/devices/wifi/nm-device-olpc-mesh.c index bcb75f84b2..770b53f60c 100644 --- a/src/devices/wifi/nm-device-olpc-mesh.c +++ b/src/devices/wifi/nm-device-olpc-mesh.c @@ -235,6 +235,9 @@ companion_cleanup (NMDeviceOlpcMesh *self) NMDeviceOlpcMeshPrivate *priv = NM_DEVICE_OLPC_MESH_GET_PRIVATE (self); if (priv->companion) { + nm_device_wifi_scanning_prohibited_track (NM_DEVICE_WIFI (priv->companion), + self, + FALSE); g_signal_handlers_disconnect_by_data (priv->companion, self); g_clear_object (&priv->companion); } @@ -288,16 +291,6 @@ companion_state_changed_cb (NMDeviceWifi *companion, NM_DEVICE_STATE_REASON_USER_REQUESTED); } -static gboolean -companion_scan_prohibited_cb (NMDeviceWifi *companion, gboolean periodic, gpointer user_data) -{ - NMDeviceOlpcMesh *self = NM_DEVICE_OLPC_MESH (user_data); - NMDeviceState state = nm_device_get_state (NM_DEVICE (self)); - - /* Don't allow the companion to scan while configuring the mesh interface */ - return (state >= NM_DEVICE_STATE_PREPARE) && (state <= NM_DEVICE_STATE_IP_CONFIG); -} - static gboolean companion_autoconnect_allowed_cb (NMDeviceWifi *companion, gpointer user_data) { @@ -323,7 +316,7 @@ check_companion (NMDeviceOlpcMesh *self, NMDevice *other) if (!nm_utils_hwaddr_matches (my_addr, -1, their_addr, -1)) return FALSE; - g_assert (priv->companion == NULL); + nm_assert (priv->companion == NULL); priv->companion = g_object_ref (other); _LOGI (LOGD_OLPC, "found companion Wi-Fi device %s", @@ -335,9 +328,6 @@ check_companion (NMDeviceOlpcMesh *self, NMDevice *other) g_signal_connect (G_OBJECT (other), "notify::" NM_DEVICE_WIFI_SCANNING, G_CALLBACK (companion_notify_cb), self); - g_signal_connect (G_OBJECT (other), NM_DEVICE_WIFI_SCANNING_PROHIBITED, - G_CALLBACK (companion_scan_prohibited_cb), self); - g_signal_connect (G_OBJECT (other), NM_DEVICE_AUTOCONNECT_ALLOWED, G_CALLBACK (companion_autoconnect_allowed_cb), self); @@ -399,8 +389,24 @@ state_changed (NMDevice *device, NMDeviceState old_state, NMDeviceStateReason reason) { + NMDeviceOlpcMesh *self = NM_DEVICE_OLPC_MESH (device); + NMDeviceOlpcMeshPrivate *priv = NM_DEVICE_OLPC_MESH_GET_PRIVATE (self); + if (new_state == NM_DEVICE_STATE_UNAVAILABLE) - find_companion (NM_DEVICE_OLPC_MESH (device)); + find_companion (self); + + if (priv->companion) { + gboolean temporarily_prohibited = FALSE; + + if ( new_state >= NM_DEVICE_STATE_PREPARE + && new_state <= NM_DEVICE_STATE_IP_CONFIG) { + /* Don't allow the companion to scan while configuring the mesh interface */ + temporarily_prohibited = TRUE; + } + nm_device_wifi_scanning_prohibited_track (NM_DEVICE_WIFI (priv->companion), + self, + temporarily_prohibited); + } } static guint32 diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index 0885f2a5ae..266a842514 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -12,6 +12,7 @@ #include #include "nm-glib-aux/nm-ref-string.h" +#include "nm-glib-aux/nm-c-list.h" #include "nm-device-wifi-p2p.h" #include "nm-wifi-ap.h" #include "nm-libnm-core-intern/nm-common-macros.h" @@ -62,7 +63,6 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMDeviceWifi, ); enum { - SCANNING_PROHIBITED, P2P_DEVICE_CREATED, LAST_SIGNAL @@ -74,6 +74,8 @@ typedef struct { CList aps_lst_head; GHashTable *aps_idx_by_supplicant_path; + CList scanning_prohibited_lst_head; + NMWifiAP * current_ap; guint32 rate; bool enabled:1; /* rfkilled or not */ @@ -123,9 +125,6 @@ struct _NMDeviceWifi struct _NMDeviceWifiClass { NMDeviceClass parent; - - /* Signals */ - gboolean (*scanning_prohibited) (NMDeviceWifi *device, gboolean periodic); }; /*****************************************************************************/ @@ -194,6 +193,45 @@ static void recheck_p2p_availability (NMDeviceWifi *self); /*****************************************************************************/ +void +nm_device_wifi_scanning_prohibited_track (NMDeviceWifi *self, + gpointer tag, + gboolean temporarily_prohibited) +{ + NMDeviceWifiPrivate *priv; + NMCListElem *elem; + + g_return_if_fail (NM_IS_DEVICE_WIFI (self)); + nm_assert (tag); + + priv = NM_DEVICE_WIFI_GET_PRIVATE (self); + + /* We track these with a simple CList. This would be not efficient, if + * there would be many users that need to be tracked at the same time (there + * aren't). In fact, most of the time there is no NMDeviceOlpcMesh and + * nobody tracks itself here. Optimize for that and simplicity. */ + + elem = nm_c_list_elem_find_first (&priv->scanning_prohibited_lst_head, + iter, + iter == tag); + + if (!temporarily_prohibited) { + if (!elem) + return; + + nm_c_list_elem_free (elem); + return; + } + + if (elem) + return; + + c_list_link_tail (&priv->scanning_prohibited_lst_head, + &nm_c_list_elem_new_stale (tag)->lst); +} + +/*****************************************************************************/ + static void _ap_dump (NMDeviceWifi *self, NMLogLevel log_level, @@ -230,7 +268,6 @@ _notify_scanning (NMDeviceWifi *self) if (scanning == priv->is_scanning) return; - _LOGD (LOGD_WIFI, "wifi-scan: scanning-state: %s", scanning ? "scanning" : "idle"); priv->is_scanning = scanning; if ( !scanning @@ -239,6 +276,11 @@ _notify_scanning (NMDeviceWifi *self) priv->last_scan_msec = nm_utils_get_monotonic_timestamp_msec (); } + _LOGD (LOGD_WIFI, + "wifi-scan: scanning-state: %s%s", + scanning ? "scanning" : "idle", + last_scan_changed ? " (notify last-scan)" : ""); + schedule_scan (self, TRUE); nm_gobject_notify_together (self, @@ -1292,12 +1334,15 @@ _nm_device_wifi_request_scan (NMDeviceWifi *self, } static gboolean -scanning_prohibited (NMDeviceWifi *self, gboolean periodic) +check_scanning_prohibited (NMDeviceWifi *self, + gboolean periodic) { NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); - NMSupplicantInterfaceState supplicant_state; - g_return_val_if_fail (priv->sup_iface != NULL, TRUE); + nm_assert (NM_IS_SUPPLICANT_INTERFACE (priv->sup_iface)); + + if (!c_list_is_empty (&priv->scanning_prohibited_lst_head)) + return TRUE; /* Don't scan when a an AP or Ad-Hoc connection is active as it will * disrupt connected clients or peers. @@ -1334,11 +1379,11 @@ scanning_prohibited (NMDeviceWifi *self, gboolean periodic) } /* Prohibit scans if the supplicant is busy */ - 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 + if ( NM_IN_SET (nm_supplicant_interface_get_state (priv->sup_iface), + NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATING, + NM_SUPPLICANT_INTERFACE_STATE_ASSOCIATED, + NM_SUPPLICANT_INTERFACE_STATE_4WAY_HANDSHAKE, + NM_SUPPLICANT_INTERFACE_STATE_GROUP_HANDSHAKE) || nm_supplicant_interface_get_scanning (priv->sup_iface)) return TRUE; @@ -1346,15 +1391,6 @@ scanning_prohibited (NMDeviceWifi *self, gboolean periodic) return FALSE; } -static gboolean -check_scanning_prohibited (NMDeviceWifi *self, gboolean periodic) -{ - gboolean prohibited = FALSE; - - g_signal_emit (self, signals[SCANNING_PROHIBITED], 0, periodic, &prohibited); - return prohibited; -} - static gboolean hidden_filter_func (NMSettings *settings, NMSettingsConnection *set_con, @@ -3324,6 +3360,7 @@ nm_device_wifi_init (NMDeviceWifi *self) NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); c_list_init (&priv->aps_lst_head); + c_list_init (&priv->scanning_prohibited_lst_head); priv->aps_idx_by_supplicant_path = g_hash_table_new (nm_direct_hash, NULL); priv->hidden_probe_scan_warn = TRUE; @@ -3365,6 +3402,8 @@ dispose (GObject *object) NMDeviceWifi *self = NM_DEVICE_WIFI (object); NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self); + nm_assert (c_list_is_empty (&priv->scanning_prohibited_lst_head)); + nm_clear_g_source (&priv->periodic_update_id); wifi_secrets_cancel (self); @@ -3443,8 +3482,6 @@ nm_device_wifi_class_init (NMDeviceWifiClass *klass) device_class->state_changed = device_state_changed; - klass->scanning_prohibited = scanning_prohibited; - obj_properties[PROP_MODE] = g_param_spec_uint (NM_DEVICE_WIFI_MODE, "", "", NM_802_11_MODE_UNKNOWN, @@ -3491,14 +3528,6 @@ nm_device_wifi_class_init (NMDeviceWifiClass *klass) g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties); - signals[SCANNING_PROHIBITED] = - g_signal_new (NM_DEVICE_WIFI_SCANNING_PROHIBITED, - G_OBJECT_CLASS_TYPE (object_class), - G_SIGNAL_RUN_LAST, - G_STRUCT_OFFSET (NMDeviceWifiClass, scanning_prohibited), - NULL, NULL, NULL, - G_TYPE_BOOLEAN, 1, G_TYPE_BOOLEAN); - signals[P2P_DEVICE_CREATED] = g_signal_new (NM_DEVICE_WIFI_P2P_DEVICE_CREATED, G_OBJECT_CLASS_TYPE (object_class), diff --git a/src/devices/wifi/nm-device-wifi.h b/src/devices/wifi/nm-device-wifi.h index 280856efaf..c9fce4e267 100644 --- a/src/devices/wifi/nm-device-wifi.h +++ b/src/devices/wifi/nm-device-wifi.h @@ -24,7 +24,6 @@ #define NM_DEVICE_WIFI_SCANNING "scanning" #define NM_DEVICE_WIFI_LAST_SCAN "last-scan" -#define NM_DEVICE_WIFI_SCANNING_PROHIBITED "scanning-prohibited" #define NM_DEVICE_WIFI_P2P_DEVICE_CREATED "p2p-device-created" typedef struct _NMDeviceWifi NMDeviceWifi; @@ -44,4 +43,8 @@ GPtrArray *nmtst_ssids_options_to_ptrarray (GVariant *value, GError **error); gboolean nm_device_wifi_get_scanning (NMDeviceWifi *self); +void nm_device_wifi_scanning_prohibited_track (NMDeviceWifi *self, + gpointer tag, + gboolean temporarily_prohibited); + #endif /* __NETWORKMANAGER_DEVICE_WIFI_H__ */