From fc624b8de838fcacd18429bee981949b18f6a9a8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 May 2023 14:19:14 +0200 Subject: core: assert for valid blocked reasons in autoconnect code We only have a few blocked reasons. Some of them can be only set on the devcon data, and some only on the settings connection. Assert that we don't mix that up. --- src/core/nm-manager.c | 26 +++++++++++++------------- src/core/nm-policy.c | 2 +- src/core/settings/nm-settings-connection.c | 5 ++++- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 02dabaf6fe..8773ce47fe 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -1252,7 +1252,7 @@ _autoconnect_retries_initial(NMSettingsConnection *sett_conn) if (retries == -1) retries = nm_config_data_get_autoconnect_retries_default(NM_CONFIG_GET_DATA); - nm_assert(retries >= 0 && ((guint) retries) <= ((guint) G_MAXINT32)); + nm_assert(retries >= 0 && retries <= G_MAXINT32); if (retries == 0) return NM_AUTOCONNECT_RETRIES_FOREVER; @@ -1401,16 +1401,15 @@ nm_manager_devcon_autoconnect_reset_reconnect_all(NMManager *self, /* we reset the tries-count and any blocked-reason */ nm_manager_devcon_autoconnect_retries_reset(self, NULL, sett_conn); - /* if there is a device and we changed the state, then something changed. */ - if (device - && nm_manager_devcon_autoconnect_blocked_reason_set( - self, - device, - sett_conn, - NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED - | NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_USER_REQUEST, - FALSE)) - changed = TRUE; + if (device) { + if (nm_manager_devcon_autoconnect_blocked_reason_set( + self, + device, + sett_conn, + NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED, + FALSE)) + changed = TRUE; + } /* we remove all the blocked reason from the connection, if something * happened, then it means the status changed */ @@ -1487,8 +1486,9 @@ nm_manager_devcon_autoconnect_blocked_reason_set(NMManager gboolean changed = FALSE; char buf[100]; - nm_assert(value); - nm_assert(sett_conn); + nm_assert(NM_IS_SETTINGS_CONNECTION(sett_conn)); + nm_assert(value != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE); + nm_assert(!NM_FLAGS_ANY(value, ~(NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED))); if (device) { data = _devcon_lookup_data(self, device, sett_conn, TRUE); diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index 9309026e56..2e7f9c605b 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -2158,7 +2158,7 @@ device_state_changed(NMDevice *device, priv->manager, device, sett_conn, - NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_ALL, + NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED, FALSE); break; case NM_DEVICE_STATE_SECONDARIES: diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c index 8d9ee08a99..8b18812255 100644 --- a/src/core/settings/nm-settings-connection.c +++ b/src/core/settings/nm-settings-connection.c @@ -2526,7 +2526,10 @@ nm_settings_connection_autoconnect_blocked_reason_set(NMSettingsConnection char buf1[100]; char buf2[100]; - nm_assert(reason); + nm_assert(reason != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE); + nm_assert(!NM_FLAGS_ANY(reason, + ~(NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_USER_REQUEST + | NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NO_SECRETS))); v = priv->autoconnect_blocked_reason; v = NM_FLAGS_ASSIGN(v, reason, set); -- cgit v1.2.1 From 87b46e166350548b518074a65ab2492825992fc0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 27 Apr 2023 17:00:10 +0200 Subject: core: improve handling for blocking autoconnect Cleanup logging to always print a "block-autoconnect:" prefix to related lines. Also, make sure that everywhere where the state changes, a line gets logged. Also, for devconf data print both the interface and the profile. --- src/core/nm-manager.c | 141 ++++++++++++++++++----------- src/core/nm-policy.c | 8 +- src/core/settings/nm-settings-connection.c | 23 +++-- 3 files changed, 110 insertions(+), 62 deletions(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 8773ce47fe..46adfd356b 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -89,6 +89,16 @@ typedef struct { } DevConData; +#define DEV_CON_DATA_LOG_FMT \ + "device[" NM_HASH_OBFUSCATE_PTR_FMT ",%s]-profile[" NM_HASH_OBFUSCATE_PTR_FMT ",%s]" + +/* This is an unsafe macro (it evaluates the macro arguments multiple times and is non-function-like). */ +#define DEV_CON_DATA_LOG_ARGS(device, sett_conn) \ + NM_HASH_OBFUSCATE_PTR(device), nm_device_get_iface(device), NM_HASH_OBFUSCATE_PTR(sett_conn), \ + nm_settings_connection_get_id(sett_conn) + +#define DEV_CON_DATA_LOG_ARGS_DATA(data) DEV_CON_DATA_LOG_ARGS((data)->device, (data)->sett_conn) + typedef enum { ASYNC_OP_TYPE_AC_AUTH_ACTIVATE_INTERNAL, ASYNC_OP_TYPE_AC_AUTH_ACTIVATE_USER, @@ -440,7 +450,8 @@ static void _rfkill_update(NMManager *self, NMRfkillType rtype); static DevConData *_devcon_lookup_data(NMManager *self, NMDevice *device, NMSettingsConnection *sett_conn, - gboolean create); + gboolean create, + gboolean log_creation); /*****************************************************************************/ @@ -1262,38 +1273,45 @@ _autoconnect_retries_initial(NMSettingsConnection *sett_conn) static void _autoconnect_retries_set(NMManager *self, DevConData *data, guint32 retries, gboolean is_reset) { + gboolean changed = FALSE; + gint32 blocked_until_sec; + nm_assert(data); - if (data->autoconnect.retries != retries || !data->autoconnect.initialized) { - _LOGT(LOGD_SETTINGS, - "autoconnect: device[%p] (%s): retries set %d%s", - data->device, - nm_device_get_ip_iface(data->device), - retries, - is_reset ? " (reset)" : ""); + if (!data->autoconnect.initialized || data->autoconnect.retries != retries) { data->autoconnect.initialized = TRUE; data->autoconnect.retries = retries; + changed = TRUE; } if (retries != 0) { - _LOGT(LOGD_SETTINGS, - "autoconnect: device[%p] (%s): no longer block autoconnect (due to retry count) %s", - data->device, - nm_device_get_ip_iface(data->device), - is_reset ? " (reset)" : ""); - data->autoconnect.blocked_until_sec = 0; + blocked_until_sec = 0; } else { /* NOTE: 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 every time a connection gets blocked). */ - data->autoconnect.blocked_until_sec = + blocked_until_sec = nm_utils_get_monotonic_timestamp_sec() + AUTOCONNECT_RESET_RETRIES_TIMER_SEC; + } + + if (data->autoconnect.blocked_until_sec != blocked_until_sec) { + data->autoconnect.blocked_until_sec = blocked_until_sec; + changed = TRUE; + } + + if (changed) { + char sbuf[200]; + _LOGT(LOGD_SETTINGS, - "autoconnect: device[%p] (%s): block autoconnect due to retry count for %d seconds", - data->device, - nm_device_get_ip_iface(data->device), - AUTOCONNECT_RESET_RETRIES_TIMER_SEC); + "block-autoconnect: " DEV_CON_DATA_LOG_FMT ": retries set %u%s%s", + DEV_CON_DATA_LOG_ARGS_DATA(data), + retries, + is_reset ? " (is-reset)" : "", + blocked_until_sec == 0 ? "" + : nm_sprintf_buf(sbuf, + " (blocked for %d sec)", + AUTOCONNECT_RESET_RETRIES_TIMER_SEC)); } } @@ -1320,11 +1338,10 @@ nm_manager_devcon_autoconnect_retries_get(NMManager *self, nm_assert(self == nm_device_get_manager(device)); nm_assert(self == nm_settings_connection_get_manager(sett_conn)); - data = _devcon_lookup_data(self, device, sett_conn, TRUE); + data = _devcon_lookup_data(self, device, sett_conn, TRUE, FALSE); - if (G_UNLIKELY(!data->autoconnect.initialized)) { + if (G_UNLIKELY(!data->autoconnect.initialized)) _autoconnect_retries_set(self, data, _autoconnect_retries_initial(sett_conn), FALSE); - } return data->autoconnect.retries; } @@ -1336,7 +1353,7 @@ nm_manager_devcon_autoconnect_retries_set(NMManager *self, guint32 retries) { _autoconnect_retries_set(self, - _devcon_lookup_data(self, device, sett_conn, TRUE), + _devcon_lookup_data(self, device, sett_conn, TRUE, FALSE), retries, FALSE); } @@ -1347,18 +1364,22 @@ nm_manager_devcon_autoconnect_retries_reset(NMManager *self, NMSettingsConnection *sett_conn) { DevConData *data; + guint32 retries_initial; nm_assert(NM_IS_SETTINGS_CONNECTION(sett_conn)); - if (device) + retries_initial = _autoconnect_retries_initial(sett_conn); + + if (device) { _autoconnect_retries_set(self, - _devcon_lookup_data(self, device, sett_conn, TRUE), - _autoconnect_retries_initial(sett_conn), + _devcon_lookup_data(self, device, sett_conn, TRUE, FALSE), + retries_initial, TRUE); - else { - c_list_for_each_entry (data, &sett_conn->devcon_con_lst_head, con_lst) - _autoconnect_retries_set(self, data, _autoconnect_retries_initial(sett_conn), TRUE); + return; } + + c_list_for_each_entry (data, &sett_conn->devcon_con_lst_head, con_lst) + _autoconnect_retries_set(self, data, retries_initial, TRUE); } /** @@ -1398,7 +1419,8 @@ nm_manager_devcon_autoconnect_reset_reconnect_all(NMManager *self, return changed; } - /* we reset the tries-count and any blocked-reason */ + /* we reset the tries-count and any blocked-reason... */ + nm_manager_devcon_autoconnect_retries_reset(self, NULL, sett_conn); if (device) { @@ -1434,18 +1456,19 @@ nm_manager_devcon_autoconnect_retries_blocked_until(NMManager *self, nm_assert(NM_IS_SETTINGS_CONNECTION(sett_conn)); if (device) { - data = _devcon_lookup_data(self, device, sett_conn, TRUE); + data = _devcon_lookup_data(self, device, sett_conn, TRUE, TRUE); return data->autoconnect.blocked_until_sec; - } else { - min_stamp = 0; - c_list_for_each_entry (data, &sett_conn->devcon_con_lst_head, con_lst) { - gint32 condev_stamp = data->autoconnect.blocked_until_sec; - if (condev_stamp == 0) - continue; + } - if (min_stamp == 0 || min_stamp > condev_stamp) - min_stamp = condev_stamp; - } + min_stamp = 0; + c_list_for_each_entry (data, &sett_conn->devcon_con_lst_head, con_lst) { + gint32 condev_stamp = data->autoconnect.blocked_until_sec; + + if (condev_stamp == 0) + continue; + + if (min_stamp == 0 || min_stamp > condev_stamp) + min_stamp = condev_stamp; } return min_stamp; @@ -1464,11 +1487,12 @@ nm_manager_devcon_autoconnect_is_blocked(NMManager *self, if (nm_settings_connection_autoconnect_is_blocked(sett_conn)) return TRUE; - data = _devcon_lookup_data(self, device, sett_conn, TRUE); + data = _devcon_lookup_data(self, device, sett_conn, TRUE, TRUE); + if (data->autoconnect.blocked_reason != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE) return TRUE; - if (data->autoconnect.retries == 0 && data->autoconnect.initialized) + if (data->autoconnect.initialized && data->autoconnect.retries == 0) return TRUE; return FALSE; @@ -1491,18 +1515,18 @@ nm_manager_devcon_autoconnect_blocked_reason_set(NMManager nm_assert(!NM_FLAGS_ANY(value, ~(NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED))); if (device) { - data = _devcon_lookup_data(self, device, sett_conn, TRUE); + data = _devcon_lookup_data(self, device, sett_conn, TRUE, TRUE); v = data->autoconnect.blocked_reason; v = NM_FLAGS_ASSIGN(v, value, set); if (data->autoconnect.blocked_reason == v) return FALSE; - _LOGT(LOGD_SETTINGS, - "autoconnect: blocked reason: %s for device %s", - nm_settings_autoconnect_blocked_reason_to_string(v, buf, sizeof(buf)), - nm_device_get_ip_iface(device)); data->autoconnect.blocked_reason = v; + _LOGT(LOGD_SETTINGS, + "block-autoconnect: " DEV_CON_DATA_LOG_FMT ": set blocked reason %s", + DEV_CON_DATA_LOG_ARGS_DATA(data), + nm_settings_autoconnect_blocked_reason_to_string(v, buf, sizeof(buf))); return TRUE; } @@ -1512,10 +1536,11 @@ nm_manager_devcon_autoconnect_blocked_reason_set(NMManager if (data->autoconnect.blocked_reason == v) continue; + _LOGT(LOGD_SETTINGS, - "autoconnect: blocked reason: %s for device %s", - nm_settings_autoconnect_blocked_reason_to_string(v, buf, sizeof(buf)), - nm_device_get_ip_iface(data->device)); + "block-autoconnect: " DEV_CON_DATA_LOG_FMT ": set blocked reason %s", + DEV_CON_DATA_LOG_ARGS_DATA(data), + nm_settings_autoconnect_blocked_reason_to_string(v, buf, sizeof(buf))); data->autoconnect.blocked_reason = v; changed = TRUE; } @@ -1554,7 +1579,8 @@ static DevConData * _devcon_lookup_data(NMManager *self, NMDevice *device, NMSettingsConnection *sett_conn, - gboolean create) + gboolean create, + gboolean log_creation) { NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE(self); DevConData *data; @@ -1581,7 +1607,10 @@ _devcon_lookup_data(NMManager *self, .sett_conn = sett_conn, .autoconnect = { - .initialized = FALSE, + .initialized = FALSE, + .retries = 0, + .blocked_until_sec = 0, + .blocked_reason = NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE, }, }; c_list_link_tail(&device->devcon_dev_lst_head, &data->dev_lst); @@ -1589,6 +1618,12 @@ _devcon_lookup_data(NMManager *self, g_hash_table_add(priv->devcon_data_dict, data); + if (log_creation) { + _LOGT(LOGD_SETTINGS, + "block-autoconnect: " DEV_CON_DATA_LOG_FMT ": entry created (not initialized)", + DEV_CON_DATA_LOG_ARGS_DATA(data)); + } + return data; } @@ -1600,7 +1635,7 @@ _devcon_remove_data(NMManager *self, DevConData *data) nm_assert(data); nm_assert(NM_IS_DEVICE(data->device)); nm_assert(NM_IS_SETTINGS_CONNECTION(data->sett_conn)); - nm_assert(data == _devcon_lookup_data(self, data->device, data->sett_conn, FALSE)); + nm_assert(data == _devcon_lookup_data(self, data->device, data->sett_conn, FALSE, FALSE)); c_list_unlink_stale(&data->dev_lst); c_list_unlink_stale(&data->con_lst); diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index 2e7f9c605b..ae2c15f22a 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -1998,7 +1998,8 @@ device_state_changed(NMDevice *device, con_v = nm_settings_connection_get_last_secret_agent_version_id(sett_conn); if (con_v == 0 || con_v == nm_agent_manager_get_agent_version_id(priv->agent_mgr)) { _LOGD(LOGD_DEVICE, - "connection '%s' now blocked from autoconnect due to no secrets", + "block-autoconnect: connection '%s' now blocked from autoconnect due to " + "no secrets", nm_settings_connection_get_id(sett_conn)); nm_settings_connection_autoconnect_blocked_reason_set( sett_conn, @@ -2017,7 +2018,8 @@ device_state_changed(NMDevice *device, * dependency-failed. */ _LOGD(LOGD_DEVICE, - "autoconnect: connection[%p] (%s) now blocked from autoconnect due to failed " + "block-autoconnect: connection[%p] (%s) now blocked from autoconnect due to " + "failed " "dependency", sett_conn, nm_settings_connection_get_id(sett_conn)); @@ -2102,7 +2104,7 @@ device_state_changed(NMDevice *device, } if (blocked_reason != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE) { _LOGD(LOGD_DEVICE, - "blocking autoconnect of connection '%s': %s", + "block-autoconnect: blocking autoconnect of connection '%s': %s", nm_settings_connection_get_id(sett_conn), NM_UTILS_LOOKUP_STR_A(nm_device_state_reason_to_string, nm_device_state_reason_check(reason))); diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c index 8b18812255..da0f272872 100644 --- a/src/core/settings/nm-settings-connection.c +++ b/src/core/settings/nm-settings-connection.c @@ -2523,8 +2523,8 @@ nm_settings_connection_autoconnect_blocked_reason_set(NMSettingsConnection { NMSettingsAutoconnectBlockedReason v; NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self); - char buf1[100]; - char buf2[100]; + char buf1[200]; + char buf2[200]; nm_assert(reason != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE); nm_assert(!NM_FLAGS_ANY(reason, @@ -2537,10 +2537,21 @@ nm_settings_connection_autoconnect_blocked_reason_set(NMSettingsConnection if (priv->autoconnect_blocked_reason == v) return FALSE; - _LOGD("autoconnect: %s blocked reason: %s (now %s)", - set ? "set" : "unset", - nm_settings_autoconnect_blocked_reason_to_string(reason, buf1, sizeof(buf1)), - nm_settings_autoconnect_blocked_reason_to_string(v, buf2, sizeof(buf2))); + if (set) { + _LOGT("block-autoconnect: profile: blocked with reason %s (%s %s)", + nm_settings_autoconnect_blocked_reason_to_string(v, buf1, sizeof(buf1)), + "just blocked", + nm_settings_autoconnect_blocked_reason_to_string(reason, buf2, sizeof(buf2))); + } else if (v != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE) { + _LOGT("block-autoconnect: profile: blocked with reason %s (%s %s)", + nm_settings_autoconnect_blocked_reason_to_string(v, buf1, sizeof(buf1)), + "just unblocked", + nm_settings_autoconnect_blocked_reason_to_string(reason, buf2, sizeof(buf2))); + } else { + _LOGT("block-autoconnect: profile: not blocked (unblocked %s)", + nm_settings_autoconnect_blocked_reason_to_string(reason, buf1, sizeof(buf1))); + } + priv->autoconnect_blocked_reason = v; return TRUE; } -- cgit v1.2.1 From a019d965f72e1f8d59413f2df9e8d94cdfff84db Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Apr 2023 11:47:20 +0200 Subject: core: avoid creating devcon data that we don't need MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise, we create device × profiles entries, most of them nonsensical. --- src/core/nm-manager.c | 11 +++++++++-- src/core/nm-policy.c | 10 ++++++---- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 46adfd356b..6d8245c7d8 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -1456,7 +1456,11 @@ nm_manager_devcon_autoconnect_retries_blocked_until(NMManager *self, nm_assert(NM_IS_SETTINGS_CONNECTION(sett_conn)); if (device) { - data = _devcon_lookup_data(self, device, sett_conn, TRUE, TRUE); + data = _devcon_lookup_data(self, device, sett_conn, FALSE, FALSE); + + if (!data) + return 0; + return data->autoconnect.blocked_until_sec; } @@ -1487,7 +1491,10 @@ nm_manager_devcon_autoconnect_is_blocked(NMManager *self, if (nm_settings_connection_autoconnect_is_blocked(sett_conn)) return TRUE; - data = _devcon_lookup_data(self, device, sett_conn, TRUE, TRUE); + data = _devcon_lookup_data(self, device, sett_conn, FALSE, FALSE); + + if (!data) + return FALSE; if (data->autoconnect.blocked_reason != NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_NONE) return TRUE; diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index ae2c15f22a..7a4cab1d21 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -1758,11 +1758,13 @@ _connection_autoconnect_retries_set(NMPolicy *self, if (tries == 0) { /* Schedule a handler to reset retries count */ if (!priv->reset_connections_retries_idle_source) { - gint32 retry_time = nm_manager_devcon_autoconnect_retries_blocked_until(priv->manager, - device, - connection); + gint32 retry_time; + + retry_time = nm_manager_devcon_autoconnect_retries_blocked_until(priv->manager, + device, + connection); + nm_assert(retry_time != 0); - g_warn_if_fail(retry_time != 0); priv->reset_connections_retries_idle_source = nm_g_timeout_add_seconds_source( MAX(0, retry_time - nm_utils_get_monotonic_timestamp_sec()), reset_connections_retries, -- cgit v1.2.1 From 5492945fdcab7027b0f7f5791e303d2a22249f9e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Apr 2023 12:12:21 +0200 Subject: core: use switch statement in device_state_changed() It seems better for readability, because reacting based on the state-reason is ugly already. This way, we access nm_device_state_reason_check(reason) only at once place. With the if, it's not immediately obvious that both if/else parts only switch on the reason too. --- src/core/nm-policy.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index 7a4cab1d21..5b64c86951 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -1983,7 +1983,8 @@ device_state_changed(NMDevice *device, gboolean blocked = FALSE; guint64 con_v; - if (nm_device_state_reason_check(reason) == NM_DEVICE_STATE_REASON_NO_SECRETS) { + switch (nm_device_state_reason_check(reason)) { + case 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 @@ -2009,8 +2010,8 @@ device_state_changed(NMDevice *device, TRUE); blocked = TRUE; } - } else if (nm_device_state_reason_check(reason) - == NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED) { + break; + case NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED: /* A connection that fails due to dependency-failed is not * able to reconnect until the master connection activates * again; when this happens, the master clears the blocked @@ -2032,6 +2033,9 @@ device_state_changed(NMDevice *device, NM_SETTINGS_AUTOCONNECT_BLOCKED_REASON_FAILED, TRUE); blocked = TRUE; + break; + default: + break; } if (!blocked) { -- cgit v1.2.1 From 7e15b4d5621fbf4b65c27d25841ee759c39fac52 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Apr 2023 12:59:49 +0200 Subject: core: return whether anything changed from nm_manager_devcon_autoconnect_retries_reset() --- src/core/nm-manager.c | 24 +++++++++++++++--------- src/core/nm-manager.h | 6 +++--- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index 6d8245c7d8..c6cf2cb943 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -1270,7 +1270,7 @@ _autoconnect_retries_initial(NMSettingsConnection *sett_conn) return (guint32) retries; } -static void +static gboolean _autoconnect_retries_set(NMManager *self, DevConData *data, guint32 retries, gboolean is_reset) { gboolean changed = FALSE; @@ -1313,6 +1313,8 @@ _autoconnect_retries_set(NMManager *self, DevConData *data, guint32 retries, gbo " (blocked for %d sec)", AUTOCONNECT_RESET_RETRIES_TIMER_SEC)); } + + return changed; } /** @@ -1358,28 +1360,32 @@ nm_manager_devcon_autoconnect_retries_set(NMManager *self, FALSE); } -void +gboolean nm_manager_devcon_autoconnect_retries_reset(NMManager *self, NMDevice *device, NMSettingsConnection *sett_conn) { DevConData *data; guint32 retries_initial; + gboolean changed = FALSE; nm_assert(NM_IS_SETTINGS_CONNECTION(sett_conn)); retries_initial = _autoconnect_retries_initial(sett_conn); if (device) { - _autoconnect_retries_set(self, - _devcon_lookup_data(self, device, sett_conn, TRUE, FALSE), - retries_initial, - TRUE); - return; + return _autoconnect_retries_set(self, + _devcon_lookup_data(self, device, sett_conn, TRUE, FALSE), + retries_initial, + TRUE); + } + + c_list_for_each_entry (data, &sett_conn->devcon_con_lst_head, con_lst) { + if (_autoconnect_retries_set(self, data, retries_initial, TRUE)) + changed = TRUE; } - c_list_for_each_entry (data, &sett_conn->devcon_con_lst_head, con_lst) - _autoconnect_retries_set(self, data, retries_initial, TRUE); + return changed; } /** diff --git a/src/core/nm-manager.h b/src/core/nm-manager.h index caa83e4546..3028eb7ebe 100644 --- a/src/core/nm-manager.h +++ b/src/core/nm-manager.h @@ -243,9 +243,9 @@ void nm_manager_devcon_autoconnect_retries_set(NMManager *self, NMSettingsConnection *sett_conn, guint32 retries); -void nm_manager_devcon_autoconnect_retries_reset(NMManager *self, - NMDevice *device, - NMSettingsConnection *sett_conn); +gboolean nm_manager_devcon_autoconnect_retries_reset(NMManager *self, + NMDevice *device, + NMSettingsConnection *sett_conn); gboolean nm_manager_devcon_autoconnect_reset_reconnect_all(NMManager *self, NMDevice *device, -- cgit v1.2.1 From 6e229a852fd93fc0dd5d52fcc0884d462a243249 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 28 Apr 2023 13:03:18 +0200 Subject: core: only trigger recheck when something changes in activate_slave_connections() We need to detect when nothing relevant changes, and shortcut doing things when they are unnecessary. --- src/core/nm-policy.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/nm-policy.c b/src/core/nm-policy.c index 5b64c86951..bad8d32f6d 100644 --- a/src/core/nm-policy.c +++ b/src/core/nm-policy.c @@ -1827,9 +1827,8 @@ activate_slave_connections(NMPolicy *self, NMDevice *device) continue; if (!internal_activation) { - nm_manager_devcon_autoconnect_retries_reset(priv->manager, NULL, sett_conn); - /* we cannot know if they changed or not, so considering we did a reset, let's consider they changed. */ - changed = TRUE; + if (nm_manager_devcon_autoconnect_retries_reset(priv->manager, NULL, sett_conn)) + changed = TRUE; } /* unblock the devices associated with that connection */ if (nm_manager_devcon_autoconnect_blocked_reason_set( -- cgit v1.2.1