From 616976d6a891c28bbdf8a6764ce1e641d75b43c8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 16:24:40 +0100 Subject: [PATCH 01/34] core: refactor NMSettingsConnectionCallId typedef not to be a pointer to struct Typedefs to structs are fine, but a typedef for a pointer seems confusing to me. Let's avoid it. --- src/nm-act-request.c | 6 +++--- src/settings/nm-settings-connection.c | 6 +++--- src/settings/nm-settings-connection.h | 22 +++++++++++----------- src/vpn/nm-vpn-connection.c | 4 ++-- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/nm-act-request.c b/src/nm-act-request.c index 25a1dd9b4f..29bb3a05bb 100644 --- a/src/nm-act-request.c +++ b/src/nm-act-request.c @@ -96,7 +96,7 @@ struct _NMActRequestGetSecretsCallId { NMActRequest *self; NMActRequestSecretsFunc callback; gpointer callback_data; - NMSettingsConnectionCallId call_id; + NMSettingsConnectionCallId *call_id; bool has_ref; }; @@ -113,7 +113,7 @@ _get_secrets_call_id_free (NMActRequestGetSecretsCallId *call_id) static void get_secrets_cb (NMSettingsConnection *connection, - NMSettingsConnectionCallId call_id_s, + NMSettingsConnectionCallId *call_id_s, const char *agent_username, const char *setting_name, GError *error, @@ -172,7 +172,7 @@ nm_act_request_get_secrets (NMActRequest *self, { NMActRequestPrivate *priv; NMActRequestGetSecretsCallId *call_id; - NMSettingsConnectionCallId call_id_s; + NMSettingsConnectionCallId *call_id_s; NMSettingsConnection *settings_connection; NMConnection *applied_connection; const char *hints[2] = { hint, NULL }; diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 1185aef905..77b10aae6e 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -1222,7 +1222,7 @@ get_secrets_idle_cb (GetSecretsInfo *info) * * Returns: a call ID which may be used to cancel the ongoing secrets request. **/ -NMSettingsConnectionCallId +NMSettingsConnectionCallId * nm_settings_connection_get_secrets (NMSettingsConnection *self, NMConnection *applied_connection, NMAuthSubject *subject, @@ -1337,7 +1337,7 @@ _get_secrets_cancel (NMSettingsConnection *self, void nm_settings_connection_cancel_secrets (NMSettingsConnection *self, - NMSettingsConnectionCallId call_id) + NMSettingsConnectionCallId *call_id) { _LOGD ("(%p) secrets canceled", call_id); @@ -1951,7 +1951,7 @@ out_err: static void dbus_get_agent_secrets_cb (NMSettingsConnection *self, - NMSettingsConnectionCallId call_id, + NMSettingsConnectionCallId *call_id, const char *agent_username, const char *setting_name, GError *error, diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index 7a9e383259..aa79628f7f 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -90,7 +90,7 @@ typedef enum { } NMSettingsAutoconnectBlockedReason; struct _NMSettingsConnectionCallId; -typedef struct _NMSettingsConnectionCallId *NMSettingsConnectionCallId; +typedef struct _NMSettingsConnectionCallId NMSettingsConnectionCallId; typedef struct _NMSettingsConnectionClass NMSettingsConnectionClass; @@ -150,7 +150,7 @@ gboolean nm_settings_connection_delete (NMSettingsConnection *self, GError **error); typedef void (*NMSettingsConnectionSecretsFunc) (NMSettingsConnection *self, - NMSettingsConnectionCallId call_id, + NMSettingsConnectionCallId *call_id, const char *agent_username, const char *setting_name, GError *error, @@ -162,17 +162,17 @@ gboolean nm_settings_connection_new_secrets (NMSettingsConnection *self, GVariant *secrets, GError **error); -NMSettingsConnectionCallId nm_settings_connection_get_secrets (NMSettingsConnection *self, - NMConnection *applied_connection, - NMAuthSubject *subject, - const char *setting_name, - NMSecretAgentGetSecretsFlags flags, - const char *const*hints, - NMSettingsConnectionSecretsFunc callback, - gpointer callback_data); +NMSettingsConnectionCallId *nm_settings_connection_get_secrets (NMSettingsConnection *self, + NMConnection *applied_connection, + NMAuthSubject *subject, + const char *setting_name, + NMSecretAgentGetSecretsFlags flags, + const char *const*hints, + NMSettingsConnectionSecretsFunc callback, + gpointer callback_data); void nm_settings_connection_cancel_secrets (NMSettingsConnection *self, - NMSettingsConnectionCallId call_id); + NMSettingsConnectionCallId *call_id); gboolean nm_settings_connection_is_visible (NMSettingsConnection *self); diff --git a/src/vpn/nm-vpn-connection.c b/src/vpn/nm-vpn-connection.c index e441bbd051..6c600c46a6 100644 --- a/src/vpn/nm-vpn-connection.c +++ b/src/vpn/nm-vpn-connection.c @@ -103,7 +103,7 @@ typedef struct { gboolean service_can_persist; gboolean connection_can_persist; - NMSettingsConnectionCallId secrets_id; + NMSettingsConnectionCallId *secrets_id; SecretsReq secrets_idx; char *username; @@ -2571,7 +2571,7 @@ plugin_new_secrets_cb (GDBusProxy *proxy, GAsyncResult *result, gpointer user_da static void get_secrets_cb (NMSettingsConnection *connection, - NMSettingsConnectionCallId call_id, + NMSettingsConnectionCallId *call_id, const char *agent_username, const char *setting_name, GError *error, From fc918049de2159a4a9cbbd923087eb30bd6d9229 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 16:24:40 +0100 Subject: [PATCH 02/34] core: drop internal typedef GetSecretsInfo for NMSettingsConnectionCallId Using an internal alias for the type is just confusing. Drop it. --- src/settings/nm-settings-connection.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 77b10aae6e..905531f25b 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -818,17 +818,15 @@ struct _NMSettingsConnectionCallId { } t; }; -typedef struct _NMSettingsConnectionCallId GetSecretsInfo; - -static GetSecretsInfo * +static NMSettingsConnectionCallId * _get_secrets_info_new (NMSettingsConnection *self, NMConnection *applied_connection, NMSettingsConnectionSecretsFunc callback, gpointer callback_data) { - GetSecretsInfo *info; + NMSettingsConnectionCallId *info; - info = g_slice_new0 (GetSecretsInfo); + info = g_slice_new0 (NMSettingsConnectionCallId); info->self = self; if (applied_connection) { @@ -843,7 +841,7 @@ _get_secrets_info_new (NMSettingsConnection *self, } static void -_get_secrets_info_callback (GetSecretsInfo *info, +_get_secrets_info_callback (NMSettingsConnectionCallId *info, const char *agent_username, const char *setting_name, GError *error) @@ -859,7 +857,7 @@ _get_secrets_info_callback (GetSecretsInfo *info, } static void -_get_secrets_info_free (GetSecretsInfo *info) +_get_secrets_info_free (NMSettingsConnectionCallId *info) { g_return_if_fail (info && info->self); @@ -870,7 +868,7 @@ _get_secrets_info_free (GetSecretsInfo *info) g_clear_error (&info->t.idle.error); memset (info, 0, sizeof (*info)); - g_slice_free (GetSecretsInfo, info); + g_slice_free (NMSettingsConnectionCallId, info); } static gboolean @@ -907,7 +905,7 @@ secret_is_system_owned (NMSettingSecretFlags flags, static void get_cmp_flags (NMSettingsConnection *self, /* only needed for logging */ - GetSecretsInfo *info, /* only needed for logging */ + NMSettingsConnectionCallId *info, /* only needed for logging */ NMConnection *connection, const char *agent_dbus_owner, gboolean agent_has_modify, @@ -1023,7 +1021,7 @@ get_secrets_done_cb (NMAgentManager *manager, GError *error, gpointer user_data) { - GetSecretsInfo *info = user_data; + NMSettingsConnectionCallId *info = user_data; NMSettingsConnection *self; NMSettingsConnectionPrivate *priv; NMConnection *applied_connection; @@ -1179,7 +1177,7 @@ out: } static gboolean -get_secrets_idle_cb (GetSecretsInfo *info) +get_secrets_idle_cb (NMSettingsConnectionCallId *info) { NMSettingsConnectionPrivate *priv; @@ -1236,7 +1234,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, GVariant *existing_secrets; NMAgentManagerCallId call_id_a; gs_free char *joined_hints = NULL; - GetSecretsInfo *info; + NMSettingsConnectionCallId *info; GError *local = NULL; g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (self), NULL); @@ -1312,7 +1310,7 @@ schedule_dummy: static void _get_secrets_cancel (NMSettingsConnection *self, - GetSecretsInfo *info, + NMSettingsConnectionCallId *info, gboolean is_disposing) { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); @@ -2777,7 +2775,7 @@ dispose (GObject *object) /* Cancel in-progress secrets requests */ if (priv->agent_mgr) { while (priv->get_secret_requests) { - GetSecretsInfo *info = priv->get_secret_requests->data; + NMSettingsConnectionCallId *info = priv->get_secret_requests->data; _get_secrets_cancel (self, info, TRUE); g_return_if_fail (!priv->get_secret_requests || (info != priv->get_secret_requests->data)); From 4e11be5ecf91d4b4d943b1a01c622306e37bb70e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 16:34:27 +0100 Subject: [PATCH 03/34] core/trivial: unify names of internal NMSettingsConnectionCallId as "call_id" --- src/settings/nm-settings-connection.c | 184 +++++++++++++------------- 1 file changed, 92 insertions(+), 92 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 905531f25b..fa526c0167 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -795,9 +795,9 @@ nm_settings_connection_delete (NMSettingsConnection *self, typedef enum { - GET_SECRETS_INFO_TYPE_REQ, - GET_SECRETS_INFO_TYPE_IDLE, -} GetSecretsInfoType; + CALL_ID_TYPE_REQ, + CALL_ID_TYPE_IDLE, +} CallIdType; struct _NMSettingsConnectionCallId { NMSettingsConnection *self; @@ -806,7 +806,7 @@ struct _NMSettingsConnectionCallId { NMSettingsConnectionSecretsFunc callback; gpointer callback_data; - GetSecretsInfoType type; + CallIdType type; union { struct { NMAgentManagerCallId id; @@ -819,56 +819,56 @@ struct _NMSettingsConnectionCallId { }; static NMSettingsConnectionCallId * -_get_secrets_info_new (NMSettingsConnection *self, - NMConnection *applied_connection, - NMSettingsConnectionSecretsFunc callback, - gpointer callback_data) +_call_id_new (NMSettingsConnection *self, + NMConnection *applied_connection, + NMSettingsConnectionSecretsFunc callback, + gpointer callback_data) { - NMSettingsConnectionCallId *info; + NMSettingsConnectionCallId *call_id; - info = g_slice_new0 (NMSettingsConnectionCallId); + call_id = g_slice_new0 (NMSettingsConnectionCallId); - info->self = self; + call_id->self = self; if (applied_connection) { - info->had_applied_connection = TRUE; - info->applied_connection = applied_connection; - g_object_add_weak_pointer (G_OBJECT (applied_connection), (gpointer *) &info->applied_connection); + call_id->had_applied_connection = TRUE; + call_id->applied_connection = applied_connection; + g_object_add_weak_pointer (G_OBJECT (applied_connection), (gpointer *) &call_id->applied_connection); } - info->callback = callback; - info->callback_data = callback_data; + call_id->callback = callback; + call_id->callback_data = callback_data; - return info; + return call_id; } static void -_get_secrets_info_callback (NMSettingsConnectionCallId *info, +_get_secrets_info_callback (NMSettingsConnectionCallId *call_id, const char *agent_username, const char *setting_name, GError *error) { - if (info->callback) { - info->callback (info->self, - info, - agent_username, - setting_name, - error, - info->callback_data); + if (call_id->callback) { + call_id->callback (call_id->self, + call_id, + agent_username, + setting_name, + error, + call_id->callback_data); } } static void -_get_secrets_info_free (NMSettingsConnectionCallId *info) +_get_secrets_info_free (NMSettingsConnectionCallId *call_id) { - g_return_if_fail (info && info->self); + g_return_if_fail (call_id && call_id->self); - if (info->applied_connection) - g_object_remove_weak_pointer (G_OBJECT (info->applied_connection), (gpointer *) &info->applied_connection); + if (call_id->applied_connection) + g_object_remove_weak_pointer (G_OBJECT (call_id->applied_connection), (gpointer *) &call_id->applied_connection); - if (info->type == GET_SECRETS_INFO_TYPE_IDLE) - g_clear_error (&info->t.idle.error); + if (call_id->type == CALL_ID_TYPE_IDLE) + g_clear_error (&call_id->t.idle.error); - memset (info, 0, sizeof (*info)); - g_slice_free (NMSettingsConnectionCallId, info); + memset (call_id, 0, sizeof (*call_id)); + g_slice_free (NMSettingsConnectionCallId, call_id); } static gboolean @@ -905,7 +905,7 @@ secret_is_system_owned (NMSettingSecretFlags flags, static void get_cmp_flags (NMSettingsConnection *self, /* only needed for logging */ - NMSettingsConnectionCallId *info, /* only needed for logging */ + NMSettingsConnectionCallId *call_id, /* only needed for logging */ NMConnection *connection, const char *agent_dbus_owner, gboolean agent_has_modify, @@ -928,7 +928,7 @@ get_cmp_flags (NMSettingsConnection *self, /* only needed for logging */ if (is_self) { _LOGD ("(%s:%p) secrets returned from agent %s", setting_name, - info, + call_id, agent_dbus_owner); } @@ -947,7 +947,7 @@ get_cmp_flags (NMSettingsConnection *self, /* only needed for logging */ if (is_self) { _LOGD ("(%s:%p) interaction forbidden but agent %s returned system secrets", setting_name, - info, + call_id, agent_dbus_owner); } @@ -959,7 +959,7 @@ get_cmp_flags (NMSettingsConnection *self, /* only needed for logging */ if (is_self) { _LOGD ("(%s:%p) agent failed to authenticate but provided system secrets", setting_name, - info); + call_id); } cmp_flags->required |= NM_SETTING_SECRET_FLAG_AGENT_OWNED; @@ -969,7 +969,7 @@ get_cmp_flags (NMSettingsConnection *self, /* only needed for logging */ if (is_self) { _LOGD ("(%s:%p) existing secrets returned", setting_name, - info); + call_id); } } @@ -1021,7 +1021,7 @@ get_secrets_done_cb (NMAgentManager *manager, GError *error, gpointer user_data) { - NMSettingsConnectionCallId *info = user_data; + NMSettingsConnectionCallId *call_id = user_data; NMSettingsConnection *self; NMSettingsConnectionPrivate *priv; NMConnection *applied_connection; @@ -1033,36 +1033,36 @@ get_secrets_done_cb (NMAgentManager *manager, if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) return; - self = info->self; + self = call_id->self; g_return_if_fail (NM_IS_SETTINGS_CONNECTION (self)); priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); - g_return_if_fail (g_slist_find (priv->get_secret_requests, info)); + g_return_if_fail (g_slist_find (priv->get_secret_requests, call_id)); - priv->get_secret_requests = g_slist_remove (priv->get_secret_requests, info); + priv->get_secret_requests = g_slist_remove (priv->get_secret_requests, call_id); if (error) { _LOGD ("(%s:%p) secrets request error: %s", - setting_name, info, error->message); + setting_name, call_id, error->message); - _get_secrets_info_callback (info, NULL, setting_name, error); + _get_secrets_info_callback (call_id, NULL, setting_name, error); goto out; } - if ( info->had_applied_connection - && !info->applied_connection) { + if ( call_id->had_applied_connection + && !call_id->applied_connection) { g_set_error_literal (&local, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND, "Applied connection deleted since requesting secrets"); - _get_secrets_info_callback (info, NULL, setting_name, local); + _get_secrets_info_callback (call_id, NULL, setting_name, local); goto out; } - if ( info->had_applied_connection - && !nm_settings_connection_has_unmodified_applied_connection (self, info->applied_connection, NM_SETTING_COMPARE_FLAG_NONE)) { + if ( call_id->had_applied_connection + && !nm_settings_connection_has_unmodified_applied_connection (self, call_id->applied_connection, NM_SETTING_COMPARE_FLAG_NONE)) { g_set_error_literal (&local, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "The connection was modified since activation"); - _get_secrets_info_callback (info, NULL, setting_name, local); + _get_secrets_info_callback (call_id, NULL, setting_name, local); goto out; } @@ -1070,12 +1070,12 @@ get_secrets_done_cb (NMAgentManager *manager, g_set_error (&local, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_SETTING_NOT_FOUND, "Connection didn't have requested setting '%s'.", setting_name); - _get_secrets_info_callback (info, NULL, setting_name, local); + _get_secrets_info_callback (call_id, NULL, setting_name, local); goto out; } get_cmp_flags (self, - info, + call_id, NM_CONNECTION (self), agent_dbus_owner, agent_has_modify, @@ -1087,7 +1087,7 @@ get_secrets_done_cb (NMAgentManager *manager, _LOGD ("(%s:%p) secrets request completed", setting_name, - info); + call_id); dict = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); @@ -1117,7 +1117,7 @@ get_secrets_done_cb (NMAgentManager *manager, if (agent_had_system) { _LOGD ("(%s:%p) saving new secrets to backing storage", setting_name, - info); + call_id); nm_settings_connection_commit_changes (self, NULL, @@ -1126,27 +1126,27 @@ get_secrets_done_cb (NMAgentManager *manager, } else { _LOGD ("(%s:%p) new agent secrets processed", setting_name, - info); + call_id); } } else { _LOGD ("(%s:%p) failed to update with agent secrets: %s", setting_name, - info, + call_id, local->message); } g_variant_unref (filtered_secrets); } else { _LOGD ("(%s:%p) failed to update with existing secrets: %s", setting_name, - info, + call_id, local->message); } - applied_connection = info->applied_connection; + applied_connection = call_id->applied_connection; if (applied_connection) { get_cmp_flags (self, - info, + call_id, applied_connection, agent_dbus_owner, agent_has_modify, @@ -1167,31 +1167,31 @@ get_secrets_done_cb (NMAgentManager *manager, } } - _get_secrets_info_callback (info, agent_username, setting_name, local); + _get_secrets_info_callback (call_id, agent_username, setting_name, local); g_clear_error (&local); if (dict) g_variant_unref (dict); out: - _get_secrets_info_free (info); + _get_secrets_info_free (call_id); } static gboolean -get_secrets_idle_cb (NMSettingsConnectionCallId *info) +get_secrets_idle_cb (NMSettingsConnectionCallId *call_id) { NMSettingsConnectionPrivate *priv; - g_return_val_if_fail (info && NM_IS_SETTINGS_CONNECTION (info->self), G_SOURCE_REMOVE); + g_return_val_if_fail (call_id && NM_IS_SETTINGS_CONNECTION (call_id->self), G_SOURCE_REMOVE); - priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (info->self); + priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (call_id->self); - g_return_val_if_fail (g_slist_find (priv->get_secret_requests, info), G_SOURCE_REMOVE); + g_return_val_if_fail (g_slist_find (priv->get_secret_requests, call_id), G_SOURCE_REMOVE); - priv->get_secret_requests = g_slist_remove (priv->get_secret_requests, info); + priv->get_secret_requests = g_slist_remove (priv->get_secret_requests, call_id); - _get_secrets_info_callback (info, NULL, NULL, info->t.idle.error); + _get_secrets_info_callback (call_id, NULL, NULL, call_id->t.idle.error); - _get_secrets_info_free (info); + _get_secrets_info_free (call_id); return G_SOURCE_REMOVE; } @@ -1234,7 +1234,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, GVariant *existing_secrets; NMAgentManagerCallId call_id_a; gs_free char *joined_hints = NULL; - NMSettingsConnectionCallId *info; + NMSettingsConnectionCallId *call_id; GError *local = NULL; g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (self), NULL); @@ -1242,12 +1242,12 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, || ( NM_IS_CONNECTION (applied_connection) && (((NMConnection *) self) != applied_connection)), NULL); - info = _get_secrets_info_new (self, - applied_connection, - callback, - callback_data); + call_id = _call_id_new (self, + applied_connection, + callback, + callback_data); - priv->get_secret_requests = g_slist_append (priv->get_secret_requests, info); + priv->get_secret_requests = g_slist_append (priv->get_secret_requests, call_id); /* Use priv->secrets to work around the fact that nm_connection_clear_secrets() * will clear secrets on this object's settings. @@ -1285,7 +1285,7 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, flags, hints, get_secrets_done_cb, - info); + call_id); g_assert (call_id_a); if (existing_secrets) g_variant_unref (existing_secrets); @@ -1297,40 +1297,40 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, (hints && hints[0]) ? (joined_hints = g_strjoinv (",", (char **) hints)) : "(none)"); if (call_id_a) { - info->type = GET_SECRETS_INFO_TYPE_REQ; - info->t.req.id = call_id_a; + call_id->type = CALL_ID_TYPE_REQ; + call_id->t.req.id = call_id_a; } else { schedule_dummy: - info->type = GET_SECRETS_INFO_TYPE_IDLE; - g_propagate_error (&info->t.idle.error, local); - info->t.idle.id = g_idle_add ((GSourceFunc) get_secrets_idle_cb, info); + call_id->type = CALL_ID_TYPE_IDLE; + g_propagate_error (&call_id->t.idle.error, local); + call_id->t.idle.id = g_idle_add ((GSourceFunc) get_secrets_idle_cb, call_id); } - return info; + return call_id; } static void _get_secrets_cancel (NMSettingsConnection *self, - NMSettingsConnectionCallId *info, + NMSettingsConnectionCallId *call_id, gboolean is_disposing) { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); gs_free_error GError *error = NULL; - if (!g_slist_find (priv->get_secret_requests, info)) + if (!g_slist_find (priv->get_secret_requests, call_id)) g_return_if_reached (); - priv->get_secret_requests = g_slist_remove (priv->get_secret_requests, info); + priv->get_secret_requests = g_slist_remove (priv->get_secret_requests, call_id); - if (info->type == GET_SECRETS_INFO_TYPE_REQ) - nm_agent_manager_cancel_secrets (priv->agent_mgr, info->t.req.id); + if (call_id->type == CALL_ID_TYPE_REQ) + nm_agent_manager_cancel_secrets (priv->agent_mgr, call_id->t.req.id); else - g_source_remove (info->t.idle.id); + g_source_remove (call_id->t.idle.id); nm_utils_error_set_cancelled (&error, is_disposing, "NMSettingsConnection"); - _get_secrets_info_callback (info, NULL, NULL, error); + _get_secrets_info_callback (call_id, NULL, NULL, error); - _get_secrets_info_free (info); + _get_secrets_info_free (call_id); } void @@ -2775,10 +2775,10 @@ dispose (GObject *object) /* Cancel in-progress secrets requests */ if (priv->agent_mgr) { while (priv->get_secret_requests) { - NMSettingsConnectionCallId *info = priv->get_secret_requests->data; + NMSettingsConnectionCallId *call_id = priv->get_secret_requests->data; - _get_secrets_cancel (self, info, TRUE); - g_return_if_fail (!priv->get_secret_requests || (info != priv->get_secret_requests->data)); + _get_secrets_cancel (self, call_id, TRUE); + g_return_if_fail (!priv->get_secret_requests || (call_id != priv->get_secret_requests->data)); } } From 310973bb642d615502bab588a8234b9791878256 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 17:48:07 +0100 Subject: [PATCH 04/34] core: use CList for call-ids in NMSettingsConnection --- src/settings/nm-settings-connection.c | 64 ++++++++++----------------- 1 file changed, 23 insertions(+), 41 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index fa526c0167..538e90ee97 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -25,6 +25,8 @@ #include +#include "nm-utils/c-list.h" + #include "nm-common-macros.h" #include "nm-config.h" #include "nm-config-data.h" @@ -88,7 +90,7 @@ typedef struct _NMSettingsConnectionPrivate { GSList *pending_auths; /* List of pending authentication requests */ - GSList *get_secret_requests; /* in-progress secrets requests */ + CList call_ids_lst_head; /* in-progress secrets requests */ /* Caches secrets from on-disk connections; were they not cached any * call to nm_connection_clear_secrets() wipes them out and we'd have @@ -801,6 +803,7 @@ typedef enum { struct _NMSettingsConnectionCallId { NMSettingsConnection *self; + CList call_ids_lst; gboolean had_applied_connection; NMConnection *applied_connection; NMSettingsConnectionSecretsFunc callback; @@ -818,28 +821,6 @@ struct _NMSettingsConnectionCallId { } t; }; -static NMSettingsConnectionCallId * -_call_id_new (NMSettingsConnection *self, - NMConnection *applied_connection, - NMSettingsConnectionSecretsFunc callback, - gpointer callback_data) -{ - NMSettingsConnectionCallId *call_id; - - call_id = g_slice_new0 (NMSettingsConnectionCallId); - - call_id->self = self; - if (applied_connection) { - call_id->had_applied_connection = TRUE; - call_id->applied_connection = applied_connection; - g_object_add_weak_pointer (G_OBJECT (applied_connection), (gpointer *) &call_id->applied_connection); - } - call_id->callback = callback; - call_id->callback_data = callback_data; - - return call_id; -} - static void _get_secrets_info_callback (NMSettingsConnectionCallId *call_id, const char *agent_username, @@ -860,6 +841,7 @@ static void _get_secrets_info_free (NMSettingsConnectionCallId *call_id) { g_return_if_fail (call_id && call_id->self); + nm_assert (!c_list_is_linked (&call_id->call_ids_lst)); if (call_id->applied_connection) g_object_remove_weak_pointer (G_OBJECT (call_id->applied_connection), (gpointer *) &call_id->applied_connection); @@ -1038,9 +1020,9 @@ get_secrets_done_cb (NMAgentManager *manager, priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); - g_return_if_fail (g_slist_find (priv->get_secret_requests, call_id)); + nm_assert (c_list_contains (&priv->call_ids_lst_head, &call_id->call_ids_lst)); - priv->get_secret_requests = g_slist_remove (priv->get_secret_requests, call_id); + c_list_unlink_init (&call_id->call_ids_lst); if (error) { _LOGD ("(%s:%p) secrets request error: %s", @@ -1185,9 +1167,9 @@ get_secrets_idle_cb (NMSettingsConnectionCallId *call_id) priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (call_id->self); - g_return_val_if_fail (g_slist_find (priv->get_secret_requests, call_id), G_SOURCE_REMOVE); + nm_assert (c_list_contains (&priv->call_ids_lst_head, &call_id->call_ids_lst)); - priv->get_secret_requests = g_slist_remove (priv->get_secret_requests, call_id); + c_list_unlink_init (&call_id->call_ids_lst); _get_secrets_info_callback (call_id, NULL, NULL, call_id->t.idle.error); @@ -1242,12 +1224,16 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, || ( NM_IS_CONNECTION (applied_connection) && (((NMConnection *) self) != applied_connection)), NULL); - call_id = _call_id_new (self, - applied_connection, - callback, - callback_data); - - priv->get_secret_requests = g_slist_append (priv->get_secret_requests, call_id); + call_id = g_slice_new0 (NMSettingsConnectionCallId); + call_id->self = self; + if (applied_connection) { + call_id->had_applied_connection = TRUE; + call_id->applied_connection = applied_connection; + g_object_add_weak_pointer (G_OBJECT (applied_connection), (gpointer *) &call_id->applied_connection); + } + call_id->callback = callback; + call_id->callback_data = callback_data; + c_list_link_tail (&priv->call_ids_lst_head, &call_id->call_ids_lst); /* Use priv->secrets to work around the fact that nm_connection_clear_secrets() * will clear secrets on this object's settings. @@ -1316,10 +1302,9 @@ _get_secrets_cancel (NMSettingsConnection *self, NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); gs_free_error GError *error = NULL; - if (!g_slist_find (priv->get_secret_requests, call_id)) - g_return_if_reached (); + nm_assert (c_list_contains (&priv->call_ids_lst_head, &call_id->call_ids_lst)); - priv->get_secret_requests = g_slist_remove (priv->get_secret_requests, call_id); + c_list_unlink_init (&call_id->call_ids_lst); if (call_id->type == CALL_ID_TYPE_REQ) nm_agent_manager_cancel_secrets (priv->agent_mgr, call_id->t.req.id); @@ -2769,17 +2754,14 @@ dispose (GObject *object) { NMSettingsConnection *self = NM_SETTINGS_CONNECTION (object); NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); + NMSettingsConnectionCallId *call_id, *call_id_safe; _LOGD ("disposing"); /* Cancel in-progress secrets requests */ if (priv->agent_mgr) { - while (priv->get_secret_requests) { - NMSettingsConnectionCallId *call_id = priv->get_secret_requests->data; - + c_list_for_each_entry_safe (call_id, call_id_safe, &priv->call_ids_lst_head, call_ids_lst) _get_secrets_cancel (self, call_id, TRUE); - g_return_if_fail (!priv->get_secret_requests || (call_id != priv->get_secret_requests->data)); - } } /* Disconnect handlers. From 31f70557f5ffc9c903e50adec9a3d3c44b207b6d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 11:02:06 +0100 Subject: [PATCH 05/34] policy: avoid cloning connection list We don't care about having a cloned list, nor do we care about a sort order. --- src/nm-policy.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 7353d635af..116f25cc7d 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1412,7 +1412,7 @@ static void reset_autoconnect_all (NMPolicy *self, NMDevice *device) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - gs_free NMSettingsConnection **connections = NULL; + NMSettingsConnection *const*connections = NULL; guint i; if (device) { @@ -1421,7 +1421,7 @@ reset_autoconnect_all (NMPolicy *self, NMDevice *device) } else _LOGD (LOGD_DEVICE, "re-enabling autoconnect for all connections"); - connections = nm_settings_get_connections_sorted (priv->settings, NULL); + connections = nm_settings_get_connections (priv->settings, NULL); for (i = 0; connections[i]; i++) { NMSettingsConnection *connection = connections[i]; @@ -1436,12 +1436,12 @@ static void reset_autoconnect_for_failed_secrets (NMPolicy *self) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - gs_free NMSettingsConnection **connections = NULL; + NMSettingsConnection *const*connections = NULL; guint i; _LOGD (LOGD_DEVICE, "re-enabling autoconnect for all connections with failed secrets"); - connections = nm_settings_get_connections_sorted (priv->settings, NULL); + connections = nm_settings_get_connections (priv->settings, NULL); for (i = 0; connections[i]; i++) { NMSettingsConnection *connection = connections[i]; @@ -1514,7 +1514,7 @@ reset_connections_retries (gpointer user_data) { NMPolicy *self = (NMPolicy *) user_data; NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - gs_free NMSettingsConnection **connections = NULL; + NMSettingsConnection *const*connections = NULL; guint i; gint32 con_stamp, min_stamp, now; gboolean changed = FALSE; @@ -1523,7 +1523,7 @@ reset_connections_retries (gpointer user_data) min_stamp = 0; now = nm_utils_get_monotonic_timestamp_s (); - connections = nm_settings_get_connections_sorted (priv->settings, NULL); + connections = nm_settings_get_connections (priv->settings, NULL); for (i = 0; connections[i]; i++) { NMSettingsConnection *connection = connections[i]; @@ -1557,7 +1557,7 @@ activate_slave_connections (NMPolicy *self, NMDevice *device) guint i; NMActRequest *req; gboolean internal_activation = FALSE; - gs_free NMSettingsConnection **connections = NULL; + NMSettingsConnection *const*connections; master_device = nm_device_get_iface (device); g_assert (master_device); @@ -1581,7 +1581,7 @@ activate_slave_connections (NMPolicy *self, NMDevice *device) internal_activation = subject && nm_auth_subject_is_internal (subject); } - connections = nm_settings_get_connections_sorted (priv->settings, NULL); + connections = nm_settings_get_connections (priv->settings, NULL); for (i = 0; connections[i]; i++) { NMConnection *slave; NMSettingConnection *s_slave_con; From cfb2af1df2f596cf1bb3653d091dac915db90387 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 12:10:50 +0100 Subject: [PATCH 06/34] policy: schedule autoactivation check after reset_autoconnect_all() When changing the autoconnect blocked settings, we must trigger a recheck-all. --- src/nm-policy.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 116f25cc7d..cbda19dcd8 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1463,8 +1463,10 @@ sleeping_changed (NMManager *manager, GParamSpec *pspec, gpointer user_data) g_object_get (G_OBJECT (manager), NM_MANAGER_NETWORKING_ENABLED, &enabled, NULL); /* Reset retries on all connections so they'll checked on wakeup */ - if (sleeping || !enabled) + if (sleeping || !enabled) { reset_autoconnect_all (self, NULL); + schedule_activate_all (self); + } } static void From 3b874554ac4e3267f1b63a58b88239ac6c69282c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 12:15:24 +0100 Subject: [PATCH 07/34] policy: use CList to track pending_activation_checks This makes link and unlink operations O(1) and safes an additional GSlice allocation for each GSList node. --- src/nm-policy.c | 47 ++++++++++++++++++++--------------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index cbda19dcd8..4bb273c552 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -65,7 +65,7 @@ typedef struct { NMManager *manager; NMNetns *netns; NMFirewallManager *firewall_manager; - GSList *pending_activation_checks; + CList pending_activation_checks; GHashTable *devices; GHashTable *pending_active_connections; @@ -1132,6 +1132,7 @@ check_activating_devices (NMPolicy *self) } typedef struct { + CList pending_lst; NMPolicy *policy; NMDevice *device; guint autoactivate_id; @@ -1140,14 +1141,10 @@ typedef struct { static void activate_data_free (ActivateData *data) { - NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (data->policy); - nm_device_remove_pending_action (data->device, NM_PENDING_ACTION_AUTOACTIVATE, TRUE); - priv->pending_activation_checks = g_slist_remove (priv->pending_activation_checks, data); - + c_list_unlink (&data->pending_lst); nm_clear_g_source (&data->autoactivate_id); g_object_unref (data->device); - g_slice_free (ActivateData, data); } @@ -1304,13 +1301,14 @@ auto_activate_device_cb (gpointer user_data) } static ActivateData * -find_pending_activation (GSList *list, NMDevice *device) +find_pending_activation (NMPolicy *self, NMDevice *device) { - GSList *iter; + NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); + ActivateData *data; - for (iter = list; iter; iter = g_slist_next (iter)) { - if (((ActivateData *) iter->data)->device == device) - return iter->data; + c_list_for_each_entry (data, &priv->pending_activation_checks, pending_lst) { + if (data->device == device) + return data; } return NULL; } @@ -1482,7 +1480,7 @@ schedule_activate_check (NMPolicy *self, NMDevice *device) if (!nm_device_autoconnect_allowed (device)) return; - if (find_pending_activation (priv->pending_activation_checks, device)) + if (find_pending_activation (self, device)) return; active_connections = nm_manager_get_active_connections (priv->manager); @@ -1497,18 +1495,7 @@ schedule_activate_check (NMPolicy *self, NMDevice *device) data->policy = self; data->device = g_object_ref (device); data->autoactivate_id = g_idle_add (auto_activate_device_cb, data); - priv->pending_activation_checks = g_slist_append (priv->pending_activation_checks, data); -} - -static void -clear_pending_activate_check (NMPolicy *self, NMDevice *device) -{ - NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - ActivateData *data; - - data = find_pending_activation (priv->pending_activation_checks, device); - if (data && data->autoactivate_id) - activate_data_free (data); + c_list_link_tail (&priv->pending_activation_checks, &data->pending_lst); } static gboolean @@ -2016,13 +2003,16 @@ device_removed (NMManager *manager, NMDevice *device, gpointer user_data) { NMPolicyPrivate *priv = user_data; NMPolicy *self = _PRIV_TO_SELF (priv); + ActivateData *data; /* XXX 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 */ - clear_pending_activate_check (self, device); + data = find_pending_activation (self, device); + if (data && data->autoactivate_id) + activate_data_free (data); if (g_hash_table_remove (priv->devices, device)) devices_list_unregister (self, device); @@ -2471,6 +2461,8 @@ nm_policy_init (NMPolicy *self) NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); const char *hostname_mode; + c_list_init (&priv->pending_activation_checks); + priv->netns = g_object_ref (nm_netns_get ()); priv->hostname_manager = g_object_ref (nm_hostname_manager_get ()); @@ -2562,6 +2554,7 @@ dispose (GObject *object) const GSList *connections; GHashTableIter h_iter; NMDevice *device; + ActivateData *data, *data_safe; nm_clear_g_cancellable (&priv->lookup.cancellable); g_clear_object (&priv->lookup.addr); @@ -2573,8 +2566,8 @@ dispose (GObject *object) nm_clear_g_object (&priv->activating_device6); g_clear_pointer (&priv->pending_active_connections, g_hash_table_unref); - while (priv->pending_activation_checks) - activate_data_free (priv->pending_activation_checks->data); + c_list_for_each_entry_safe (data, data_safe, &priv->pending_activation_checks, pending_lst) + activate_data_free (data); g_slist_free_full (priv->pending_secondaries, (GDestroyNotify) pending_secondary_data_free); priv->pending_secondaries = NULL; From 51531c953972215e0b15900463378fb8524b4232 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 12:46:58 +0100 Subject: [PATCH 08/34] core: merge nm_settings_get_connections_sorted() with nm_settings_get_connections_clone() --- src/devices/wifi/nm-device-wifi.c | 4 ++-- src/nm-checkpoint.c | 4 +++- src/nm-manager.c | 40 +++++++++++++++---------------- src/settings/nm-settings.c | 33 +++++++++---------------- src/settings/nm-settings.h | 7 +++--- 5 files changed, 39 insertions(+), 49 deletions(-) diff --git a/src/devices/wifi/nm-device-wifi.c b/src/devices/wifi/nm-device-wifi.c index b13ae2c77e..bf021095a5 100644 --- a/src/devices/wifi/nm-device-wifi.c +++ b/src/devices/wifi/nm-device-wifi.c @@ -1428,8 +1428,8 @@ build_hidden_probe_list (NMDeviceWifi *self) connections = nm_settings_get_connections_clone (nm_device_get_settings ((NMDevice *) self), &len, - hidden_filter_func, - NULL); + hidden_filter_func, NULL, + NULL, NULL); if (!connections[0]) return NULL; diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c index 54b577642e..0390255ddc 100644 --- a/src/nm-checkpoint.c +++ b/src/nm-checkpoint.c @@ -335,7 +335,9 @@ next_dev: guint i; g_return_val_if_fail (priv->connection_uuids, NULL); - list = nm_settings_get_connections_sorted (nm_settings_get (), NULL); + list = nm_settings_get_connections_clone (nm_settings_get (), NULL, + NULL, NULL, + nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL); for (i = 0; list[i]; i++) { con = list[i]; diff --git a/src/nm-manager.c b/src/nm-manager.c index 1ab6e39e01..f45771ca37 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -511,26 +511,16 @@ _get_activatable_connections_filter (NMSettings *settings, return !active_connection_find_first (user_data, connection, NULL, NM_ACTIVE_CONNECTION_STATE_DEACTIVATING); } -/* Filter out connections that are already active. - * nm_settings_get_connections_sorted() returns sorted list. We need to preserve the - * order so that we didn't change auto-activation order (recent timestamps - * are first). - * Caller is responsible for freeing the returned list with g_slist_free(). - */ NMSettingsConnection ** nm_manager_get_activatable_connections (NMManager *manager, guint *out_len, gboolean sort) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); - NMSettingsConnection **connections; - guint len; - connections = nm_settings_get_connections_clone (priv->settings, &len, - _get_activatable_connections_filter, - manager); - if (sort && len > 1) - g_qsort_with_data (connections, len, sizeof (connections[0]), nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL); - NM_SET_OUT (out_len, len); - return connections; + return nm_settings_get_connections_clone (priv->settings, out_len, + _get_activatable_connections_filter, + manager, + sort ? nm_settings_connection_cmp_autoconnect_priority_p_with_data : NULL, + NULL); } static NMActiveConnection * @@ -1392,7 +1382,9 @@ system_create_virtual_device (NMManager *self, NMConnection *connection) } /* Create backing resources if the device has any autoconnect connections */ - connections = nm_settings_get_connections_sorted (priv->settings, NULL); + connections = nm_settings_get_connections_clone (priv->settings, NULL, + NULL, NULL, + nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL); for (i = 0; connections[i]; i++) { NMConnection *candidate = NM_CONNECTION (connections[i]); NMSettingConnection *s_con; @@ -1436,7 +1428,9 @@ retry_connections_for_parent_device (NMManager *self, NMDevice *device) g_return_if_fail (device); - connections = nm_settings_get_connections_sorted (priv->settings, NULL); + connections = nm_settings_get_connections_clone (priv->settings, NULL, + NULL, NULL, + nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL); for (i = 0; connections[i]; i++) { NMConnection *candidate = NM_CONNECTION (connections[i]); gs_free_error GError *error = NULL; @@ -3022,7 +3016,9 @@ find_slaves (NMManager *manager, * even if a slave was already active, it might be deactivated during * master reactivation. */ - all_connections = nm_settings_get_connections_sorted (priv->settings, &n_all_connections); + all_connections = nm_settings_get_connections_clone (priv->settings, &n_all_connections, + NULL, NULL, + nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL); for (i = 0; i < n_all_connections; i++) { NMSettingsConnection *master_connection = NULL; NMDevice *master_device = NULL, *slave_device; @@ -4167,7 +4163,9 @@ impl_manager_add_and_activate_connection (NMManager *self, gs_free NMSettingsConnection **connections = NULL; guint i, len; - connections = nm_settings_get_connections_sorted (priv->settings, &len); + connections = nm_settings_get_connections_clone (priv->settings, &len, + NULL, NULL, + nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL); all_connections = NULL; for (i = len; i > 0; ) { i--; @@ -5226,7 +5224,9 @@ nm_manager_start (NMManager *self, GError **error) * connection-added signals thus devices have to be created manually. */ _LOGD (LOGD_CORE, "creating virtual devices..."); - connections = nm_settings_get_connections_sorted (priv->settings, NULL); + connections = nm_settings_get_connections_clone (priv->settings, NULL, + NULL, NULL, + nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL); for (i = 0; connections[i]; i++) connection_changed (self, NM_CONNECTION (connections[i])); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 4eeee31554..d52592dd60 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -431,6 +431,9 @@ nm_settings_get_connections (NMSettings *self, guint *out_len) * @out_len: (allow-none): optional output argument * @func: caller-supplied function for filtering connections * @func_data: caller-supplied data passed to @func + * @sort_compare_func: (allow-none): optional function pointer for + * sorting the returned list. + * @sort_data: user data for @sort_compare_func. * * Returns: (transfer container) (element-type NMSettingsConnection): * an NULL terminated array of #NMSettingsConnection objects that were @@ -443,7 +446,9 @@ NMSettingsConnection ** nm_settings_get_connections_clone (NMSettings *self, guint *out_len, NMSettingsConnectionFilterFunc func, - gpointer func_data) + gpointer func_data, + GCompareDataFunc sort_compare_func, + gpointer sort_data) { NMSettingsConnection *const*list_cached; NMSettingsConnection **list; @@ -471,31 +476,15 @@ nm_settings_get_connections_clone (NMSettings *self, } else memcpy (list, list_cached, sizeof (list[0]) * ((gsize) len + 1)); + if ( len > 1 + && sort_compare_func) { + g_qsort_with_data (list, len, sizeof (NMSettingsConnection *), + sort_compare_func, sort_data); + } NM_SET_OUT (out_len, len); return list; } -/* Returns a list of NMSettingsConnections. - * The list is sorted in the order suitable for auto-connecting, i.e. - * first go connections with autoconnect=yes and most recent timestamp. - * Caller must free the list with g_free(), but not the list items. - */ -NMSettingsConnection ** -nm_settings_get_connections_sorted (NMSettings *self, guint *out_len) -{ - NMSettingsConnection **connections; - guint len; - - g_return_val_if_fail (NM_IS_SETTINGS (self), NULL); - - connections = nm_settings_get_connections_clone (self, &len, NULL, NULL); - if (len > 1) - g_qsort_with_data (connections, len, sizeof (NMSettingsConnection *), nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL); - - NM_SET_OUT (out_len, len); - return connections; -} - NMSettingsConnection * nm_settings_get_connection_by_path (NMSettings *self, const char *path) { diff --git a/src/settings/nm-settings.h b/src/settings/nm-settings.h index eede76b029..0e01f6b5fd 100644 --- a/src/settings/nm-settings.h +++ b/src/settings/nm-settings.h @@ -100,10 +100,9 @@ NMSettingsConnection *const* nm_settings_get_connections (NMSettings *settings, NMSettingsConnection **nm_settings_get_connections_clone (NMSettings *self, guint *out_len, NMSettingsConnectionFilterFunc func, - gpointer func_data); - -NMSettingsConnection **nm_settings_get_connections_sorted (NMSettings *self, - guint *out_len); + gpointer func_data, + GCompareDataFunc sort_compare_func, + gpointer sort_data); NMSettingsConnection *nm_settings_add_connection (NMSettings *settings, NMConnection *connection, From 3a907377ac360325e8dea3f5ff864ce55a4950c1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 23 Nov 2017 21:30:09 +0100 Subject: [PATCH 09/34] core: track NMActiveConnection in manager with CList Using CList, we embed the list element in NMActiveConnection struct itself. That means for example, that you couldn't track a NMActiveConnection more then once. But we anyway never want that. The advantage is, that removing an active connection from the list is O(1), and we safe additional GSlice allocations for each node element. --- src/nm-active-connection.c | 4 + src/nm-active-connection.h | 6 ++ src/nm-checkpoint.c | 10 +-- src/nm-manager.c | 171 +++++++++++++++++-------------------- src/nm-manager.h | 15 +++- src/nm-policy.c | 123 +++++++++++++------------- 6 files changed, 166 insertions(+), 163 deletions(-) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 002f11f7e9..8efb7a3fa3 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -1289,6 +1289,8 @@ nm_active_connection_init (NMActiveConnection *self) priv = G_TYPE_INSTANCE_GET_PRIVATE (self, NM_TYPE_ACTIVE_CONNECTION, NMActiveConnectionPrivate); self->_priv = priv; + c_list_init (&self->active_connections_lst); + _LOGT ("creating"); priv->activation_type = NM_ACTIVATION_TYPE_MANAGED; @@ -1331,6 +1333,8 @@ dispose (GObject *object) NMActiveConnection *self = NM_ACTIVE_CONNECTION (object); NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); + nm_assert (!c_list_is_linked (&self->active_connections_lst)); + _LOGD ("disposing"); if (priv->chain) { diff --git a/src/nm-active-connection.h b/src/nm-active-connection.h index 5562b42f07..a4f6c12464 100644 --- a/src/nm-active-connection.h +++ b/src/nm-active-connection.h @@ -24,6 +24,8 @@ #include "nm-exported-object.h" #include "nm-connection.h" +#include "nm-utils/c-list.h" + #define NM_TYPE_ACTIVE_CONNECTION (nm_active_connection_get_type ()) #define NM_ACTIVE_CONNECTION(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_ACTIVE_CONNECTION, NMActiveConnection)) #define NM_ACTIVE_CONNECTION_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_ACTIVE_CONNECTION, NMActiveConnectionClass)) @@ -71,6 +73,10 @@ struct _NMActiveConnectionPrivate; struct _NMActiveConnection { NMExportedObject parent; struct _NMActiveConnectionPrivate *_priv; + + /* active connection can be tracked in a list by NMManager. This is + * the list node. */ + CList active_connections_lst; }; typedef struct { diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c index 0390255ddc..af56bf26e6 100644 --- a/src/nm-checkpoint.c +++ b/src/nm-checkpoint.c @@ -125,10 +125,10 @@ find_settings_connection (NMCheckpoint *self, gboolean *need_activation) { NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self); - const GSList *active_connections, *iter; - NMActiveConnection *active = NULL; + NMActiveConnection *active; NMSettingsConnection *connection; const char *uuid, *ac_uuid; + const CList *tmp_clist; *need_activation = FALSE; *need_update = FALSE; @@ -149,9 +149,7 @@ find_settings_connection (NMCheckpoint *self, } /* ... is active, ... */ - active_connections = nm_manager_get_active_connections (priv->manager); - for (iter = active_connections; iter; iter = g_slist_next (iter)) { - active = iter->data; + nm_manager_for_each_active_connection (priv->manager, active, tmp_clist) { ac_uuid = nm_settings_connection_get_uuid (nm_active_connection_get_settings_connection (active)); if (nm_streq (uuid, ac_uuid)) { _LOGT ("rollback: connection %s is active", uuid); @@ -159,7 +157,7 @@ find_settings_connection (NMCheckpoint *self, } } - if (!iter) { + if (!active) { _LOGT ("rollback: connection %s is not active", uuid); *need_activation = TRUE; return connection; diff --git a/src/nm-manager.c b/src/nm-manager.c index f45771ca37..4766e3c8e3 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -114,7 +114,7 @@ typedef struct { GArray *capabilities; - GSList *active_connections; + CList active_connections_lst_head; GSList *authorizing_connections; guint ac_cleanup_id; NMActiveConnection *primary_connection; @@ -314,39 +314,38 @@ static gboolean active_connection_remove (NMManager *self, NMActiveConnection *active) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - gboolean notify = nm_exported_object_is_exported (NM_EXPORTED_OBJECT (active)); - GSList *found; + NMSettingsConnection *connection; + gboolean notify; - /* FIXME: switch to a GList for faster removal */ - found = g_slist_find (priv->active_connections, active); - if (found) { - NMSettingsConnection *connection; + nm_assert (NM_IS_ACTIVE_CONNECTION (active)); + nm_assert (c_list_contains (&priv->active_connections_lst_head, &active->active_connections_lst)); - priv->active_connections = g_slist_remove (priv->active_connections, active); - g_signal_emit (self, signals[ACTIVE_CONNECTION_REMOVED], 0, active); - g_signal_handlers_disconnect_by_func (active, active_connection_state_changed, self); - g_signal_handlers_disconnect_by_func (active, active_connection_default_changed, self); - g_signal_handlers_disconnect_by_func (active, active_connection_parent_active, self); + notify = nm_exported_object_is_exported (NM_EXPORTED_OBJECT (active)); - if ( (connection = nm_active_connection_get_settings_connection (active)) - && nm_settings_connection_get_volatile (connection)) - g_object_ref (connection); - else - connection = NULL; + c_list_unlink_init (&active->active_connections_lst); + g_signal_emit (self, signals[ACTIVE_CONNECTION_REMOVED], 0, active); + g_signal_handlers_disconnect_by_func (active, active_connection_state_changed, self); + g_signal_handlers_disconnect_by_func (active, active_connection_default_changed, self); + g_signal_handlers_disconnect_by_func (active, active_connection_parent_active, self); - nm_exported_object_clear_and_unexport (&active); + if ( (connection = nm_active_connection_get_settings_connection (active)) + && nm_settings_connection_get_volatile (connection)) + g_object_ref (connection); + else + connection = NULL; - if (connection) { - if (nm_settings_has_connection (priv->settings, connection)) { - _LOGD (LOGD_DEVICE, "assumed connection disconnected. Deleting generated connection '%s' (%s)", - nm_settings_connection_get_id (connection), nm_settings_connection_get_uuid (connection)); - nm_settings_connection_delete (connection, NULL); - } - g_object_unref (connection); + nm_exported_object_clear_and_unexport (&active); + + if (connection) { + if (nm_settings_has_connection (priv->settings, connection)) { + _LOGD (LOGD_DEVICE, "assumed connection disconnected. Deleting generated connection '%s' (%s)", + nm_settings_connection_get_id (connection), nm_settings_connection_get_uuid (connection)); + nm_settings_connection_delete (connection, NULL); } + g_object_unref (connection); } - return found && notify; + return notify; } static gboolean @@ -354,16 +353,12 @@ _active_connection_cleanup (gpointer user_data) { NMManager *self = NM_MANAGER (user_data); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - GSList *iter; + NMActiveConnection *ac, *ac_safe; priv->ac_cleanup_id = 0; g_object_freeze_notify (G_OBJECT (self)); - iter = priv->active_connections; - while (iter) { - NMActiveConnection *ac = iter->data; - - iter = iter->next; + c_list_for_each_entry_safe (ac, ac_safe, &priv->active_connections_lst_head, active_connections_lst) { if (nm_active_connection_get_state (ac) == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED) { if (active_connection_remove (self, ac)) _notify (self, PROP_ACTIVE_CONNECTIONS); @@ -421,10 +416,11 @@ active_connection_add (NMManager *self, NMActiveConnection *active) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - g_return_if_fail (g_slist_find (priv->active_connections, active) == FALSE); + nm_assert (NM_IS_ACTIVE_CONNECTION (active)); + nm_assert (!c_list_is_linked (&active->active_connections_lst)); - priv->active_connections = g_slist_prepend (priv->active_connections, - g_object_ref (active)); + c_list_link_front (&priv->active_connections_lst_head, &active->active_connections_lst); + g_object_ref (active); g_signal_connect (active, "notify::" NM_ACTIVE_CONNECTION_STATE, @@ -446,10 +442,10 @@ active_connection_add (NMManager *self, NMActiveConnection *active) _notify (self, PROP_ACTIVE_CONNECTIONS); } -const GSList * +const CList * nm_manager_get_active_connections (NMManager *manager) { - return NM_MANAGER_GET_PRIVATE (manager)->active_connections; + return &NM_MANAGER_GET_PRIVATE (manager)->active_connections_lst_head; } static NMActiveConnection * @@ -458,16 +454,12 @@ active_connection_find_first (NMManager *self, const char *uuid, NMActiveConnectionState max_state) { - NMManagerPrivate *priv; - GSList *iter; + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMActiveConnection *ac; - g_return_val_if_fail (NM_IS_MANAGER (self), NULL); - g_return_val_if_fail (!settings_connection || NM_IS_SETTINGS_CONNECTION (settings_connection), NULL); + nm_assert (!settings_connection || NM_IS_SETTINGS_CONNECTION (settings_connection)); - priv = NM_MANAGER_GET_PRIVATE (self); - - for (iter = priv->active_connections; iter; iter = iter->next) { - NMActiveConnection *ac = iter->data; + c_list_for_each_entry (ac, &priv->active_connections_lst_head, active_connections_lst) { NMSettingsConnection *con; con = nm_active_connection_get_settings_connection (ac); @@ -527,16 +519,13 @@ static NMActiveConnection * active_connection_get_by_path (NMManager *manager, const char *path) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); - GSList *iter; + NMActiveConnection *ac; - g_return_val_if_fail (manager != NULL, NULL); - g_return_val_if_fail (path != NULL, NULL); + nm_assert (path); - for (iter = priv->active_connections; iter; iter = g_slist_next (iter)) { - NMActiveConnection *candidate = iter->data; - - if (g_strcmp0 (path, nm_exported_object_get_path (NM_EXPORTED_OBJECT (candidate))) == 0) - return candidate; + c_list_for_each_entry (ac, &priv->active_connections_lst_head, active_connections_lst) { + if (nm_streq0 (path, nm_exported_object_get_path (NM_EXPORTED_OBJECT (ac)))) + return ac; } return NULL; } @@ -818,10 +807,9 @@ find_best_device_state (NMManager *manager) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); NMState best_state = NM_STATE_DISCONNECTED; - GSList *iter; + NMActiveConnection *ac; - for (iter = priv->active_connections; iter; iter = iter->next) { - NMActiveConnection *ac = NM_ACTIVE_CONNECTION (iter->data); + c_list_for_each_entry (ac, &priv->active_connections_lst_head, active_connections_lst) { NMActiveConnectionState ac_state = nm_active_connection_get_state (ac); switch (ac_state) { @@ -3624,11 +3612,11 @@ _internal_activation_auth_done (NMActiveConnection *active, gpointer user_data1, gpointer user_data2) { + _nm_unused gs_unref_object NMActiveConnection *active_to_free = active; NMManager *self = user_data1; NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); - NMActiveConnection *candidate; - GError *error = NULL; - GSList *iter; + NMActiveConnection *ac; + gs_free_error GError *error = NULL; priv->authorizing_connections = g_slist_remove (priv->authorizing_connections, active); @@ -3637,12 +3625,12 @@ _internal_activation_auth_done (NMActiveConnection *active, * detect a duplicate if the existing active connection is undergoing * authorization in impl_manager_activate_connection(). */ - if (success && nm_auth_subject_is_internal (nm_active_connection_get_subject (active))) { - for (iter = priv->active_connections; iter; iter = iter->next) { - candidate = iter->data; - if ( nm_active_connection_get_device (candidate) == nm_active_connection_get_device (active) - && nm_active_connection_get_settings_connection (candidate) == nm_active_connection_get_settings_connection (active) - && NM_IN_SET (nm_active_connection_get_state (candidate), + if ( success + && nm_auth_subject_is_internal (nm_active_connection_get_subject (active))) { + c_list_for_each_entry (ac, &priv->active_connections_lst_head, active_connections_lst) { + if ( nm_active_connection_get_device (ac) == nm_active_connection_get_device (active) + && nm_active_connection_get_settings_connection (ac) == nm_active_connection_get_settings_connection (active) + && NM_IN_SET (nm_active_connection_get_state (ac), NM_ACTIVE_CONNECTION_STATE_ACTIVATING, NM_ACTIVE_CONNECTION_STATE_ACTIVATED)) { g_set_error (&error, @@ -3657,16 +3645,12 @@ _internal_activation_auth_done (NMActiveConnection *active, } if (success) { - if (_internal_activate_generic (self, active, &error)) { - g_object_unref (active); + if (_internal_activate_generic (self, active, &error)) return; - } } - g_assert (error_desc || error); + nm_assert (error_desc || error); _internal_activation_failed (self, active, error_desc ? error_desc : error->message); - g_object_unref (active); - g_clear_error (&error); } /** @@ -5990,19 +5974,15 @@ periodic_update_active_connection_timestamps (gpointer user_data) { NMManager *manager = NM_MANAGER (user_data); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (manager); - GSList *iter; - - for (iter = priv->active_connections; iter; iter = g_slist_next (iter)) { - NMActiveConnection *ac = iter->data; - NMSettingsConnection *connection; + NMActiveConnection *ac; + c_list_for_each_entry (ac, &priv->active_connections_lst_head, active_connections_lst) { if (nm_active_connection_get_state (ac) == NM_ACTIVE_CONNECTION_STATE_ACTIVATED) { - connection = nm_active_connection_get_settings_connection (ac); - nm_settings_connection_update_timestamp (connection, (guint64) time (NULL), FALSE); + nm_settings_connection_update_timestamp (nm_active_connection_get_settings_connection (ac), + (guint64) time (NULL), FALSE); } } - - return TRUE; + return G_SOURCE_CONTINUE; } static void @@ -6164,6 +6144,7 @@ nm_manager_init (NMManager *self) GFile *file; c_list_init (&priv->link_cb_lst); + c_list_init (&priv->active_connections_lst_head); priv->platform = g_object_ref (NM_PLATFORM_GET); @@ -6252,6 +6233,10 @@ get_property (GObject *object, guint prop_id, const NMGlobalDnsConfig *dns_config; const char *type; char **strv; + const char *path; + NMActiveConnection *ac; + GPtrArray *ptrarr; + gboolean vbool; switch (prop_id) { case PROP_VERSION: @@ -6291,7 +6276,14 @@ get_property (GObject *object, guint prop_id, g_value_set_boolean (value, FALSE); break; case PROP_ACTIVE_CONNECTIONS: - nm_utils_g_value_set_object_path_array (value, priv->active_connections, NULL, NULL); + ptrarr = g_ptr_array_new (); + c_list_for_each_entry (ac, &priv->active_connections_lst_head, active_connections_lst) { + path = nm_exported_object_get_path (NM_EXPORTED_OBJECT (ac)); + if (path) + g_ptr_array_add (ptrarr, g_strdup (path)); + } + g_ptr_array_add (ptrarr, NULL); + g_value_take_boxed (value, g_ptr_array_free (ptrarr, FALSE)); break; case PROP_CONNECTIVITY: g_value_set_uint (value, priv->connectivity_state); @@ -6302,15 +6294,11 @@ get_property (GObject *object, guint prop_id, break; case PROP_CONNECTIVITY_CHECK_ENABLED: #if WITH_CONCHECK - { - NMConnectivity *connectivity; - - connectivity = nm_connectivity_get (); - g_value_set_boolean (value, nm_connectivity_check_enabled (connectivity)); - } + vbool = nm_connectivity_check_enabled (nm_connectivity_get ()); #else - g_value_set_boolean (value, FALSE); + vbool = FALSE; #endif + g_value_set_boolean (value, FALSE); break; case PROP_PRIMARY_CONNECTION: nm_utils_g_value_set_object_path (value, priv->primary_connection); @@ -6415,6 +6403,7 @@ dispose (GObject *object) NMManager *self = NM_MANAGER (object); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); CList *iter, *iter_safe; + NMActiveConnection *ac, *ac_safe; g_signal_handlers_disconnect_by_func (priv->platform, G_CALLBACK (platform_link_cb), @@ -6448,9 +6437,9 @@ dispose (GObject *object) nm_clear_g_source (&priv->ac_cleanup_id); - while (priv->active_connections) - active_connection_remove (self, NM_ACTIVE_CONNECTION (priv->active_connections->data)); - g_clear_pointer (&priv->active_connections, g_slist_free); + c_list_for_each_entry_safe (ac, ac_safe, &priv->active_connections_lst_head, active_connections_lst) + active_connection_remove (self, ac); + nm_assert (c_list_is_empty (&priv->active_connections_lst_head)); g_clear_object (&priv->primary_connection); g_clear_object (&priv->activating_connection); diff --git a/src/nm-manager.h b/src/nm-manager.h index b4ec7b2688..d52b2655de 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -24,6 +24,7 @@ #include "nm-exported-object.h" #include "settings/nm-settings-connection.h" +#include "nm-utils/c-list.h" #define NM_TYPE_MANAGER (nm_manager_get_type ()) #define NM_MANAGER(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_MANAGER, NMManager)) @@ -85,7 +86,19 @@ gboolean nm_manager_start (NMManager *manager, GError **error); void nm_manager_stop (NMManager *manager); NMState nm_manager_get_state (NMManager *manager); -const GSList *nm_manager_get_active_connections (NMManager *manager); +const CList * nm_manager_get_active_connections (NMManager *manager); + +#define nm_manager_for_each_active_connection(manager, iter, tmp_list) \ + for (tmp_list = nm_manager_get_active_connections (manager), \ + iter = c_list_entry (tmp_list->next, NMActiveConnection, active_connections_lst); \ + ({ \ + gboolean _has_next = (&iter->active_connections_lst != tmp_list); \ + \ + if (!_has_next) \ + iter = NULL; \ + _has_next; \ + }); \ + iter = c_list_entry (iter->active_connections_lst.next, NMActiveConnection, active_connections_lst)) NMSettingsConnection **nm_manager_get_activatable_connections (NMManager *manager, guint *out_len, diff --git a/src/nm-policy.c b/src/nm-policy.c index 4bb273c552..c65e6d3a4a 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -312,8 +312,9 @@ device_ip6_prefix_delegated (NMDevice *device, NMPolicyPrivate *priv = user_data; NMPolicy *self = _PRIV_TO_SELF (priv); IP6PrefixDelegation *delegation = NULL; - const GSList *connections, *iter; guint i; + const CList *tmp_list; + NMActiveConnection *ac; _LOGI (LOGD_IP6, "ipv6-pd: received a prefix %s/%d from %s", nm_utils_inet6_ntop (&prefix->address, NULL), @@ -344,10 +345,10 @@ device_ip6_prefix_delegated (NMDevice *device, * so traversing it from the beginning makes it likely for newly * activated connections that have no subnet assigned to be served * first. That is a simple yet fair policy, which is good. */ - connections = nm_manager_get_active_connections (priv->manager); - for (iter = connections; iter; iter = g_slist_next (iter)) { - NMDevice *to_device = nm_active_connection_get_device (iter->data); + nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { + NMDevice *to_device; + to_device = nm_active_connection_get_device (ac); if (nm_device_needs_ip6_subnet (to_device)) ip6_subnet_from_delegation (delegation, to_device); } @@ -822,16 +823,16 @@ update_default_ac (NMPolicy *self, void (*set_active_func)(NMActiveConnection*, gboolean)) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - const GSList *connections, *iter; + const CList *tmp_list; + NMActiveConnection *ac; /* Clear the 'default[6]' flag on all active connections that aren't the new * default active connection. We'll set the new default after; this ensures * we don't ever have two marked 'default[6]' simultaneously. */ - connections = nm_manager_get_active_connections (priv->manager); - for (iter = connections; iter; iter = g_slist_next (iter)) { - if (NM_ACTIVE_CONNECTION (iter->data) != best) - set_active_func (NM_ACTIVE_CONNECTION (iter->data), FALSE); + nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { + if (ac != best) + set_active_func (ac, FALSE); } /* Mark new default active connection */ @@ -850,19 +851,19 @@ get_best_ip_config (NMPolicy *self, NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); NMDevice *device; gpointer conf; - const GSList *iter; + const CList *tmp_list; + NMActiveConnection *ac; nm_assert (NM_IN_SET (addr_family, AF_INET, AF_INET6)); - for (iter = nm_manager_get_active_connections (priv->manager); iter; iter = iter->next) { - NMActiveConnection *active = NM_ACTIVE_CONNECTION (iter->data); + nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { NMVpnConnection *candidate; NMVpnConnectionState vpn_state; - if (!NM_IS_VPN_CONNECTION (active)) + if (!NM_IS_VPN_CONNECTION (ac)) continue; - candidate = NM_VPN_CONNECTION (active); + candidate = NM_VPN_CONNECTION (ac); vpn_state = nm_vpn_connection_get_vpn_state (candidate); if (vpn_state != NM_VPN_CONNECTION_STATE_ACTIVATED) @@ -887,7 +888,7 @@ get_best_ip_config (NMPolicy *self, * best metric. */ NM_SET_OUT (out_device, NULL); NM_SET_OUT (out_vpn, candidate); - NM_SET_OUT (out_ac, active); + NM_SET_OUT (out_ac, ac); NM_SET_OUT (out_ip_iface, nm_vpn_connection_get_ip_iface (candidate, TRUE)); return conf; } @@ -926,6 +927,8 @@ update_ip4_routing (NMPolicy *self, gboolean force_update) NMVpnConnection *vpn = NULL; NMActiveConnection *best_ac = NULL; const char *ip_iface = NULL; + const CList *tmp_list; + NMActiveConnection *ac; /* Note that we might have an IPv4 VPN tunneled over an IPv6-only device, * so we can get (vpn != NULL && best == NULL). @@ -945,16 +948,11 @@ update_ip4_routing (NMPolicy *self, gboolean force_update) return; if (best) { - const GSList *connections, *iter; - - connections = nm_manager_get_active_connections (priv->manager); - for (iter = connections; iter; iter = g_slist_next (iter)) { - NMActiveConnection *active = iter->data; - - if ( NM_IS_VPN_CONNECTION (active) - && nm_vpn_connection_get_ip4_config (NM_VPN_CONNECTION (active)) - && !nm_active_connection_get_device (active)) - nm_active_connection_set_device (active, best); + nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { + if ( NM_IS_VPN_CONNECTION (ac) + && nm_vpn_connection_get_ip4_config (NM_VPN_CONNECTION (ac)) + && !nm_active_connection_get_device (ac)) + nm_active_connection_set_device (ac, best); } } @@ -977,12 +975,12 @@ static void update_ip6_dns_delegation (NMPolicy *self) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - const GSList *connections, *iter; - - connections = nm_manager_get_active_connections (priv->manager); - for (iter = connections; iter; iter = g_slist_next (iter)) { - NMDevice *device = nm_active_connection_get_device (iter->data); + NMDevice *device; + NMActiveConnection *ac; + const CList *tmp_list; + nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { + device = nm_active_connection_get_device (ac); if (device && nm_device_needs_ip6_subnet (device)) nm_device_copy_ip6_dns_config (device, priv->default_device6); } @@ -992,13 +990,13 @@ static void update_ip6_prefix_delegation (NMPolicy *self) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - const GSList *connections, *iter; + NMDevice *device; + NMActiveConnection *ac; + const CList *tmp_list; /* There's new default IPv6 connection, try to get a prefix for everyone. */ - connections = nm_manager_get_active_connections (priv->manager); - for (iter = connections; iter; iter = g_slist_next (iter)) { - NMDevice *device = nm_active_connection_get_device (iter->data); - + nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { + device = nm_active_connection_get_device (ac); if (device && nm_device_needs_ip6_subnet (device)) ip6_subnet_from_device (self, priv->default_device6, device); } @@ -1012,6 +1010,8 @@ update_ip6_routing (NMPolicy *self, gboolean force_update) NMVpnConnection *vpn = NULL; NMActiveConnection *best_ac = NULL; const char *ip_iface = NULL; + NMActiveConnection *ac; + const CList *tmp_list; /* Note that we might have an IPv6 VPN tunneled over an IPv4-only device, * so we can get (vpn != NULL && best == NULL). @@ -1031,16 +1031,11 @@ update_ip6_routing (NMPolicy *self, gboolean force_update) return; if (best) { - const GSList *connections, *iter; - - connections = nm_manager_get_active_connections (priv->manager); - for (iter = connections; iter; iter = g_slist_next (iter)) { - NMActiveConnection *active = iter->data; - - if ( NM_IS_VPN_CONNECTION (active) - && nm_vpn_connection_get_ip6_config (NM_VPN_CONNECTION (active)) - && !nm_active_connection_get_device (active)) - nm_active_connection_set_device (active, best); + nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { + if ( NM_IS_VPN_CONNECTION (ac) + && nm_vpn_connection_get_ip6_config (NM_VPN_CONNECTION (ac)) + && !nm_active_connection_get_device (ac)) + nm_active_connection_set_device (ac, best); } } @@ -1472,7 +1467,8 @@ schedule_activate_check (NMPolicy *self, NMDevice *device) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); ActivateData *data; - const GSList *active_connections, *iter; + NMActiveConnection *ac; + const CList *tmp_list; if (nm_manager_get_state (priv->manager) == NM_STATE_ASLEEP) return; @@ -1483,9 +1479,8 @@ schedule_activate_check (NMPolicy *self, NMDevice *device) if (find_pending_activation (self, device)) return; - active_connections = nm_manager_get_active_connections (priv->manager); - for (iter = active_connections; iter; iter = iter->next) { - if (nm_active_connection_get_device (NM_ACTIVE_CONNECTION (iter->data)) == device) + nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { + if (nm_active_connection_get_device (ac) == device) return; } @@ -2302,19 +2297,21 @@ connection_updated (NMSettings *settings, } static void -_deactivate_if_active (NMManager *manager, NMSettingsConnection *connection) +_deactivate_if_active (NMPolicy *self, NMSettingsConnection *connection) { - const GSList *active, *iter; + NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); + NMActiveConnection *ac; + const CList *tmp_list; + GError *error = NULL; - active = nm_manager_get_active_connections (manager); - for (iter = active; iter; iter = g_slist_next (iter)) { - NMActiveConnection *ac = iter->data; + nm_assert (NM_IS_SETTINGS_CONNECTION (connection)); + + nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { NMActiveConnectionState state = nm_active_connection_get_state (ac); - GError *error = NULL; - if (nm_active_connection_get_settings_connection (ac) == connection && - (state <= NM_ACTIVE_CONNECTION_STATE_ACTIVATED)) { - if (!nm_manager_deactivate_connection (manager, + if ( nm_active_connection_get_settings_connection (ac) == connection + && (state <= NM_ACTIVE_CONNECTION_STATE_ACTIVATED)) { + if (!nm_manager_deactivate_connection (priv->manager, ac, NM_DEVICE_STATE_REASON_CONNECTION_REMOVED, &error)) { @@ -2333,9 +2330,7 @@ connection_removed (NMSettings *settings, NMSettingsConnection *connection, gpointer user_data) { - NMPolicyPrivate *priv = user_data; - - _deactivate_if_active (priv->manager, connection); + _deactivate_if_active (user_data, connection); } static void @@ -2349,7 +2344,7 @@ connection_visibility_changed (NMSettings *settings, if (nm_settings_connection_is_visible (connection)) schedule_activate_all (self); else - _deactivate_if_active (priv->manager, connection); + _deactivate_if_active (self, connection); } static void @@ -2551,7 +2546,6 @@ dispose (GObject *object) { NMPolicy *self = NM_POLICY (object); NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - const GSList *connections; GHashTableIter h_iter; NMDevice *device; ActivateData *data, *data_safe; @@ -2592,8 +2586,7 @@ dispose (GObject *object) * will have called active_connection_removed() and thus we don't need * to clean anything up. Assert that this is TRUE. */ - connections = nm_manager_get_active_connections (priv->manager); - g_assert (connections == NULL); + 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); From 10a46c5ae2a488dc933d2e4a510bfb5d033f72bb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 23 Nov 2017 21:53:04 +0100 Subject: [PATCH 10/34] core: merge IPv4 and IPv6 versions of nm_active_connection_get_default() --- src/devices/nm-device.c | 2 +- src/nm-act-request.c | 6 ++-- src/nm-active-connection.c | 59 +++++++++++++++++--------------------- src/nm-active-connection.h | 8 ++---- src/nm-manager.c | 3 +- src/nm-policy.c | 12 ++++---- 6 files changed, 38 insertions(+), 52 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index f8c2adfd19..f1643ea772 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -12446,7 +12446,7 @@ _cleanup_generic_post (NMDevice *self, CleanupType cleanup_type) nm_assert (priv->needs_ip6_subnet == FALSE); if (priv->act_request) { - nm_active_connection_set_default (NM_ACTIVE_CONNECTION (priv->act_request), FALSE); + nm_active_connection_set_default (NM_ACTIVE_CONNECTION (priv->act_request), AF_INET, FALSE); priv->master_ready_handled = FALSE; nm_clear_g_signal_handler (priv->act_request, &priv->master_ready_id); diff --git a/src/nm-act-request.c b/src/nm-act-request.c index 29bb3a05bb..164f44d2cf 100644 --- a/src/nm-act-request.c +++ b/src/nm-act-request.c @@ -454,10 +454,8 @@ device_state_changed (NMActiveConnection *active, } if ( ac_state == NM_ACTIVE_CONNECTION_STATE_DEACTIVATED - || ac_state == NM_ACTIVE_CONNECTION_STATE_UNKNOWN) { - nm_active_connection_set_default (active, FALSE); - nm_active_connection_set_default6 (active, FALSE); - } + || ac_state == NM_ACTIVE_CONNECTION_STATE_UNKNOWN) + nm_active_connection_set_default (active, AF_UNSPEC, FALSE); nm_active_connection_set_state (active, ac_state, ac_state_reason); } diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 8efb7a3fa3..c27f6548eb 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -494,53 +494,46 @@ nm_active_connection_set_specific_object (NMActiveConnection *self, } void -nm_active_connection_set_default (NMActiveConnection *self, gboolean is_default) +nm_active_connection_set_default (NMActiveConnection *self, + int addr_family, + gboolean is_default) { NMActiveConnectionPrivate *priv; g_return_if_fail (NM_IS_ACTIVE_CONNECTION (self)); + nm_assert (NM_IN_SET (addr_family, AF_UNSPEC, AF_INET, AF_INET6)); is_default = !!is_default; priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); - if (priv->is_default == is_default) - return; - - priv->is_default = is_default; - _notify (self, PROP_DEFAULT); + if (NM_IN_SET (addr_family, AF_UNSPEC, AF_INET)) { + if (priv->is_default != is_default) { + priv->is_default = is_default; + _notify (self, PROP_DEFAULT); + } + } + if (NM_IN_SET (addr_family, AF_UNSPEC, AF_INET6)) { + if (priv->is_default6 != is_default) { + priv->is_default6 = is_default; + _notify (self, PROP_DEFAULT6); + } + } } gboolean -nm_active_connection_get_default (NMActiveConnection *self) -{ - g_return_val_if_fail (NM_IS_ACTIVE_CONNECTION (self), FALSE); - - return NM_ACTIVE_CONNECTION_GET_PRIVATE (self)->is_default; -} - -void -nm_active_connection_set_default6 (NMActiveConnection *self, gboolean is_default6) +nm_active_connection_get_default (NMActiveConnection *self, int addr_family) { NMActiveConnectionPrivate *priv; - g_return_if_fail (NM_IS_ACTIVE_CONNECTION (self)); - - is_default6 = !!is_default6; + g_return_val_if_fail (NM_IS_ACTIVE_CONNECTION (self), FALSE); + nm_assert (NM_IN_SET (addr_family, AF_UNSPEC, AF_INET, AF_INET6)); priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); - if (priv->is_default6 == is_default6) - return; - - priv->is_default6 = is_default6; - _notify (self, PROP_DEFAULT6); -} - -gboolean -nm_active_connection_get_default6 (NMActiveConnection *self) -{ - g_return_val_if_fail (NM_IS_ACTIVE_CONNECTION (self), FALSE); - - return NM_ACTIVE_CONNECTION_GET_PRIVATE (self)->is_default6; + switch (addr_family) { + case AF_INET: return priv->is_default; + case AF_INET6: return priv->is_default6; + default: return priv->is_default || priv->is_default6; + } } NMAuthSubject * @@ -1263,10 +1256,10 @@ set_property (GObject *object, guint prop_id, priv->specific_object = g_strdup (tmp); break; case PROP_DEFAULT: - priv->is_default = !!g_value_get_boolean (value); + priv->is_default = g_value_get_boolean (value); break; case PROP_DEFAULT6: - priv->is_default6 = !!g_value_get_boolean (value); + priv->is_default6 = g_value_get_boolean (value); break; case PROP_VPN: priv->vpn = g_value_get_boolean (value); diff --git a/src/nm-active-connection.h b/src/nm-active-connection.h index a4f6c12464..5ede2b04ff 100644 --- a/src/nm-active-connection.h +++ b/src/nm-active-connection.h @@ -137,14 +137,10 @@ void nm_active_connection_set_specific_object (NMActiveConnection *self const char *specific_object); void nm_active_connection_set_default (NMActiveConnection *self, + int addr_family, gboolean is_default); -gboolean nm_active_connection_get_default (NMActiveConnection *self); - -void nm_active_connection_set_default6 (NMActiveConnection *self, - gboolean is_default6); - -gboolean nm_active_connection_get_default6 (NMActiveConnection *self); +gboolean nm_active_connection_get_default (NMActiveConnection *self, int addr_family); NMActiveConnectionState nm_active_connection_get_state (NMActiveConnection *self); diff --git a/src/nm-manager.c b/src/nm-manager.c index 4766e3c8e3..d4d4ba610c 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -814,8 +814,7 @@ find_best_device_state (NMManager *manager) switch (ac_state) { case NM_ACTIVE_CONNECTION_STATE_ACTIVATED: - if ( nm_active_connection_get_default (ac) - || nm_active_connection_get_default6 (ac)) { + if (nm_active_connection_get_default (ac, AF_UNSPEC)) { if (priv->connectivity_state == NM_CONNECTIVITY_FULL) return NM_STATE_CONNECTED_GLOBAL; diff --git a/src/nm-policy.c b/src/nm-policy.c index c65e6d3a4a..71415c4ffa 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -819,8 +819,8 @@ update_system_hostname (NMPolicy *self, const char *msg) static void update_default_ac (NMPolicy *self, - NMActiveConnection *best, - void (*set_active_func)(NMActiveConnection*, gboolean)) + int addr_family, + NMActiveConnection *best) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); const CList *tmp_list; @@ -832,12 +832,12 @@ update_default_ac (NMPolicy *self, */ nm_manager_for_each_active_connection (priv->manager, ac, tmp_list) { if (ac != best) - set_active_func (ac, FALSE); + nm_active_connection_set_default (ac, addr_family, FALSE); } /* Mark new default active connection */ if (best) - set_active_func (best, TRUE); + nm_active_connection_set_default (best, addr_family, TRUE); } static gpointer @@ -959,7 +959,7 @@ update_ip4_routing (NMPolicy *self, gboolean force_update) if (vpn) best = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (vpn)); - update_default_ac (self, best_ac, nm_active_connection_set_default); + update_default_ac (self, AF_INET, best_ac); if (!nm_g_object_ref_set (&priv->default_device4, best)) return; @@ -1042,7 +1042,7 @@ update_ip6_routing (NMPolicy *self, gboolean force_update) if (vpn) best = nm_active_connection_get_device (NM_ACTIVE_CONNECTION (vpn)); - update_default_ac (self, best_ac, nm_active_connection_set_default6); + update_default_ac (self, AF_INET6, best_ac); if (!nm_g_object_ref_set (&priv->default_device6, best)) return; From 1f3f142fedb7e2414032b14ed9240f6f9e880667 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 11:02:06 +0100 Subject: [PATCH 11/34] core/trivial: add code comment --- src/settings/nm-settings-connection.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 538e90ee97..510046fa4d 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2574,8 +2574,13 @@ nm_settings_connection_autoconnect_retries_set (NMSettingsConnection *self, } if (retries) priv->autoconnect_blocked_until = 0; - else + else { + /* XXX: the blocked time must be identical for all connections, otherwise + * the tracking of resetting the retry count in NMPolicy needs adjustment + * (as it would need to re-evaluate the next-timeout everytime a + * connection gets blocked). */ priv->autoconnect_blocked_until = nm_utils_get_monotonic_timestamp_s () + AUTOCONNECT_RESET_RETRIES_TIMER; + } } void From 4e7b05de7981c28eba6db48eb16476372594fed2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 14:52:54 +0100 Subject: [PATCH 12/34] policy: merge reset_autoconnect_*() functions --- src/nm-policy.c | 66 +++++++++++++++++++++++-------------------------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 71415c4ffa..52656e5781 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1401,48 +1401,44 @@ hostname_changed (NMHostnameManager *hostname_manager, GParamSpec *pspec, gpoint update_system_hostname (self, "hostname changed"); } -static void -reset_autoconnect_all (NMPolicy *self, NMDevice *device) +static gboolean +reset_autoconnect_all (NMPolicy *self, + NMDevice *device, /* if present, only reset connections compatible with @device */ + gboolean only_for_failed_secrets) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); NMSettingsConnection *const*connections = NULL; guint i; + gboolean changed; - if (device) { - _LOGD (LOGD_DEVICE, "re-enabling autoconnect for all connections on %s", - nm_device_get_iface (device)); - } else - _LOGD (LOGD_DEVICE, "re-enabling autoconnect for all connections"); + _LOGD (LOGD_DEVICE, "re-enabling autoconnect for all connections%s%s%s", + device ? " on " : "", + device ? nm_device_get_iface (device) : "", + only_for_failed_secrets ? " only for failed secrets" : ""); connections = nm_settings_get_connections (priv->settings, NULL); for (i = 0; connections[i]; i++) { NMSettingsConnection *connection = connections[i]; - if (!device || nm_device_check_connection_compatible (device, NM_CONNECTION (connection))) { - nm_settings_connection_autoconnect_retries_reset (connection); - nm_settings_connection_autoconnect_blocked_reason_set (connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); - } - } -} - -static void -reset_autoconnect_for_failed_secrets (NMPolicy *self) -{ - NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); - NMSettingsConnection *const*connections = NULL; - guint i; - - _LOGD (LOGD_DEVICE, "re-enabling autoconnect for all connections with failed secrets"); - - connections = nm_settings_get_connections (priv->settings, NULL); - for (i = 0; connections[i]; i++) { - NMSettingsConnection *connection = connections[i]; - - if (nm_settings_connection_autoconnect_blocked_reason_get (connection) == NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS) { - nm_settings_connection_autoconnect_retries_reset (connection); - nm_settings_connection_autoconnect_blocked_reason_set (connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); + if ( only_for_failed_secrets + && nm_settings_connection_autoconnect_blocked_reason_get (connection) != NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS) + continue; + + if ( device + && !nm_device_check_connection_compatible (device, NM_CONNECTION (connection))) + continue; + + if (nm_settings_connection_autoconnect_retries_get (connection) == 0) + changed = TRUE; + nm_settings_connection_autoconnect_retries_reset (connection); + + if (nm_settings_connection_autoconnect_blocked_reason_get (connection) != NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE) { + changed = TRUE; + nm_settings_connection_autoconnect_blocked_reason_set (connection, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); } } + return changed; } static void @@ -1457,8 +1453,8 @@ sleeping_changed (NMManager *manager, GParamSpec *pspec, gpointer user_data) /* Reset retries on all connections so they'll checked on wakeup */ if (sleeping || !enabled) { - reset_autoconnect_all (self, NULL); - schedule_activate_all (self); + if (reset_autoconnect_all (self, NULL, FALSE)) + schedule_activate_all (self); } } @@ -1809,7 +1805,7 @@ device_state_changed (NMDevice *device, */ if ( nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_CARRIER && old_state == NM_DEVICE_STATE_UNAVAILABLE) - reset_autoconnect_all (self, device); + reset_autoconnect_all (self, device, FALSE); if (old_state > NM_DEVICE_STATE_DISCONNECTED) update_routing_and_dns (self, FALSE); @@ -2359,8 +2355,8 @@ secret_agent_registered (NMSettings *settings, * reset retries count here and schedule activation, so that the * connections failed due to missing secrets may re-try auto-connection. */ - reset_autoconnect_for_failed_secrets (self); - schedule_activate_all (self); + if (reset_autoconnect_all (self, NULL, TRUE)) + schedule_activate_all (self); } NMDevice * From 955432ca8795dd8164e9a823b5945c12ccfa7b73 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 14:58:09 +0100 Subject: [PATCH 13/34] settings/trivial: rename nm_settings_connection_autoconnect_retries_blocked_until() NMSettingsConnection has 3 properties that are related to autoconnect: - autoconnect_retries - autoconnect_blocked_until - autoconnect_blocked_reason autoconnect_blocked_reason is entirely independent from the other two. A connection have have autoconnect blocked via a blocked-reason, but the retry count is not affected by that. The retry count is an independent mechanism, that may additionally prevent autoconnect. However autoconnect_retries and autoconnect_retries_blocked_until are strongly related. The latter is set if and only if autoconnect_retries is at zero. Rename to reflect that better. --- src/nm-policy.c | 4 ++-- src/settings/nm-settings-connection.c | 16 ++++++++-------- src/settings/nm-settings-connection.h | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 52656e5781..daab2255eb 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1507,7 +1507,7 @@ reset_connections_retries (gpointer user_data) for (i = 0; connections[i]; i++) { NMSettingsConnection *connection = connections[i]; - con_stamp = nm_settings_connection_autoconnect_blocked_until_get (connection); + con_stamp = nm_settings_connection_autoconnect_retries_blocked_until (connection); if (con_stamp == 0) continue; @@ -1737,7 +1737,7 @@ device_state_changed (NMDevice *device, nm_settings_connection_get_id (connection)); /* Schedule a handler to reset retries count */ if (!priv->reset_retries_id) { - gint32 retry_time = nm_settings_connection_autoconnect_blocked_until_get (connection); + 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_s ()), reset_connections_retries, self); diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 510046fa4d..e376e6abf0 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -44,9 +44,9 @@ #define SETTINGS_TIMESTAMPS_FILE NMSTATEDIR "/timestamps" #define SETTINGS_SEEN_BSSIDS_FILE NMSTATEDIR "/seen-bssids" -#define AUTOCONNECT_RETRIES_UNSET -2 -#define AUTOCONNECT_RETRIES_FOREVER -1 -#define AUTOCONNECT_RETRIES_DEFAULT 4 +#define AUTOCONNECT_RETRIES_UNSET -2 +#define AUTOCONNECT_RETRIES_FOREVER -1 +#define AUTOCONNECT_RETRIES_DEFAULT 4 #define AUTOCONNECT_RESET_RETRIES_TIMER 300 /*****************************************************************************/ @@ -111,7 +111,7 @@ typedef struct _NMSettingsConnectionPrivate { GHashTable *seen_bssids; /* Up-to-date BSSIDs that's been seen for the connection */ int autoconnect_retries; - gint32 autoconnect_blocked_until; + gint32 autoconnect_retries_blocked_until; char *filename; } NMSettingsConnectionPrivate; @@ -2573,13 +2573,13 @@ nm_settings_connection_autoconnect_retries_set (NMSettingsConnection *self, priv->autoconnect_retries = retries; } if (retries) - priv->autoconnect_blocked_until = 0; + priv->autoconnect_retries_blocked_until = 0; else { /* XXX: the blocked time must be identical for all connections, otherwise * the tracking of resetting the retry count in NMPolicy needs adjustment * (as it would need to re-evaluate the next-timeout everytime a * connection gets blocked). */ - priv->autoconnect_blocked_until = nm_utils_get_monotonic_timestamp_s () + AUTOCONNECT_RESET_RETRIES_TIMER; + priv->autoconnect_retries_blocked_until = nm_utils_get_monotonic_timestamp_s () + AUTOCONNECT_RESET_RETRIES_TIMER; } } @@ -2590,9 +2590,9 @@ nm_settings_connection_autoconnect_retries_reset (NMSettingsConnection *self) } gint32 -nm_settings_connection_autoconnect_blocked_until_get (NMSettingsConnection *self) +nm_settings_connection_autoconnect_retries_blocked_until (NMSettingsConnection *self) { - return NM_SETTINGS_CONNECTION_GET_PRIVATE (self)->autoconnect_blocked_until; + return NM_SETTINGS_CONNECTION_GET_PRIVATE (self)->autoconnect_retries_blocked_until; } NMSettingsAutoconnectBlockedReason diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index aa79628f7f..78a93fcaae 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -218,7 +218,7 @@ void nm_settings_connection_autoconnect_retries_set (NMSettingsConnection *self, int retries); void nm_settings_connection_autoconnect_retries_reset (NMSettingsConnection *self); -gint32 nm_settings_connection_autoconnect_blocked_until_get (NMSettingsConnection *self); +gint32 nm_settings_connection_autoconnect_retries_blocked_until (NMSettingsConnection *self); NMSettingsAutoconnectBlockedReason nm_settings_connection_autoconnect_blocked_reason_get (NMSettingsConnection *self); void nm_settings_connection_autoconnect_blocked_reason_set (NMSettingsConnection *self, From 071495c754fa4610cf957641272377b689356701 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 16:33:27 +0100 Subject: [PATCH 14/34] policy: minor code cleanup in activate_slave_connections() --- src/nm-policy.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index daab2255eb..121ef877dd 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1579,15 +1579,13 @@ activate_slave_connections (NMPolicy *self, NMDevice *device) || nm_streq0 (slave_master, master_uuid_applied) || nm_streq0 (slave_master, master_uuid_settings)) { NMSettingsConnection *settings = NM_SETTINGS_CONNECTION (slave); - NMSettingsAutoconnectBlockedReason reason; if (!internal_activation) nm_settings_connection_autoconnect_retries_reset (settings); - reason = nm_settings_connection_autoconnect_blocked_reason_get (settings); - if (reason == NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED) { - reason = NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE; - nm_settings_connection_autoconnect_blocked_reason_set (settings, reason); + if (nm_settings_connection_autoconnect_blocked_reason_get (settings) == NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED) { + nm_settings_connection_autoconnect_blocked_reason_set (settings, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); } } } From 3177b18aabc1e2a1a890c6d7a7d05d34c627eb73 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 16:45:27 +0100 Subject: [PATCH 15/34] core: log autoconnect properties of NMSettingsConnection --- src/settings/nm-settings-connection.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index e376e6abf0..ec937f989b 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2550,7 +2550,7 @@ nm_settings_connection_autoconnect_retries_get (NMSettingsConnection *self) if (retries == 0) retries = AUTOCONNECT_RETRIES_FOREVER; - _LOGT ("autoconnect-retries: init %d", retries); + _LOGT ("autoconnect: retries init %d", retries); priv->autoconnect_retries = retries; } @@ -2569,7 +2569,7 @@ nm_settings_connection_autoconnect_retries_set (NMSettingsConnection *self, priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); if (priv->autoconnect_retries != retries) { - _LOGT ("autoconnect-retries: set %d", retries); + _LOGT ("autoconnect: retries set %d", retries); priv->autoconnect_retries = retries; } if (retries) @@ -2605,12 +2605,19 @@ void nm_settings_connection_autoconnect_blocked_reason_set (NMSettingsConnection *self, NMSettingsAutoconnectBlockedReason reason) { - g_return_if_fail (NM_IN_SET (reason, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS)); - NM_SETTINGS_CONNECTION_GET_PRIVATE (self)->autoconnect_blocked_reason = reason; + NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); + + nm_assert (NM_IN_SET (reason, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS)); + + if (priv->autoconnect_blocked_reason == reason) + return; + + _LOGT ("autoconnect: blocked reason: %d", (int) reason); + priv->autoconnect_blocked_reason = reason; } /*****************************************************************************/ From 124b905f97aeb6a82c9bb03076fa58464c7e8ead Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 17:03:22 +0100 Subject: [PATCH 16/34] policy: move setting autoconnect retries to a separate function Note that for the if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_NO_SECRETS) case we no longer do the if (nm_settings_connection_autoconnect_retries_get (connection) == 0) check. But that is fine, because we only skip schedling a reset_connections_retries() action. But note, that that previously we also would never actually scheudle a new timeout, because - either nm_settings_connection_autoconnect_retries_get (connection) != 0 - or the retries count was zero, in which case we already have a reset_connections_retries action pending (from the time when we set it to zero. So, there is no change in behavior at all except dropping of a redundant logging line. --- src/nm-policy.c | 36 ++++++++++++++++++--------- src/settings/nm-settings-connection.c | 4 +-- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 121ef877dd..1d90844ee4 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1529,6 +1529,29 @@ reset_connections_retries (gpointer user_data) return FALSE; } +static void +_connection_autoconnect_retries_set (NMPolicy *self, + NMSettingsConnection *connection, + int tries) +{ + NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); + + nm_assert (NM_IS_SETTINGS_CONNECTION (connection)); + nm_assert (tries >= 0); + + nm_settings_connection_autoconnect_retries_set (connection, tries); + + if (tries == 0) { + /* Schedule a handler to reset retries count */ + if (!priv->reset_retries_id) { + 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_s ()), reset_connections_retries, self); + } + } +} + static void activate_slave_connections (NMPolicy *self, NMDevice *device) { @@ -1723,24 +1746,13 @@ device_state_changed (NMDevice *device, if (tries > 0) { _LOGD (LOGD_DEVICE, "connection '%s' failed to autoconnect; %d tries left", nm_settings_connection_get_id (connection), tries); - nm_settings_connection_autoconnect_retries_set (connection, --tries); + _connection_autoconnect_retries_set (self, connection, tries - 1); } else { _LOGD (LOGD_DEVICE, "connection '%s' failed to autoconnect; infinite tries left", nm_settings_connection_get_id (connection)); } } - if (nm_settings_connection_autoconnect_retries_get (connection) == 0) { - _LOGI (LOGD_DEVICE, "disabling autoconnect for connection '%s'.", - nm_settings_connection_get_id (connection)); - /* Schedule a handler to reset retries count */ - if (!priv->reset_retries_id) { - 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_s ()), reset_connections_retries, self); - } - } nm_connection_clear_secrets (NM_CONNECTION (connection)); } break; diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index ec937f989b..3babbf9b76 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2577,8 +2577,8 @@ nm_settings_connection_autoconnect_retries_set (NMSettingsConnection *self, else { /* XXX: the blocked time must be identical for all connections, otherwise * the tracking of resetting the retry count in NMPolicy needs adjustment - * (as it would need to re-evaluate the next-timeout everytime a - * connection gets blocked). */ + * in _connection_autoconnect_retries_set() (as it would need to re-evaluate + * the next-timeout everytime a connection gets blocked). */ priv->autoconnect_retries_blocked_until = nm_utils_get_monotonic_timestamp_s () + AUTOCONNECT_RESET_RETRIES_TIMER; } } From 3d118f5b4921e538e6885162af73fd4a67c40f3c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 20:09:47 +0100 Subject: [PATCH 17/34] policy: don't log GError code Our GError codes are mostly meaningless, only the message is interesting. And our messages should anyway be unique, so that one could understand which was the corresponding error code (by inspecting the source code). While at it, use gs_free_error. --- src/nm-policy.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 1d90844ee4..65bbeada7a 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1191,7 +1191,7 @@ auto_activate_device (NMPolicy *self, gs_free char *specific_object = NULL; gs_free NMSettingsConnection **connections = NULL; guint i, len; - GError *error = NULL; + gs_free_error GError *error = NULL; NMAuthSubject *subject; NMActiveConnection *ac; @@ -1256,11 +1256,9 @@ auto_activate_device (NMPolicy *self, NM_ACTIVATION_TYPE_MANAGED, &error); if (!ac) { - _LOGI (LOGD_DEVICE, "connection '%s' auto-activation failed: (%d) %s", + _LOGI (LOGD_DEVICE, "connection '%s' auto-activation failed: %s", nm_settings_connection_get_id (best_connection), - error->code, error->message); - g_error_free (error); nm_settings_connection_autoconnect_blocked_reason_set (best_connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED); schedule_activate_check (self, device); From a91dfa6a27487617f57de89e8e5a9e8b97a45396 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 21:06:31 +0100 Subject: [PATCH 18/34] core: don't explicitly unset autoconnect retry counter NMPolicy would at various time call nm_settings_connection_autoconnect_retries_reset() followed by nm_settings_connection_autoconnect_retries_get(). This resulted in two logging messages, first to indicate that the value was unset, and then reset it to the value from configuration. While that is correct, it causes a lot of verbose logging. Especially for all connections which autoconnect retry counter didn't actually change. The advantage of that was, that we only loaded the actual value when we need it the first time (during get()). That means, the user could reload the configuration, and the value would be loaded and cached at a later pointer. However, the duplicate logging was annoying, but we still want to see a message about the resetting. So, now during reset load the value setting from NetworkManager.conf and set it right away. Skip the intermediate UNSET value. In most cases nothing changed now, and we don't log anything for most connections. --- src/settings/nm-settings-connection.c | 104 +++++++++++++++----------- 1 file changed, 62 insertions(+), 42 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 3babbf9b76..38e9ea3235 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2516,6 +2516,58 @@ nm_settings_connection_read_and_fill_seen_bssids (NMSettingsConnection *self) /*****************************************************************************/ +static int +_autoconnect_retries_initial (NMSettingsConnection *self) +{ + NMSettingConnection *s_con; + int retries = -1; + + s_con = nm_connection_get_setting_connection ((NMConnection *) self); + if (s_con) + retries = nm_setting_connection_get_autoconnect_retries (s_con); + + /* -1 means 'default' */ + if (retries == -1) { + retries = nm_config_data_get_value_int64 (NM_CONFIG_GET_DATA, + NM_CONFIG_KEYFILE_GROUP_MAIN, + "autoconnect-retries-default", + 10, 0, G_MAXINT32, + AUTOCONNECT_RETRIES_DEFAULT); + } + + /* 0 means 'forever', which is translated to a retry count of -1 */ + if (retries == 0) + retries = AUTOCONNECT_RETRIES_FOREVER; + + return retries; +} + +static void +_autoconnect_retries_set (NMSettingsConnection *self, + int retries, + gboolean is_reset) +{ + NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); + + g_return_if_fail (retries == AUTOCONNECT_RETRIES_FOREVER || retries >= 0); + + if (priv->autoconnect_retries != retries) { + _LOGT ("autoconnect: retries set %d%s", retries, + is_reset ? " (reset)" : ""); + priv->autoconnect_retries = retries; + } + + if (retries) + priv->autoconnect_retries_blocked_until = 0; + else { + /* XXX: the blocked time must be identical for all connections, otherwise + * the tracking of resetting the retry count in NMPolicy needs adjustment + * in _connection_autoconnect_retries_set() (as it would need to re-evaluate + * the next-timeout everytime a connection gets blocked). */ + priv->autoconnect_retries_blocked_until = nm_utils_get_monotonic_timestamp_s () + AUTOCONNECT_RESET_RETRIES_TIMER; + } +} + /** * nm_settings_connection_autoconnect_retries_get: * @self: the settings connection @@ -2530,30 +2582,10 @@ nm_settings_connection_autoconnect_retries_get (NMSettingsConnection *self) NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); if (G_UNLIKELY (priv->autoconnect_retries == AUTOCONNECT_RETRIES_UNSET)) { - NMSettingConnection *s_con; - int retries = -1; - - s_con = nm_connection_get_setting_connection ((NMConnection *) self); - if (s_con) - retries = nm_setting_connection_get_autoconnect_retries (s_con); - - /* -1 means 'default' */ - if (retries == -1) { - retries = nm_config_data_get_value_int64 (NM_CONFIG_GET_DATA, - NM_CONFIG_KEYFILE_GROUP_MAIN, - "autoconnect-retries-default", - 10, 0, G_MAXINT32, - AUTOCONNECT_RETRIES_DEFAULT); - } - - /* 0 means 'forever', which is translated to a retry count of -1 */ - if (retries == 0) - retries = AUTOCONNECT_RETRIES_FOREVER; - - _LOGT ("autoconnect: retries init %d", retries); - priv->autoconnect_retries = retries; + _autoconnect_retries_set (self, + _autoconnect_retries_initial (self), + TRUE); } - return priv->autoconnect_retries; } @@ -2561,32 +2593,20 @@ void nm_settings_connection_autoconnect_retries_set (NMSettingsConnection *self, int retries) { - NMSettingsConnectionPrivate *priv; - g_return_if_fail (NM_IS_SETTINGS_CONNECTION (self)); - nm_assert (retries == AUTOCONNECT_RETRIES_UNSET || retries >= 0); + g_return_if_fail (retries >= 0); - priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); - - if (priv->autoconnect_retries != retries) { - _LOGT ("autoconnect: retries set %d", retries); - priv->autoconnect_retries = retries; - } - if (retries) - priv->autoconnect_retries_blocked_until = 0; - else { - /* XXX: the blocked time must be identical for all connections, otherwise - * the tracking of resetting the retry count in NMPolicy needs adjustment - * in _connection_autoconnect_retries_set() (as it would need to re-evaluate - * the next-timeout everytime a connection gets blocked). */ - priv->autoconnect_retries_blocked_until = nm_utils_get_monotonic_timestamp_s () + AUTOCONNECT_RESET_RETRIES_TIMER; - } + _autoconnect_retries_set (self, retries, FALSE); } void nm_settings_connection_autoconnect_retries_reset (NMSettingsConnection *self) { - nm_settings_connection_autoconnect_retries_set (self, AUTOCONNECT_RETRIES_UNSET); + g_return_if_fail (NM_IS_SETTINGS_CONNECTION (self)); + + _autoconnect_retries_set (self, + _autoconnect_retries_initial (self), + TRUE); } gint32 From 1c631bda4e38371cc5aec1c7bbdfd05c1a8ff14d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 21:11:54 +0100 Subject: [PATCH 19/34] core: use #define for "autoconnect-retries-default" config All our known configuration keys should have a #define, so that all keys are collected in the header file. --- src/nm-config.h | 1 + src/settings/nm-settings-connection.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nm-config.h b/src/nm-config.h index 47e929884c..17912185f7 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -60,6 +60,7 @@ #define NM_CONFIG_KEYFILE_GROUP_IFNET "ifnet" #define NM_CONFIG_KEYFILE_KEY_MAIN_AUTH_POLKIT "auth-polkit" +#define NM_CONFIG_KEYFILE_KEY_MAIN_AUTOCONNECT_RETRIES_DEFAULT "autoconnect-retries-default" #define NM_CONFIG_KEYFILE_KEY_MAIN_DHCP "dhcp" #define NM_CONFIG_KEYFILE_KEY_MAIN_DEBUG "debug" #define NM_CONFIG_KEYFILE_KEY_MAIN_HOSTNAME_MODE "hostname-mode" diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 38e9ea3235..6bca549b66 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2530,7 +2530,7 @@ _autoconnect_retries_initial (NMSettingsConnection *self) if (retries == -1) { retries = nm_config_data_get_value_int64 (NM_CONFIG_GET_DATA, NM_CONFIG_KEYFILE_GROUP_MAIN, - "autoconnect-retries-default", + NM_CONFIG_KEYFILE_KEY_MAIN_AUTOCONNECT_RETRIES_DEFAULT, 10, 0, G_MAXINT32, AUTOCONNECT_RETRIES_DEFAULT); } From af703ba99061d0ca4c7704b69558179b5da13798 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 21:19:40 +0100 Subject: [PATCH 20/34] core: cache "autoconnect-retries-default" in NMConfigData It's not ever going to change(*), and NMPolicy calls reset() a lot. No need to lookup the configuration in the GKeyFile every time. (*) per NMConfigData instance. The config may be reloaded, in which case NMConfig creates a new NMConfigData instance, but the NMConfigData instance itself is immutable. --- src/nm-config-data.c | 24 ++++++++++++++++++------ src/nm-config-data.h | 2 ++ src/settings/nm-settings-connection.c | 11 +++-------- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/nm-config-data.c b/src/nm-config-data.c index 5b06e00a16..00e5c635b5 100644 --- a/src/nm-config-data.c +++ b/src/nm-config-data.c @@ -95,6 +95,8 @@ typedef struct { guint interval; } connectivity; + int autoconnect_retries_default; + struct { char **arr; GSList *specs; @@ -275,6 +277,14 @@ nm_config_data_get_connectivity_response (const NMConfigData *self) return NM_CONFIG_DATA_GET_PRIVATE (self)->connectivity.response; } +int +nm_config_data_get_autoconnect_retries_default (const NMConfigData *self) +{ + g_return_val_if_fail (self, FALSE); + + return NM_CONFIG_DATA_GET_PRIVATE (self)->autoconnect_retries_default; +} + const char *const* nm_config_data_get_no_auto_default (const NMConfigData *self) { @@ -1527,7 +1537,7 @@ constructed (GObject *object) { NMConfigData *self = NM_CONFIG_DATA (object); NMConfigDataPrivate *priv = NM_CONFIG_DATA_GET_PRIVATE (self); - char *interval; + char *str; priv->keyfile = _merge_keyfiles (priv->keyfile_user, priv->keyfile_intern); @@ -1538,13 +1548,15 @@ constructed (GObject *object) priv->connectivity.uri = nm_strstrip (g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "uri", NULL)); priv->connectivity.response = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "response", NULL); + str = nm_config_keyfile_get_value (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, NM_CONFIG_KEYFILE_KEY_MAIN_AUTOCONNECT_RETRIES_DEFAULT, NM_CONFIG_GET_VALUE_NONE); + priv->autoconnect_retries_default = _nm_utils_ascii_str_to_int64 (str, 10, 0, G_MAXINT32, 4); + g_free (str); + /* On missing config value, fallback to 300. On invalid value, disable connectivity checking by setting * the interval to zero. */ - interval = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "interval", NULL); - priv->connectivity.interval = interval - ? _nm_utils_ascii_str_to_int64 (interval, 10, 0, G_MAXUINT, 0) - : NM_CONFIG_DEFAULT_CONNECTIVITY_INTERVAL; - g_free (interval); + str = g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_CONNECTIVITY, "interval", NULL); + priv->connectivity.interval = _nm_utils_ascii_str_to_int64 (str, 10, 0, G_MAXUINT, NM_CONFIG_DEFAULT_CONNECTIVITY_INTERVAL); + g_free (str); priv->dns_mode = nm_strstrip (g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "dns", NULL)); priv->rc_manager = nm_strstrip (g_key_file_get_string (priv->keyfile, NM_CONFIG_KEYFILE_GROUP_MAIN, "rc-manager", NULL)); diff --git a/src/nm-config-data.h b/src/nm-config-data.h index 3efe4259d5..3092a87f6c 100644 --- a/src/nm-config-data.h +++ b/src/nm-config-data.h @@ -165,6 +165,8 @@ const char *nm_config_data_get_connectivity_uri (const NMConfigData *config_data guint nm_config_data_get_connectivity_interval (const NMConfigData *config_data); const char *nm_config_data_get_connectivity_response (const NMConfigData *config_data); +int nm_config_data_get_autoconnect_retries_default (const NMConfigData *config_data); + const char *const*nm_config_data_get_no_auto_default (const NMConfigData *config_data); gboolean nm_config_data_get_no_auto_default_for_device (const NMConfigData *self, NMDevice *device); diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 6bca549b66..544837123d 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -46,7 +46,6 @@ #define AUTOCONNECT_RETRIES_UNSET -2 #define AUTOCONNECT_RETRIES_FOREVER -1 -#define AUTOCONNECT_RETRIES_DEFAULT 4 #define AUTOCONNECT_RESET_RETRIES_TIMER 300 /*****************************************************************************/ @@ -2527,18 +2526,14 @@ _autoconnect_retries_initial (NMSettingsConnection *self) retries = nm_setting_connection_get_autoconnect_retries (s_con); /* -1 means 'default' */ - if (retries == -1) { - retries = nm_config_data_get_value_int64 (NM_CONFIG_GET_DATA, - NM_CONFIG_KEYFILE_GROUP_MAIN, - NM_CONFIG_KEYFILE_KEY_MAIN_AUTOCONNECT_RETRIES_DEFAULT, - 10, 0, G_MAXINT32, - AUTOCONNECT_RETRIES_DEFAULT); - } + if (retries == -1) + retries = nm_config_data_get_autoconnect_retries_default (NM_CONFIG_GET_DATA); /* 0 means 'forever', which is translated to a retry count of -1 */ if (retries == 0) retries = AUTOCONNECT_RETRIES_FOREVER; + nm_assert (retries == AUTOCONNECT_RETRIES_FOREVER || retries >= 0); return retries; } From 5fc4b326299c24ac06feb7f55d58134522f032e0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 21:27:47 +0100 Subject: [PATCH 21/34] policy: fix logging about autoconnect retries number NMPolicy printed policy: connection 'a' failed to autoconnect; 1 tries left settings-connection[0x55a485553b60,ab9f3891-3420-335e-89da-f14c1b94c540]: autoconnect: retries set 0 That is, it claimed there was one more try, when in fact there wasn't. --- src/nm-policy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 65bbeada7a..6696c6c4b4 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1743,7 +1743,7 @@ device_state_changed (NMDevice *device, } else if (tries != 0) { if (tries > 0) { _LOGD (LOGD_DEVICE, "connection '%s' failed to autoconnect; %d tries left", - nm_settings_connection_get_id (connection), tries); + nm_settings_connection_get_id (connection), tries - 1); _connection_autoconnect_retries_set (self, connection, tries - 1); } else { _LOGD (LOGD_DEVICE, "connection '%s' failed to autoconnect; infinite tries left", From 40b534c5d8a2be48742bf44576381a73f6e05fb6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Nov 2017 22:23:13 +0100 Subject: [PATCH 22/34] policy: minor cleanup of loop in activate_slave_connections() --- src/nm-policy.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 6696c6c4b4..cc5fd1b095 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1584,30 +1584,24 @@ activate_slave_connections (NMPolicy *self, NMDevice *device) connections = nm_settings_get_connections (priv->settings, NULL); for (i = 0; connections[i]; i++) { - NMConnection *slave; + NMSettingsConnection *connection = connections[i]; NMSettingConnection *s_slave_con; const char *slave_master; - slave = NM_CONNECTION (connections[i]); - - s_slave_con = nm_connection_get_setting_connection (slave); - g_assert (s_slave_con); + s_slave_con = nm_connection_get_setting_connection (NM_CONNECTION (connection)); slave_master = nm_setting_connection_get_master (s_slave_con); if (!slave_master) continue; + if (!NM_IN_STRSET (slave_master, master_device, + master_uuid_applied, + master_uuid_settings)) + continue; - if ( nm_streq0 (slave_master, master_device) - || nm_streq0 (slave_master, master_uuid_applied) - || nm_streq0 (slave_master, master_uuid_settings)) { - NMSettingsConnection *settings = NM_SETTINGS_CONNECTION (slave); - - if (!internal_activation) - nm_settings_connection_autoconnect_retries_reset (settings); - - if (nm_settings_connection_autoconnect_blocked_reason_get (settings) == NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED) { - nm_settings_connection_autoconnect_blocked_reason_set (settings, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); - } + if (!internal_activation) + nm_settings_connection_autoconnect_retries_reset (connection); + if (nm_settings_connection_autoconnect_blocked_reason_get (connection) == NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED) { + nm_settings_connection_autoconnect_blocked_reason_set (connection, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); } } From ddb8713d443d3953594feddbdd88b3f40cf97524 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 10:33:23 +0100 Subject: [PATCH 23/34] policy: minor refactoring of device_state_changed() --- src/nm-policy.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index cc5fd1b095..cbfc25ee8c 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1781,21 +1781,24 @@ device_state_changed (NMDevice *device, update_routing_and_dns (self, FALSE); break; case NM_DEVICE_STATE_DEACTIVATING: - if (NM_IN_SET (nm_device_state_reason_check (reason), - NM_DEVICE_STATE_REASON_USER_REQUESTED, - NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED)) { - if (connection) { - NMSettingsAutoconnectBlockedReason blocked_reason; + if (connection) { + NMSettingsAutoconnectBlockedReason blocked_reason = NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE; - /* The connection was deactivated, so block just this connection */ + switch (nm_device_state_reason_check (reason)) { + case NM_DEVICE_STATE_REASON_USER_REQUESTED: + blocked_reason = NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST; + break; + case NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED: + blocked_reason = NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED; + break; + default: + break; + } + if (blocked_reason != NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE) { _LOGD (LOGD_DEVICE, "blocking autoconnect of connection '%s': %s", nm_settings_connection_get_id (connection), NM_UTILS_LOOKUP_STR (nm_device_state_reason_to_str, nm_device_state_reason_check (reason))); - if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_USER_REQUESTED) - blocked_reason = NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST; - else - blocked_reason = NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED; nm_settings_connection_autoconnect_blocked_reason_set (connection, blocked_reason); } } From 8d2d9b0748b856023eb5790043e3143cd8bb8450 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 10:53:39 +0100 Subject: [PATCH 24/34] policy: track autoconnect-blocked-reasons as flags Extend the enum and API to use flags for the blocked reasons. A connection is blocked from autoconnect if it has any reason set. There is no behavioral change in this patch beyond that, because where we previously would set blocked-reason NONE, we would still clear all flags, and not only a particular one. Later of course, we want to set and clear individual flags independently. --- src/nm-policy.c | 32 ++++++++++++--------- src/settings/nm-settings-connection.c | 41 +++++++++++++++++---------- src/settings/nm-settings-connection.h | 27 ++++++++++++++---- 3 files changed, 65 insertions(+), 35 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index cbfc25ee8c..4e7b9a0bcf 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1170,7 +1170,7 @@ pending_ac_state_changed (NMActiveConnection *ac, guint state, guint reason, NMP * loop. */ con = nm_active_connection_get_settings_connection (ac); - nm_settings_connection_autoconnect_blocked_reason_set (con, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED); + nm_settings_connection_autoconnect_blocked_reason_set (con, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, TRUE); schedule_activate_check (self, nm_active_connection_get_device (ac)); /* Cleanup */ @@ -1223,7 +1223,7 @@ auto_activate_device (NMPolicy *self, if ( !nm_settings_connection_is_visible (candidate) || nm_settings_connection_autoconnect_retries_get (candidate) == 0 - || nm_settings_connection_autoconnect_blocked_reason_get (candidate) != NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE) + || nm_settings_connection_autoconnect_blocked_reason_get (candidate, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL)) continue; s_con = nm_connection_get_setting_connection (NM_CONNECTION (candidate)); @@ -1260,7 +1260,8 @@ auto_activate_device (NMPolicy *self, nm_settings_connection_get_id (best_connection), error->message); nm_settings_connection_autoconnect_blocked_reason_set (best_connection, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED); + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, + TRUE); schedule_activate_check (self, device); return; } @@ -1419,7 +1420,8 @@ reset_autoconnect_all (NMPolicy *self, NMSettingsConnection *connection = connections[i]; if ( only_for_failed_secrets - && nm_settings_connection_autoconnect_blocked_reason_get (connection) != NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS) + && !nm_settings_connection_autoconnect_blocked_reason_get (connection, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS)) continue; if ( device @@ -1430,11 +1432,10 @@ reset_autoconnect_all (NMPolicy *self, changed = TRUE; nm_settings_connection_autoconnect_retries_reset (connection); - if (nm_settings_connection_autoconnect_blocked_reason_get (connection) != NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE) { + if (nm_settings_connection_autoconnect_blocked_reason_set (connection, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL, + FALSE)) changed = TRUE; - nm_settings_connection_autoconnect_blocked_reason_set (connection, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); - } } return changed; } @@ -1599,9 +1600,11 @@ activate_slave_connections (NMPolicy *self, NMDevice *device) if (!internal_activation) nm_settings_connection_autoconnect_retries_reset (connection); - if (nm_settings_connection_autoconnect_blocked_reason_get (connection) == NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED) { + if (nm_settings_connection_autoconnect_blocked_reason_get (connection, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED)) { nm_settings_connection_autoconnect_blocked_reason_set (connection, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL, + FALSE); } } @@ -1711,7 +1714,8 @@ device_state_changed (NMDevice *device, */ if (connection) { nm_settings_connection_autoconnect_blocked_reason_set (connection, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED); + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, + TRUE); } break; default: @@ -1733,7 +1737,7 @@ device_state_changed (NMDevice *device, _LOGD (LOGD_DEVICE, "connection '%s' now blocked from autoconnect due to no secrets", nm_settings_connection_get_id (connection)); - nm_settings_connection_autoconnect_blocked_reason_set (connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS); + nm_settings_connection_autoconnect_blocked_reason_set (connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS, TRUE); } else if (tries != 0) { if (tries > 0) { _LOGD (LOGD_DEVICE, "connection '%s' failed to autoconnect; %d tries left", @@ -1799,7 +1803,7 @@ device_state_changed (NMDevice *device, nm_settings_connection_get_id (connection), NM_UTILS_LOOKUP_STR (nm_device_state_reason_to_str, nm_device_state_reason_check (reason))); - nm_settings_connection_autoconnect_blocked_reason_set (connection, blocked_reason); + nm_settings_connection_autoconnect_blocked_reason_set (connection, blocked_reason, TRUE); } } ip6_remove_device_prefix_delegations (self, device); @@ -1836,7 +1840,7 @@ device_state_changed (NMDevice *device, case NM_DEVICE_STATE_IP_CONFIG: /* We must have secrets if we got here. */ if (connection) - nm_settings_connection_autoconnect_blocked_reason_set (connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); + nm_settings_connection_autoconnect_blocked_reason_set (connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL, FALSE); break; case NM_DEVICE_STATE_SECONDARIES: if (connection) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 544837123d..b7f16f21b2 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -85,7 +85,7 @@ typedef struct _NMSettingsConnectionPrivate { bool timestamp_set:1; - NMSettingsAutoconnectBlockedReason autoconnect_blocked_reason:3; + NMSettingsAutoconnectBlockedReason autoconnect_blocked_reason:4; GSList *pending_auths; /* List of pending authentication requests */ @@ -2610,29 +2610,40 @@ nm_settings_connection_autoconnect_retries_blocked_until (NMSettingsConnection * return NM_SETTINGS_CONNECTION_GET_PRIVATE (self)->autoconnect_retries_blocked_until; } +NM_UTILS_FLAGS2STR_DEFINE_STATIC (_autoconnect_blocked_reason_to_string, NMSettingsAutoconnectBlockedReason, + NM_UTILS_FLAGS2STR (NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE, "none"), + NM_UTILS_FLAGS2STR (NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST, "user-request"), + NM_UTILS_FLAGS2STR (NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, "failed"), + NM_UTILS_FLAGS2STR (NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS, "no-secrets"), +); + NMSettingsAutoconnectBlockedReason -nm_settings_connection_autoconnect_blocked_reason_get (NMSettingsConnection *self) +nm_settings_connection_autoconnect_blocked_reason_get (NMSettingsConnection *self, NMSettingsAutoconnectBlockedReason mask) { - return NM_SETTINGS_CONNECTION_GET_PRIVATE (self)->autoconnect_blocked_reason; + return NM_SETTINGS_CONNECTION_GET_PRIVATE (self)->autoconnect_blocked_reason & (mask ?: NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL); } -void -nm_settings_connection_autoconnect_blocked_reason_set (NMSettingsConnection *self, - NMSettingsAutoconnectBlockedReason reason) +gboolean +nm_settings_connection_autoconnect_blocked_reason_set_full (NMSettingsConnection *self, + NMSettingsAutoconnectBlockedReason mask, + NMSettingsAutoconnectBlockedReason value) { + NMSettingsAutoconnectBlockedReason v; NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); + char buf[100]; - nm_assert (NM_IN_SET (reason, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS)); + nm_assert (mask); + nm_assert (!NM_FLAGS_ANY (value, ~mask)); - if (priv->autoconnect_blocked_reason == reason) - return; + v = priv->autoconnect_blocked_reason; + v = (v & ~mask) | (value & mask); - _LOGT ("autoconnect: blocked reason: %d", (int) reason); - priv->autoconnect_blocked_reason = reason; + if (priv->autoconnect_blocked_reason == v) + return FALSE; + + _LOGT ("autoconnect: blocked reason: %s", _autoconnect_blocked_reason_to_string (v, buf, sizeof (buf))); + priv->autoconnect_blocked_reason = v; + return TRUE; } /*****************************************************************************/ diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index 78a93fcaae..5950ef4267 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -84,9 +84,14 @@ typedef enum { /*< skip >*/ typedef enum { NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE = 0, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST = 1, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED = 2, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS = 3, + + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST = (1LL << 0), + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED = (1LL << 1), + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS = (1LL << 2), + + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL = ( NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST + | NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED + | NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS), } NMSettingsAutoconnectBlockedReason; struct _NMSettingsConnectionCallId; @@ -220,9 +225,19 @@ void nm_settings_connection_autoconnect_retries_reset (NMSettingsConnection *sel gint32 nm_settings_connection_autoconnect_retries_blocked_until (NMSettingsConnection *self); -NMSettingsAutoconnectBlockedReason nm_settings_connection_autoconnect_blocked_reason_get (NMSettingsConnection *self); -void nm_settings_connection_autoconnect_blocked_reason_set (NMSettingsConnection *self, - NMSettingsAutoconnectBlockedReason reason); +NMSettingsAutoconnectBlockedReason nm_settings_connection_autoconnect_blocked_reason_get (NMSettingsConnection *self, + NMSettingsAutoconnectBlockedReason mask); +gboolean nm_settings_connection_autoconnect_blocked_reason_set_full (NMSettingsConnection *self, + NMSettingsAutoconnectBlockedReason mask, + NMSettingsAutoconnectBlockedReason value); + +static inline gboolean +nm_settings_connection_autoconnect_blocked_reason_set (NMSettingsConnection *self, + NMSettingsAutoconnectBlockedReason mask, + gboolean set) +{ + return nm_settings_connection_autoconnect_blocked_reason_set_full (self, mask, set ? mask : NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); +} gboolean nm_settings_connection_get_nm_generated (NMSettingsConnection *self); gboolean nm_settings_connection_get_volatile (NMSettingsConnection *self); From b154f29b2be8e8c11ab0a8efef4c8216c9fb2445 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 12:14:42 +0100 Subject: [PATCH 25/34] policy: only reset no-secrets autoconnect block flag when secret agent registers The other blocked reasons still hold. If autoconnect was blocked for other reasons then no-secrets, a secret agent should not unblock them all. --- src/nm-policy.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 4e7b9a0bcf..d39f8cc848 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1403,7 +1403,7 @@ hostname_changed (NMHostnameManager *hostname_manager, GParamSpec *pspec, gpoint static gboolean reset_autoconnect_all (NMPolicy *self, NMDevice *device, /* if present, only reset connections compatible with @device */ - gboolean only_for_failed_secrets) + gboolean only_no_secrets) { NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self); NMSettingsConnection *const*connections = NULL; @@ -1413,29 +1413,37 @@ reset_autoconnect_all (NMPolicy *self, _LOGD (LOGD_DEVICE, "re-enabling autoconnect for all connections%s%s%s", device ? " on " : "", device ? nm_device_get_iface (device) : "", - only_for_failed_secrets ? " only for failed secrets" : ""); + only_no_secrets ? " (only clear no-secrets flag)" : ""); connections = nm_settings_get_connections (priv->settings, NULL); for (i = 0; connections[i]; i++) { NMSettingsConnection *connection = connections[i]; - if ( only_for_failed_secrets - && !nm_settings_connection_autoconnect_blocked_reason_get (connection, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS)) - continue; - if ( device && !nm_device_check_connection_compatible (device, NM_CONNECTION (connection))) continue; - if (nm_settings_connection_autoconnect_retries_get (connection) == 0) - changed = TRUE; - nm_settings_connection_autoconnect_retries_reset (connection); + if (only_no_secrets) { + /* we only reset the no-secrets blocked flag. */ + if (nm_settings_connection_autoconnect_blocked_reason_set (connection, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS, + FALSE)) { + /* maybe the connection is still blocked afterwards for other reasons + * and in the larger picture nothing changed. But it's too complicated + * to find out exactly. Just assume, something changed to be sure. */ + changed = TRUE; + } + } else { + /* we reset the tries-count and any blocked-reason */ + if (nm_settings_connection_autoconnect_retries_get (connection) == 0) + changed = TRUE; + nm_settings_connection_autoconnect_retries_reset (connection); - if (nm_settings_connection_autoconnect_blocked_reason_set (connection, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL, - FALSE)) - changed = TRUE; + if (nm_settings_connection_autoconnect_blocked_reason_set (connection, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL, + FALSE)) + changed = TRUE; + } } return changed; } From 3a826a83df6b3cc36b2f57654864c3a37ca8ec25 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 12:45:23 +0100 Subject: [PATCH 26/34] policy: only reset autoconnect-blocked-reason FAILED in activate_slave_connections() The reasons no-secret and user-request still hold. --- src/nm-policy.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index d39f8cc848..7cb8379587 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1608,12 +1608,9 @@ activate_slave_connections (NMPolicy *self, NMDevice *device) if (!internal_activation) nm_settings_connection_autoconnect_retries_reset (connection); - if (nm_settings_connection_autoconnect_blocked_reason_get (connection, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED)) { - nm_settings_connection_autoconnect_blocked_reason_set (connection, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL, - FALSE); - } + nm_settings_connection_autoconnect_blocked_reason_set (connection, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, + FALSE); } schedule_activate_all (self); From 5e5121a4838cd82537351b28fd027673e7a50ddd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 12:47:59 +0100 Subject: [PATCH 27/34] policy: only selectively schedule autoconnect check in activate_slave_connections() schedule_activate_all() is expensive. It iterates over all devices, and asks them to autoactivated (which might involve iterating over all connections for each device). Avoid it if nothing changed. --- src/nm-policy.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 7cb8379587..f31b1cbb36 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1568,6 +1568,7 @@ activate_slave_connections (NMPolicy *self, NMDevice *device) NMActRequest *req; gboolean internal_activation = FALSE; NMSettingsConnection *const*connections; + gboolean changed; master_device = nm_device_get_iface (device); g_assert (master_device); @@ -1591,6 +1592,7 @@ activate_slave_connections (NMPolicy *self, NMDevice *device) internal_activation = subject && nm_auth_subject_is_internal (subject); } + changed = FALSE; connections = nm_settings_get_connections (priv->settings, NULL); for (i = 0; connections[i]; i++) { NMSettingsConnection *connection = connections[i]; @@ -1606,14 +1608,19 @@ activate_slave_connections (NMPolicy *self, NMDevice *device) master_uuid_settings)) continue; - if (!internal_activation) + if (!internal_activation) { + if (nm_settings_connection_autoconnect_retries_get (connection) == 0) + changed = TRUE; nm_settings_connection_autoconnect_retries_reset (connection); - nm_settings_connection_autoconnect_blocked_reason_set (connection, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, - FALSE); + } + if (nm_settings_connection_autoconnect_blocked_reason_set (connection, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, + FALSE)) + changed = TRUE; } - schedule_activate_all (self); + if (changed) + schedule_activate_all (self); } static gboolean From d8b4403f6bf46b209071c138ef388737ee5a8770 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 12:54:31 +0100 Subject: [PATCH 28/34] policy: don't clear autoconnect-blocked-reason user-request on internal reasons reset_autoconnect_all() as two callers with only_no_secrets=FALSE: - sleeping_changed() when NM returns from sleep. - when device changes to state DISCONNECTED with reason CARRIER. In both cases, this should not overwrite a previous decision by the user that the connection should not autoconnect. --- src/nm-policy.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index f31b1cbb36..c9a9934854 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1440,7 +1440,8 @@ reset_autoconnect_all (NMPolicy *self, nm_settings_connection_autoconnect_retries_reset (connection); if (nm_settings_connection_autoconnect_blocked_reason_set (connection, - NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL + & ~NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST, FALSE)) changed = TRUE; } From 36ac08c092afe7a14b0fe64ada8b6d6f43a1b8c7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 13:20:55 +0100 Subject: [PATCH 29/34] policy: add nm_settings_connection_autoconnect_is_blocked() helper function --- src/nm-policy.c | 3 +-- src/settings/nm-settings-connection.c | 9 +++++++++ src/settings/nm-settings-connection.h | 2 ++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index c9a9934854..81d4db9376 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1222,8 +1222,7 @@ auto_activate_device (NMPolicy *self, const char *permission; if ( !nm_settings_connection_is_visible (candidate) - || nm_settings_connection_autoconnect_retries_get (candidate) == 0 - || nm_settings_connection_autoconnect_blocked_reason_get (candidate, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL)) + || nm_settings_connection_autoconnect_is_blocked (candidate)) continue; s_con = nm_connection_get_setting_connection (NM_CONNECTION (candidate)); diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index b7f16f21b2..71ef2e49ad 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2646,6 +2646,15 @@ nm_settings_connection_autoconnect_blocked_reason_set_full (NMSettingsConnection return TRUE; } +gboolean +nm_settings_connection_autoconnect_is_blocked (NMSettingsConnection *self) +{ + NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); + + return !priv->autoconnect_blocked_reason + && priv->autoconnect_retries != 0; +} + /*****************************************************************************/ /** diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index 5950ef4267..7caf42b717 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -239,6 +239,8 @@ nm_settings_connection_autoconnect_blocked_reason_set (NMSettingsConnection *sel return nm_settings_connection_autoconnect_blocked_reason_set_full (self, mask, set ? mask : NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NONE); } +gboolean nm_settings_connection_autoconnect_is_blocked (NMSettingsConnection *self); + gboolean nm_settings_connection_get_nm_generated (NMSettingsConnection *self); gboolean nm_settings_connection_get_volatile (NMSettingsConnection *self); From f0147a9c476e49aad373591f7ef1029b683cd560 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 12:59:15 +0100 Subject: [PATCH 30/34] policy: avoid false positives for checking for autoactivation We want to only check for autoconnect all, if something happend that makes it possible that we can autoconnect now (while we couldn't previously). It's not a real problem to check more often then strictly necessary. But add a check to rule out a few false-positives to avoid the overhead of checking all devices for autoconnect. --- src/nm-policy.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 81d4db9376..d04727daec 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1430,7 +1430,8 @@ reset_autoconnect_all (NMPolicy *self, /* maybe the connection is still blocked afterwards for other reasons * and in the larger picture nothing changed. But it's too complicated * to find out exactly. Just assume, something changed to be sure. */ - changed = TRUE; + if (!nm_settings_connection_autoconnect_is_blocked (connection)) + changed = TRUE; } } else { /* we reset the tries-count and any blocked-reason */ @@ -1441,8 +1442,10 @@ reset_autoconnect_all (NMPolicy *self, if (nm_settings_connection_autoconnect_blocked_reason_set (connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_ALL & ~NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST, - FALSE)) - changed = TRUE; + FALSE)) { + if (!nm_settings_connection_autoconnect_is_blocked (connection)) + changed = TRUE; + } } } return changed; @@ -1615,8 +1618,10 @@ activate_slave_connections (NMPolicy *self, NMDevice *device) } if (nm_settings_connection_autoconnect_blocked_reason_set (connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED, - FALSE)) - changed = TRUE; + FALSE)) { + if (!nm_settings_connection_autoconnect_is_blocked (connection)) + changed = TRUE; + } } if (changed) From 6ab0ff8a7c183d50bf464cac7952187d6608b371 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 14:13:24 +0100 Subject: [PATCH 31/34] settings: use slice allocator for UpdateInfo data --- src/settings/nm-settings-connection.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 71ef2e49ad..4fae50ed0c 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -1638,8 +1638,7 @@ update_complete (NMSettingsConnection *self, g_clear_object (&info->agent_mgr); g_clear_object (&info->new_settings); g_free (info->audit_args); - memset (info, 0, sizeof (*info)); - g_free (info); + g_slice_free (UpdateInfo, info); } static void @@ -1814,7 +1813,7 @@ settings_connection_update_helper (NMSettingsConnection *self, goto error; } - info = g_malloc0 (sizeof (*info)); + info = g_slice_new0 (UpdateInfo); info->context = context; info->agent_mgr = g_object_ref (priv->agent_mgr); info->subject = subject; From c3cae3d0dc0d46bc6fff4ae221e3be588da2dc37 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 24 Nov 2017 18:17:26 +0100 Subject: [PATCH 32/34] core: use #define for "agent-registered" signal name --- src/settings/nm-agent-manager.c | 2 +- src/settings/nm-agent-manager.h | 2 ++ src/settings/nm-settings.c | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index 42df9a62a7..86de21a99a 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -1627,7 +1627,7 @@ nm_agent_manager_class_init (NMAgentManagerClass *agent_manager_class) object_class->dispose = dispose; signals[AGENT_REGISTERED] = - g_signal_new ("agent-registered", + g_signal_new (NM_AGENT_MANAGER_AGENT_REGISTERED, G_OBJECT_CLASS_TYPE (object_class), G_SIGNAL_RUN_FIRST, 0, diff --git a/src/settings/nm-agent-manager.h b/src/settings/nm-agent-manager.h index c2a2ec1d8f..ddbece7c56 100644 --- a/src/settings/nm-agent-manager.h +++ b/src/settings/nm-agent-manager.h @@ -33,6 +33,8 @@ #define NM_IS_AGENT_MANAGER_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_AGENT_MANAGER)) #define NM_AGENT_MANAGER_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_AGENT_MANAGER, NMAgentManagerClass)) +#define NM_AGENT_MANAGER_AGENT_REGISTERED "agent-registered" + typedef struct _NMAgentManagerCallId *NMAgentManagerCallId; typedef struct _NMAgentManagerClass NMAgentManagerClass; diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index d52592dd60..c29419e7b4 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -1897,7 +1897,7 @@ nm_settings_init (NMSettings *self) priv->config = g_object_ref (nm_config_get ()); - g_signal_connect (priv->agent_mgr, "agent-registered", G_CALLBACK (secret_agent_registered), self); + g_signal_connect (priv->agent_mgr, NM_AGENT_MANAGER_AGENT_REGISTERED, G_CALLBACK (secret_agent_registered), self); } NMSettings * From 46af70b508cdb19ce8fd6eafd01575d39124ed0e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 27 Nov 2017 09:07:28 +0100 Subject: [PATCH 33/34] policy: use "agent-registered" signal directly from NMAgentManager instead of NMSettings --- src/nm-policy.c | 16 +++++++++++++--- src/settings/nm-settings.c | 30 ------------------------------ src/settings/nm-settings.h | 1 - 3 files changed, 13 insertions(+), 34 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index d04727daec..98c45daa11 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -44,6 +44,7 @@ #include "nm-manager.h" #include "settings/nm-settings.h" #include "settings/nm-settings-connection.h" +#include "settings/nm-agent-manager.h" #include "nm-dhcp4-config.h" #include "nm-dhcp6-config.h" #include "nm-config.h" @@ -67,6 +68,8 @@ typedef struct { NMFirewallManager *firewall_manager; CList pending_activation_checks; + NMAgentManager *agent_mgr; + GHashTable *devices; GHashTable *pending_active_connections; @@ -2374,8 +2377,7 @@ secret_agent_registered (NMSettings *settings, NMSecretAgent *agent, gpointer user_data) { - NMPolicyPrivate *priv = user_data; - NMPolicy *self = _PRIV_TO_SELF (priv); + NMPolicy *self = NM_POLICY (user_data); /* The registered secret agent may provide some missing secrets. Thus we * reset retries count here and schedule activation, so that the @@ -2520,6 +2522,8 @@ constructed (GObject *object) _LOGT (LOGD_DNS, "hostname-original: set to %s%s%s", NM_PRINT_FMT_QUOTE_STRING (priv->orig_hostname)); + priv->agent_mgr = g_object_ref (nm_agent_manager_get ()); + priv->firewall_manager = g_object_ref (nm_firewall_manager_get ()); g_signal_connect (priv->firewall_manager, NM_FIREWALL_MANAGER_STATE_CHANGED, G_CALLBACK (firewall_state_changed), self); @@ -2544,7 +2548,8 @@ constructed (GObject *object) g_signal_connect (priv->settings, NM_SETTINGS_SIGNAL_CONNECTION_UPDATED, (GCallback) connection_updated, priv); g_signal_connect (priv->settings, NM_SETTINGS_SIGNAL_CONNECTION_REMOVED, (GCallback) connection_removed, priv); g_signal_connect (priv->settings, NM_SETTINGS_SIGNAL_CONNECTION_VISIBILITY_CHANGED, (GCallback) connection_visibility_changed, priv); - g_signal_connect (priv->settings, NM_SETTINGS_SIGNAL_AGENT_REGISTERED, (GCallback) secret_agent_registered, priv); + + g_signal_connect (priv->agent_mgr, NM_AGENT_MANAGER_AGENT_REGISTERED, G_CALLBACK (secret_agent_registered), self); G_OBJECT_CLASS (nm_policy_parent_class)->constructed (object); @@ -2593,6 +2598,11 @@ dispose (GObject *object) g_clear_object (&priv->firewall_manager); } + if (priv->agent_mgr) { + g_signal_handlers_disconnect_by_func (priv->agent_mgr, secret_agent_registered, self); + g_clear_object (&priv->agent_mgr); + } + if (priv->dns_manager) { nm_clear_g_signal_handler (priv->dns_manager, &priv->config_changed_id); g_clear_object (&priv->dns_manager); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index c29419e7b4..081997058f 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -128,7 +128,6 @@ enum { CONNECTION_UPDATED, CONNECTION_REMOVED, CONNECTION_VISIBILITY_CHANGED, - AGENT_REGISTERED, NEW_CONNECTION, /* exported, not used internally */ LAST_SIGNAL }; @@ -878,18 +877,6 @@ connection_removed (NMSettingsConnection *connection, gpointer user_data) g_object_unref (connection); } -static void -secret_agent_registered (NMAgentManager *agent_mgr, - NMSecretAgent *agent, - gpointer user_data) -{ - /* Re-emit for listeners like NMPolicy */ - g_signal_emit (NM_SETTINGS (user_data), - signals[AGENT_REGISTERED], - 0, - agent); -} - #define NM_DBUS_SERVICE_OPENCONNECT "org.freedesktop.NetworkManager.openconnect" #define NM_OPENCONNECT_KEY_GATEWAY "gateway" #define NM_OPENCONNECT_KEY_COOKIE "cookie" @@ -1888,16 +1875,8 @@ nm_settings_init (NMSettings *self) priv->connections = g_hash_table_new_full (nm_str_hash, g_str_equal, NULL, g_object_unref); - /* Hold a reference to the agent manager so it stays alive; the only - * other holders are NMSettingsConnection objects which are often - * transient, and we don't want the agent manager to get destroyed and - * recreated often. - */ priv->agent_mgr = g_object_ref (nm_agent_manager_get ()); - priv->config = g_object_ref (nm_config_get ()); - - g_signal_connect (priv->agent_mgr, NM_AGENT_MANAGER_AGENT_REGISTERED, G_CALLBACK (secret_agent_registered), self); } NMSettings * @@ -2022,15 +2001,6 @@ nm_settings_class_init (NMSettingsClass *class) g_cclosure_marshal_VOID__OBJECT, G_TYPE_NONE, 1, NM_TYPE_SETTINGS_CONNECTION); - signals[AGENT_REGISTERED] = - g_signal_new (NM_SETTINGS_SIGNAL_AGENT_REGISTERED, - G_OBJECT_CLASS_TYPE (object_class), - G_SIGNAL_RUN_FIRST, - 0, NULL, NULL, - g_cclosure_marshal_VOID__OBJECT, - G_TYPE_NONE, 1, NM_TYPE_SECRET_AGENT); - - signals[NEW_CONNECTION] = g_signal_new ("new-connection", G_OBJECT_CLASS_TYPE (object_class), diff --git a/src/settings/nm-settings.h b/src/settings/nm-settings.h index 0e01f6b5fd..cbd8afa9c3 100644 --- a/src/settings/nm-settings.h +++ b/src/settings/nm-settings.h @@ -47,7 +47,6 @@ #define NM_SETTINGS_SIGNAL_CONNECTION_UPDATED "connection-updated" #define NM_SETTINGS_SIGNAL_CONNECTION_REMOVED "connection-removed" #define NM_SETTINGS_SIGNAL_CONNECTION_VISIBILITY_CHANGED "connection-visibility-changed" -#define NM_SETTINGS_SIGNAL_AGENT_REGISTERED "agent-registered" /** * NMConnectionFilterFunc: From e2c8ef45ac9fba8d4f5722ab10831bf42085a110 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 27 Nov 2017 09:33:32 +0100 Subject: [PATCH 34/34] core: fix race of blocking autoconnect for no-secrets when a new secret-agent registers When activation of the connection fails with no-secrets, we block autoconnect due to that. However, NMPolicy also unblocks such autoconnect, whenever a new secret-agent registers. The reason is obviously, that the new secret-agent might be able to provide the previously missing secrets. However, there is a race between - making the secret request, failing activation and blocking autoconnect - new secret-agent registers If the secret-agent registers after making the request, but before we block autoconnect, then autoconnect stays blocked. [1511468634.5759] device (wlp4s0): state change: config -> need-auth (reason 'none', sys-iface-state: 'managed') [1511468634.5772] device (wlp4s0): No agents were available for this request. [1511468638.4082] agent-manager: req[0x55ea7e58a5d0, :1.32/org.kde.plasma.networkmanagement/1000]: agent registered [1511468638.4082] policy: re-enabling autoconnect for all connections with failed secrets [1511468664.6280] device (wlp4s0): state change: need-auth -> failed (reason 'no-secrets', sys-iface-state: 'managed') [1511468664.6287] policy: connection 'tuxmobil' now blocked from autoconnect due to no secrets Note the long timing between making the secret request and the activation failure. This race already existed before, but now with WPS push-button method enabled by default, the duraction of the activation is much longer and the race is easy to hit. https://bugzilla.gnome.org/show_bug.cgi?id=790571 --- src/nm-policy.c | 23 +++++++++++++++++++---- src/settings/nm-agent-manager.c | 14 ++++++++++++++ src/settings/nm-agent-manager.h | 2 ++ src/settings/nm-secret-agent.c | 7 ++----- src/settings/nm-settings-connection.c | 26 ++++++++++++++++++++++++-- src/settings/nm-settings-connection.h | 2 ++ 6 files changed, 63 insertions(+), 11 deletions(-) diff --git a/src/nm-policy.c b/src/nm-policy.c index 98c45daa11..6b46ed1469 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -1750,20 +1750,35 @@ device_state_changed (NMDevice *device, if ( connection && old_state >= NM_DEVICE_STATE_PREPARE && old_state <= NM_DEVICE_STATE_ACTIVATED) { + gboolean block_no_secrets = FALSE; int tries; + guint64 con_v; - tries = nm_settings_connection_autoconnect_retries_get (connection); if (nm_device_state_reason_check (reason) == NM_DEVICE_STATE_REASON_NO_SECRETS) { + /* we want to block the connection from auto-connect if it failed due to no-secrets. + * However, if a secret-agent registered, since the connection made the last + * secret-request, we do not block it. The new secret-agent might not yet + * been consulted, and it may be able to provide the secrets. + * + * We detect this by using a version-id of the agent-manager, which increments + * whenever new agents register. */ + con_v = nm_settings_connection_get_last_secret_agent_version_id (connection); + if ( con_v == 0 + || con_v != nm_agent_manager_get_agent_version_id (priv->agent_mgr)) + block_no_secrets = TRUE; + } + + if (block_no_secrets) { _LOGD (LOGD_DEVICE, "connection '%s' now blocked from autoconnect due to no secrets", nm_settings_connection_get_id (connection)); - nm_settings_connection_autoconnect_blocked_reason_set (connection, NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS, TRUE); - } else if (tries != 0) { + } else { + tries = nm_settings_connection_autoconnect_retries_get (connection); if (tries > 0) { _LOGD (LOGD_DEVICE, "connection '%s' failed to autoconnect; %d tries left", nm_settings_connection_get_id (connection), tries - 1); _connection_autoconnect_retries_set (self, connection, tries - 1); - } else { + } else if (tries != 0) { _LOGD (LOGD_DEVICE, "connection '%s' failed to autoconnect; infinite tries left", nm_settings_connection_get_id (connection)); } diff --git a/src/settings/nm-agent-manager.c b/src/settings/nm-agent-manager.c index 86de21a99a..345d6e6611 100644 --- a/src/settings/nm-agent-manager.c +++ b/src/settings/nm-agent-manager.c @@ -62,6 +62,8 @@ typedef struct { GHashTable *agents; CList requests; + + guint64 agent_version_id; } NMAgentManagerPrivate; struct _NMAgentManager { @@ -137,6 +139,16 @@ static gboolean _con_get_try_complete_early (Request *req); /*****************************************************************************/ +guint64 +nm_agent_manager_get_agent_version_id (NMAgentManager *self) +{ + g_return_val_if_fail (NM_IS_AGENT_MANAGER (self), 0); + + return NM_AGENT_MANAGER_GET_PRIVATE (self)->agent_version_id; +} + +/*****************************************************************************/ + typedef enum { REQUEST_TYPE_INVALID, REQUEST_TYPE_CON_GET, @@ -336,6 +348,7 @@ agent_register_permissions_done (NMAuthChain *chain, if (result == NM_AUTH_CALL_RESULT_YES) nm_secret_agent_add_permission (agent, NM_AUTH_PERMISSION_WIFI_SHARE_OPEN, TRUE); + priv->agent_version_id += 1; sender = nm_secret_agent_get_dbus_owner (agent); g_hash_table_insert (priv->agents, g_strdup (sender), agent); _LOGD (agent, "agent registered"); @@ -1558,6 +1571,7 @@ nm_agent_manager_init (NMAgentManager *self) { NMAgentManagerPrivate *priv = NM_AGENT_MANAGER_GET_PRIVATE (self); + priv->agent_version_id = 1; c_list_init (&priv->requests); priv->agents = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_object_unref); } diff --git a/src/settings/nm-agent-manager.h b/src/settings/nm-agent-manager.h index ddbece7c56..f6845818dd 100644 --- a/src/settings/nm-agent-manager.h +++ b/src/settings/nm-agent-manager.h @@ -43,6 +43,8 @@ GType nm_agent_manager_get_type (void); NMAgentManager *nm_agent_manager_get (void); +guint64 nm_agent_manager_get_agent_version_id (NMAgentManager *self); + /* If no agent fulfilled the secrets request, agent_dbus_owner will be NULL */ typedef void (*NMAgentSecretsResultFunc) (NMAgentManager *manager, NMAgentManagerCallId call_id, diff --git a/src/settings/nm-secret-agent.c b/src/settings/nm-secret-agent.c index 99b955ed4a..5cb0ce4538 100644 --- a/src/settings/nm-secret-agent.c +++ b/src/settings/nm-secret-agent.c @@ -50,16 +50,13 @@ typedef struct { char *owner_username; char *dbus_owner; NMSecretAgentCapabilities capabilities; - GSList *permissions; - NMDBusSecretAgent *proxy; NMBusManager *bus_mgr; GDBusConnection *connection; - gboolean connection_is_private; - gulong on_disconnected_id; - CList requests; + gulong on_disconnected_id; + bool connection_is_private:1; } NMSecretAgentPrivate; struct _NMSecretAgent { diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index 4fae50ed0c..ec73174a4d 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -106,13 +106,17 @@ typedef struct _NMSettingsConnectionPrivate { */ NMConnection *agent_secrets; - guint64 timestamp; /* Up-to-date timestamp of connection use */ + char *filename; + GHashTable *seen_bssids; /* Up-to-date BSSIDs that's been seen for the connection */ + guint64 timestamp; /* Up-to-date timestamp of connection use */ + + guint64 last_secret_agent_version_id; + int autoconnect_retries; gint32 autoconnect_retries_blocked_until; - char *filename; } NMSettingsConnectionPrivate; G_DEFINE_TYPE_WITH_CODE (NMSettingsConnection, nm_settings_connection, NM_TYPE_EXPORTED_OBJECT, @@ -171,6 +175,16 @@ nm_settings_connection_has_unmodified_applied_connection (NMSettingsConnection * /*****************************************************************************/ +guint64 +nm_settings_connection_get_last_secret_agent_version_id (NMSettingsConnection *self) +{ + g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (self), 0); + + return NM_SETTINGS_CONNECTION_GET_PRIVATE (self)->last_secret_agent_version_id; +} + +/*****************************************************************************/ + /* Return TRUE to keep, FALSE to drop */ typedef gboolean (*ForEachSecretFunc) (NMSettingSecretFlags flags, gpointer user_data); @@ -1261,6 +1275,14 @@ nm_settings_connection_get_secrets (NMSettingsConnection *self, existing_secrets = nm_connection_to_dbus (priv->system_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS); if (existing_secrets) g_variant_ref_sink (existing_secrets); + + /* we remember the current version-id of the secret-agents. The version-id is strictly increasing, + * as new agents register the number. We know hence, that this request was made against a certain + * set of secret-agents. + * If after making this request a new secret-agent registeres, the version-id increases. + * Then we know that the this request probably did not yet include the latest secret-agent. */ + priv->last_secret_agent_version_id = nm_agent_manager_get_agent_version_id (priv->agent_mgr); + call_id_a = nm_agent_manager_get_secrets (priv->agent_mgr, nm_connection_get_path (NM_CONNECTION (self)), NM_CONNECTION (self), diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index 7caf42b717..3672bc08c1 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -125,6 +125,8 @@ struct _NMSettingsConnectionClass { GType nm_settings_connection_get_type (void); +guint64 nm_settings_connection_get_last_secret_agent_version_id (NMSettingsConnection *self); + gboolean nm_settings_connection_has_unmodified_applied_connection (NMSettingsConnection *self, NMConnection *applied_connection, NMSettingCompareFlags compare_flage);