From fa86fe4ee23067870e2daa6966e8b59817cbd18d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 20 Mar 2020 17:53:34 +0100 Subject: [PATCH] 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__ */