iwd: Track InterfacesAdded/Removed signals for Networks

Until now we didn't rely on InterfacesAdded and InterfacesRemoved
signals for tracking when IWD finds new Wi-Fi networks or expires
networks not seen in the latest scans.  Instead we'd request the whole
list of networks currently seen by IWD every time the Station.Scanning
property would go from true to false.  However the
Station.GetOrderedNetworks() IWD method that we use has a deficiency
up until 1.9 (I plan to fix it soon) where it won't show the hidden
network discovered in the course of the last ConnectHiddenNetwork() call
if that call was unsuccessful, in other words where the new network has
not been saved as a Known Network.  A new ConnectHiddenNetwork() will
fail with the "NotHidden" error, so we have to use the Network.Connect()
call for such a network but to find it out we need to track the
InterfacesAdded signals.  Doing this may also improve autoconnect speed
in some cases so overall I think it's a good idea.

3 files changed, 185 insertions, 77 deletions

diff --git a/src/devices/wifi/nm-device-iwd.c b/src/devices/wifi/nm-device-iwd.c
index a5dc7784ff..89cfb2cd28 100644
--- a/src/devices/wifi/nm-device-iwd.c
+++ b/src/devices/wifi/nm-device-iwd.c
@@ -61,7 +61,10 @@ typedef struct {
     bool                          scan_requested : 1;
     bool                          act_mode_switch : 1;
     bool                          secrets_failed : 1;
+    bool                          networks_requested : 1;
+    bool                          networks_changed : 1;
     gint64                        last_scan;
+    uint32_t                      ap_id;
 } NMDeviceIwdPrivate;
 
 struct _NMDeviceIwd {
@@ -200,49 +203,36 @@ ap_security_flags_from_network_type(const char *type)
     return flags;
 }
 
