From 72f9cfe8bcdc06da5730313fc7a2a08b5d310686 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 18 Apr 2023 13:16:59 +0200 Subject: [PATCH 01/13] core: don't trigger recheck to auto activate for deleted devices The delete_on_deactivate_link_delete() handler may be called after the device was already removed from NMManager. Don't allow that. Check whether the device is still exported on D-Bus as indication. (cherry picked from commit 49c1e0151900c2c3dea0ed8a8aab57bd91f28c5e) --- src/core/devices/nm-device.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 0e029de97e..3c6addc5e9 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -12628,7 +12628,11 @@ delete_on_deactivate_link_delete(gpointer user_data) if (!nm_device_unrealize(self, TRUE, &error)) _LOGD(LOGD_DEVICE, "delete_on_deactivate: unrealizing failed (%s)", error->message); - nm_device_emit_recheck_auto_activate(self); + if (nm_dbus_object_is_exported(NM_DBUS_OBJECT(self))) { + /* The device is still alive. We may need to autoactivate virtual + * devices again. */ + nm_device_emit_recheck_auto_activate(self); + } g_free(data); return FALSE; From 073d3a619fa15db7de429cf54dbdec04542b7f2a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Apr 2023 10:57:57 +0200 Subject: [PATCH 02/13] core: expose and rename nm_policy_device_recheck_auto_activate_schedule() Let's simplify this part of the code. This is the first step. (cherry picked from commit f1c15f0ae79cee417766f31eb94e004f52971f05) --- src/core/nm-policy.c | 25 +++++++++++++++---------- src/core/nm-policy.h | 2 ++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index d7e05b7b5b..621e1600dd 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -136,7 +136,6 @@ _PRIV_TO_SELF(NMPolicyPrivate *priv) static void update_system_hostname(NMPolicy *self, const char *msg); static void schedule_activate_all(NMPolicy *self); -static void schedule_activate_check(NMPolicy *self, NMDevice *device); static NMDevice *get_default_device(NMPolicy *self, int addr_family); /*****************************************************************************/ @@ -1332,7 +1331,8 @@ pending_ac_state_changed(NMActiveConnection *ac, guint state, guint reason, NMPo con, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, TRUE); - schedule_activate_check(self, nm_active_connection_get_device(ac)); + nm_policy_device_recheck_auto_activate_schedule(self, + nm_active_connection_get_device(ac)); } /* Cleanup */ @@ -1439,7 +1439,7 @@ auto_activate_device(NMPolicy *self, NMDevice *device) best_connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, TRUE); - schedule_activate_check(self, device); + nm_policy_device_recheck_auto_activate_schedule(self, device); return; } @@ -1683,14 +1683,19 @@ sleeping_changed(NMManager *manager, GParamSpec *pspec, gpointer user_data) reset_autoconnect_all(self, NULL, FALSE); } -static void -schedule_activate_check(NMPolicy *self, NMDevice *device) +void +nm_policy_device_recheck_auto_activate_schedule(NMPolicy *self, NMDevice *device) { - NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); + NMPolicyPrivate *priv; ActivateData *data; NMActiveConnection *ac; const CList *tmp_list; + g_return_if_fail(NM_IS_POLICY(self)); + g_return_if_fail(NM_IS_DEVICE(device)); + + priv = NM_POLICY_GET_PRIVATE(self); + if (nm_manager_get_state(priv->manager) == NM_STATE_ASLEEP) return; @@ -2126,7 +2131,7 @@ device_state_changed(NMDevice *device, update_routing_and_dns(self, FALSE, device); /* Device is now available for auto-activation */ - schedule_activate_check(self, device); + nm_policy_device_recheck_auto_activate_schedule(self, device); break; case NM_DEVICE_STATE_PREPARE: @@ -2248,7 +2253,7 @@ device_autoconnect_changed(NMDevice *device, GParamSpec *pspec, gpointer user_da NMPolicyPrivate *priv = user_data; NMPolicy *self = _PRIV_TO_SELF(priv); - schedule_activate_check(self, device); + nm_policy_device_recheck_auto_activate_schedule(self, device); } static void @@ -2257,7 +2262,7 @@ device_recheck_auto_activate(NMDevice *device, gpointer user_data) NMPolicyPrivate *priv = user_data; NMPolicy *self = _PRIV_TO_SELF(priv); - schedule_activate_check(self, device); + nm_policy_device_recheck_auto_activate_schedule(self, device); } static void @@ -2518,7 +2523,7 @@ schedule_activate_all_cb(gpointer user_data) priv->schedule_activate_all_id = 0; nm_manager_for_each_device (priv->manager, device, tmp_lst) - schedule_activate_check(self, device); + nm_policy_device_recheck_auto_activate_schedule(self, device); return G_SOURCE_REMOVE; } diff --git a/src/core/nm-policy.h b/src/core/nm-policy.h index f2b98dbd34..9cfb0b2448 100644 --- a/src/core/nm-policy.h +++ b/src/core/nm-policy.h @@ -34,6 +34,8 @@ NMActiveConnection *nm_policy_get_activating_ip6_ac(NMPolicy *policy); void nm_policy_unblock_failed_ovs_interfaces(NMPolicy *self); +void nm_policy_device_recheck_auto_activate_schedule(NMPolicy *self, NMDevice *device); + /** * NMPolicyHostnameMode * @NM_POLICY_HOSTNAME_MODE_NONE: never update the transient hostname. From a446ee2e1b1c1701c3d463bfb96fc55cfa874009 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Apr 2023 11:02:36 +0200 Subject: [PATCH 03/13] core: rename internal function nm_policy_device_recheck_auto_activate_all_schedule() The "all" variant is strongly related to nm_policy_device_recheck_auto_activate_schedule(). Rename, so that the names express that better. (cherry picked from commit 886786ee0b6e899bc9c9083334968cad18b9e4e8) --- src/core/nm-policy.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index 621e1600dd..af4a9196f9 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -72,7 +72,8 @@ typedef struct { guint reset_retries_id; /* idle handler for resetting the retries count */ - guint schedule_activate_all_id; /* idle handler for schedule_activate_all(). */ + guint + schedule_activate_all_id; /* idle handler for nm_policy_device_recheck_auto_activate_all_schedule(). */ NMPolicyHostnameMode hostname_mode; char *orig_hostname; /* hostname at NM start time */ @@ -135,7 +136,7 @@ _PRIV_TO_SELF(NMPolicyPrivate *priv) /*****************************************************************************/ static void update_system_hostname(NMPolicy *self, const char *msg); -static void schedule_activate_all(NMPolicy *self); +static void nm_policy_device_recheck_auto_activate_all_schedule(NMPolicy *self); static NMDevice *get_default_device(NMPolicy *self, int addr_family); /*****************************************************************************/ @@ -1760,7 +1761,7 @@ reset_connections_retries(gpointer user_data) /* If anything changed, try to activate the newly re-enabled connections */ if (changed) - schedule_activate_all(self); + nm_policy_device_recheck_auto_activate_all_schedule(self); return FALSE; } @@ -1858,7 +1859,7 @@ activate_slave_connections(NMPolicy *self, NMDevice *device) } if (changed) - schedule_activate_all(self); + nm_policy_device_recheck_auto_activate_all_schedule(self); } static gboolean @@ -2513,7 +2514,7 @@ active_connection_removed(NMManager *manager, NMActiveConnection *active, gpoint /*****************************************************************************/ static gboolean -schedule_activate_all_cb(gpointer user_data) +_device_recheck_auto_activate_all_cb(gpointer user_data) { NMPolicy *self = user_data; NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); @@ -2529,23 +2530,25 @@ schedule_activate_all_cb(gpointer user_data) } static void -schedule_activate_all(NMPolicy *self) +nm_policy_device_recheck_auto_activate_all_schedule(NMPolicy *self) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); /* always restart the idle handler. That way, we settle * all other events before restarting to activate them. */ nm_clear_g_source(&priv->schedule_activate_all_id); - priv->schedule_activate_all_id = g_idle_add(schedule_activate_all_cb, self); + priv->schedule_activate_all_id = g_idle_add(_device_recheck_auto_activate_all_cb, self); } +/*****************************************************************************/ + static void connection_added(NMSettings *settings, NMSettingsConnection *connection, gpointer user_data) { NMPolicyPrivate *priv = user_data; NMPolicy *self = _PRIV_TO_SELF(priv); - schedule_activate_all(self); + nm_policy_device_recheck_auto_activate_all_schedule(self); } static void @@ -2613,7 +2616,7 @@ connection_updated(NMSettings *settings, } } - schedule_activate_all(self); + nm_policy_device_recheck_auto_activate_all_schedule(self); } static void @@ -2662,7 +2665,7 @@ connection_flags_changed(NMSettings *settings, NMSettingsConnection *connection, if (NM_FLAGS_HAS(nm_settings_connection_get_flags(connection), NM_SETTINGS_CONNECTION_INT_FLAGS_VISIBLE)) { if (!nm_settings_connection_autoconnect_is_blocked(connection)) - schedule_activate_all(self); + nm_policy_device_recheck_auto_activate_all_schedule(self); } } @@ -2676,7 +2679,7 @@ secret_agent_registered(NMSettings *settings, NMSecretAgent *agent, gpointer use * connections failed due to missing secrets may re-try auto-connection. */ if (reset_autoconnect_all(self, NULL, TRUE)) - schedule_activate_all(self); + nm_policy_device_recheck_auto_activate_all_schedule(self); } NMActiveConnection * From 7e49e32f18d8ba5d30764c597d61c1fbbb414326 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Apr 2023 12:55:57 +0200 Subject: [PATCH 04/13] core: use GSource for tracking _device_recheck_auto_activate_all_cb idle action The numeric source IDs are discouraged. Use a GSource instead. (cherry picked from commit 1559c37b9f5f4a449413341275a331ef426f9d88) --- src/core/nm-policy.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index af4a9196f9..d312da34af 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -62,6 +62,8 @@ typedef struct { NMSettings *settings; + GSource *device_recheck_auto_activate_all_idle_source; + NMHostnameManager *hostname_manager; NMActiveConnection *default_ac4, *activating_ac4; @@ -72,9 +74,6 @@ typedef struct { guint reset_retries_id; /* idle handler for resetting the retries count */ - guint - schedule_activate_all_id; /* idle handler for nm_policy_device_recheck_auto_activate_all_schedule(). */ - NMPolicyHostnameMode hostname_mode; char *orig_hostname; /* hostname at NM start time */ char *cur_hostname; /* hostname we want to assign */ @@ -2521,12 +2520,12 @@ _device_recheck_auto_activate_all_cb(gpointer user_data) const CList *tmp_lst; NMDevice *device; - priv->schedule_activate_all_id = 0; + nm_clear_g_source_inst(&priv->device_recheck_auto_activate_all_idle_source); nm_manager_for_each_device (priv->manager, device, tmp_lst) nm_policy_device_recheck_auto_activate_schedule(self, device); - return G_SOURCE_REMOVE; + return G_SOURCE_CONTINUE; } static void @@ -2536,8 +2535,10 @@ nm_policy_device_recheck_auto_activate_all_schedule(NMPolicy *self) /* always restart the idle handler. That way, we settle * all other events before restarting to activate them. */ - nm_clear_g_source(&priv->schedule_activate_all_id); - priv->schedule_activate_all_id = g_idle_add(_device_recheck_auto_activate_all_cb, self); + nm_clear_g_source_inst(&priv->device_recheck_auto_activate_all_idle_source); + + priv->device_recheck_auto_activate_all_idle_source = + nm_g_idle_add_source(_device_recheck_auto_activate_all_cb, self); } /*****************************************************************************/ @@ -2953,7 +2954,7 @@ dispose(GObject *object) nm_assert(c_list_is_empty(nm_manager_get_active_connections(priv->manager))); nm_clear_g_source(&priv->reset_retries_id); - nm_clear_g_source(&priv->schedule_activate_all_id); + nm_clear_g_source_inst(&priv->device_recheck_auto_activate_all_idle_source); nm_clear_g_free(&priv->orig_hostname); nm_clear_g_free(&priv->cur_hostname); From 65d127461b0fdf404d402405e8b194e7742d4ad1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Apr 2023 13:08:14 +0200 Subject: [PATCH 05/13] core: use GSource for tracking reset_connections_retries idle action The numeric source IDs are discouraged. Use a GSource instead. (cherry picked from commit aa2569a9cdff9e9a0afbe99a394b5f351a7b1506) --- src/core/nm-policy.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index d312da34af..116302d8a5 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -64,6 +64,8 @@ typedef struct { GSource *device_recheck_auto_activate_all_idle_source; + GSource *reset_connections_retries_idle_source; + NMHostnameManager *hostname_manager; NMActiveConnection *default_ac4, *activating_ac4; @@ -72,8 +74,6 @@ typedef struct { NMDnsManager *dns_manager; gulong config_changed_id; - guint reset_retries_id; /* idle handler for resetting the retries count */ - NMPolicyHostnameMode hostname_mode; char *orig_hostname; /* hostname at NM start time */ char *cur_hostname; /* hostname we want to assign */ @@ -1734,7 +1734,7 @@ reset_connections_retries(gpointer user_data) gint32 con_stamp, min_stamp, now; gboolean changed = FALSE; - priv->reset_retries_id = 0; + nm_clear_g_source_inst(&priv->reset_connections_retries_idle_source); min_stamp = 0; now = nm_utils_get_monotonic_timestamp_sec(); @@ -1754,15 +1754,16 @@ reset_connections_retries(gpointer user_data) } /* Schedule the handler again if there are some stamps left */ - if (min_stamp != 0) - priv->reset_retries_id = - g_timeout_add_seconds(min_stamp - now, reset_connections_retries, self); + if (min_stamp != 0) { + priv->reset_connections_retries_idle_source = + nm_g_timeout_add_seconds_source(min_stamp - now, reset_connections_retries, self); + } /* If anything changed, try to activate the newly re-enabled connections */ if (changed) nm_policy_device_recheck_auto_activate_all_schedule(self); - return FALSE; + return G_SOURCE_CONTINUE; } static void @@ -1777,15 +1778,15 @@ _connection_autoconnect_retries_set(NMPolicy *self, NMSettingsConnection *connec if (tries == 0) { /* Schedule a handler to reset retries count */ - if (!priv->reset_retries_id) { + if (!priv->reset_connections_retries_idle_source) { gint32 retry_time = nm_settings_connection_autoconnect_retries_blocked_until(connection); g_warn_if_fail(retry_time != 0); - priv->reset_retries_id = - g_timeout_add_seconds(MAX(0, retry_time - nm_utils_get_monotonic_timestamp_sec()), - reset_connections_retries, - self); + priv->reset_connections_retries_idle_source = nm_g_timeout_add_seconds_source( + MAX(0, retry_time - nm_utils_get_monotonic_timestamp_sec()), + reset_connections_retries, + self); } } } @@ -2953,7 +2954,7 @@ dispose(GObject *object) */ nm_assert(c_list_is_empty(nm_manager_get_active_connections(priv->manager))); - nm_clear_g_source(&priv->reset_retries_id); + nm_clear_g_source_inst(&priv->reset_connections_retries_idle_source); nm_clear_g_source_inst(&priv->device_recheck_auto_activate_all_idle_source); nm_clear_g_free(&priv->orig_hostname); From 3382e666b23d6289a458ecc237c45d72029e7eef Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Apr 2023 11:12:46 +0200 Subject: [PATCH 06/13] core: drop NM_DEVICE_RECHECK_AUTO_ACTIVATE signal and call policy directly GObject signals don't make the code easier to understand, on the contrary. They may have their purpose, when objects truly must/should not be aware of each other, and need to be composed very loosely. That is not the case here. There really is only one subscriber to NM_DEVICE_RECHECK_AUTO_ACTIVATE signal, and it only makes sense this way. Instead of going through a signal invocation, just call the well known method directly. It becomes clearer who calls this code (and it has a lower overhead). When using cscope/ctags it also is easier to follow the code because the tools understand function calls. (cherry picked from commit 3c59c6b393c1ffb92ff3d2a4e471f8c6b740336a) --- src/core/devices/nm-device.c | 13 +------------ src/core/devices/nm-device.h | 1 - src/core/nm-manager.c | 10 ++++++++++ src/core/nm-manager.h | 2 ++ src/core/nm-policy.c | 13 ------------- 5 files changed, 13 insertions(+), 26 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 3c6addc5e9..2bcebf3fa1 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -335,7 +335,6 @@ enum { IP6_PREFIX_DELEGATED, IP6_SUBNET_NEEDED, REMOVED, - RECHECK_AUTO_ACTIVATE, RECHECK_ASSUME, DNS_LOOKUP_DONE, PLATFORM_ADDRESS_CHANGED, @@ -9083,7 +9082,7 @@ nm_device_queue_recheck_available(NMDevice *self, void nm_device_emit_recheck_auto_activate(NMDevice *self) { - g_signal_emit(self, signals[RECHECK_AUTO_ACTIVATE], 0); + nm_manager_device_recheck_auto_activate_schedule(nm_device_get_manager(self), self); } void @@ -18554,16 +18553,6 @@ nm_device_class_init(NMDeviceClass *klass) G_TYPE_NONE, 0); - signals[RECHECK_AUTO_ACTIVATE] = g_signal_new(NM_DEVICE_RECHECK_AUTO_ACTIVATE, - G_OBJECT_CLASS_TYPE(object_class), - G_SIGNAL_RUN_FIRST, - 0, - NULL, - NULL, - NULL, - G_TYPE_NONE, - 0); - signals[RECHECK_ASSUME] = g_signal_new(NM_DEVICE_RECHECK_ASSUME, G_OBJECT_CLASS_TYPE(object_class), G_SIGNAL_RUN_FIRST, diff --git a/src/core/devices/nm-device.h b/src/core/devices/nm-device.h index a9dc18b19d..6b3d1dde70 100644 --- a/src/core/devices/nm-device.h +++ b/src/core/devices/nm-device.h @@ -75,7 +75,6 @@ #define NM_DEVICE_IP6_PREFIX_DELEGATED "ip6-prefix-delegated" #define NM_DEVICE_IP6_SUBNET_NEEDED "ip6-subnet-needed" #define NM_DEVICE_REMOVED "removed" -#define NM_DEVICE_RECHECK_AUTO_ACTIVATE "recheck-auto-activate" #define NM_DEVICE_RECHECK_ASSUME "recheck-assume" #define NM_DEVICE_STATE_CHANGED "state-changed" #define NM_DEVICE_LINK_INITIALIZED "link-initialized" diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 19ca1d1e9b..816c7efa72 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -2666,6 +2666,16 @@ _rfkill_update_from_user(NMManager *self, NMRfkillType rtype, gboolean enabled) /*****************************************************************************/ +void +nm_manager_device_recheck_auto_activate_schedule(NMManager *self, NMDevice *device) +{ + g_return_if_fail(NM_IS_MANAGER(self)); + + nm_policy_device_recheck_auto_activate_schedule(NM_MANAGER_GET_PRIVATE(self)->policy, device); +} + +/*****************************************************************************/ + static void device_auth_done_cb(NMAuthChain *chain, GDBusMethodInvocation *context, gpointer user_data) { diff --git a/src/core/nm-manager.h b/src/core/nm-manager.h index 0cbbcbf0ae..d4d3197aeb 100644 --- a/src/core/nm-manager.h +++ b/src/core/nm-manager.h @@ -121,6 +121,8 @@ NMSettingsConnection **nm_manager_get_activatable_connections(NMManager *manager gboolean sort, guint *out_len); +void nm_manager_device_recheck_auto_activate_schedule(NMManager *self, NMDevice *device); + void nm_manager_write_device_state_all(NMManager *manager); gboolean nm_manager_write_device_state(NMManager *manager, NMDevice *device, int *out_ifindex); diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index 116302d8a5..786584b4d8 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -2257,15 +2257,6 @@ device_autoconnect_changed(NMDevice *device, GParamSpec *pspec, gpointer user_da nm_policy_device_recheck_auto_activate_schedule(self, device); } -static void -device_recheck_auto_activate(NMDevice *device, gpointer user_data) -{ - NMPolicyPrivate *priv = user_data; - NMPolicy *self = _PRIV_TO_SELF(priv); - - nm_policy_device_recheck_auto_activate_schedule(self, device); -} - static void devices_list_unregister(NMPolicy *self, NMDevice *device) { @@ -2298,10 +2289,6 @@ devices_list_register(NMPolicy *self, NMDevice *device) "notify::" NM_DEVICE_AUTOCONNECT, G_CALLBACK(device_autoconnect_changed), priv); - g_signal_connect(device, - NM_DEVICE_RECHECK_AUTO_ACTIVATE, - G_CALLBACK(device_recheck_auto_activate), - priv); } static void From 0c449ef4d84b3954aa99c7d2504795f5ddef4fb8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Apr 2023 11:14:39 +0200 Subject: [PATCH 07/13] core: rename nm_device_emit_recheck_auto_activate() to nm_device_recheck_auto_activate_schedule() It's the better name. Especially since there is no more signal involved, the term "emit" doesn't match. Note also how the previous approach using a signal tried to abstract what is happening. So we were no longer rechecking-autoconnect, instead, we were emitting-a-signal-to-recheck-autoconnect. Just be plain about what it is doing and don't go through a layer of signal. (cherry picked from commit 751b927cf21aed1cae448d5ff86e058880419e2e) --- src/core/devices/nm-device.c | 12 ++++++------ src/core/devices/nm-device.h | 4 ++-- src/core/devices/ovs/nm-device-ovs-interface.c | 2 +- src/core/devices/wifi/nm-device-iwd.c | 10 +++++----- src/core/devices/wifi/nm-device-olpc-mesh.c | 2 +- src/core/devices/wifi/nm-device-wifi.c | 4 ++-- src/core/nm-manager.c | 2 +- 7 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 2bcebf3fa1..d73f9ac890 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -6443,7 +6443,7 @@ carrier_changed(NMDevice *self, gboolean carrier) * when the carrier appears, auto connections are rechecked for * the device. */ - nm_device_emit_recheck_auto_activate(self); + nm_device_recheck_auto_activate_schedule(self); } } else { if (priv->state == NM_DEVICE_STATE_UNAVAILABLE) { @@ -6767,7 +6767,7 @@ device_link_changed(gpointer user_data) /* Let any connections that use the new interface name have a chance * to auto-activate on the device. */ - nm_device_emit_recheck_auto_activate(self); + nm_device_recheck_auto_activate_schedule(self); } if (priv->ipac6_data.ndisc && pllink->inet6_token.id) { @@ -7702,7 +7702,7 @@ nm_device_unrealize(NMDevice *self, gboolean remove_resources, GError **error) /* In case the unrealized device is not going away, it may need to * autoactivate. Schedule also a check for that. */ - nm_device_emit_recheck_auto_activate(self); + nm_device_recheck_auto_activate_schedule(self); return TRUE; } @@ -7724,7 +7724,7 @@ nm_device_notify_availability_maybe_changed(NMDevice *self) * available. */ nm_device_recheck_available_connections(self); if (g_hash_table_size(priv->available_connections) > 0) - nm_device_emit_recheck_auto_activate(self); + nm_device_recheck_auto_activate_schedule(self); } /** @@ -9080,7 +9080,7 @@ nm_device_queue_recheck_available(NMDevice *self, } void -nm_device_emit_recheck_auto_activate(NMDevice *self) +nm_device_recheck_auto_activate_schedule(NMDevice *self) { nm_manager_device_recheck_auto_activate_schedule(nm_device_get_manager(self), self); } @@ -12630,7 +12630,7 @@ delete_on_deactivate_link_delete(gpointer user_data) if (nm_dbus_object_is_exported(NM_DBUS_OBJECT(self))) { /* The device is still alive. We may need to autoactivate virtual * devices again. */ - nm_device_emit_recheck_auto_activate(self); + nm_device_recheck_auto_activate_schedule(self); } g_free(data); diff --git a/src/core/devices/nm-device.h b/src/core/devices/nm-device.h index 6b3d1dde70..39151a20ac 100644 --- a/src/core/devices/nm-device.h +++ b/src/core/devices/nm-device.h @@ -293,7 +293,7 @@ typedef struct _NMDeviceClass { GPtrArray *(*get_extra_rules)(NMDevice *self); /* allow derived classes to override the result of nm_device_autoconnect_allowed(). - * If the value changes, the class should call nm_device_emit_recheck_auto_activate(), + * If the value changes, the class should call nm_device_recheck_auto_activate_schedule(), * which emits NM_DEVICE_RECHECK_AUTO_ACTIVATE signal. */ gboolean (*get_autoconnect_allowed)(NMDevice *self); @@ -704,7 +704,7 @@ nm_device_autoconnect_blocked_unset(NMDevice *device, NMDeviceAutoconnectBlocked nm_device_autoconnect_blocked_set_full(device, mask, NM_DEVICE_AUTOCONNECT_BLOCKED_NONE); } -void nm_device_emit_recheck_auto_activate(NMDevice *device); +void nm_device_recheck_auto_activate_schedule(NMDevice *device); NMDeviceSysIfaceState nm_device_sys_iface_state_get(NMDevice *device); diff --git a/src/core/devices/ovs/nm-device-ovs-interface.c b/src/core/devices/ovs/nm-device-ovs-interface.c index 711f65cb5b..8b0fc7f79c 100644 --- a/src/core/devices/ovs/nm-device-ovs-interface.c +++ b/src/core/devices/ovs/nm-device-ovs-interface.c @@ -479,7 +479,7 @@ ovsdb_ready(NMOvsdb *ovsdb, NMDeviceOvsInterface *self) NM_DEVICE_STATE_REASON_NONE, NM_DEVICE_STATE_REASON_NONE); nm_device_recheck_available_connections(device); - nm_device_emit_recheck_auto_activate(device); + nm_device_recheck_auto_activate_schedule(device); } static void diff --git a/src/core/devices/wifi/nm-device-iwd.c b/src/core/devices/wifi/nm-device-iwd.c index e03227cd03..6bb420354f 100644 --- a/src/core/devices/wifi/nm-device-iwd.c +++ b/src/core/devices/wifi/nm-device-iwd.c @@ -159,7 +159,7 @@ ap_add_remove(NMDeviceIwd *self, } if (priv->enabled && !priv->iwd_autoconnect) - nm_device_emit_recheck_auto_activate(NM_DEVICE(self)); + nm_device_recheck_auto_activate_schedule(NM_DEVICE(self)); if (recheck_available_connections) nm_device_recheck_available_connections(NM_DEVICE(self)); @@ -208,7 +208,7 @@ remove_all_aps(NMDeviceIwd *self) ap_add_remove(self, FALSE, ap, FALSE); if (!priv->iwd_autoconnect) - nm_device_emit_recheck_auto_activate(NM_DEVICE(self)); + nm_device_recheck_auto_activate_schedule(NM_DEVICE(self)); nm_device_recheck_available_connections(NM_DEVICE(self)); } @@ -401,7 +401,7 @@ get_ordered_networks_cb(GObject *source, GAsyncResult *res, gpointer user_data) if (changed) { if (!priv->iwd_autoconnect) - nm_device_emit_recheck_auto_activate(NM_DEVICE(self)); + nm_device_recheck_auto_activate_schedule(NM_DEVICE(self)); nm_device_recheck_available_connections(NM_DEVICE(self)); } @@ -1682,7 +1682,7 @@ failed: if (!priv->nm_autoconnect) { priv->nm_autoconnect = true; - nm_device_emit_recheck_auto_activate(device); + nm_device_recheck_auto_activate_schedule(device); } } g_variant_unref(value); @@ -2908,7 +2908,7 @@ state_changed(NMDeviceIwd *self, const char *new_state) if (!priv->iwd_autoconnect && NM_IN_STRSET(new_state, "disconnected")) { priv->nm_autoconnect = TRUE; if (!can_connect) - nm_device_emit_recheck_auto_activate(device); + nm_device_recheck_auto_activate_schedule(device); } } diff --git a/src/core/devices/wifi/nm-device-olpc-mesh.c b/src/core/devices/wifi/nm-device-olpc-mesh.c index 4705f75ce3..436c7847ab 100644 --- a/src/core/devices/wifi/nm-device-olpc-mesh.c +++ b/src/core/devices/wifi/nm-device-olpc-mesh.c @@ -270,7 +270,7 @@ companion_state_changed_cb(NMDeviceWifi *companion, NMDeviceState self_state = nm_device_get_state(NM_DEVICE(self)); if (old_state > NM_DEVICE_STATE_DISCONNECTED && state <= NM_DEVICE_STATE_DISCONNECTED) { - nm_device_emit_recheck_auto_activate(NM_DEVICE(self)); + nm_device_recheck_auto_activate_schedule(NM_DEVICE(self)); } if (self_state < NM_DEVICE_STATE_PREPARE || self_state > NM_DEVICE_STATE_ACTIVATED diff --git a/src/core/devices/wifi/nm-device-wifi.c b/src/core/devices/wifi/nm-device-wifi.c index 03625f8d6d..9012a59bef 100644 --- a/src/core/devices/wifi/nm-device-wifi.c +++ b/src/core/devices/wifi/nm-device-wifi.c @@ -483,7 +483,7 @@ _scan_notify_is_scanning(NMDeviceWifi *self) if (!_scan_is_scanning_eval(priv)) { if (state <= NM_DEVICE_STATE_DISCONNECTED || state > NM_DEVICE_STATE_ACTIVATED) - nm_device_emit_recheck_auto_activate(NM_DEVICE(self)); + nm_device_recheck_auto_activate_schedule(NM_DEVICE(self)); nm_device_remove_pending_action(NM_DEVICE(self), NM_PENDING_ACTION_WIFI_SCAN, FALSE); } @@ -843,7 +843,7 @@ ap_add_remove(NMDeviceWifi *self, nm_dbus_object_clear_and_unexport(&ap); } - nm_device_emit_recheck_auto_activate(NM_DEVICE(self)); + nm_device_recheck_auto_activate_schedule(NM_DEVICE(self)); if (recheck_available_connections) nm_device_recheck_available_connections(NM_DEVICE(self)); } diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 816c7efa72..54be060774 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -3459,7 +3459,7 @@ _device_realize_finish(NMManager *self, NMDevice *device, const NMPlatformLink * nm_device_state_changed(device, NM_DEVICE_STATE_UNAVAILABLE, NM_DEVICE_STATE_REASON_NOW_MANAGED); - nm_device_emit_recheck_auto_activate(device); + nm_device_recheck_auto_activate_schedule(device); } /** From 4558c23209e39a2a6ebfd1b31018b292860c827f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Apr 2023 11:16:45 +0200 Subject: [PATCH 08/13] core: call nm_manager_device_recheck_auto_activate_schedule() from "nm-manager.c" No need to call down to the device, to call back up to the NMManager. (cherry picked from commit a81925ad324ca92655db17b80d04ed65df56fc0b) --- src/core/nm-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 54be060774..bd95581613 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -3459,7 +3459,7 @@ _device_realize_finish(NMManager *self, NMDevice *device, const NMPlatformLink * nm_device_state_changed(device, NM_DEVICE_STATE_UNAVAILABLE, NM_DEVICE_STATE_REASON_NOW_MANAGED); - nm_device_recheck_auto_activate_schedule(device); + nm_manager_device_recheck_auto_activate_schedule(self, device); } /** From 35946a9fb6b08e0846a89e082ed396e93133f227 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Apr 2023 12:07:10 +0200 Subject: [PATCH 09/13] core: add nm_manager_get_policy() accessor NMPolicy really should be merged into NMManager. It has not a clear responsiblity so that there are two separate objects only makes things confusing. Anyway. It is permissible to look up the NMPolicy instance of a NMManager. Add an accessor. (cherry picked from commit 520fcc8667e9939275f51da355fceb964815c42b) --- src/core/nm-manager.c | 8 ++++++++ src/core/nm-manager.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index bd95581613..c0af846c1f 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -7973,6 +7973,14 @@ nm_settings_get(void) return NM_MANAGER_GET_PRIVATE(singleton_instance)->settings; } +NMPolicy * +nm_manager_get_policy(NMManager *self) +{ + g_return_val_if_fail(NM_IS_MANAGER(self), NULL); + + return NM_MANAGER_GET_PRIVATE(self)->policy; +} + NMManager * nm_manager_setup(void) { diff --git a/src/core/nm-manager.h b/src/core/nm-manager.h index d4d3197aeb..1bf8ded3af 100644 --- a/src/core/nm-manager.h +++ b/src/core/nm-manager.h @@ -69,6 +69,8 @@ NMManager *nm_manager_setup(void); NMManager *nm_manager_get(void); #define NM_MANAGER_GET (nm_manager_get()) +NMPolicy *nm_manager_get_policy(NMManager *self); + gboolean nm_manager_start(NMManager *manager, GError **error); void nm_manager_stop(NMManager *manager); NMState nm_manager_get_state(NMManager *manager); From 5fddc6d72308e719f6cbf3c8fb3064f97e7b5cce Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 5 Apr 2023 11:55:28 +0200 Subject: [PATCH 10/13] core: rework tracking of auto-activating devices in NMPolicy Hook the information for tracking the activation of a device, to the NMDevice itself. Sure, that slightly couples the NMPolicy closer to NMDevice, but the result is still simpler code because we don't need a separate ActivateData. It also means we can immediately tell whether the auto activation check for NMDevice is already scheduled and don't need to search through the list. (cherry picked from commit a22e5080a0a67a76fd9fa17f69491267fbe89f72) --- src/core/devices/nm-device.c | 3 ++ src/core/devices/nm-device.h | 3 ++ src/core/nm-policy.c | 94 +++++++++++++++--------------------- 3 files changed, 44 insertions(+), 56 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index d73f9ac890..e065eb63b5 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -17771,6 +17771,7 @@ nm_device_init(NMDevice *self) c_list_init(&priv->concheck_lst_head); c_list_init(&self->devices_lst); + c_list_init(&self->policy_auto_activate_lst); c_list_init(&priv->slaves); priv->ipdhcp_data_6.v6.mode = NM_NDISC_DHCP_LEVEL_NONE; @@ -17891,6 +17892,8 @@ dispose(GObject *object) _LOGD(LOGD_DEVICE, "disposing"); nm_assert(c_list_is_empty(&self->devices_lst)); + nm_assert(c_list_is_empty(&self->policy_auto_activate_lst)); + nm_assert(!self->policy_auto_activate_idle_source); while ((con_handle = c_list_first_entry(&priv->concheck_lst_head, NMDeviceConnectivityHandle, diff --git a/src/core/devices/nm-device.h b/src/core/devices/nm-device.h index 39151a20ac..ac843a39cb 100644 --- a/src/core/devices/nm-device.h +++ b/src/core/devices/nm-device.h @@ -143,6 +143,9 @@ struct _NMDevice { NMDBusObject parent; struct _NMDevicePrivate *_priv; CList devices_lst; + + CList policy_auto_activate_lst; + GSource *policy_auto_activate_idle_source; }; /* The flags have an relaxing meaning, that means, specifying more flags, can make diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index 786584b4d8..c342fcc53d 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -51,7 +51,7 @@ typedef struct { NMManager *manager; NMNetns *netns; NMFirewalldManager *firewalld_manager; - CList pending_activation_checks; + CList policy_auto_activate_lst_head; NMAgentManager *agent_mgr; @@ -1282,23 +1282,6 @@ check_activating_active_connections(NMPolicy *self) g_object_thaw_notify(G_OBJECT(self)); } -typedef struct { - CList pending_lst; - NMPolicy *policy; - NMDevice *device; - guint autoactivate_id; -} ActivateData; - -static void -activate_data_free(ActivateData *data) -{ - nm_device_remove_pending_action(data->device, NM_PENDING_ACTION_AUTOACTIVATE, TRUE); - c_list_unlink_stale(&data->pending_lst); - nm_clear_g_source(&data->autoactivate_id); - g_object_unref(data->device); - g_slice_free(ActivateData, data); -} - static void pending_ac_gone(gpointer data, GObject *where_the_object_was) { @@ -1345,7 +1328,7 @@ pending_ac_state_changed(NMActiveConnection *ac, guint state, guint reason, NMPo } static void -auto_activate_device(NMPolicy *self, NMDevice *device) +_auto_activate_device(NMPolicy *self, NMDevice *device) { NMPolicyPrivate *priv; NMSettingsConnection *best_connection; @@ -1456,32 +1439,34 @@ auto_activate_device(NMPolicy *self, NMDevice *device) } } -static gboolean -auto_activate_device_cb(gpointer user_data) +static void +_auto_activate_device_clear(NMPolicy *self, NMDevice *device, gboolean do_activate) { - ActivateData *data = user_data; + nm_assert(NM_IS_DEVICE(device)); + nm_assert(NM_IS_POLICY(self)); + nm_assert(c_list_is_linked(&device->policy_auto_activate_lst)); + nm_assert(c_list_contains(&NM_POLICY_GET_PRIVATE(self)->policy_auto_activate_lst_head, + &device->policy_auto_activate_lst)); - g_assert(data); - g_assert(NM_IS_POLICY(data->policy)); - g_assert(NM_IS_DEVICE(data->device)); + c_list_unlink(&device->policy_auto_activate_lst); + nm_clear_g_source_inst(&device->policy_auto_activate_idle_source); - data->autoactivate_id = 0; - auto_activate_device(data->policy, data->device); - activate_data_free(data); - return G_SOURCE_REMOVE; + if (do_activate) + _auto_activate_device(self, device); + + nm_device_remove_pending_action(device, NM_PENDING_ACTION_AUTOACTIVATE, TRUE); + g_object_unref(device); } -static ActivateData * -find_pending_activation(NMPolicy *self, NMDevice *device) +static gboolean +_auto_activate_idle_cb(gpointer user_data) { - NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); - ActivateData *data; + NMDevice *device = user_data; - c_list_for_each_entry (data, &priv->pending_activation_checks, pending_lst) { - if (data->device == device) - return data; - } - return NULL; + nm_assert(NM_IS_DEVICE(device)); + + _auto_activate_device_clear(nm_manager_get_policy(nm_device_get_manager(device)), device, TRUE); + return G_SOURCE_CONTINUE; } /*****************************************************************************/ @@ -1687,13 +1672,17 @@ void nm_policy_device_recheck_auto_activate_schedule(NMPolicy *self, NMDevice *device) { NMPolicyPrivate *priv; - ActivateData *data; NMActiveConnection *ac; const CList *tmp_list; g_return_if_fail(NM_IS_POLICY(self)); g_return_if_fail(NM_IS_DEVICE(device)); + if (!c_list_is_empty(&device->policy_auto_activate_lst)) { + /* already queued. Return. */ + return; + } + priv = NM_POLICY_GET_PRIVATE(self); if (nm_manager_get_state(priv->manager) == NM_STATE_ASLEEP) @@ -1702,9 +1691,6 @@ nm_policy_device_recheck_auto_activate_schedule(NMPolicy *self, NMDevice *device if (!nm_device_autoconnect_allowed(device)) return; - if (find_pending_activation(self, device)) - return; - nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { if (nm_active_connection_get_device(ac) == device) { if (nm_device_sys_iface_state_is_external(device) @@ -1717,11 +1703,9 @@ nm_policy_device_recheck_auto_activate_schedule(NMPolicy *self, NMDevice *device nm_device_add_pending_action(device, NM_PENDING_ACTION_AUTOACTIVATE, TRUE); - data = g_slice_new0(ActivateData); - data->policy = self; - data->device = g_object_ref(device); - data->autoactivate_id = g_idle_add(auto_activate_device_cb, data); - c_list_link_tail(&priv->pending_activation_checks, &data->pending_lst); + c_list_link_tail(&priv->policy_auto_activate_lst_head, &device->policy_auto_activate_lst); + device->policy_auto_activate_idle_source = nm_g_idle_add_source(_auto_activate_idle_cb, device); + g_object_ref(device); } static gboolean @@ -2312,16 +2296,13 @@ device_removed(NMManager *manager, NMDevice *device, gpointer user_data) { NMPolicyPrivate *priv = user_data; NMPolicy *self = _PRIV_TO_SELF(priv); - ActivateData *data; /* TODO: is this needed? The delegations are cleaned up * on transition to deactivated too. */ ip6_remove_device_prefix_delegations(self, device); - /* Clear any idle callbacks for this device */ - data = find_pending_activation(self, device); - if (data && data->autoactivate_id) - activate_data_free(data); + if (c_list_is_linked(&device->policy_auto_activate_lst)) + _auto_activate_device_clear(self, device, FALSE); if (g_hash_table_remove(priv->devices, device)) devices_list_unregister(self, device); @@ -2762,7 +2743,7 @@ nm_policy_init(NMPolicy *self) NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); gs_free char *hostname_mode = NULL; - c_list_init(&priv->pending_activation_checks); + c_list_init(&priv->policy_auto_activate_lst_head); priv->netns = g_object_ref(nm_netns_get()); @@ -2900,7 +2881,6 @@ dispose(GObject *object) NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); GHashTableIter h_iter; NMDevice *device; - ActivateData *data, *data_safe; nm_clear_g_object(&priv->default_ac4); nm_clear_g_object(&priv->default_ac6); @@ -2908,8 +2888,10 @@ dispose(GObject *object) nm_clear_g_object(&priv->activating_ac6); nm_clear_pointer(&priv->pending_active_connections, g_hash_table_unref); - c_list_for_each_entry_safe (data, data_safe, &priv->pending_activation_checks, pending_lst) - activate_data_free(data); + while ((device = c_list_first_entry(&priv->policy_auto_activate_lst_head, + NMDevice, + policy_auto_activate_lst))) + _auto_activate_device_clear(self, device, FALSE); g_slist_free_full(priv->pending_secondaries, (GDestroyNotify) pending_secondary_data_free); priv->pending_secondaries = NULL; From 3b05cbc33b01231b320f513deb1ed320a41f9b52 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Apr 2023 13:23:59 +0200 Subject: [PATCH 11/13] core: don't take reference on NMDevice to track auto-activate Add an assertion to nm_policy_device_recheck_auto_activate_schedule(), that the device is currently registered in NMPolicy. Calling it outside would be odd, and likely a bug. But if we only register the auto-activate while being registered, we don't need to take an additional reference. We know that the object must be be alive (also, we have assertions that in fact it is still alive). (cherry picked from commit 0dd4724446fdef043fddc9f1e20569cacb78474d) --- src/core/nm-policy.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index c342fcc53d..f7e1e97344 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -1455,7 +1455,6 @@ _auto_activate_device_clear(NMPolicy *self, NMDevice *device, gboolean do_activa _auto_activate_device(self, device); nm_device_remove_pending_action(device, NM_PENDING_ACTION_AUTOACTIVATE, TRUE); - g_object_unref(device); } static gboolean @@ -1677,6 +1676,14 @@ nm_policy_device_recheck_auto_activate_schedule(NMPolicy *self, NMDevice *device g_return_if_fail(NM_IS_POLICY(self)); g_return_if_fail(NM_IS_DEVICE(device)); + nm_assert(g_signal_handler_find(device, + G_SIGNAL_MATCH_DATA, + 0, + 0, + NULL, + NULL, + NM_POLICY_GET_PRIVATE(self)) + != 0); if (!c_list_is_empty(&device->policy_auto_activate_lst)) { /* already queued. Return. */ @@ -1705,7 +1712,6 @@ nm_policy_device_recheck_auto_activate_schedule(NMPolicy *self, NMDevice *device c_list_link_tail(&priv->policy_auto_activate_lst_head, &device->policy_auto_activate_lst); device->policy_auto_activate_idle_source = nm_g_idle_add_source(_auto_activate_idle_cb, device); - g_object_ref(device); } static gboolean From d412eb8aca3c3eec94d8be16d6a7636794312441 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 7 Apr 2023 13:34:34 +0200 Subject: [PATCH 12/13] core: assert that devices are not registered when disposing NMPolicy NMDevice holds a reference to NMManager, which holds a reference to NMPolicy. It is not possible that we try to dispose NMPolicy while there are still devices registered. That would be a bug, that we need to find and solve differently. Add an assertion instead of trying to handle it. (cherry picked from commit aede228974beed3764352e0e31096a70b8d2a49f) --- src/core/nm-policy.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index f7e1e97344..6dcdeb4c66 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -2885,8 +2885,9 @@ dispose(GObject *object) { NMPolicy *self = NM_POLICY(object); NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE(self); - GHashTableIter h_iter; - NMDevice *device; + + nm_assert(!c_list_is_empty(&priv->policy_auto_activate_lst_head)); + nm_assert(g_hash_table_size(priv->devices) == 0); nm_clear_g_object(&priv->default_ac4); nm_clear_g_object(&priv->default_ac6); @@ -2894,11 +2895,6 @@ dispose(GObject *object) nm_clear_g_object(&priv->activating_ac6); nm_clear_pointer(&priv->pending_active_connections, g_hash_table_unref); - while ((device = c_list_first_entry(&priv->policy_auto_activate_lst_head, - NMDevice, - policy_auto_activate_lst))) - _auto_activate_device_clear(self, device, FALSE); - g_slist_free_full(priv->pending_secondaries, (GDestroyNotify) pending_secondary_data_free); priv->pending_secondaries = NULL; @@ -2917,12 +2913,6 @@ dispose(GObject *object) g_clear_object(&priv->dns_manager); } - g_hash_table_iter_init(&h_iter, priv->devices); - while (g_hash_table_iter_next(&h_iter, (gpointer *) &device, NULL)) { - g_hash_table_iter_remove(&h_iter); - devices_list_unregister(self, device); - } - /* The manager should have disposed of ActiveConnections already, which * will have called active_connection_removed() and thus we don't need * to clean anything up. Assert that this is TRUE. From cef0c31b2ca7b22fcdeb0208e84ab04cf3a2a0ef Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 18 Apr 2023 13:24:34 +0200 Subject: [PATCH 13/13] core: simplify tracking of delete_on_deactivate idle action Before commit a42682d44fe2 ('device: take reference to device object before 'delete_on_deactivate''), we used a weak pointer to track the idle action. As we now use a strong reference, we can store all data about the idle action in NMDevice itself. Drop DeleteOnDeactivateData. (cherry picked from commit b48c31432864f4e075f74b2e68710404d8c74049) --- src/core/devices/nm-device.c | 51 +++++++++++------------------------- 1 file changed, 15 insertions(+), 36 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index e065eb63b5..ef00a4b26f 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -131,11 +131,6 @@ typedef struct { bool configure; } SlaveInfo; -typedef struct { - NMDevice *device; - guint idle_add_id; -} DeleteOnDeactivateData; - typedef struct { NMDevice *device; GCancellable *cancellable; @@ -512,8 +507,8 @@ typedef struct _NMDevicePrivate { NMUnmanagedFlags unmanaged_mask; NMUnmanagedFlags unmanaged_flags; - DeleteOnDeactivateData - *delete_on_deactivate_data; /* data for scheduled cleanup when deleting link (g_idle_add) */ + + GSource *delete_on_deactivate_idle_source; GCancellable *deactivating_cancellable; @@ -8334,7 +8329,7 @@ nm_device_autoconnect_allowed(NMDevice *self) return FALSE; } - if (priv->delete_on_deactivate_data) + if (priv->delete_on_deactivate_idle_source) return FALSE; /* The 'autoconnect-allowed' signal is emitted on a device to allow @@ -12613,16 +12608,13 @@ nm_device_is_nm_owned(NMDevice *self) static gboolean delete_on_deactivate_link_delete(gpointer user_data) { - DeleteOnDeactivateData *data = user_data; - nm_auto_unref_object NMDevice *self = data->device; + nm_auto_unref_object NMDevice *self = user_data; NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); gs_free_error GError *error = NULL; - _LOGD(LOGD_DEVICE, - "delete_on_deactivate: cleanup and delete virtual link (id=%u)", - data->idle_add_id); + _LOGD(LOGD_DEVICE, "delete_on_deactivate: cleanup and delete virtual link"); - priv->delete_on_deactivate_data = NULL; + nm_clear_g_source_inst(&priv->delete_on_deactivate_idle_source); if (!nm_device_unrealize(self, TRUE, &error)) _LOGD(LOGD_DEVICE, "delete_on_deactivate: unrealizing failed (%s)", error->message); @@ -12633,8 +12625,7 @@ delete_on_deactivate_link_delete(gpointer user_data) nm_device_recheck_auto_activate_schedule(self); } - g_free(data); - return FALSE; + return G_SOURCE_CONTINUE; } static void @@ -12642,25 +12633,16 @@ delete_on_deactivate_unschedule(NMDevice *self) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - if (priv->delete_on_deactivate_data) { - DeleteOnDeactivateData *data = priv->delete_on_deactivate_data; - - priv->delete_on_deactivate_data = NULL; - - g_source_remove(data->idle_add_id); - _LOGD(LOGD_DEVICE, - "delete_on_deactivate: cancel cleanup and delete virtual link (id=%u)", - data->idle_add_id); - g_object_unref(data->device); - g_free(data); + if (nm_clear_g_source_inst(&priv->delete_on_deactivate_idle_source)) { + _LOGD(LOGD_DEVICE, "delete_on_deactivate: cancel cleanup and delete virtual link"); + g_object_unref(self); } } static void delete_on_deactivate_check_and_schedule(NMDevice *self) { - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - DeleteOnDeactivateData *data; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); if (!priv->nm_owned) return; @@ -12674,14 +12656,11 @@ delete_on_deactivate_check_and_schedule(NMDevice *self) return; delete_on_deactivate_unschedule(self); /* always cancel and reschedule */ - data = g_new(DeleteOnDeactivateData, 1); - data->device = g_object_ref(self); - data->idle_add_id = g_idle_add(delete_on_deactivate_link_delete, data); - priv->delete_on_deactivate_data = data; + g_object_ref(self); + priv->delete_on_deactivate_idle_source = + nm_g_idle_add_source(delete_on_deactivate_link_delete, self); - _LOGD(LOGD_DEVICE, - "delete_on_deactivate: schedule cleanup and delete virtual link (id=%u)", - data->idle_add_id); + _LOGD(LOGD_DEVICE, "delete_on_deactivate: schedule cleanup and delete virtual link"); } static void