-static void
-insert_ap_from_network(NMDeviceIwd *self,
-                       GHashTable * aps,
-                       const char * path,
-                       gint64       last_seen_msec,
-                       int16_t      signal,
-                       uint32_t     ap_id)
-{
-    gs_unref_object GDBusProxy *network_proxy = NULL;
-    gs_unref_variant GVariant *name_value     = NULL;
-    gs_unref_variant GVariant *type_value     = NULL;
-    nm_auto_ref_string NMRefString *bss_path  = NULL;
-    const char *                    name;
-    const char *                    type;
-    NMSupplicantBssInfo             bss_info;
-    uint8_t                         bssid[6];
-    NMWifiAP *                      ap;
+static NMWifiAP *
+ap_from_network(NMDeviceIwd *self,
+                GDBusProxy * network,
+                NMRefString *bss_path,
+                gint64       last_seen_msec,
+                int16_t      signal)
+{
+    NMDeviceIwdPrivate *priv              = NM_DEVICE_IWD_GET_PRIVATE(self);
+    gs_unref_variant GVariant *name_value = NULL;
+    gs_unref_variant GVariant *type_value = NULL;
+    const char *               name;
+    const char *               type;
+    uint32_t                   ap_id;
+    uint8_t                    bssid[6];
     gs_unref_bytes GBytes *ssid = NULL;
+    NMWifiAP *             ap;
+    NMSupplicantBssInfo    bss_info;
 
-    bss_path = nm_ref_string_new(path);
-
-    if (g_hash_table_lookup(aps, path)) {
-        _LOGD(LOGD_WIFI, "Duplicate network at %s", path);
-        return;
-    }
-
-    network_proxy =
-        nm_iwd_manager_get_dbus_interface(nm_iwd_manager_get(), path, NM_IWD_NETWORK_INTERFACE);
-    if (!network_proxy)
-        return;
-
-    name_value = g_dbus_proxy_get_cached_property(network_proxy, "Name");
-    type_value = g_dbus_proxy_get_cached_property(network_proxy, "Type");
+    name_value = g_dbus_proxy_get_cached_property(network, "Name");
+    type_value = g_dbus_proxy_get_cached_property(network, "Type");
     if (!name_value || !g_variant_is_of_type(name_value, G_VARIANT_TYPE_STRING) || !type_value
         || !g_variant_is_of_type(type_value, G_VARIANT_TYPE_STRING))
-        return;
+        return NULL;
 
     name = g_variant_get_string(name_value, NULL);
     type = g_variant_get_string(type_value, NULL);
 
     if (nm_streq(type, "wep")) {
         /* WEP not supported */
-        return;
+        return NULL;
     }
 
     /* What we get from IWD are networks, or ESSs, that may contain
@@ -253,6 +243,7 @@ insert_ap_from_network(NMDeviceIwd *self,
      * already does that.  We fake the BSSIDs as they don't play any
      * role either.
      */
+    ap_id    = priv->ap_id++;
     bssid[0] = 0x00;
     bssid[1] = 0x01;
     bssid[2] = 0x02;
@@ -279,6 +270,34 @@ insert_ap_from_network(NMDeviceIwd *self,
 
     nm_assert(bss_path == nm_wifi_ap_get_supplicant_path(ap));
 
+    return ap;
+}
+
+static void
+insert_ap_from_network(NMDeviceIwd *self,
+                       GHashTable * aps,
+                       const char * path,
+                       gint64       last_seen_msec,
+                       int16_t      signal)
+{
+    gs_unref_object GDBusProxy *network_proxy = NULL;
+    nm_auto_ref_string NMRefString *bss_path  = nm_ref_string_new(path);
+    NMWifiAP *                      ap;
+
+    if (g_hash_table_lookup(aps, bss_path)) {
+        _LOGD(LOGD_WIFI, "Duplicate network at %s", path);
+        return;
+    }
+
+    network_proxy =
+        nm_iwd_manager_get_dbus_interface(nm_iwd_manager_get(), path, NM_IWD_NETWORK_INTERFACE);
+    if (!network_proxy)
+        return;
+
+    ap = ap_from_network(self, network_proxy, bss_path, last_seen_msec, signal);
+    if (!ap)
+        return;
+
     g_hash_table_insert(aps, bss_path, ap);
 }
 
@@ -293,20 +312,23 @@ get_ordered_networks_cb(GObject *source, GAsyncResult *res, gpointer user_data)
     const char *               path;
     int16_t                    signal;
     NMWifiAP *                 ap, *ap_safe, *new_ap;
-    gboolean                   changed = FALSE;
+    gboolean                   changed;
     GHashTableIter             ap_iter;
     gs_unref_hashtable GHashTable *new_aps = NULL;
-    static uint32_t                ap_id   = 0;
     gint64                         last_seen_msec;
 
     variant = g_dbus_proxy_call_finish(G_DBUS_PROXY(source), res, &error);
+    if (!variant && nm_utils_error_is_cancelled(error))
+        return;
+
+    priv                     = NM_DEVICE_IWD_GET_PRIVATE(self);
+    priv->networks_requested = FALSE;
+
     if (!variant) {
         _LOGE(LOGD_WIFI, "Station.GetOrderedNetworks failed: %s", error->message);
         return;
     }
 
-    priv = NM_DEVICE_IWD_GET_PRIVATE(self);
-
     if (!g_variant_is_of_type(variant, G_VARIANT_TYPE("(a(on))"))) {
         _LOGE(LOGD_WIFI,
               "Station.GetOrderedNetworks returned type %s instead of (a(on))",
@@ -319,10 +341,13 @@ get_ordered_networks_cb(GObject *source, GAsyncResult *res, gpointer user_data)
 
     last_seen_msec = nm_utils_get_monotonic_timestamp_msec();
     while (g_variant_iter_next(networks, "(&on)", &path, &signal))
-        insert_ap_from_network(self, new_aps, path, last_seen_msec, signal, ap_id++);
+        insert_ap_from_network(self, new_aps, path, last_seen_msec, signal);
 
     g_variant_iter_free(networks);
 
+    changed                = priv->networks_changed;
+    priv->networks_changed = FALSE;
+
     c_list_for_each_entry_safe (ap, ap_safe, &priv->aps_lst_head, aps_lst) {
         new_ap = g_hash_table_lookup(new_aps, nm_wifi_ap_get_supplicant_path(ap));
         if (new_ap) {
@@ -375,6 +400,7 @@ update_aps(NMDeviceIwd *self)
                       priv->cancellable,
                       get_ordered_networks_cb,
                       self);
+    priv->networks_requested = TRUE;
 }
 
 static void
@@ -2547,6 +2573,58 @@ nm_device_iwd_agent_query(NMDeviceIwd *self, GDBusMethodInvocation *invocation)
     return TRUE;
 }
 
+void
+nm_device_iwd_network_add_remove(NMDeviceIwd *self, GDBusProxy *network, bool add)
+{
+    NMDeviceIwdPrivate *priv = NM_DEVICE_IWD_GET_PRIVATE(self);
+    NMWifiAP *          ap   = NULL;
+    NMWifiAP *          tmp;
+    bool                recheck;
+    nm_auto_ref_string NMRefString *bss_path = NULL;
+
+    bss_path = nm_ref_string_new(g_dbus_proxy_get_object_path(network));
+    c_list_for_each_entry (tmp, &priv->aps_lst_head, aps_lst)
+        if (nm_wifi_ap_get_supplicant_path(tmp) == bss_path) {
+            ap = tmp;
+            break;
+        }
+
+    /* We could schedule an update_aps(self) idle call here but up to IWD 1.9
+     * when a hidden network connection is attempted, that network is initially
+     * only added as a Network object but not shown in GetOrderedNetworks()
+     * return values, and for some corner case scenarios it's beneficial to
+     * have that Network reflected in our ap list so that we don't attempt
+     * calling ConnectHiddenNetwork() on it, as that will fail in 1.9.  But we
+     * can skip recheck-available if we're currently scanning or in the middle
+     * of a GetOrderedNetworks() call as that will trigger the recheck too.
+     */
+    recheck = !priv->scanning && !priv->networks_requested;
+
+    if (!add) {
+        if (ap) {
+            ap_add_remove(self, FALSE, ap, recheck);
+            priv->networks_changed |= !recheck;
+        }
+
+        return;
+    }
+
+    if (!ap) {
+        ap = ap_from_network(self,
+                             network,
+                             bss_path,
+                             nm_utils_get_monotonic_timestamp_msec(),
+                             -10000);
+        if (!ap)
+            return;
+
+        ap_add_remove(self, TRUE, ap, recheck);
+        g_object_unref(ap);
+        priv->networks_changed |= !recheck;
+        return;
+    }
+}
+
 /*****************************************************************************/
 
 static const char *
diff --git a/src/devices/wifi/nm-device-iwd.h b/src/devices/wifi/nm-device-iwd.h
index e364beaf3d..d4bf5dd61d 100644
--- a/src/devices/wifi/nm-device-iwd.h
+++ b/src/devices/wifi/nm-device-iwd.h
@@ -44,4 +44,6 @@ void _nm_device_iwd_request_scan(NMDeviceIwd *          self,
                                  GVariant *             options,
                                  GDBusMethodInvocation *invocation);
 
+void nm_device_iwd_network_add_remove(NMDeviceIwd *device, GDBusProxy *network, bool add);
+
 #endif /* __NETWORKMANAGER_DEVICE_IWD_H__ */
diff --git a/src/devices/wifi/nm-iwd-manager.c b/src/devices/wifi/nm-iwd-manager.c
index c5d29f0838..a19c8b2afd 100644
--- a/src/devices/wifi/nm-iwd-manager.c
+++ b/src/devices/wifi/nm-iwd-manager.c
@@ -117,6 +117,44 @@ get_property_string_or_null(GDBusProxy *proxy, const char *property)
     return get_variant_string_or_null(value);
 }
 
+static NMDeviceIwd *
+get_device_from_network(NMIwdManager *self, GDBusProxy *network)
+{
+    NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE(self);
+    const char *         ifname;
+    const char *         device_path;
+    NMDevice *           device;
+    gs_unref_object GDBusInterface *device_obj = NULL;
+
+    /* Try not to rely on the path of the Device being a prefix of the
+     * Network's object path.
+     */
+
+    device_path = get_property_string_or_null(network, "Device");
+    if (!device_path) {
+        _LOGD("Device not cached for network at %s", g_dbus_proxy_get_object_path(network));
+        return NULL;
+    }
+
+    device_obj = g_dbus_object_manager_get_interface(priv->object_manager,
+                                                     device_path,
+                                                     NM_IWD_DEVICE_INTERFACE);
+
+    ifname = get_property_string_or_null(G_DBUS_PROXY(device_obj), "Name");
+    if (!ifname) {
+        _LOGD("Name not cached for device at %s", device_path);
+        return NULL;
+    }
+
+    device = nm_manager_get_device(priv->manager, ifname, NM_DEVICE_TYPE_WIFI);
+    if (!device || !NM_IS_DEVICE_IWD(device)) {
+        _LOGD("NM device %s is not an IWD-managed device", ifname);
+        return NULL;
+    }
+
+    return NM_DEVICE_IWD(device);
+}
+
 static void
 agent_dbus_method_cb(GDBusConnection *      connection,
                      const char *           sender,
@@ -129,12 +167,10 @@ agent_dbus_method_cb(GDBusConnection *      connection,
 {
     NMIwdManager *       self = user_data;
     NMIwdManagerPrivate *priv = NM_IWD_MANAGER_GET_PRIVATE(self);
-    const char *         network_path, *device_path, *ifname;
-    gs_unref_object GDBusInterface *network = NULL, *device_obj = NULL;
-    int                             ifindex;
-    NMDevice *                      device;
-    gs_free char *                  name_owner = NULL;
-    int                             errsv;
+    const char *         network_path;
+    NMDeviceIwd *        device;
+    gs_free char *       name_owner         = NULL;
+    gs_unref_object GDBusInterface *network = NULL;
 
     /* Be paranoid and check the sender address */
     name_owner = g_dbus_object_manager_client_get_name_owner(
@@ -151,47 +187,21 @@ agent_dbus_method_cb(GDBusConnection *      connection,
                                                   network_path,
                                                   NM_IWD_NETWORK_INTERFACE);
     if (!network) {
-        _LOGE("unable to find the network object");
-        return;
-    }
-
-    device_path = get_property_string_or_null(G_DBUS_PROXY(network), "Device");
-    if (!device_path) {
-        _LOGD("agent-request: device not cached for network %s in IWD Agent request", network_path);
+        _LOGE("agent-request: unable to find the network object");
         goto return_error;
     }
 
-    device_obj = g_dbus_object_manager_get_interface(priv->object_manager,
-                                                     device_path,
-                                                     NM_IWD_DEVICE_INTERFACE);
-
-    ifname = get_property_string_or_null(G_DBUS_PROXY(device_obj), "Name");
-    if (!ifname) {
-        _LOGD("agent-request: name not cached for device %s in IWD Agent request", device_path);
+    device = get_device_from_network(self, G_DBUS_PROXY(network));
+    if (!device) {
+        _LOGD("agent-request: device not found in IWD Agent request");
         goto return_error;
     }
 
-    ifindex = if_nametoindex(ifname);
-    if (!ifindex) {
-        errsv = errno;
-        _LOGD("agent-request: if_nametoindex failed for Name %s for Device at %s: %i",
-              ifname,
-              device_path,
-              errsv);
-        goto return_error;
-    }
-
-    device = nm_manager_get_device_by_ifindex(priv->manager, ifindex);
-    if (!NM_IS_DEVICE_IWD(device)) {
-        _LOGD("agent-request: IWD device named %s is not a Wifi device in IWD Agent request",
-              ifname);
-        goto return_error;
-    }
-
-    if (nm_device_iwd_agent_query(NM_DEVICE_IWD(device), invocation))
+    if (nm_device_iwd_agent_query(device, invocation))
         return;
 
-    _LOGD("agent-request: device %s did not handle the IWD Agent request", ifname);
+    _LOGD("agent-request: device %s did not handle the IWD Agent request",
+          nm_device_get_iface(NM_DEVICE(device)));
 
 return_error:
     /* IWD doesn't look at the specific error */
@@ -576,6 +586,15 @@ interface_added(GDBusObjectManager *object_manager,
 
         return;
     }
+
+    if (nm_streq(iface_name, NM_IWD_NETWORK_INTERFACE)) {
+        NMDeviceIwd *device = get_device_from_network(self, proxy);
+
+        if (device)
+            nm_device_iwd_network_add_remove(device, proxy, TRUE);
+
+        return;
+    }
 }
 
 static void
@@ -620,6 +639,15 @@ interface_removed(GDBusObjectManager *object_manager,
         g_hash_table_remove(priv->known_networks, &id);
         return;
     }
+
+    if (nm_streq(iface_name, NM_IWD_NETWORK_INTERFACE)) {
+        NMDeviceIwd *device = get_device_from_network(self, proxy);
+
+        if (device)
+            nm_device_iwd_network_add_remove(device, proxy, FALSE);
+
+        return;
+    }
 }
 
 static void